diff --git a/src/app/api/coach/about-me/__tests__/route-concurrency.test.ts b/src/app/api/coach/about-me/__tests__/route-concurrency.test.ts index 77954ad35..022796ebb 100644 --- a/src/app/api/coach/about-me/__tests__/route-concurrency.test.ts +++ b/src/app/api/coach/about-me/__tests__/route-concurrency.test.ts @@ -50,6 +50,7 @@ const { profile, userUpdate, transaction } = vi.hoisted(() => { }; const userUpdate = vi.fn(async () => ({})); const tx = { + $queryRaw: vi.fn(async () => []), userHealthProfile: profile, user: { update: userUpdate }, }; diff --git a/src/app/api/coach/about-me/__tests__/route-module-gate.test.ts b/src/app/api/coach/about-me/__tests__/route-module-gate.test.ts index 402286c7d..95dcfe264 100644 --- a/src/app/api/coach/about-me/__tests__/route-module-gate.test.ts +++ b/src/app/api/coach/about-me/__tests__/route-module-gate.test.ts @@ -44,6 +44,7 @@ const { const profileUpsert = vi.fn(async () => ({ updatedAt: new Date(0) })); const userUpdate = vi.fn(async () => ({})); const tx = { + $queryRaw: vi.fn(async () => []), userHealthProfile: { findUnique: profileFindUnique, upsert: profileUpsert, @@ -246,6 +247,7 @@ describe("about-me module gate", () => { transaction.mockImplementationOnce( async (callback: (client: unknown) => unknown) => { const result = await callback({ + $queryRaw: vi.fn(async () => []), userHealthProfile: { findUnique: profileFindUnique, upsert: txProfileUpsert, diff --git a/src/app/api/coach/about-me/route.ts b/src/app/api/coach/about-me/route.ts index 5cc84ed6c..bacf26cad 100644 --- a/src/app/api/coach/about-me/route.ts +++ b/src/app/api/coach/about-me/route.ts @@ -42,6 +42,7 @@ import { annotate } from "@/lib/logging/context"; import { auditLog } from "@/lib/auth/audit"; import { invalidateUserInsights } from "@/lib/cache/invalidate"; import { prisma } from "@/lib/db"; +import { lockAiSections } from "@/lib/profile/life-circumstances"; import { takeBaseToken, invalidBaseTokenError } from "@/lib/optimistic-lock"; import { checkRateLimit, rateLimitHeaders } from "@/lib/rate-limit"; import { encryptToBytes } from "@/lib/ai/coach/bytes-codec"; @@ -246,6 +247,9 @@ export const PUT = apiHandler(async (req: Request) => { // A PUT here replaces the eight sections it lists and leaves the // life-circumstance switches (v1.43) exactly as stored. if (parsed.data.aiIncludedSections !== undefined) { + // Taken before the read, so a switch changed concurrently is read + // after it commits and carried over as it now stands. + await lockAiSections(tx, user.id); const stored = await tx.userHealthProfile.findUnique({ where: { userId: user.id }, select: { aiIncludedSections: true }, diff --git a/src/lib/profile/life-circumstances.ts b/src/lib/profile/life-circumstances.ts index bab7ee582..3ee481dd5 100644 --- a/src/lib/profile/life-circumstances.ts +++ b/src/lib/profile/life-circumstances.ts @@ -445,6 +445,25 @@ function lifeSwitches( ) as Record; } +/** + * Serialise every read-modify-write of a profile's AI switches. Two routes + * write the one array: this module's switch route and the about-me PUT, which + * replaces its eight sections and carries the life switches over as stored. + * Without a lock, a PUT that read the array before a concurrent switch-off + * committed wrote the old value back and turned the section on again. A + * transaction-scoped advisory lock covers the first write too, when there is + * no row yet to lock. + */ +export async function lockAiSections( + tx: Prisma.TransactionClient, + userId: string, +): Promise { + await tx.$queryRaw` + SELECT 1 AS locked + FROM pg_advisory_xact_lock(hashtextextended(${`ai-sections:${userId}`}, 0)) + `; +} + /** * Switch life-circumstance sections in or out of the AI inclusion list. Only * the named sections change; the other stored values, the five about-me @@ -459,6 +478,7 @@ export async function updateLifeAiSections( base?: Date, ): Promise { return prisma.$transaction(async (tx) => { + await lockAiSections(tx, userId); const row = await tx.userHealthProfile.findUnique({ where: { userId }, select: { aiIncludedSections: true, updatedAt: true }, diff --git a/tests/integration/life-circumstances.test.ts b/tests/integration/life-circumstances.test.ts index 9c1cd8d12..508c8a53c 100644 --- a/tests/integration/life-circumstances.test.ts +++ b/tests/integration/life-circumstances.test.ts @@ -578,6 +578,53 @@ describe("AI switches and confirmations", () => { ]); }); + it("never turns a section on again that a concurrent switch turned off", async () => { + const user = await makeUser({ + modulePreferencesJson: { mentalHealth: true, coach: true }, + }); + await signIn(user.id); + const prisma = getPrismaClient(); + await prisma.userHealthProfile.create({ + data: { userId: user.id, aiIncludedSections: ["ABOUT_ME", "HABITS"] }, + }); + const { lockAiSections } = await import("@/lib/profile/life-circumstances"); + const aboutMe = await import("@/app/api/coach/about-me/route"); + + // The switch-off holds its transaction open while the PUT starts. + let release!: () => void; + const held = new Promise((resolve) => (release = resolve)); + let locked!: () => void; + const isLocked = new Promise((resolve) => (locked = resolve)); + const switchOff = prisma.$transaction(async (tx) => { + await lockAiSections(tx, user.id); + await tx.userHealthProfile.update({ + where: { userId: user.id }, + data: { aiIncludedSections: ["ABOUT_ME"] }, + }); + locked(); + await held; + }); + await isLocked; + const put = (aboutMe.PUT as unknown as Handler)( + request("PUT", "/api/coach/about-me", { + aiIncludedSections: ["ABOUT_ME", "CONDITIONS"], + }), + ); + // Long enough for an unlocked PUT to have read the old array. + await new Promise((resolve) => setTimeout(resolve, 300)); + release(); + await switchOff; + await data(await put); + + const stored = await prisma.userHealthProfile.findUniqueOrThrow({ + where: { userId: user.id }, + }); + expect([...stored.aiIncludedSections].sort()).toEqual([ + "ABOUT_ME", + "CONDITIONS", + ]); + }); + it("answers a stale token with about_me_conflict", async () => { const user = await makeUser(); await signIn(user.id);