From 06251d72197c1e229fb673ad3d659ef32b3abbff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 24 Sep 2026 12:08:39 +0200 Subject: [PATCH 1/9] fix(ai): read AI switches, modules and consent afresh inside background jobs A background job opens one wide event for its whole run, and the per-request memo cached the operator switches, module availability, module preferences, provider presence and consent kinds on it. The nightly status passes loop over every candidate inside that one event, so a switch turned off mid-run did not stop the remaining users, and the wire re-check read the same cached answer. memoizePerRequest takes a freshInBackground option; every input that decides whether AI work may run sets it, so a background caller reads them on every call while a request still reads them once. --- src/lib/__tests__/request-cache.test.ts | 27 +++++ src/lib/ai/capabilities/load.ts | 19 ++-- src/lib/ai/provider.ts | 1 + src/lib/feature-flags/__tests__/index.test.ts | 8 +- src/lib/feature-flags/index.ts | 59 ++++++----- src/lib/logging/event-builder.ts | 5 + src/lib/modules/gate.ts | 48 ++++----- src/lib/modules/operator-availability.ts | 18 ++-- src/lib/request-cache.ts | 20 ++++ .../background-switch-freshness.test.ts | 98 +++++++++++++++++++ 10 files changed, 236 insertions(+), 67 deletions(-) create mode 100644 src/lib/__tests__/request-cache.test.ts create mode 100644 tests/integration/background-switch-freshness.test.ts diff --git a/src/lib/__tests__/request-cache.test.ts b/src/lib/__tests__/request-cache.test.ts new file mode 100644 index 000000000..0e2cd1d2a --- /dev/null +++ b/src/lib/__tests__/request-cache.test.ts @@ -0,0 +1,27 @@ +import { describe, expect, it, vi } from "vitest"; + +import { eventStorage } from "@/lib/logging/context"; +import { WideEventBuilder } from "@/lib/logging/event-builder"; +import { memoizePerRequest } from "@/lib/request-cache"; + +function twice(kind: "http" | "background", fresh: boolean) { + const factory = vi.fn(async () => Math.random()); + return eventStorage.run(new WideEventBuilder(kind), async () => { + const options = fresh ? { freshInBackground: true } : {}; + await memoizePerRequest("k", factory, options); + await memoizePerRequest("k", factory, options); + return factory.mock.calls.length; + }); +} + +describe("memoizePerRequest", () => { + it("reads once per request", async () => { + expect(await twice("http", false)).toBe(1); + expect(await twice("http", true)).toBe(1); + }); + + it("reads once per background event unless the input must stay fresh", async () => { + expect(await twice("background", false)).toBe(1); + expect(await twice("background", true)).toBe(2); + }); +}); diff --git a/src/lib/ai/capabilities/load.ts b/src/lib/ai/capabilities/load.ts index 7a940a76b..9e43dc842 100644 --- a/src/lib/ai/capabilities/load.ts +++ b/src/lib/ai/capabilities/load.ts @@ -56,13 +56,17 @@ export interface AiCapabilityScope { /** The kinds of the record's active consent receipts. */ function loadActiveConsentKinds(recordId: string): Promise> { - return memoizePerRequest(`ai-consent-kinds:${recordId}`, async () => { - const rows = await prisma.consentReceipt.findMany({ - where: { userId: recordId, revokedAt: null }, - select: { kind: true }, - }); - return new Set(rows.map((row) => row.kind)); - }); + return memoizePerRequest( + `ai-consent-kinds:${recordId}`, + async () => { + const rows = await prisma.consentReceipt.findMany({ + where: { userId: recordId, revokedAt: null }, + select: { kind: true }, + }); + return new Set(rows.map((row) => row.kind)); + }, + { freshInBackground: true }, + ); } function scopeKey(scope: AiCapabilityScope): string { @@ -114,6 +118,7 @@ export function loadAiCapabilityInputs( return null; } }, + { freshInBackground: true }, ); } diff --git a/src/lib/ai/provider.ts b/src/lib/ai/provider.ts index 6d788d6d2..7586504b2 100644 --- a/src/lib/ai/provider.ts +++ b/src/lib/ai/provider.ts @@ -985,6 +985,7 @@ export function probeProviderChain( managedBy: entries.length > 0 ? managedBy : null, }; }, + { freshInBackground: true }, ); } diff --git a/src/lib/feature-flags/__tests__/index.test.ts b/src/lib/feature-flags/__tests__/index.test.ts index 67d146391..226614216 100644 --- a/src/lib/feature-flags/__tests__/index.test.ts +++ b/src/lib/feature-flags/__tests__/index.test.ts @@ -16,7 +16,7 @@ vi.mock("@/lib/db", () => ({ // pre-v1.4.33 assertion semantics. let mockEvent: object | null = null; vi.mock("@/lib/logging/context", () => ({ - getEvent: () => mockEvent ?? { addWarning: vi.fn() }, + getEvent: () => mockEvent ?? { getKind: () => "http", addWarning: vi.fn() }, })); import { prisma } from "@/lib/db"; @@ -154,7 +154,7 @@ describe("per-request memoisation", () => { // Pin a stable stub object so every `getEvent()` inside this test // returns the same reference — the memo keys on identity, so a // shared reference == shared cache. - mockEvent = { addWarning: vi.fn() }; + mockEvent = { getKind: () => "http", addWarning: vi.fn() }; const a = await getAssistantFlags(); const b = await getAssistantFlags(); @@ -174,9 +174,9 @@ describe("per-request memoisation", () => { assistantDocumentAiEnabled: true, }); - mockEvent = { addWarning: vi.fn(), id: "req-1" }; + mockEvent = { getKind: () => "http", addWarning: vi.fn(), id: "req-1" }; await getAssistantFlags(); - mockEvent = { addWarning: vi.fn(), id: "req-2" }; + mockEvent = { getKind: () => "http", addWarning: vi.fn(), id: "req-2" }; await getAssistantFlags(); expect(FIND).toHaveBeenCalledTimes(2); diff --git a/src/lib/feature-flags/index.ts b/src/lib/feature-flags/index.ts index a89e91587..5d1b613bc 100644 --- a/src/lib/feature-flags/index.ts +++ b/src/lib/feature-flags/index.ts @@ -49,35 +49,40 @@ const ASSISTANT_FLAGS_OFF: AssistantFlagSet = Object.freeze({ * switch; failing open meant a database blip switched every AI egress on. * * Memoised per request, so a page that fires several gated reads at once reads - * the singleton row once. + * the singleton row once. Read afresh on every call inside a background job, + * whose one event spans a whole multi-user pass. */ export function loadAssistantSwitches(): Promise { - return memoizePerRequest("assistant-flags", async () => { - try { - const settings = await prisma.appSettings.findUnique({ - where: { id: "singleton" }, - select: { - assistantEnabled: true, - assistantCoachEnabled: true, - assistantBriefingEnabled: true, - assistantInsightStatusEnabled: true, - assistantDocumentAiEnabled: true, - }, - }); - return resolveAssistantFlags({ - enabled: settings?.assistantEnabled ?? true, - coach: settings?.assistantCoachEnabled ?? true, - briefing: settings?.assistantBriefingEnabled ?? true, - insightStatus: settings?.assistantInsightStatusEnabled ?? true, - documentAi: settings?.assistantDocumentAiEnabled ?? true, - }); - } catch { - getEvent()?.addWarning( - "Failed to load assistant switches; every AI capability is off for this request", - ); - return null; - } - }); + return memoizePerRequest( + "assistant-flags", + async () => { + try { + const settings = await prisma.appSettings.findUnique({ + where: { id: "singleton" }, + select: { + assistantEnabled: true, + assistantCoachEnabled: true, + assistantBriefingEnabled: true, + assistantInsightStatusEnabled: true, + assistantDocumentAiEnabled: true, + }, + }); + return resolveAssistantFlags({ + enabled: settings?.assistantEnabled ?? true, + coach: settings?.assistantCoachEnabled ?? true, + briefing: settings?.assistantBriefingEnabled ?? true, + insightStatus: settings?.assistantInsightStatusEnabled ?? true, + documentAi: settings?.assistantDocumentAiEnabled ?? true, + }); + } catch { + getEvent()?.addWarning( + "Failed to load assistant switches; every AI capability is off for this request", + ); + return null; + } + }, + { freshInBackground: true }, + ); } /** diff --git a/src/lib/logging/event-builder.ts b/src/lib/logging/event-builder.ts index 9a89a6dd4..1bcc0596c 100644 --- a/src/lib/logging/event-builder.ts +++ b/src/lib/logging/event-builder.ts @@ -288,6 +288,11 @@ export class WideEventBuilder { return structuredClone(this.event) as WideEvent; } + /** `http` for a request, `background` for a job or a hand-rolled route. */ + getKind(): EventKind { + return this.event.kind ?? "http"; + } + getDurationMs(): number { return Math.round(performance.now() - this.startTime); } diff --git a/src/lib/modules/gate.ts b/src/lib/modules/gate.ts index 61bb9bf04..a4da80212 100644 --- a/src/lib/modules/gate.ts +++ b/src/lib/modules/gate.ts @@ -169,28 +169,32 @@ export function resolveModuleEnabled( /** Internal: load the gate inputs once per request. */ function loadInputs(userId: string): Promise { - return memoizePerRequest(`module-gate-inputs:${userId}`, async () => { - const [user, cycleProfile] = await Promise.all([ - prisma.user.findUnique({ - where: { id: userId }, - select: { - gender: true, - disableCoach: true, - modulePreferencesJson: true, - }, - }), - prisma.cycleProfile.findUnique({ - where: { userId }, - select: { cycleTrackingEnabled: true }, - }), - ]); - return { - gender: user?.gender ?? null, - disableCoach: user?.disableCoach ?? false, - modulePreferences: normalisePrefs(user?.modulePreferencesJson), - cycleTrackingEnabled: cycleProfile?.cycleTrackingEnabled ?? null, - }; - }); + return memoizePerRequest( + `module-gate-inputs:${userId}`, + async () => { + const [user, cycleProfile] = await Promise.all([ + prisma.user.findUnique({ + where: { id: userId }, + select: { + gender: true, + disableCoach: true, + modulePreferencesJson: true, + }, + }), + prisma.cycleProfile.findUnique({ + where: { userId }, + select: { cycleTrackingEnabled: true }, + }), + ]); + return { + gender: user?.gender ?? null, + disableCoach: user?.disableCoach ?? false, + modulePreferences: normalisePrefs(user?.modulePreferencesJson), + cycleTrackingEnabled: cycleProfile?.cycleTrackingEnabled ?? null, + }; + }, + { freshInBackground: true }, + ); } /** diff --git a/src/lib/modules/operator-availability.ts b/src/lib/modules/operator-availability.ts index 022f6e36c..90007c04e 100644 --- a/src/lib/modules/operator-availability.ts +++ b/src/lib/modules/operator-availability.ts @@ -104,13 +104,17 @@ async function loadAvailabilityBlob(): Promise { * all-available so the modules stay visible on first boot. */ export async function getOperatorModuleAvailability(): Promise { - return memoizePerRequest("operator-module-availability", async () => { - const [blob, switches] = await Promise.all([ - loadAvailabilityBlob(), - getAssistantFlags(), - ]); - return resolveOperatorAvailability(blob, switches.coach); - }); + return memoizePerRequest( + "operator-module-availability", + async () => { + const [blob, switches] = await Promise.all([ + loadAvailabilityBlob(), + getAssistantFlags(), + ]); + return resolveOperatorAvailability(blob, switches.coach); + }, + { freshInBackground: true }, + ); } /** A module key whose operator layer the blob owns (every key but the Coach). */ diff --git a/src/lib/request-cache.ts b/src/lib/request-cache.ts index ec40f6a9a..eff4191d8 100644 --- a/src/lib/request-cache.ts +++ b/src/lib/request-cache.ts @@ -35,6 +35,22 @@ function getCache(builder: WideEventBuilder): Map { return cache; } +export interface MemoizeOptions { + /** + * Read afresh on every call inside a background event. + * + * A request is short and acts for one record, so one read of the operator's + * switches serves all of it. A background job opens one wide event for its + * whole run, and a nightly pass loops over every user inside it: a cached + * switch, module map, provider presence or consent set would be the answer + * from the start of the run, served to every later user and to the wire + * re-check, so turning the Assistant off at 02:05 would not stop the pass + * that began at 02:00. Every input that decides whether AI work may run + * sets this. + */ + freshInBackground?: boolean; +} + /** * Memoise `factory()` for the lifetime of the current request. Returns * the factory's result directly when no event context is active so @@ -49,11 +65,15 @@ function getCache(builder: WideEventBuilder): Map { export function memoizePerRequest( key: string, factory: () => Promise, + options: MemoizeOptions = {}, ): Promise { const event = getEvent(); if (!event) { return factory(); } + if (options.freshInBackground && event.getKind() === "background") { + return factory(); + } const cache = getCache(event); const existing = cache.get(key); if (existing !== undefined) { diff --git a/tests/integration/background-switch-freshness.test.ts b/tests/integration/background-switch-freshness.test.ts new file mode 100644 index 000000000..98140c248 --- /dev/null +++ b/tests/integration/background-switch-freshness.test.ts @@ -0,0 +1,98 @@ +/** + * A switch turned off while a nightly job is running stops the rest of it. + * + * Reads inside one request are memoised on the request's wide event, so a + * page that resolves the AI capabilities five times reads the settings row + * once. A background job opens one wide event for its whole run, and the + * status crons loop over every candidate inside it: memoising there meant the + * switches, the modules and the consent kinds were read once at the start of + * the run and served to every later user, and the wire re-check read the same + * cached answer. Turning the Assistant off at 02:05 did not stop the 02:00 + * pass. + * + * Driven here over the real loop, the real capability loader and Postgres: + * the first user's generation turns the switch off, and the remaining users + * must not reach a generator, while the wire re-check inside the first user's + * own run must see the change too. + */ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { getPrismaClient, truncateAllTables } from "./setup"; + +const candidates = vi.hoisted(() => ({ ids: [] as string[] })); + +vi.mock("@/lib/jobs/status-cron-candidates", () => ({ + findStatusCronCandidates: vi.fn(async () => + candidates.ids.map((id) => ({ id, locale: "en" })), + ), +})); + +import { runStatusCronGenerate } from "@/lib/jobs/reminder/insights-handlers"; +import { aiEgressRefusal } from "@/lib/ai/capabilities/egress"; + +let counter = 0; + +async function seedUserWithProvider(): Promise { + const n = counter++; + const user = await getPrismaClient().user.create({ + data: { + username: `fresh-switch-${n}`, + email: `fresh-switch-${n}@example.test`, + role: "USER", + timezone: "UTC", + locale: "en", + aiProvider: "ANTHROPIC", + aiAnthropicKeyEncrypted: "v1:presence-only", + consentReceipts: { + create: { kind: "ai_full", artefact: "test", signedAt: new Date() }, + }, + }, + }); + return user.id; +} + +async function setInsightStatusSwitch(enabled: boolean): Promise { + await getPrismaClient().appSettings.upsert({ + where: { id: "singleton" }, + create: { id: "singleton", assistantInsightStatusEnabled: enabled }, + update: { assistantInsightStatusEnabled: enabled }, + }); +} + +beforeEach(async () => { + await truncateAllTables(getPrismaClient()); + await setInsightStatusSwitch(true); +}); + +describe("a switch turned off mid-run", () => { + it("stops the remaining users of the same background pass", async () => { + candidates.ids = [ + await seedUserWithProvider(), + await seedUserWithProvider(), + await seedUserWithProvider(), + ]; + const reached: string[] = []; + const wire: Array = []; + + const generate = vi.fn(async (userId: string) => { + reached.push(userId); + // The wire re-check before the switch moves: open. + wire.push( + (await aiEgressRefusal("statusText", userId, ["anthropic"]))?.reason ?? + null, + ); + await setInsightStatusSwitch(false); + // The same user's wire re-check after it moved: closed. + wire.push( + (await aiEgressRefusal("statusText", userId, ["anthropic"]))?.reason ?? + null, + ); + }); + + await runStatusCronGenerate("job.test.fresh_switch", generate); + + expect(reached).toEqual([candidates.ids[0]]); + expect(wire[0]).toBeNull(); + expect(wire[1]).not.toBeNull(); + }); +}); From bd58571d460273a109270c4423412b0e5787b3ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 24 Sep 2026 12:11:21 +0200 Subject: [PATCH 2/9] fix(coach): hold reminders while the Coach is unavailable The daily sweep and the measurement-ingest hook surfaced due reminders into a Coach conversation whatever the record's coach capability said, and the nag cap then dismissed them after three ticks although no badge ever showed. The app-open path already waited on the capability. surfaceCoachReminders now holds every row while the coach capability is unavailable (the same answer that keeps the badge dark), and the nag cap neither charges nor dismisses a held user's surfaced rows. They surface on the first tick after the Coach is back. The integration fixture gives its users a provider and consent, and a held case covers the sweep, the nag cap and the ingest hook. --- .../coach/__tests__/context-reminders.test.ts | 11 ++ src/lib/ai/coach/reminder-surface.ts | 17 ++- .../__tests__/coach-reminder-sweep.test.ts | 11 ++ src/lib/jobs/coach-reminder-sweep.ts | 25 +++- .../coach-reminder-surfacing.test.ts | 113 +++++++++++++++++- 5 files changed, 173 insertions(+), 4 deletions(-) diff --git a/src/lib/ai/coach/__tests__/context-reminders.test.ts b/src/lib/ai/coach/__tests__/context-reminders.test.ts index dfccf9437..407f7857e 100644 --- a/src/lib/ai/coach/__tests__/context-reminders.test.ts +++ b/src/lib/ai/coach/__tests__/context-reminders.test.ts @@ -9,6 +9,17 @@ */ import { describe, it, expect, vi } from "vitest"; +// Reminders are held while the Coach is unavailable; these cases are about +// surfacing, so the Coach answers. The hold is pinned against Postgres in +// `tests/integration/coach-reminder-surfacing.test.ts`. +vi.mock("@/lib/ai/capabilities/gate", () => ({ + aiCapabilityToServe: vi.fn(async () => ({ + available: true, + reason: null, + onDeviceAllowed: true, + })), +})); + vi.mock("@/lib/ai/coach/bytes-codec", () => ({ decryptFromBytes: (buf: Uint8Array) => Buffer.from(buf).toString("utf8"), encryptToBytes: (s: string) => new Uint8Array(Buffer.from(`enc:${s}`)), diff --git a/src/lib/ai/coach/reminder-surface.ts b/src/lib/ai/coach/reminder-surface.ts index f10056678..9b7ed4719 100644 --- a/src/lib/ai/coach/reminder-surface.ts +++ b/src/lib/ai/coach/reminder-surface.ts @@ -15,6 +15,7 @@ */ import type { PrismaClient } from "@/generated/prisma/client"; +import { aiCapabilityToServe } from "@/lib/ai/capabilities/gate"; import { decryptFromBytes, encryptToBytes } from "@/lib/ai/coach/bytes-codec"; import { getServerTranslator } from "@/lib/i18n/server-translator"; import type { Locale } from "@/lib/i18n/config"; @@ -56,6 +57,18 @@ export function buildReminderSurfaceMessage( return { title, body }; } +/** + * Whether a user's Coach reminders are held: the record's `coach` capability + * is unavailable (operator switch, module off, opt-out, no provider, no + * consent), which is the same answer that keeps the badge dark. A reminder + * surfaced into a Coach nobody can open is a message that never shows, and + * the nag cap would then dismiss it unseen; held, it waits and surfaces once + * the Coach is back. + */ +export async function coachRemindersHeld(userId: string): Promise { + return !(await aiCapabilityToServe(userId, "coach")).available; +} + export interface SurfaceOutcome { /** Reminders that reached the conversation in this call. */ surfaced: number; @@ -70,7 +83,8 @@ export interface SurfaceOutcome { * cost the user the reminders beside it. At most `MAX_NOTES_PER_MESSAGE` * notes ride one message; the remainder keep their status and surface on * the next trigger. A write failure leaves every row untouched: no badge - * claims a message that was never written. + * claims a message that was never written. While the Coach is unavailable + * nothing is written and every row keeps its status. */ export async function surfaceCoachReminders( prisma: ReminderSurfacePrisma, @@ -80,6 +94,7 @@ export async function surfaceCoachReminders( ): Promise { const outcome: SurfaceOutcome = { surfaced: 0, errored: 0 }; if (rows.length === 0) return outcome; + if (await coachRemindersHeld(userId)) return outcome; const usable: { id: string; note: string }[] = []; for (const row of rows) { diff --git a/src/lib/jobs/__tests__/coach-reminder-sweep.test.ts b/src/lib/jobs/__tests__/coach-reminder-sweep.test.ts index 015b10d74..f71009da6 100644 --- a/src/lib/jobs/__tests__/coach-reminder-sweep.test.ts +++ b/src/lib/jobs/__tests__/coach-reminder-sweep.test.ts @@ -19,6 +19,17 @@ vi.mock("@/lib/ai/coach/bytes-codec", () => ({ encryptToBytes: (s: string) => new Uint8Array(Buffer.from(`enc:${s}`)), })); +// Reminders are held while the Coach is unavailable; these cases are about +// surfacing, so the Coach answers. The hold is pinned against Postgres in +// `tests/integration/coach-reminder-surfacing.test.ts`. +vi.mock("@/lib/ai/capabilities/gate", () => ({ + aiCapabilityToServe: vi.fn(async () => ({ + available: true, + reason: null, + onDeviceAllowed: true, + })), +})); + // The context-cue backstop delegates to the shared evaluator; its own // behaviour is pinned in `ai/coach/__tests__/context-reminders.test.ts`. const { contextEvaluateMock } = vi.hoisted(() => ({ diff --git a/src/lib/jobs/coach-reminder-sweep.ts b/src/lib/jobs/coach-reminder-sweep.ts index a15ff20a5..850f053db 100644 --- a/src/lib/jobs/coach-reminder-sweep.ts +++ b/src/lib/jobs/coach-reminder-sweep.ts @@ -34,6 +34,10 @@ * that has been stuck on an empty conversation resolves into the message it * always implied rather than staying stuck. * + * While the record's `coach` capability is unavailable its reminders are held: + * none is surfaced, charged or auto-dismissed, and they resume once the Coach + * is back (`coachRemindersHeld`, the same answer that keeps the badge dark). + * * The sweep still never calls a provider and never pushes. The message body is * a deterministic localized template around the user's OWN note text — the * words they asked to be reminded of, never anything fabricated. @@ -50,7 +54,10 @@ import type { PrismaClient } from "@/generated/prisma/client"; import { decryptFromBytes, encryptToBytes } from "@/lib/ai/coach/bytes-codec"; import { evaluateCoachContextReminders } from "@/lib/ai/coach/context-reminders"; -import { surfaceCoachReminders } from "@/lib/ai/coach/reminder-surface"; +import { + coachRemindersHeld, + surfaceCoachReminders, +} from "@/lib/ai/coach/reminder-surface"; export const COACH_REMINDER_SWEEP_QUEUE = "coach-reminder-sweep"; // Daily at 05:20 Europe/Berlin — just after the 05:15 nudge tick so both @@ -258,11 +265,26 @@ export async function runCoachReminderSweep( // it has already sat there for a day as one more surfacing, then // auto-dismiss the rows that reached the cap. Two bulk statements, no // message: dismissal is silence, not another nag. + // A day the Coach was unavailable is not a day the reminder sat ignored: + // the badge was dark and there was nothing to open. Those users' rows are + // neither charged nor dismissed. + const surfacedOwners = await prisma.coachReminder.findMany({ + where: { deletedAt: null, status: "surfaced" }, + distinct: ["userId"], + select: { userId: true }, + }); + const heldOwners: string[] = []; + for (const { userId } of surfacedOwners) { + if (await coachRemindersHeld(userId)) heldOwners.push(userId); + } + const notHeld = + heldOwners.length > 0 ? { userId: { notIn: heldOwners } } : {}; await prisma.coachReminder.updateMany({ where: { deletedAt: null, status: "surfaced", lastSurfacedAt: { lte: new Date(now.getTime() - NAG_STALE_MS) }, + ...notHeld, }, data: { surfaceCount: { increment: 1 } }, }); @@ -271,6 +293,7 @@ export async function runCoachReminderSweep( deletedAt: null, status: "surfaced", surfaceCount: { gte: REMINDER_NAG_CAP }, + ...notHeld, }, data: { status: "dismissed" }, }); diff --git a/tests/integration/coach-reminder-surfacing.test.ts b/tests/integration/coach-reminder-surfacing.test.ts index cbd927b0a..06293671e 100644 --- a/tests/integration/coach-reminder-surfacing.test.ts +++ b/tests/integration/coach-reminder-surfacing.test.ts @@ -23,6 +23,7 @@ import { beforeEach, describe, expect, it } from "vitest"; import { getPrismaClient, truncateAllTables } from "./setup"; import { runCoachReminderSweep } from "@/lib/jobs/coach-reminder-sweep"; import { readCoachNudgeStatus } from "@/lib/ai/coach/nudge-status"; +import { evaluateCoachContextReminders } from "@/lib/ai/coach/context-reminders"; import { decryptFromBytes, encryptToBytes } from "@/lib/ai/coach/bytes-codec"; const NOW = new Date("2026-08-07T05:20:00.000Z"); @@ -32,17 +33,34 @@ beforeEach(async () => { await truncateAllTables(getPrismaClient()); }); -async function seedUser(username: string, locale = "en") { +/** + * A user whose Coach can answer: a provider (presence only) and a Coach + * consent receipt. Reminders are held while the Coach is unavailable, so a + * user without them would prove only the hold. + */ +async function seedUser( + username: string, + { coach = true }: { coach?: boolean } = {}, +) { return getPrismaClient().user.create({ data: { username, email: `${username}@example.test`, timezone: "UTC", - locale, + locale: "en", + ...(coach ? COACH_READY : {}), }, }); } +const COACH_READY = { + aiProvider: "ANTHROPIC", + aiAnthropicKeyEncrypted: "v1:presence-only", + consentReceipts: { + create: { kind: "ai_full", artefact: "test", signedAt: new Date() }, + }, +} as const; + describe("a due Coach reminder reaches the conversation the badge points at", () => { it("writes the note as an assistant message, and that message is what makes the badge unread", async () => { const prisma = getPrismaClient(); @@ -236,4 +254,95 @@ describe("a due Coach reminder reaches the conversation the badge points at", () expect(untouched.status).toBe("active"); expect((await readCoachNudgeStatus(user.id)).unread).toBe(false); }); + + it("holds every reminder while the Coach is unavailable, and resumes when it is back", async () => { + const prisma = getPrismaClient(); + const user = await seedUser("reminder-held", { coach: false }); + const due = await prisma.coachReminder.create({ + data: { + userId: user.id, + noteEncrypted: encryptToBytes("held until the Coach is back"), + triggerKind: "date", + dueAt: YESTERDAY, + status: "active", + source: "user", + }, + }); + // One more ignored day would reach the nag cap. + const nagging = await prisma.coachReminder.create({ + data: { + userId: user.id, + noteEncrypted: encryptToBytes("surfaced before the Coach went away"), + triggerKind: "date", + dueAt: new Date("2026-08-01T09:00:00.000Z"), + status: "surfaced", + lastSurfacedAt: new Date("2026-08-04T05:20:00.000Z"), + surfaceCount: 2, + source: "user", + }, + }); + const context = await prisma.coachReminder.create({ + data: { + userId: user.id, + noteEncrypted: encryptToBytes("ask about the weigh-in"), + triggerKind: "context", + contextCue: "NEXT_WEIGHT_LOGGED", + status: "active", + source: "user", + }, + }); + await prisma.measurement.create({ + data: { + userId: user.id, + type: "WEIGHT", + value: 70, + unit: "kg", + measuredAt: YESTERDAY, + }, + }); + + // Three daily ticks: none surfaces, none is charged or dismissed. + for (let day = 0; day < 3; day++) { + const summary = await runCoachReminderSweep( + prisma, + new Date(NOW.getTime() + day * 86_400_000), + ); + expect(summary.remindersDue).toBe(0); + expect(summary.contextSurfaced).toBe(0); + expect(summary.nagDismissed).toBe(0); + } + // The ingest hook holds too. + expect( + (await evaluateCoachContextReminders(prisma, user.id, "measurement", NOW)) + .surfaced, + ).toBe(0); + + expect( + await prisma.coachMessage.count({ + where: { conversation: { userId: user.id } }, + }), + ).toBe(0); + const rows = await prisma.coachReminder.findMany({ + where: { userId: user.id }, + orderBy: { createdAt: "asc" }, + }); + const byId = new Map(rows.map((r) => [r.id, r])); + expect(byId.get(due.id)?.status).toBe("active"); + expect(byId.get(nagging.id)?.status).toBe("surfaced"); + expect(byId.get(nagging.id)?.surfaceCount).toBe(2); + expect(byId.get(context.id)?.status).toBe("active"); + + // The Coach comes back: the held reminders surface on the next tick. + await prisma.user.update({ + where: { id: user.id }, + data: { + aiProvider: COACH_READY.aiProvider, + aiAnthropicKeyEncrypted: COACH_READY.aiAnthropicKeyEncrypted, + consentReceipts: { create: COACH_READY.consentReceipts.create }, + }, + }); + const resumed = await runCoachReminderSweep(prisma, NOW); + expect(resumed.remindersDue).toBe(1); + expect(resumed.contextSurfaced).toBe(1); + }); }); From 2c1e0f5bca63ad9cb95b82152561a44065173571 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 24 Sep 2026 12:15:56 +0200 Subject: [PATCH 3/9] fix(ai): drop model text whose consent was withdrawn while the call was in flight A consent withdrawal purges the stored status notes, briefing, reaction lines and workout paragraphs in its own transaction, but a provider call that was already in flight still persisted its reply afterwards, bringing the text back under a consent that no longer existed. The consent rule now runs once more after the reply arrives, against the provider that actually answered and with a fresh receipt read: in runStatusCompletion (status notes, workout paragraphs, narratives, derived assessments), before the briefing commit, and before the reaction-line commit. A refused reply is dropped; the spent tokens stay on the ledger. --- .../comprehensive-privacy-flip.test.ts | 44 ++++++++++++++++++- .../__tests__/status-provider-budget.test.ts | 25 +++++++++++ src/lib/insights/comprehensive-generate.ts | 23 +++++++++- src/lib/insights/status-provider.ts | 22 ++++++++++ .../reaction-line-degradation.test.ts | 34 ++++++++++++++ src/lib/jobs/reaction-line.ts | 11 +++++ 6 files changed, 157 insertions(+), 2 deletions(-) diff --git a/src/lib/insights/__tests__/comprehensive-privacy-flip.test.ts b/src/lib/insights/__tests__/comprehensive-privacy-flip.test.ts index 83d13f02f..0af17d76b 100644 --- a/src/lib/insights/__tests__/comprehensive-privacy-flip.test.ts +++ b/src/lib/insights/__tests__/comprehensive-privacy-flip.test.ts @@ -41,8 +41,9 @@ vi.mock("@/lib/db", () => ({ // path has its own test in comprehensive-generate-force.test.ts. // The wire re-check (`aiEgressRefusal`) passes in these fixtures; its refusal // arm has its own test in comprehensive-budget-refusal.test.ts. +const aiEgressRefusal = vi.fn(); vi.mock("@/lib/ai/capabilities/egress", () => ({ - aiEgressRefusal: vi.fn(async () => null), + aiEgressRefusal: (...a: unknown[]) => aiEgressRefusal(...a), })); vi.mock("@/lib/ai/capabilities/gate", () => ({ aiCapabilityForRecord: vi.fn(async () => ({ @@ -139,6 +140,47 @@ beforeEach(() => { extractFeatures.mockResolvedValue(FEATURES); userUpdate.mockResolvedValue({}); getSelfContextTextForUser.mockResolvedValue(null); + aiEgressRefusal.mockResolvedValue(null); +}); + +describe("generateComprehensiveInsight — consent withdrawn in flight", () => { + it("drops the reply instead of writing it back over the purge", async () => { + findUnique.mockResolvedValue({ + insightsPrivacyMode: "aggregated", + insightsCachedAt: new Date(Date.now() - 26 * 60 * 60 * 1000), + insightsCachedText: JSON.stringify({ dailyBriefing: { p: "old" } }), + insightsExcludeMetrics: [], + insightsSnapshotHash: "0".repeat(64), + }); + runRawCompletionWithFallback.mockResolvedValue({ + result: { + content: JSON.stringify({ dailyBriefing: { p: "new" } }), + tokensUsed: 10, + providerType: "openai", + model: "m", + }, + workingProvider: { providerType: "openai" }, + fallbackHops: [], + }); + userUpdateMany.mockResolvedValue({ count: 1 }); + const { AiUnavailableError } = + await import("@/lib/ai/capabilities/refusal"); + // Open at the wire; withdrawn by the time the reply is back. + aiEgressRefusal + .mockResolvedValueOnce(null) + .mockResolvedValueOnce( + new AiUnavailableError("briefing", "consent_required"), + ); + + const outcome = await generateComprehensiveInsight("u1", { locale: "de" }); + + expect(outcome).toEqual({ status: "skipped", reason: "no-consent" }); + expect(aiEgressRefusal).toHaveBeenLastCalledWith("briefing", "u1", [ + "openai", + ]); + expect(userUpdateMany).not.toHaveBeenCalled(); + expect(invalidateUserInsights).not.toHaveBeenCalled(); + }); }); describe("generateComprehensiveInsight — scope/version commit guard", () => { diff --git a/src/lib/insights/__tests__/status-provider-budget.test.ts b/src/lib/insights/__tests__/status-provider-budget.test.ts index c50af944a..bc6bdedb7 100644 --- a/src/lib/insights/__tests__/status-provider-budget.test.ts +++ b/src/lib/insights/__tests__/status-provider-budget.test.ts @@ -188,6 +188,31 @@ describe("runStatusCompletion — capability at the wire", () => { ); }); + it("drops a reply whose consent was withdrawn while the call was in flight", async () => { + mockProviderReply(40); + // Open when the request leaves; withdrawn by the time the reply is back. + aiEgressRefusal + .mockResolvedValueOnce(null) + .mockResolvedValueOnce( + new AiUnavailableError("statusText", "consent_required"), + ); + + const outcome = await runStatusCompletion(completionArgs() as never); + + expect(outcome).toEqual({ kind: "none", reason: "consent_required" }); + // Re-checked against the provider that actually answered. + expect(aiEgressRefusal).toHaveBeenLastCalledWith("statusText", "u1", [ + "admin-openai", + ]); + // The tokens were spent upstream and stay on the ledger. + expect(ledgerTotal).toBe(40); + expect(annotate).toHaveBeenCalledWith( + expect.objectContaining({ + meta: expect.objectContaining({ at: "reply" }), + }), + ); + }); + it("returns none with reason no_provider when the chain is empty", async () => { resolveProviderChain.mockResolvedValue([]); const outcome = await runStatusCompletion(completionArgs() as never); diff --git a/src/lib/insights/comprehensive-generate.ts b/src/lib/insights/comprehensive-generate.ts index cc29fcfa8..8d4d2ae40 100644 --- a/src/lib/insights/comprehensive-generate.ts +++ b/src/lib/insights/comprehensive-generate.ts @@ -508,7 +508,11 @@ function failBeforeProvider( } type InsightCommitResult = - "committed" | "scope-changed" | "profile-scope-changed"; + | "committed" + | "scope-changed" + | "profile-scope-changed" + | "no-consent" + | "unavailable"; /** * The exact User columns (besides `insightsPrivacyMode`, guarded @@ -530,6 +534,12 @@ type PromptScopeAtRead = { * was built from — without keying on the broad `updatedAt` timestamp, which * a provider credential refresh (e.g. `resolveProviderChain()`'s token * maintenance) also advances and would otherwise false-positive here. + * + * The consent rule runs once more too, against the provider that answered + * (`servedBy`, `null` for a timestamp-only stamp that writes no text), with a + * fresh receipt read: a withdrawal that landed while the + * call was in flight already purged the stored briefing, and this write would + * bring it back. */ async function commitInsightUnderScope( userId: string, @@ -537,8 +547,15 @@ async function commitInsightUnderScope( selfContextAtRead: string | null, promptScopeAtRead: PromptScopeAtRead, locale: SupportedLocale, + servedBy: string | null, data: Prisma.UserUpdateInput, ): Promise { + if (servedBy !== null) { + const late = await aiEgressRefusal("briefing", userId, [servedBy]); + if (late) { + return late.reason === "consent_required" ? "no-consent" : "unavailable"; + } + } const currentSelfContext = await getSelfContextTextForUser(userId, locale); if (currentSelfContext !== selfContextAtRead) { return "profile-scope-changed"; @@ -930,6 +947,7 @@ export async function generateComprehensiveInsight( aboutMe, promptScopeAtRead, locale, + rerolled.providerType, { insightsCachedAt: new Date(), insightsCachedText: rerolled.text, @@ -956,6 +974,7 @@ export async function generateComprehensiveInsight( aboutMe, promptScopeAtRead, locale, + null, { insightsCachedAt: new Date(), // The fingerprint covers the generation locale, so a matching hash @@ -979,6 +998,7 @@ export async function generateComprehensiveInsight( aboutMe, promptScopeAtRead, locale, + null, { insightsCachedAt: new Date(), insightsCachedLocale: locale, @@ -1322,6 +1342,7 @@ export async function generateComprehensiveInsight( aboutMe, promptScopeAtRead, locale, + workingProviderType, { insightsCachedAt: new Date(), insightsCachedText: JSON.stringify(insights), diff --git a/src/lib/insights/status-provider.ts b/src/lib/insights/status-provider.ts index bb0342bca..a5b0ff31d 100644 --- a/src/lib/insights/status-provider.ts +++ b/src/lib/insights/status-provider.ts @@ -321,6 +321,28 @@ export async function runStatusCompletion( return { kind: "error" }; } + // Consent withdrawn while the call was in flight. The withdrawal purged the + // stored text in its own transaction; a reply persisted after it would + // bring that text back under a consent that no longer exists. So the rule + // runs once more, against the provider that actually answered, with a fresh + // receipt read, and a refused reply is dropped as if it had never arrived. + // The tokens stay on the ledger: they were spent upstream. + const late = await aiEgressRefusal(args.capability, userId, [ + workingProvider.providerType, + ]); + if (late) { + annotate({ + action: { name: "insights.status.capability_unavailable" }, + meta: { + cacheAction, + capability: args.capability, + reason: late.reason, + at: "reply", + }, + }); + return { kind: "none", reason: late.reason }; + } + return { kind: "ok", content, diff --git a/src/lib/jobs/__tests__/reaction-line-degradation.test.ts b/src/lib/jobs/__tests__/reaction-line-degradation.test.ts index aed073f31..f9ba8ad97 100644 --- a/src/lib/jobs/__tests__/reaction-line-degradation.test.ts +++ b/src/lib/jobs/__tests__/reaction-line-degradation.test.ts @@ -709,6 +709,40 @@ describe("reaction line — degradation", () => { expectSurfaceStillWorks(); }); + it("consent withdrawn while the call was in flight: writes no line", async () => { + resolveProviderChain.mockResolvedValue([ + { + providerType: "openai", + instance: { + generateCompletion: vi.fn().mockResolvedValue({ + content: "A solid night, deeper than your recent stretch.", + tokensUsed: 1_100, + cachedInputTokens: 100, + }), + }, + }, + ]); + aiEgressRefusal + .mockResolvedValueOnce(null) + .mockResolvedValueOnce( + new AiUnavailableError("reactionLines", "consent_required"), + ); + + const outcome = await runReactionLine(JOB); + + expect(outcome).toEqual({ status: "skipped", reason: "consent_required" }); + expect(aiEgressRefusal).toHaveBeenLastCalledWith("reactionLines", "u1", [ + "openai", + ]); + expect( + updateMany.mock.calls.some( + ([arg]) => + (arg as { data?: { lineEncrypted?: unknown } }).data + ?.lineEncrypted instanceof Uint8Array, + ), + ).toBe(false); + }); + it("a committed line is never regenerated — the unique row is the throttle", async () => { findUnique.mockResolvedValue({ id: "r1", generatedAt: new Date() }); resolveProviderChain.mockResolvedValue([ diff --git a/src/lib/jobs/reaction-line.ts b/src/lib/jobs/reaction-line.ts index a036f0025..5d3084acf 100644 --- a/src/lib/jobs/reaction-line.ts +++ b/src/lib/jobs/reaction-line.ts @@ -710,6 +710,17 @@ export async function runReactionLine( return { status: "skipped", reason: "ungrounded_output" }; } + // Consent withdrawn while the call was in flight: the withdrawal cleared + // stored lines in its own transaction, and this commit would write one + // back. The rule once more, for the provider that answered, fresh. + const late = await aiEgressRefusal("reactionLines", job.userId, [ + chain[0].providerType, + ]); + if (late) { + await finishTerminalAttempt().catch(() => {}); + return { status: "skipped", reason: late.reason }; + } + const committed = await prisma.arrivalReaction.updateMany({ where: { id: row.id, From 091e4b84cb679e70926dd44a78ea3c3995a84a09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 24 Sep 2026 12:18:00 +0200 Subject: [PATCH 4/9] fix(backup): fold a pre-0343 Coach availability key on instance restore Migration 0343 moved the operator's Coach answer from module availability into the Coach switch and folded every live row. Restoring the instance settings from an older file wrote the dead coach key back into module availability and left the Coach switch on, so a Coach the operator had off came back. The restore now applies the same fold: coach false in the file's module availability switches the Coach off, and the key is dropped either way. --- .../api/admin/backups/[id]/restore/route.ts | 11 +++- .../legacy-coach-availability.test.ts | 34 ++++++++++++ src/lib/modules/operator-availability.ts | 28 ++++++++++ .../integration/admin-backups-restore.test.ts | 53 +++++++++++++++++++ 4 files changed, 124 insertions(+), 2 deletions(-) create mode 100644 src/lib/modules/__tests__/legacy-coach-availability.test.ts diff --git a/src/app/api/admin/backups/[id]/restore/route.ts b/src/app/api/admin/backups/[id]/restore/route.ts index cbcf966e9..7e45e49f9 100644 --- a/src/app/api/admin/backups/[id]/restore/route.ts +++ b/src/app/api/admin/backups/[id]/restore/route.ts @@ -71,6 +71,7 @@ import { restoreAwardsData } from "@/lib/export/awards-backup"; import { restoreEnvironmentData } from "@/lib/export/environment-backup"; import { restoreEcgData } from "@/lib/export/ecg-backup"; import { invalidateUserData } from "@/lib/cache/invalidate"; +import { foldLegacyCoachAvailability } from "@/lib/modules/operator-availability"; export const dynamic = "force-dynamic"; @@ -483,6 +484,12 @@ const handler = apiHandler( // the account restore assumes. if (payload.appSettings && restoreInstanceSettings) { const settings = payload.appSettings; + // A file from before 0343 carries the Coach's operator answer in + // module availability; carry it into the Coach switch. + const coachFold = foldLegacyCoachAvailability( + settings.moduleAvailabilityJson, + settings.assistantCoachEnabled, + ); const settingsData = { registrationEnabled: settings.registrationEnabled, mfaRequired: settings.mfaRequired, @@ -523,13 +530,13 @@ const handler = apiHandler( settings.adminAiInsightsFeedbackSummary as never, defaultUserTimezone: settings.defaultUserTimezone, assistantEnabled: settings.assistantEnabled, - assistantCoachEnabled: settings.assistantCoachEnabled, + assistantCoachEnabled: coachFold.assistantCoachEnabled, assistantBriefingEnabled: settings.assistantBriefingEnabled, assistantInsightStatusEnabled: settings.assistantInsightStatusEnabled, assistantDocumentAiEnabled: settings.assistantDocumentAiEnabled ?? true, - moduleAvailabilityJson: settings.moduleAvailabilityJson as never, + moduleAvailabilityJson: coachFold.moduleAvailabilityJson as never, documentMaxFileBytes: settings.documentMaxFileBytes, documentQuotaBytes: BigInt(settings.documentQuotaBytes), }; diff --git a/src/lib/modules/__tests__/legacy-coach-availability.test.ts b/src/lib/modules/__tests__/legacy-coach-availability.test.ts new file mode 100644 index 000000000..b72c54fc5 --- /dev/null +++ b/src/lib/modules/__tests__/legacy-coach-availability.test.ts @@ -0,0 +1,34 @@ +import { describe, expect, it, vi } from "vitest"; + +vi.mock("@/lib/db", () => ({ prisma: {} })); + +import { foldLegacyCoachAvailability } from "../operator-availability"; + +describe("foldLegacyCoachAvailability", () => { + it("carries a pre-0343 `coach: false` into the Coach switch and drops the key", () => { + expect( + foldLegacyCoachAvailability({ coach: false, labs: true }, true), + ).toEqual({ + moduleAvailabilityJson: { labs: true }, + assistantCoachEnabled: false, + }); + }); + + it("drops a `coach: true` key without switching the Coach on", () => { + expect(foldLegacyCoachAvailability({ coach: true }, false)).toEqual({ + moduleAvailabilityJson: {}, + assistantCoachEnabled: false, + }); + }); + + it("leaves a current blob and a missing blob as they are", () => { + expect(foldLegacyCoachAvailability({ labs: false }, true)).toEqual({ + moduleAvailabilityJson: { labs: false }, + assistantCoachEnabled: true, + }); + expect(foldLegacyCoachAvailability(null, true)).toEqual({ + moduleAvailabilityJson: null, + assistantCoachEnabled: true, + }); + }); +}); diff --git a/src/lib/modules/operator-availability.ts b/src/lib/modules/operator-availability.ts index 90007c04e..d90b44a10 100644 --- a/src/lib/modules/operator-availability.ts +++ b/src/lib/modules/operator-availability.ts @@ -117,6 +117,34 @@ export async function getOperatorModuleAvailability(): Promise; + return { + moduleAvailabilityJson: rest, + assistantCoachEnabled: coach === false ? false : assistantCoachEnabled, + }; +} + /** A module key whose operator layer the blob owns (every key but the Coach). */ function isBlobOwnedKey(key: string): key is ModuleKey { return isModuleKey(key) && !SWITCH_OWNED_MODULE_KEYS.includes(key); diff --git a/tests/integration/admin-backups-restore.test.ts b/tests/integration/admin-backups-restore.test.ts index 3eecc7a2a..fed756f18 100644 --- a/tests/integration/admin-backups-restore.test.ts +++ b/tests/integration/admin-backups-restore.test.ts @@ -385,6 +385,59 @@ describe("POST /api/admin/backups/[id]/restore", () => { expect(rewritten.documentMaxFileBytes).toBe(4 * 1_048_576); }); + it("folds a pre-0343 Coach availability key into the Coach switch", async () => { + const prisma = getPrismaClient(); + const admin = await seedAdminSession(); + const live = await prisma.appSettings.create({ + data: { id: "singleton" }, + }); + + // A file written before the Coach's operator layer moved from module + // availability to the Coach switch: the operator had the Coach off there. + const { assistantDocumentAiEnabled: _dropped, ...beforeTheSwitchExisted } = + live; + void _dropped; + const backup = await prisma.dataBackup.create({ + data: { + userId: admin.id, + type: "MANUAL_UPLOAD_SETTINGS_TEST", + data: encrypt( + JSON.stringify({ + schemaVersion: "1", + exportedAt: "2026-05-09T00:00:00.000Z", + userId: admin.id, + measurements: [], + medications: [], + intakeEvents: [], + moodEntries: [], + appSettings: { + ...beforeTheSwitchExisted, + documentQuotaBytes: String(live.documentQuotaBytes), + assistantCoachEnabled: true, + moduleAvailabilityJson: { coach: false, labs: false }, + }, + }), + ), + }, + }); + + const { POST } = await import("@/app/api/admin/backups/[id]/restore/route"); + const res = await POST( + makeRequest(backup.id, { + confirm: "RESTORE", + restoreInstanceSettings: true, + }) as unknown as Parameters[0], + { params: Promise.resolve({ id: backup.id }) }, + ); + expect(res.status).toBe(200); + + const restored = await prisma.appSettings.findUniqueOrThrow({ + where: { id: "singleton" }, + }); + expect(restored.assistantCoachEnabled).toBe(false); + expect(restored.moduleAvailabilityJson).toEqual({ labs: false }); + }); + it("returns 404 when the backup id is unknown", async () => { const admin = await seedAdminSession(); void admin; From f700a7c0f104e63efdc4b69709aae19a20dc15f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 24 Sep 2026 12:19:11 +0200 Subject: [PATCH 5/9] fix(coach): let the capability table decide the about-me question consent The aboutMeQuestions capability admits an ai_insights_only receipt, but the question generator ran its own gate that demanded a Coach receipt, so a person whose capability read available still got the fallback questions on an operator-held chain. The generator now runs the shared wire re-check (aiEgressRefusal) for the aboutMeQuestions key, so the receipt kinds and the rule for when one is needed come from the capability table alone. --- .../self-context-questions-consent.test.ts | 75 +++++++++++++++---- src/lib/ai/coach/self-context-questions.ts | 33 ++++---- 2 files changed, 77 insertions(+), 31 deletions(-) diff --git a/src/lib/ai/coach/__tests__/self-context-questions-consent.test.ts b/src/lib/ai/coach/__tests__/self-context-questions-consent.test.ts index 7c2ca4a71..5931f593c 100644 --- a/src/lib/ai/coach/__tests__/self-context-questions-consent.test.ts +++ b/src/lib/ai/coach/__tests__/self-context-questions-consent.test.ts @@ -14,16 +14,33 @@ * - the budget runs through the ATOMIC reserve/reconcile pair, not a * read-then-write check. * - * `@/lib/consent/receipts` is mocked rather than the consent guard itself, so - * the real `chainRequiresServerManagedConsent` + `hasActiveConsentForSurface` - * logic is what these tests exercise. + * Only the receipt read is mocked, so the real wire re-check + * (`aiEgressRefusal`) and the capability table's `aboutMeQuestions` row are + * what these tests exercise: which receipts cover the prompt is the table's + * answer, not this module's. */ import { beforeEach, describe, expect, it, vi } from "vitest"; -vi.mock("@/lib/db", () => ({ prisma: {} })); - -const latestActiveReceipt = vi.hoisted(() => vi.fn()); -vi.mock("@/lib/consent/receipts", () => ({ latestActiveReceipt })); +const activeKinds = vi.hoisted(() => ({ kinds: new Set() })); +const consentFindFirst = vi.hoisted(() => + vi.fn(async (args: { where: { kind: { in: string[] } } }) => + args.where.kind.in.some((kind) => activeKinds.kinds.has(kind)) + ? { id: "receipt" } + : null, + ), +); +vi.mock("@/lib/db", () => ({ + prisma: { consentReceipt: { findFirst: consentFindFirst } }, +})); +// The capability itself is open; the route resolved it before calling in. +vi.mock("@/lib/ai/capabilities/gate", () => ({ + aiCapabilityForJob: vi.fn(async () => ({ + available: true, + reason: null, + onDeviceAllowed: true, + })), + requireAiCapability: vi.fn(async () => undefined), +})); vi.mock("@/lib/documents/document-settings", () => ({ documentAutoReadEnabled: vi.fn(async () => false), })); @@ -54,9 +71,13 @@ vi.mock("@/lib/ai/coach/budget", () => budgetMocks); const buildCoachSnapshot = vi.hoisted(() => vi.fn()); vi.mock("@/lib/ai/coach/snapshot", () => ({ buildCoachSnapshot })); -vi.mock("@/lib/logging/context", () => ({ annotate: vi.fn() })); +vi.mock("@/lib/logging/context", () => ({ + annotate: vi.fn(), + getEvent: () => null, +})); import { deriveClarifyingQuestions } from "../self-context-questions"; +import { AI_CAPABILITIES } from "@/lib/ai/capabilities/types"; import type { SelfContext } from "../about-me"; import { DEFAULT_HEALTH_PROFILE_AI_SECTIONS } from "@/lib/validations/health-profile-facts"; @@ -85,7 +106,7 @@ let provider: ReturnType; beforeEach(() => { vi.clearAllMocks(); provider = makeProvider(); - latestActiveReceipt.mockResolvedValue(null); + activeKinds.kinds = new Set(); providerMocks.hasAnyConfiguredProvider.mockResolvedValue(true); providerMocks.resolveProvider.mockResolvedValue({ type: "none" }); budgetMocks.reserveBudget.mockResolvedValue({ @@ -215,9 +236,7 @@ describe("deriveClarifyingQuestions — server-managed consent gate", () => { providerMocks.resolveProviderChain.mockResolvedValue([ { providerType: "admin-openai", instance: provider }, ]); - latestActiveReceipt.mockImplementation(async (_userId, kind) => - kind === "ai_coach" ? { id: "receipt-1" } : null, - ); + activeKinds.kinds = new Set(["ai_coach"]); const out = await deriveClarifyingQuestions( "user-1", @@ -236,9 +255,7 @@ describe("deriveClarifyingQuestions — server-managed consent gate", () => { providerMocks.resolveProviderChain.mockResolvedValue([ { providerType: "admin-openai", instance: provider }, ]); - latestActiveReceipt.mockImplementation(async (_userId, kind) => - kind === "ai_full" ? { id: "receipt-2" } : null, - ); + activeKinds.kinds = new Set(["ai_full"]); const out = await deriveClarifyingQuestions( "user-1", @@ -252,6 +269,32 @@ describe("deriveClarifyingQuestions — server-managed consent gate", () => { expect(provider.generateCompletion).toHaveBeenCalledTimes(1); }); + it("accepts the analysis-only grant the capability table lists", async () => { + // The capability the route resolved admits `ai_insights_only`; the + // question path used to demand a Coach receipt on top and fell back. + providerMocks.resolveProviderChain.mockResolvedValue([ + { providerType: "admin-openai", instance: provider }, + ]); + activeKinds.kinds = new Set(["ai_insights_only"]); + + const out = await deriveClarifyingQuestions( + "user-1", + ctx, + "en", + DEFAULT_HEALTH_PROFILE_AI_SECTIONS, + { aiAvailable: true }, + ); + + expect(out.source).toBe("ai"); + expect(consentFindFirst).toHaveBeenCalledWith( + expect.objectContaining({ + where: expect.objectContaining({ + kind: { in: [...AI_CAPABILITIES.aboutMeQuestions.consent.kinds] }, + }), + }), + ); + }); + it("leaves a BYOK chain ungated — the user's own egress needs no receipt", async () => { providerMocks.resolveProviderChain.mockResolvedValue([ { providerType: "openai", instance: provider }, @@ -267,7 +310,7 @@ describe("deriveClarifyingQuestions — server-managed consent gate", () => { expect(out.source).toBe("ai"); expect(provider.generateCompletion).toHaveBeenCalledTimes(1); - expect(latestActiveReceipt).not.toHaveBeenCalled(); + expect(consentFindFirst).not.toHaveBeenCalled(); }); it("leaves a local chain ungated", async () => { diff --git a/src/lib/ai/coach/self-context-questions.ts b/src/lib/ai/coach/self-context-questions.ts index 4b8b175af..9131feb90 100644 --- a/src/lib/ai/coach/self-context-questions.ts +++ b/src/lib/ai/coach/self-context-questions.ts @@ -36,10 +36,7 @@ import { resolveProviderChain, } from "@/lib/ai/provider"; import type { ProviderChainResolved } from "@/lib/ai/provider-runner"; -import { - chainRequiresServerManagedConsent, - hasActiveConsentForSurface, -} from "@/lib/ai/consent-guard"; +import { aiEgressRefusal } from "@/lib/ai/capabilities/egress"; import { singleUserTurn } from "@/lib/ai/types"; import { buildDateKey, @@ -213,18 +210,24 @@ export async function deriveClarifyingQuestions( const chain = await resolveQuestionsChain(userId); if (chain === null) return fallback(); - // Consent gate — BEFORE the snapshot is built, let alone sent. This - // prompt ships the complete Coach snapshot, so a chain that could egress - // via the operator's server-managed credential needs an active - // `ai_coach` / `ai_full` receipt. BYOK / local / ChatGPT-OAuth chains are - // the user's own egress and stay ungated, matching every other surface. - if ( - chainRequiresServerManagedConsent(chain) && - !(await hasActiveConsentForSurface(userId, "coach")) - ) { + // The wire re-check — BEFORE the snapshot is built, let alone sent. The + // capability table's `aboutMeQuestions` row decides which receipts cover + // this prompt and when one is needed (a chain that could reach the + // operator's key or the shared central Codex; a person's own key, their + // ChatGPT account or a local model need none), so this path and the + // capability the route resolved cannot disagree. + const refusal = await aiEgressRefusal( + "aboutMeQuestions", + userId, + chain.map((entry) => entry.providerType), + ); + if (refusal) { annotate({ - action: { name: "coach.self_context.consent_required" }, - meta: { self_context_questions_source: "fallback" }, + action: { name: "coach.self_context.egress_refused" }, + meta: { + self_context_questions_source: "fallback", + reason: refusal.reason, + }, }); return fallback(); } From f28153c832d7eb67ecccde1e0402666d10f5d33c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 24 Sep 2026 12:23:00 +0200 Subject: [PATCH 6/9] fix(tz): refuse bare UTC offsets as a timezone The runtime accepts "+05:30" as a zone id, so isValidTimezone let it through, but Postgres reads a bare offset in AT TIME ZONE with the POSIX sign (east of UTC is negative), and the day-grain SQL aggregates would cut every day on the wrong side of UTC. isValidTimezone now accepts named zones only: whatever the runtime resolves to a bare offset is refused, while named zones, their aliases, UTC and the Etc/GMT family stay accepted. Every write that already validated through it refuses offsets now, and the screener and mood linked-context request schemas validate their zone too. A value stored before this change can still sit in the database, so the SQL call sites that read the stored zone directly (the dashboard summary, achievements, the doctor report, the tiered AI context) fall back rather than splice it, as the resolver and the day aggregates already did. --- src/app/api/dashboard/summary/route.ts | 3 +- src/app/api/mood/linked-context/route.ts | 8 +++- src/lib/doctor-report/collect.ts | 3 +- src/lib/gamification/achievements-result.ts | 4 +- src/lib/rollups/tiered-context.ts | 9 ++++- src/lib/tz/__tests__/resolver.test.ts | 42 +++++++++++++++++++++ src/lib/tz/format.ts | 37 +++++++++++++++--- src/lib/validations/mental-health.ts | 7 +++- 8 files changed, 100 insertions(+), 13 deletions(-) diff --git a/src/app/api/dashboard/summary/route.ts b/src/app/api/dashboard/summary/route.ts index a80dbc6de..95d65fdbf 100644 --- a/src/app/api/dashboard/summary/route.ts +++ b/src/app/api/dashboard/summary/route.ts @@ -64,6 +64,7 @@ import { getServerTranslator } from "@/lib/i18n/server-translator"; import { type Locale } from "@/lib/i18n/config"; import { resolveJobLocale } from "@/lib/i18n/job-locale"; import { userDayKey, DEFAULT_TIMEZONE } from "@/lib/tz/resolver"; +import { validTimezoneOr } from "@/lib/tz/format"; import { cachedSwr, caches, type ServerCache } from "@/lib/cache/server-cache"; import { buildMedsTodayBlock } from "@/lib/dashboard/meds-today"; import { @@ -321,7 +322,7 @@ export const GET = apiHandler(async () => { // timezone. Falls back to Europe/Berlin when the column is somehow // missing (defensive — the schema's NOT NULL default normally pins // it). - const userTz = user.timezone ?? DEFAULT_TIMEZONE; + const userTz = validTimezoneOr(user.timezone, DEFAULT_TIMEZONE); // v1.4.38 W-F — wrap the whole response in the analytics LRU. // Subsequent iOS polls inside the TTL hit memory; measurement / diff --git a/src/app/api/mood/linked-context/route.ts b/src/app/api/mood/linked-context/route.ts index 97e13d4c0..c66e0030c 100644 --- a/src/app/api/mood/linked-context/route.ts +++ b/src/app/api/mood/linked-context/route.ts @@ -39,6 +39,7 @@ import { annotate } from "@/lib/logging/context"; import { requireModuleEnabled } from "@/lib/modules/gate"; import { resolveLinkedDayContext } from "@/lib/mood/linked-context"; import { DEFAULT_TIMEZONE } from "@/lib/mood/date-key"; +import { isValidTimezone } from "@/lib/tz/format"; export const dynamic = "force-dynamic"; @@ -47,7 +48,12 @@ const querySchema = z.object({ // The zone the day is anchored to. Absent falls back to the account's own // display timezone; the edit dialog passes the entry's stored `tz` so a row // logged elsewhere keeps its own day boundaries. - tz: z.string().min(1).max(64).optional(), + tz: z + .string() + .min(1) + .max(64) + .refine(isValidTimezone, "Invalid IANA timezone") + .optional(), }); export const GET = apiHandler(async (request: NextRequest) => { diff --git a/src/lib/doctor-report/collect.ts b/src/lib/doctor-report/collect.ts index e932224c8..d6bccfced 100644 --- a/src/lib/doctor-report/collect.ts +++ b/src/lib/doctor-report/collect.ts @@ -61,6 +61,7 @@ import { loadDenseMeasurementBuckets } from "./dense-buckets"; import { summariseDenseBuckets } from "./measurement-series"; import { emptyGlucoseClinical } from "./glucose-panel"; import { buildAdministrationLedger, buildGlp1Block } from "./medications"; +import { validTimezoneOr } from "@/lib/tz/format"; import { loadAllergies, loadAnamnesis, @@ -145,7 +146,7 @@ export async function collectDoctorReportData( insurerIkNumber: true, }, }); - const reportTz = userProfile?.timezone ?? "Europe/Berlin"; + const reportTz = validTimezoneOr(userProfile?.timezone, "Europe/Berlin"); const aggregateDenseTypes = days > DENSE_REPORT_RAW_WINDOW_DAYS; const densePulse = aggregateDenseTypes && !excluded.includes("PULSE"); diff --git a/src/lib/gamification/achievements-result.ts b/src/lib/gamification/achievements-result.ts index 0ae30939a..de50bdfb5 100644 --- a/src/lib/gamification/achievements-result.ts +++ b/src/lib/gamification/achievements-result.ts @@ -40,7 +40,7 @@ import { } from "@/lib/analytics/classifications"; import { classifyIntakeTiming } from "@/lib/analytics/compliance"; import { wallClockInTz } from "@/lib/tz/wall-clock"; -import { DEFAULT_TIMEZONE, userDayKey } from "@/lib/tz/format"; +import { DEFAULT_TIMEZONE, userDayKey, validTimezoneOr } from "@/lib/tz/format"; export type AuthedUser = Awaited>["user"]; @@ -555,7 +555,7 @@ export async function buildAchievementsResult( // day-streak badge (login, entry, weekend, miss-free, sleep-log, on-time, // compliance-80) plus the hidden night-owl / early-bird / leap-day // triggers for every non-CET account. - const tz = user.timezone || DEFAULT_TIMEZONE; + const tz = validTimezoneOr(user.timezone, DEFAULT_TIMEZONE); const [ vitalsBuckets, diff --git a/src/lib/rollups/tiered-context.ts b/src/lib/rollups/tiered-context.ts index fdf43b62c..28ec0a6a6 100644 --- a/src/lib/rollups/tiered-context.ts +++ b/src/lib/rollups/tiered-context.ts @@ -33,7 +33,7 @@ import type { MeasurementType, RollupGranularity, } from "@/generated/prisma/client"; -import { isNearUtc } from "@/lib/tz/format"; +import { isNearUtc, isValidTimezone } from "@/lib/tz/format"; import { ensureUserRollupsFresh, readRollupBuckets, @@ -400,7 +400,12 @@ async function readBand( // straight to the live path keyed in their zone. The coarser bands stay // on the UTC-anchored rollups — a few hours of offset moves bucket // MEMBERSHIP at week/month/year grain by at most one edge reading. - if (granularity === "DAY" && tz && !isNearUtc(tz, new Date(now))) { + if ( + granularity === "DAY" && + tz && + isValidTimezone(tz) && + !isNearUtc(tz, new Date(now)) + ) { return readBandLive(userId, type, granularity, from, to, tz); } // `readRollupBuckets` never throws — an empty / missing partition yields diff --git a/src/lib/tz/__tests__/resolver.test.ts b/src/lib/tz/__tests__/resolver.test.ts index 0f81b2aef..c6d8c6d62 100644 --- a/src/lib/tz/__tests__/resolver.test.ts +++ b/src/lib/tz/__tests__/resolver.test.ts @@ -55,6 +55,24 @@ describe("isValidTimezone", () => { it("rejects strings longer than 64 chars", () => { expect(isValidTimezone("A".repeat(65))).toBe(false); }); + + // The runtime accepts UTC offsets as zone ids, but Postgres reads a bare + // offset in `AT TIME ZONE` with the POSIX sign (east is negative), so a day + // cut in SQL would land on the wrong side of UTC. + it("rejects bare UTC offsets", () => { + expect(isValidTimezone("+05:30")).toBe(false); + expect(isValidTimezone("-0800")).toBe(false); + expect(isValidTimezone("+05")).toBe(false); + expect(isValidTimezone("\u221205:00")).toBe(false); + }); + + it("keeps accepting named zones, aliases and the Etc family", () => { + expect(isValidTimezone("Asia/Kolkata")).toBe(true); + expect(isValidTimezone("Asia/Calcutta")).toBe(true); + expect(isValidTimezone("Europe/Kyiv")).toBe(true); + expect(isValidTimezone("Etc/UTC")).toBe(true); + expect(isValidTimezone("Etc/GMT+5")).toBe(true); + }); }); describe("resolveServerDefaultTimezone", () => { @@ -109,6 +127,16 @@ describe("resolveUserTimezone", () => { expect(await resolveUserTimezone("user-1")).toBe("Pacific/Auckland"); }); + it("falls back to the server default for a stored bare offset", async () => { + vi.mocked(prisma.user.findUnique).mockResolvedValue({ + timezone: "+05:30", + } as never); + vi.mocked(prisma.appSettings.findUnique).mockResolvedValue({ + defaultUserTimezone: "Asia/Tokyo", + } as never); + expect(await resolveUserTimezone("user-1")).toBe("Asia/Tokyo"); + }); + it("falls back to the server default when the user row is missing", async () => { vi.mocked(prisma.user.findUnique).mockResolvedValue(null); vi.mocked(prisma.appSettings.findUnique).mockResolvedValue({ @@ -337,3 +365,17 @@ describe("DEFAULT_TIMEZONE", () => { expect(DEFAULT_TIMEZONE).toBe("Europe/Berlin"); }); }); + +describe("request schemas refuse a bare offset zone", () => { + it("the screener submission", async () => { + const { createAssessmentSchema } = + await import("@/lib/validations/mental-health"); + const base = { instrument: "PHQ9", items: [0, 0, 0, 0, 0, 0, 0, 0, 0] }; + expect( + createAssessmentSchema.safeParse({ ...base, tz: "Asia/Kolkata" }).success, + ).toBe(true); + expect( + createAssessmentSchema.safeParse({ ...base, tz: "+05:30" }).success, + ).toBe(false); + }); +}); diff --git a/src/lib/tz/format.ts b/src/lib/tz/format.ts index 1fa9ba047..4277a48a0 100644 --- a/src/lib/tz/format.ts +++ b/src/lib/tz/format.ts @@ -48,9 +48,20 @@ const WALL_CLOCK_PARTS_OPTIONS: Omit = }); /** - * Validate a timezone string by asking `Intl.DateTimeFormat` to use it. - * Returns `true` if the runtime accepts the zone, `false` otherwise. - * Cheap (microseconds) — call freely at write paths. + * A zone id the runtime resolved to a bare UTC offset (`+05:30`, `-08:00`). + */ +const OFFSET_ZONE = /^[+-]\d/; + +/** + * Validate a timezone string: a named IANA zone (or `UTC`) the runtime + * accepts. Returns `false` for anything else. Cheap (microseconds) — call + * freely at write paths. + * + * Bare UTC offsets are refused although the runtime accepts them as zone ids. + * The zone ends up in Postgres `AT TIME ZONE`, which reads a bare offset with + * the POSIX sign (east of UTC is negative), so `+05:30` would cut every day + * eleven hours off. Named zones, their aliases and the `Etc/GMT±N` family + * mean the same thing to both. */ export function isValidTimezone(tz: string): boolean { if (!tz || typeof tz !== "string" || tz.length === 0 || tz.length > 64) { @@ -60,13 +71,29 @@ export function isValidTimezone(tz: string): boolean { // The memo only caches successful constructions, so probing an // invalid zone through it cannot poison the formatter map — and a // valid zone leaves its formatter warm for the real callers. - getDateTimeFormat("en-US", tz, VALIDATION_OPTIONS); - return true; + const resolved = getDateTimeFormat( + "en-US", + tz, + VALIDATION_OPTIONS, + ).resolvedOptions().timeZone; + return !OFFSET_ZONE.test(resolved); } catch { return false; } } +/** + * `tz` when it is a valid zone, else `fallback`. For every place a stored or + * supplied zone reaches SQL: a value written before offsets were refused may + * still sit in the database, and it must never be spliced. + */ +export function validTimezoneOr( + tz: string | null | undefined, + fallback: string, +): string { + return tz && isValidTimezone(tz) ? tz : fallback; +} + /** * The list of every IANA zone the current Node runtime accepts. * Returns an empty array on engines older than Node 22 / V8 12.5 diff --git a/src/lib/validations/mental-health.ts b/src/lib/validations/mental-health.ts index 9331f9f85..ca6b70784 100644 --- a/src/lib/validations/mental-health.ts +++ b/src/lib/validations/mental-health.ts @@ -12,6 +12,7 @@ import { type InstrumentId, } from "@/lib/mental-health/instruments"; import { assertStableExternalId } from "@/lib/validations/external-id"; +import { isValidTimezone } from "@/lib/tz/format"; export const assessmentInstrumentEnum = z.enum(["PHQ9", "GAD7", "WHO5", "SCI"]); @@ -38,7 +39,11 @@ export const createAssessmentSchema = z functionalDifficulty: z.number().int().min(0).max(3).optional(), /** ISO instant the screener was taken; defaults to now server-side. */ takenAt: z.string().datetime().optional(), - tz: z.string().max(64).optional(), + tz: z + .string() + .max(64) + .refine(isValidTimezone, "Invalid IANA timezone") + .optional(), /** Locale of the validated wording actually presented. */ locale: z.string().max(16).optional(), /** Client provenance; defaults to WEB. */ From e82215493655153c2b8a3994afce627fd0d63faf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 24 Sep 2026 12:30:39 +0200 Subject: [PATCH 7/9] fix(cycle): give back a start that a mis-tapped earlier start folded in A start logged within one period before an existing start folds that start into itself and soft-deletes it, since the two are one period entered out of order. Nothing recorded which start was folded, so taking the earlier start back could not bring the later one back: a mis-tap on the 5th with a real start on the 10th, then deleted, left the 10th gone and the cycle before it reopened. Migration 0345 adds menstrual_cycles.absorbed_into_id. The folded row names the start that absorbed it; removing that start restores it, re-derives its own end and hands its days back to it. The column is needed because a soft-deleted row alone cannot tell a folded start from one the person deleted, and the audit trail is not state a removal can read. Deleting a cycle directly now runs the same shared removal as deleting its day-log. --- .../0345_cycle_absorbed_start/migration.sql | 8 ++ prisma/schema.prisma | 11 ++- src/app/api/cycle/cycles/[id]/route.ts | 19 +++-- src/lib/cycle/cycle-boundaries.ts | 74 +++++++++++++++++-- .../cycle-start-round-trip.test.ts | 60 +++++++++++++++ 5 files changed, 155 insertions(+), 17 deletions(-) create mode 100644 prisma/migrations/0345_cycle_absorbed_start/migration.sql diff --git a/prisma/migrations/0345_cycle_absorbed_start/migration.sql b/prisma/migrations/0345_cycle_absorbed_start/migration.sql new file mode 100644 index 000000000..96abb90f7 --- /dev/null +++ b/prisma/migrations/0345_cycle_absorbed_start/migration.sql @@ -0,0 +1,8 @@ +-- A period start logged a few days before an existing start folds that start +-- into itself (the same period, entered out of order), soft-deleting it. +-- Nothing recorded which start was folded, so taking the earlier start back +-- could not bring the later one back: a mis-tapped "started" wiped the real +-- start and reopened the cycle before it. The folded row now names the start +-- that absorbed it, and removing that start restores it. +ALTER TABLE "menstrual_cycles" + ADD COLUMN IF NOT EXISTS "absorbed_into_id" TEXT; diff --git a/prisma/schema.prisma b/prisma/schema.prisma index 66075b211..c6c619041 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -7113,9 +7113,14 @@ model MenstrualCycle { /// Per-row IANA timezone the date strings are anchored to (NULL = /// legacy Europe/Berlin), mirroring MoodEntry.tz. - tz String? - syncVersion Int @default(0) @map("sync_version") - deletedAt DateTime? @map("deleted_at") + tz String? + syncVersion Int @default(0) @map("sync_version") + deletedAt DateTime? @map("deleted_at") + /// The start that folded this one in (a start logged a few days earlier is + /// the same period entered out of order). Set only on the soft-deleted, + /// folded row; removing the absorbing start restores every row naming it + /// (`removeCycleStartedOn`). Cleared when the row is live again. + absorbedIntoId String? @map("absorbed_into_id") createdAt DateTime @default(now()) @map("created_at") updatedAt DateTime @updatedAt @map("updated_at") diff --git a/src/app/api/cycle/cycles/[id]/route.ts b/src/app/api/cycle/cycles/[id]/route.ts index 4e8483cc2..1a163c633 100644 --- a/src/app/api/cycle/cycles/[id]/route.ts +++ b/src/app/api/cycle/cycles/[id]/route.ts @@ -19,7 +19,7 @@ import { auditLog } from "@/lib/auth/audit"; import { overwriteDetails } from "@/lib/sharing/audit-details"; import { apiError, getClientIp } from "@/lib/api-response"; import { requireCycleEnabled } from "@/lib/cycle/gate"; -import { reanchorAfterRemovedStart } from "@/lib/cycle/cycle-boundaries"; +import { removeCycleStartedOn } from "@/lib/cycle/cycle-boundaries"; type RouteParams = { params: Promise<{ id: string }> }; @@ -45,13 +45,13 @@ export const DELETE = apiHandler( // Tombstone + re-anchor as one unit: a neighbour re-derived against a // half-applied delete would read the row being removed as still live. - const reanchored = await prisma.$transaction(async (db) => { - await db.menstrualCycle.update({ - where: { id }, - data: { deletedAt: new Date(), syncVersion: { increment: 1 } }, - }); - return reanchorAfterRemovedStart(db, user.id, existing.startDate); - }); + // The shared removal also gives back a start this one folded in and + // hands the removed cycle's days back. `null` is a cycle already + // tombstoned: nothing left to move. + const removed = await prisma.$transaction((db) => + removeCycleStartedOn(db, user.id, existing.startDate), + ); + const reanchored = removed?.reanchored ?? null; await auditLog("cycle.cycle.delete", { userId: user.id, @@ -61,6 +61,9 @@ export const DELETE = apiHandler( details: { cycleId: id, startDate: existing.startDate, + ...(removed && removed.restoredCycleIds.length > 0 + ? { restoredCycleIds: removed.restoredCycleIds } + : {}), ...(reanchored ? { reanchoredCycleId: reanchored.cycleId, diff --git a/src/lib/cycle/cycle-boundaries.ts b/src/lib/cycle/cycle-boundaries.ts index 9626d1239..660c5ed73 100644 --- a/src/lib/cycle/cycle-boundaries.ts +++ b/src/lib/cycle/cycle-boundaries.ts @@ -114,10 +114,6 @@ export async function openCycleAt( if (absorbed) { moved.absorbedCycleId = absorbed.id; moved.absorbedStartDate = absorbed.startDate; - await db.menstrualCycle.update({ - where: { id: absorbed.id }, - data: { deletedAt: new Date(), syncVersion: { increment: 1 } }, - }); } // Close the prior open cycle: its end is the day before this start. @@ -160,12 +156,26 @@ export async function openCycleAt( create: { userId, startDate: date, tz, isPredicted: false, ...carried }, update: { deletedAt: null, + absorbedIntoId: null, isPredicted: false, ...carried, syncVersion: { increment: 1 }, }, }); + // The folded start is soft-deleted and names the start that absorbed it, + // so taking this start back (a mis-tap) can give it back. + if (absorbed) { + await db.menstrualCycle.update({ + where: { id: absorbed.id }, + data: { + deletedAt: new Date(), + absorbedIntoId: cycle.id, + syncVersion: { increment: 1 }, + }, + }); + } + // Re-anchor the FOLLOWING neighbour's side of the boundary. const next = await db.menstrualCycle.findFirst({ where: { userId, deletedAt: null, startDate: { gt: date } }, @@ -342,13 +352,21 @@ export async function ensureCycleForBleedingDay( * body behind every way of taking a period start back: deleting the day-log * that opened it, clearing that day's flow, and deleting the cycle itself. * + * A start this one folded in when it was opened (a later start within one + * period, see `openCycleAt`) comes back: the earlier start was the mistake, + * so the later one was the real first day all along. + * * Returns null when no live cycle starts on that date. */ export async function removeCycleStartedOn( db: CycleDb, userId: string, date: string, -): Promise<{ cycleId: string; reanchored: ReanchoredCycle | null } | null> { +): Promise<{ + cycleId: string; + reanchored: ReanchoredCycle | null; + restoredCycleIds: string[]; +} | null> { const opened = await db.menstrualCycle.findFirst({ where: { userId, deletedAt: null, startDate: date }, select: { id: true }, @@ -359,6 +377,23 @@ export async function removeCycleStartedOn( where: { id: opened.id }, data: { deletedAt: new Date(), syncVersion: { increment: 1 } }, }); + // Give back the starts it folded in, before any boundary is re-derived, so + // the cycle before this one closes against them rather than reopening. + const restored = await db.menstrualCycle.findMany({ + where: { userId, absorbedIntoId: opened.id, deletedAt: { not: null } }, + orderBy: { startDate: "asc" }, + select: { id: true, startDate: true }, + }); + for (const row of restored) { + await db.menstrualCycle.update({ + where: { id: row.id }, + data: { + deletedAt: null, + absorbedIntoId: null, + syncVersion: { increment: 1 }, + }, + }); + } const reanchored = await reanchorAfterRemovedStart(db, userId, date); // The removed cycle's days go back to the cycle before it (or to none). const prior = await db.menstrualCycle.findFirst({ @@ -370,7 +405,34 @@ export async function removeCycleStartedOn( where: { userId, cycleId: opened.id }, data: { cycleId: prior?.id ?? null, syncVersion: { increment: 1 } }, }); - return { cycleId: opened.id, reanchored }; + // A restored start owns its span again, and its own end is re-derived from + // whatever was logged after it while it was folded. + for (const row of restored) { + const next = await db.menstrualCycle.findFirst({ + where: { userId, deletedAt: null, startDate: { gt: row.startDate } }, + orderBy: { startDate: "asc" }, + select: { startDate: true }, + }); + await db.menstrualCycle.update({ + where: { id: row.id }, + data: { + endDate: next ? addDays(next.startDate, -1) : null, + lengthDays: next ? dayDiff(next.startDate, row.startDate) : null, + }, + }); + await attributeDaysToCycle( + db, + userId, + row.id, + row.startDate, + next?.startDate ?? null, + ); + } + return { + cycleId: opened.id, + reanchored, + restoredCycleIds: restored.map((row) => row.id), + }; } /** The boundary columns the re-derivation moved, for the audit row. */ diff --git a/tests/integration/cycle-start-round-trip.test.ts b/tests/integration/cycle-start-round-trip.test.ts index 8595df6f1..bbb458a1e 100644 --- a/tests/integration/cycle-start-round-trip.test.ts +++ b/tests/integration/cycle-start-round-trip.test.ts @@ -236,6 +236,66 @@ describe("logging a period start and taking it back", () => { expect(await liveShapes()).toEqual(before); }); + it("gives back the start a mis-tapped earlier start folded in", async () => { + // A start a few days after another is the same period entered late, so + // the earlier tap folds it in. When the earlier tap was the mistake, + // taking it back has to bring the real start back too, not reopen the + // cycle before it. + await loginAs(USER_ID); + await seedHistory(); + await startPeriod("2026-08-10"); + const before = await liveShapes(); + const realStart = await getPrismaClient().menstrualCycle.findFirstOrThrow({ + where: { userId: USER_ID, startDate: "2026-08-10" }, + select: { id: true }, + }); + + await startPeriod("2026-08-05"); + const during = await liveShapes(); + expect(during.map((c) => c.startDate)).not.toContain("2026-08-10"); + expect(during.at(-1)?.startDate).toBe("2026-08-05"); + + const misTap = await getPrismaClient().menstrualCycle.findFirstOrThrow({ + where: { userId: USER_ID, startDate: "2026-08-05" }, + select: { id: true }, + }); + const { DELETE } = await import("@/app/api/cycle/cycles/[id]/route"); + const res = await DELETE( + jsonRequest(`/api/cycle/cycles/${misTap.id}`, "DELETE"), + { params: Promise.resolve({ id: misTap.id }) }, + ); + expect(res.status).toBe(204); + + expect(await liveShapes()).toEqual(before); + // The real start's own day is its own again. + const dayLog = await getPrismaClient().cycleDayLog.findFirstOrThrow({ + where: { userId: USER_ID, date: "2026-08-10" }, + select: { cycleId: true }, + }); + expect(dayLog.cycleId).toBe(realStart.id); + }); + + it("gives back the folded start when the mis-tap's day-log is deleted", async () => { + await loginAs(USER_ID); + await seedHistory(); + await startPeriod("2026-08-10"); + const before = await liveShapes(); + + await startPeriod("2026-08-05"); + const dayLog = await getPrismaClient().cycleDayLog.findFirstOrThrow({ + where: { userId: USER_ID, date: "2026-08-05" }, + select: { id: true }, + }); + const { DELETE } = await import("@/app/api/cycle/day-logs/[id]/route"); + const res = await DELETE( + jsonRequest(`/api/cycle/day-logs/${dayLog.id}`, "DELETE"), + { params: Promise.resolve({ id: dayLog.id }) }, + ); + expect(res.status).toBe(204); + + expect(await liveShapes()).toEqual(before); + }); + it("leaves a day-log that opened no cycle alone", async () => { // A plain entry on a day that is not a cycle start must not take a cycle // with it when it is deleted. From 70dba821be0b3cbbf1b7fb80da66e8336735b3fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 24 Sep 2026 12:37:51 +0200 Subject: [PATCH 8/9] perf(measurements): fold each source before picking the canonical one The live daily fold ranked every row by source before folding, which sorted the whole window twice. A year of per-minute heart rate (525,600 rows) took about 0.55 s per series on Postgres 16, with both sorts spilling to disk. The fold now aggregates per source and bucket in one hashed pass and then keeps each bucket's ladder-canonical source. Folding the canonical source's rows is the same as folding that source's group, so the numbers do not change; the same year reads in about 0.29 s. A new integration case pins the canonical pick, mean, count and spread for a day with three sources. --- src/lib/measurements/daily-series-read.ts | 64 ++++++++++--------- .../integration/daily-series-user-tz.test.ts | 60 +++++++++++++++++ 2 files changed, 94 insertions(+), 30 deletions(-) diff --git a/src/lib/measurements/daily-series-read.ts b/src/lib/measurements/daily-series-read.ts index 8c20993ab..0ce67d26f 100644 --- a/src/lib/measurements/daily-series-read.ts +++ b/src/lib/measurements/daily-series-read.ts @@ -121,8 +121,8 @@ export async function readDailySeries(opts: { // every bucket's UTC-midnight start read as the previous local day west of // UTC. The 7-day chart buckets raw rows by the local day, so switching // ranges moved daily totals between neighbouring days. The live fold is - // one indexed pass per window (about 0.1 s for 90 days of per-minute - // samples) and returns at most one row per day. + // one pass per window (about 0.3 s for a year of per-minute samples, + // measured against Postgres 16) and returns at most one row per day. const timeZone = opts.timeZone ?? (await resolveUserTimezone(userId)); const rows = await readLiveBuckets({ userId, @@ -185,24 +185,31 @@ export async function readLiveBuckets(opts: { // `TRUNC_UNIT` map keyed by the Zod-constrained grain, never from input. const unit = Prisma.raw(`'${TRUNC_UNIT[grain]}'`); // Cumulative types SUM per bucket, every other type averages. The type list - // is the closed code constant, spliced as literals; `s."type"` is a GROUP BY + // is the closed code constant, spliced as literals; `m."type"` is a GROUP BY // column, so choosing the aggregate per group is legal. const cumulativeList = [...CUMULATIVE_HK_TYPES] .map((t) => `'${t}'`) .join(","); const aggregator = Prisma.raw( - `(CASE WHEN s."type"::text IN (${cumulativeList}) THEN SUM(s."value") ELSE AVG(s."value") END)::double precision`, + `(CASE WHEN m."type"::text IN (${cumulativeList}) THEN SUM(m."value") ELSE AVG(m."value") END)::double precision`, ); const rankRaw = Prisma.raw( - buildSourceRankCase(priorityJson, 'm."type"', 'm."source"'), + buildSourceRankCase(priorityJson, 'p."type"', 'p."source"'), ); const typeFilter = type ? Prisma.sql`AND m."type" = ${type}::measurement_type` : Prisma.empty; // measured_at is a UTC wall-clock timestamp: pin it to UTC, re-read it in - // the user's zone and truncate there. The bucket is computed once in `src` - // and referenced by name afterwards, so the zone parameter never has to - // match itself across SELECT and GROUP BY. + // the user's zone and truncate there. + // + // Fold first, pick second. Every source is folded per bucket in one hashed + // pass (a few rows per day, however dense the stream), and only then does + // each bucket keep its ladder-canonical source. The fold of the canonical + // source's rows is exactly the fold of that source's group, so the result + // is the same as collapsing the rows first; what changes is the cost. A + // year of per-minute heart rate is half a million rows, and ranking them + // before the fold sorted them twice (about 0.55 s); folding first never + // sorts more than one row per source per day. const buckets = await prisma.$queryRaw< Array<{ type: string; @@ -213,41 +220,38 @@ export async function readLiveBuckets(opts: { max_value: number | null; }> >` - WITH src AS ( + WITH per_source AS ( SELECT m."type", m."source", - m."value", - date_trunc(${unit}, (m."measured_at" AT TIME ZONE 'UTC') AT TIME ZONE ${timeZone}) AS d + date_trunc(${unit}, (m."measured_at" AT TIME ZONE 'UTC') AT TIME ZONE ${timeZone}) AS d, + ${aggregator} AS avg, + COUNT(*)::int AS cnt, + MIN(m."value") AS min_value, + MAX(m."value") AS max_value FROM measurements m WHERE m."user_id" = ${userId} AND m."measured_at" >= ${from} AND m."measured_at" <= ${to} AND m."deleted_at" IS NULL ${typeFilter} + GROUP BY m."type", m."source", 3 ), canon AS ( - SELECT DISTINCT ON (m."type", m.d) - m."type" AS t, - m.d AS d, - m."source" AS canon - FROM src m - ORDER BY m."type", m.d, (${rankRaw}), m."source" + SELECT DISTINCT ON (p."type", p.d) + p."type", p.d, p.avg, p.cnt, p.min_value, p.max_value + FROM per_source p + ORDER BY p."type", p.d, (${rankRaw}), p."source" ) SELECT - s."type"::text AS type, - s.d AT TIME ZONE ${timeZone} AS bucket_start, - ${aggregator} AS avg, - COUNT(*)::int AS cnt, - MIN(s."value") AS min_value, - MAX(s."value") AS max_value - FROM src s - JOIN canon c - ON c.t = s."type" - AND c.d = s.d - AND c.canon = s."source" - GROUP BY s."type", s.d - ORDER BY s.d ASC + c."type"::text AS type, + c.d AT TIME ZONE ${timeZone} AS bucket_start, + c.avg, + c.cnt, + c.min_value, + c.max_value + FROM canon c + ORDER BY c.d ASC LIMIT ${cap} `; return buckets.map((b) => { diff --git a/tests/integration/daily-series-user-tz.test.ts b/tests/integration/daily-series-user-tz.test.ts index e96270b81..4ae147839 100644 --- a/tests/integration/daily-series-user-tz.test.ts +++ b/tests/integration/daily-series-user-tz.test.ts @@ -86,6 +86,12 @@ function byLocalDay( const EXPECTED = { "2026-09-10": 1100, "2026-09-11": 10 }; +/** The ladder puts Withings first for pulse, so its two readings are the day. */ +const CANON_EXPECTED = [ + { day: "2026-09-10", value: 95, count: 2, min: 90, max: 100 }, + { day: "2026-09-11", value: 64, count: 1, min: 64, max: 64 }, +]; + describe("daily series buckets by the user's calendar day (#1026)", () => { for (const tz of ["Europe/Berlin", "Asia/Kolkata", "America/New_York"]) { it(`chart daily read agrees with the local day (${tz})`, async () => { @@ -124,4 +130,58 @@ describe("daily series buckets by the user's calendar day (#1026)", () => { expect(byLocalDay(rows, tz)).toEqual(EXPECTED); }); } + + it("collapses a day to its canonical source before folding it", async () => { + // Two sources on one day: only the ladder's pick is folded, and the + // mean, count and spread all come from that one source. + const prisma = getPrismaClient(); + const user = await prisma.user.create({ + data: { + username: "tz-canon", + email: "tz-canon@example.test", + role: "USER", + timezone: "Europe/Berlin", + }, + }); + const at = (h: number) => localHmAsUtc(DAY, "Europe/Berlin", h, 0); + await prisma.measurement.createMany({ + data: [ + { value: 60, source: "APPLE_HEALTH" as const, measuredAt: at(8) }, + { value: 70, source: "APPLE_HEALTH" as const, measuredAt: at(9) }, + { value: 90, source: "WITHINGS" as const, measuredAt: at(10) }, + { value: 100, source: "WITHINGS" as const, measuredAt: at(12) }, + { value: 50, source: "MANUAL" as const, measuredAt: at(11) }, + // The next day has one source only. + { + value: 64, + source: "WITHINGS" as const, + measuredAt: localHmAsUtc(NEXT_DAY, "Europe/Berlin", 7, 0), + }, + ].map((r) => ({ + ...r, + userId: user.id, + type: "PULSE" as const, + unit: "bpm", + })), + }); + const rows = await readLiveBuckets({ + userId: user.id, + type: "PULSE", + from: new Date("2026-09-05T00:00:00Z"), + to: new Date("2026-09-15T00:00:00Z"), + cap: 366, + priorityJson: null, + grain: "daily", + timeZone: "Europe/Berlin", + }); + expect( + rows.map((r) => ({ + day: userDayKey(new Date(r.measuredAt), "Europe/Berlin"), + value: r.value, + count: r.count, + min: r.minValue, + max: r.maxValue, + })), + ).toEqual(CANON_EXPECTED); + }); }); From 8a09adc1c0092f740b1420a36b111b9ffe1c90d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 24 Sep 2026 12:43:41 +0200 Subject: [PATCH 9/9] fix(cycle): answer a delete of an already deleted cycle without moving anything The cycle delete now runs the shared removal, which finds live starts only. A second delete of the same cycle read nothing to remove; the route now says so explicitly and answers 204 before the transaction, and the tombstone the first delete left stays as it was. --- src/app/api/cycle/cycles/[id]/route.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/app/api/cycle/cycles/[id]/route.ts b/src/app/api/cycle/cycles/[id]/route.ts index 1a163c633..088c6c7a0 100644 --- a/src/app/api/cycle/cycles/[id]/route.ts +++ b/src/app/api/cycle/cycles/[id]/route.ts @@ -37,17 +37,21 @@ export const DELETE = apiHandler( const existing = await prisma.menstrualCycle.findUnique({ where: { id }, - select: { id: true, userId: true, startDate: true }, + select: { id: true, userId: true, startDate: true, deletedAt: true }, }); if (!existing || existing.userId !== user.id) { return apiError("Cycle not found", 404); } + // Already a tombstone: the delete is done, and nothing is left to move. + if (existing.deletedAt !== null) { + return new Response(null, { status: 204 }); + } // Tombstone + re-anchor as one unit: a neighbour re-derived against a // half-applied delete would read the row being removed as still live. // The shared removal also gives back a start this one folded in and - // hands the removed cycle's days back. `null` is a cycle already - // tombstoned: nothing left to move. + // hands the removed cycle's days back. `null` is a concurrent delete that + // tombstoned the row first: nothing left to move. const removed = await prisma.$transaction((db) => removeCycleStartedOn(db, user.id, existing.startDate), );