diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index 644022880..a4f296586 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -28647,7 +28647,9 @@ components: anyOf: - type: string - type: "null" - description: Provider that produced the reply (e.g. anthropic, openai, local, refusal); null for user turns. + description: Provider that produced the reply (e.g. anthropic, openai, local, refusal). The sentinel `cancelled` marks + an empty assistant row closing a turn the client aborted mid-generation (navigation away); clients should + render it as an interrupted turn with a retry affordance. Null for user turns. promptVersion: anyOf: - type: string diff --git a/src/app/api/insights/chat/__tests__/route-cancelled-turn.test.ts b/src/app/api/insights/chat/__tests__/route-cancelled-turn.test.ts new file mode 100644 index 000000000..593a9fb7a --- /dev/null +++ b/src/app/api/insights/chat/__tests__/route-cancelled-turn.test.ts @@ -0,0 +1,246 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +/** + * #781 — a client that navigates away mid-generation aborts the SSE fetch. + * The route must close the turn honestly: persist an EMPTY assistant marker + * with `providerType: "cancelled"` so the conversation shows the interrupted + * turn (and the client can offer a retry), instead of leaving the user + * message dangling unanswered. The reservation is refunded in full, and the + * abort is never misreported as a provider failure. + * + * Route-level on purpose: the abort branch lives in `route.ts`'s catch, a + * place no pure-module test can see (the #591 lesson). + */ + +const SNAPSHOT_JSON = '{"bp":{"aggregate":{"mean":128}}}'; + +vi.mock("@/lib/api-handler", () => ({ + apiHandler: unknown>(fn: T) => fn, + requireAuth: vi.fn(async () => ({ user: { id: "u1", locale: "en" } })), + HttpError: class HttpError extends Error { + status: number; + constructor(status: number, message: string) { + super(message); + this.status = status; + } + }, +})); +vi.mock("@/lib/modules/gate", () => ({ + requireModuleEnabled: vi.fn(async () => ({ enabled: true })), + isModuleEnabled: vi.fn(async () => true), +})); +vi.mock("@/lib/feature-flags", () => ({ + requireAssistantSurface: vi.fn(async () => undefined), +})); +vi.mock("@/lib/api-response", () => ({ + apiError: (error: string, status: number) => ({ data: null, error, status }), + apiSuccess: (data: unknown) => ({ data, error: null, status: 200 }), +})); +const { annotate } = vi.hoisted(() => ({ annotate: vi.fn() })); +vi.mock("@/lib/logging/context", () => ({ + annotate, + getEvent: vi.fn(() => null), +})); +vi.mock("@/lib/auth/audit", () => ({ auditLog: vi.fn() })); +vi.mock("@/lib/db", () => ({ + prisma: { + user: { findUnique: vi.fn(async () => ({ coachPrefsJson: null })) }, + coachConversation: { findFirst: vi.fn(async () => ({ id: "c1" })) }, + workout: { findFirst: vi.fn(async () => null) }, + }, +})); +vi.mock("@/lib/rate-limit", () => ({ + checkRateLimit: vi.fn(async () => ({ allowed: true })), +})); +vi.mock("@/lib/i18n/server-locale", () => ({ + resolveServerLocale: vi.fn(async () => "en"), +})); + +const { runStreamingRawCompletionWithFallback } = vi.hoisted(() => ({ + runStreamingRawCompletionWithFallback: vi.fn(), +})); +vi.mock("@/lib/ai/provider-runner", () => ({ + AllProvidersFailedError: class extends Error {}, + runStreamingRawCompletionWithFallback, +})); + +const { resolveProviderChain } = vi.hoisted(() => ({ + resolveProviderChain: vi.fn(), +})); +vi.mock("@/lib/ai/provider", () => ({ + resolveProviderChain, + resolveProvider: vi.fn(), +})); +vi.mock("@/lib/ai/consent-guard", () => ({ + assertConsentForChain: vi.fn(async () => undefined), +})); +vi.mock("@/lib/ai/prompts/insight-generator", () => ({ PROMPT_VERSION: "x" })); +vi.mock("@/lib/ai/ai-budgets", () => ({ + AI_BUDGETS: { coach: { maxTokens: 1500, temperature: 0.4 } }, +})); + +vi.mock("@/lib/ai/coach/persistence", () => ({ + appendMessage: vi.fn(async () => ({ id: "m1" })), + createConversation: vi.fn(async () => ({ id: "c1" })), + fetchConversationWithMessages: vi.fn(), + listConversations: vi.fn(), +})); +vi.mock("@/lib/ai/coach/coach-memory-shared", () => ({ + enqueueCoachMemoryRefresh: vi.fn(), +})); +vi.mock("@/lib/ai/coach/facts", () => ({ + storeDeterministicFacts: vi.fn(async () => undefined), +})); + +const { reserveBudget, reconcileSpend } = vi.hoisted(() => ({ + reserveBudget: vi.fn(async () => ({ allowed: true, reserved: 3000 })), + reconcileSpend: vi.fn(async () => undefined), +})); +vi.mock("@/lib/ai/coach/budget", () => ({ + buildDateKey: vi.fn(() => "2026-08-13"), + reserveBudget, + reconcileSpend, + resolveDailyCap: vi.fn(() => 2_000_000), +})); + +vi.mock("@/lib/ai/coach/refusal", () => ({ + detectRefusal: vi.fn(() => ({ refuse: false })), +})); +vi.mock("@/lib/ai/coach/system-prompt", () => ({ + getCoachSystemPrompt: vi.fn(() => "SYSTEM"), +})); +vi.mock("@/lib/ai/coach/about-me", () => ({ + getSelfContextTextForUser: vi.fn(async () => null), +})); +vi.mock("@/lib/ai/coach/snapshot", () => ({ + buildCoachSnapshot: vi.fn(async () => ({ + snapshotJson: SNAPSHOT_JSON, + sections: { bloodPressure: { aggregate: { mean: 128 } } }, + provenance: { windows: ["last30days"], metrics: ["bp"] }, + referenceGrounding: "REFERENCE RANGES", + })), +})); +vi.mock("@/lib/workouts/hr-series", () => ({ + buildWorkoutHrSeries: vi.fn(async () => null), +})); +vi.mock("@/lib/workouts/zones", () => ({ + computeZones: vi.fn(() => null), + hrMaxFromAge: vi.fn(() => 185), + parseWhoopZoneDurations: vi.fn(() => null), +})); +vi.mock("@/lib/workouts/sport-context", () => ({ + buildSportContext: vi.fn(async () => null), +})); + +vi.mock("@/lib/ai/coach/tools", () => ({ + COACH_TOOL_DEFS: [{ name: "get_metric_series" }], + MAX_ROUNDS: 3, + buildCoachDataInventory: vi.fn(async () => ({ + entries: [], + restMode: false, + cycleEnabled: false, + window: "last30days", + probeScope: { sources: ["bp"], window: "last30days" }, + })), + renderDataInventory: vi.fn(() => "DATA INVENTORY"), + renderFocusHint: vi.fn(() => ""), + buildToolModeAddendum: vi.fn(() => "TOOL ADDENDUM"), + runCoachToolLoop: vi.fn(), +})); + +vi.mock("@/lib/ai/coach/keyvalues", () => ({ + parseKeyValuesSentinel: vi.fn(), +})); +vi.mock("@/lib/ai/coach/suggest-reminder", () => ({ + parseSuggestReminder: vi.fn(), +})); +vi.mock("@/lib/ai/coach/suggest-gate", () => ({ gateSuggestion: vi.fn() })); +vi.mock("@/lib/validations/coach-prefs", () => ({ + parseCoachPrefs: vi.fn(() => ({ defaultWindow: undefined })), + DEFAULT_REMINDER_SUGGESTION_PREFS: {}, +})); + +const { appendMessage } = await import("@/lib/ai/coach/persistence"); + +const sse = vi.hoisted(() => ({ done: Promise.resolve() as Promise })); +vi.mock("@/lib/sse/create-stream", () => ({ + createSseStream: ( + producer: (c: { + signal: { aborted: boolean }; + enqueue: () => void; + }) => void | Promise, + ) => { + sse.done = Promise.resolve( + producer({ signal: { aborted: true }, enqueue: () => {} }), + ); + return new ReadableStream(); + }, +})); + +import { POST } from "../route"; + +const post = POST as unknown as (req: Request) => Promise; + +function abortedChatReq(body: Record): Request { + const controller = new AbortController(); + const req = new Request("http://localhost/api/insights/chat", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify(body), + signal: controller.signal, + }); + // The body is read synchronously by the handler before the provider call; + // aborting here models the client tearing the connection down while the + // provider is generating (the runner rejects below). + controller.abort(); + return req; +} + +function abortError(): Error { + const err = new Error("This operation was aborted"); + err.name = "AbortError"; + return err; +} + +describe("coach chat — client abort mid-generation (#781)", () => { + beforeEach(() => { + vi.clearAllMocks(); + reserveBudget.mockResolvedValue({ allowed: true, reserved: 3000 }); + // No-tools chain so the turn runs the streaming runner (which rejects + // with the abort the torn-down connection produces). + resolveProviderChain.mockResolvedValue([ + { providerType: "local", instance: { supportsTools: false } }, + ]); + runStreamingRawCompletionWithFallback.mockRejectedValue(abortError()); + }); + + it("persists an empty assistant marker with providerType 'cancelled'", async () => { + const res = await post(abortedChatReq({ message: "How is my BP?" })); + expect(res.status).toBe(200); + await sse.done; + + const calls = (appendMessage as ReturnType).mock.calls; + // The user turn is persisted first, then the cancelled marker. + const roles = calls.map((c) => (c[0] as { role: string }).role); + expect(roles).toEqual(["user", "assistant"]); + const marker = calls[1][0] as { + role: string; + content: string; + providerType: string | null; + }; + expect(marker.providerType).toBe("cancelled"); + expect(marker.content).toBe(""); + }); + + it("refunds the full reservation and annotates the abort as cancelled, not a provider failure", async () => { + await post(abortedChatReq({ message: "How is my BP?" })); + await sse.done; + + expect(reconcileSpend).toHaveBeenCalledWith("u1", 3000, 0, "2026-08-13"); + const actions = annotate.mock.calls + .map((c) => (c[0] as { action?: { name?: string } }).action?.name) + .filter(Boolean); + expect(actions).toContain("insights.coach.cancelled"); + expect(actions).not.toContain("insights.coach.providerFailed"); + }); +}); diff --git a/src/app/api/insights/chat/route.ts b/src/app/api/insights/chat/route.ts index 2f03c16ae..d10447086 100644 --- a/src/app/api/insights/chat/route.ts +++ b/src/app/api/insights/chat/route.ts @@ -325,10 +325,16 @@ async function handleChatRequest(request: NextRequest): Promise { } workingConversationId = existing.id; priorSummary = existing.summary ?? null; - priorTurns = existing.messages.map((m) => ({ - role: m.role, - content: m.content, - })); + // #781 — cancelled-turn markers (`providerType: "cancelled"`, empty body) + // are UI rows, not conversation content: keep them out of the provider + // transcript so an interrupted turn adds no empty assistant line to the + // prompt. They stay in the DTO the client renders. + priorTurns = existing.messages + .filter((m) => m.providerType !== "cancelled") + .map((m) => ({ + role: m.role, + content: m.content, + })); // Ledger cross-turn sources (D3-safe): user-authored numbers + the // persisted tool figures of prior turns. Assistant PROSE is never read. priorUserMessages = existing.messages @@ -761,6 +767,35 @@ async function handleChatRequest(request: NextRequest): Promise { await reconcileSpend(userId, reservation.reserved, 0, reqDateKey).catch( () => {}, ); + // #781 — the client walked away mid-generation. The request's abort + // signal is threaded into every provider call, so the teardown surfaces + // here as an abort-shaped failure with `request.signal` already flipped. + // Close the turn honestly instead of letting the user message dangle + // unanswered: persist an EMPTY assistant marker tagged + // `providerType: "cancelled"` (the same channel "refusal" and "nudge" + // already use for non-provider rows) so the thread shows the turn as + // interrupted and offers a retry on reload. The content is empty by + // construction — nothing guarded was produced, and anything the client + // ever SAW was persisted before the first token frame left, so no + // unguarded partial text is ever written. The reservation was refunded + // above; a retry pays exactly what a fresh turn pays. + if (request.signal.aborted) { + await appendMessage({ + conversationId: workingConversationId, + role: "assistant", + content: "", + providerType: "cancelled", + promptVersion: PROMPT_VERSION, + }).catch(() => { + // Marker persistence is best-effort — a failure leaves the + // dangling user turn, which is exactly the pre-#781 state. + }); + annotate({ + action: { name: "insights.coach.cancelled" }, + meta: { conversationId: workingConversationId }, + }); + return { ok: false, code: "coach.cancelled" }; + } if (err instanceof AllProvidersFailedError) { annotate({ action: { name: "insights.coach.providerFailed" }, @@ -1241,6 +1276,17 @@ async function handleChatRequest(request: NextRequest): Promise { outcome = await produceReply(); } catch (err) { clearInterval(heartbeat); + // #781 — a client disconnect can also surface as an abort-shaped error + // on a path `produceReply`'s own catch does not own (e.g. a teardown + // rejection out of persistence). That is not a defect and nobody is + // listening: skip the GlitchTip forward and the error frame. + if (request.signal.aborted) { + annotate({ + action: { name: "insights.coach.cancelled" }, + meta: { conversationId: workingConversationId, lateAbort: true }, + }); + return; + } // The stream is already open, so we cannot 500: a tagged provider error // surfaces a graceful `coach.provider.*` frame; anything else degrades to // a generic unavailable frame. A genuine defect is still annotated. diff --git a/src/lib/openapi/routes/coach.ts b/src/lib/openapi/routes/coach.ts index 814cdfbf0..9e0336e3e 100644 --- a/src/lib/openapi/routes/coach.ts +++ b/src/lib/openapi/routes/coach.ts @@ -540,7 +540,7 @@ const coachMessageSchema = z .string() .nullable() .describe( - "Provider that produced the reply (e.g. anthropic, openai, local, refusal); null for user turns.", + "Provider that produced the reply (e.g. anthropic, openai, local, refusal). The sentinel `cancelled` marks an empty assistant row closing a turn the client aborted mid-generation (navigation away); clients should render it as an interrupted turn with a retry affordance. Null for user turns.", ), promptVersion: z .string()