From 6d250d092d9e51d29190201b16a34e79caea247c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 22 Oct 2026 13:01:20 +0200 Subject: [PATCH] fix(insights): wait as long as the response timeout allows in the browser The regenerate button aborted its inline POST after a fixed 45 s, so on a slow local model every regenerate was cut while the server still had minutes left on the person's setting. It now uses clientAbortMs over the route's sequential model calls, never less per call than the briefing's own 180 s default, because the published value cannot say whether the person set it. A save of the about-me self-context derives the clarifying questions inline and ran under the 15 s request default, which reported a failed save while the server went on to commit it. All six save sites now take their abort from the same setting. Every other browser request that waits on a model either has no client timeout or only queues the work. Refs #1267 --- src/__tests__/ai-client-abort-guard.test.ts | 91 +++++++++++++++++++ .../use-insights-advisor-force.test.ts | 57 +++++++++++- .../insights/insights-tab-strip.tsx | 2 +- .../insights/use-insights-advisor.ts | 52 ++++++++--- src/components/onboarding/baseline-form.tsx | 6 +- .../__tests__/height-entry-surfaces.test.tsx | 7 ++ .../ai-profile-inclusion-manager.test.tsx | 8 ++ .../records/about-me-note-manager.tsx | 7 +- .../records/ai-profile-inclusion-manager.tsx | 3 + .../records/allergy-free-text-note.tsx | 3 + src/components/records/conditions-manager.tsx | 14 ++- .../records/free-text-take-over.tsx | 3 + .../coach-links-without-coach.test.tsx | 2 + .../settings-read-error-family.test.tsx | 2 + src/hooks/use-ai-capability.ts | 17 +++- .../ai/__tests__/effective-timeout.test.ts | 32 ++++++- src/lib/ai/effective-timeout.ts | 10 +- 17 files changed, 289 insertions(+), 27 deletions(-) create mode 100644 src/__tests__/ai-client-abort-guard.test.ts diff --git a/src/__tests__/ai-client-abort-guard.test.ts b/src/__tests__/ai-client-abort-guard.test.ts new file mode 100644 index 000000000..4fabbc194 --- /dev/null +++ b/src/__tests__/ai-client-abort-guard.test.ts @@ -0,0 +1,91 @@ +/** + * Structural guard (#1267): a browser request that waits on a model call + * takes its abort from the person's response-timeout setting, never from a + * constant. + * + * The force regenerate aborted at a fixed 45 s while the server still had + * minutes left on the person's 600 s setting, and a save of the about-me + * self-context (which derives the clarifying questions inline) ran under the + * 15 s request default. Both now read `clientAbortMs` through the account + * payload's `ai.provider.responseTimeoutMs`; the inventory of every such + * request is in the release notes for #1267. + * + * Its limit: it reads source text. A new synchronous AI route called from the + * browser with a fixed `AbortSignal.timeout(...)` is not found by name here; + * `use-insights-advisor-force.test.ts` proves the behaviour for the + * regenerate with fake timers. + * + * Mutation check: put `FORCE_ADVISOR_TIMEOUT_MS = 45_000` back into the + * advisor, or drop the `signal` from one about-me save, and this goes red. + */ +import { readFileSync } from "node:fs"; +import { join } from "node:path"; + +import { describe, expect, it } from "vitest"; + +import { walkSourceFiles } from "./helpers/source-files"; + +const SRC = join(process.cwd(), "src"); + +function stripComments(source: string): string { + return source + .replace(/\/\*[\s\S]*?\*\//g, "") + .replace(/(^|[^:])\/\/.*$/gm, "$1"); +} + +/** The text of the call that starts at `open` (the index of its `(`). */ +function callText(source: string, open: number): string { + let depth = 0; + for (let i = open; i < source.length; i += 1) { + if (source[i] === "(") depth += 1; + else if (source[i] === ")") { + depth -= 1; + if (depth === 0) return source.slice(open, i + 1); + } + } + return source.slice(open); +} + +describe("browser aborts for model-backed requests", () => { + it("the force regenerate derives its abort from the setting", () => { + const source = stripComments( + readFileSync( + join(SRC, "components/insights/use-insights-advisor.ts"), + "utf8", + ), + ); + expect(source).not.toMatch(/FORCE_ADVISOR_TIMEOUT_MS/); + expect(source).toMatch( + /options\.force\s*\?\s*forceAdvisorTimeoutMs\(options\.responseTimeoutMs\)/, + ); + expect(source).toMatch( + /fetchAdvisor\(\{\s*force:\s*true,\s*responseTimeoutMs\s*\}\)/, + ); + }); + + it("every about-me save carries the setting-derived signal", () => { + const sites: string[] = []; + const missing: string[] = []; + for (const rel of walkSourceFiles(SRC, { floor: 3000 })) { + if (rel.includes("__tests__/") || rel.startsWith("generated/")) continue; + if (!rel.startsWith("components/") && !rel.startsWith("app/")) continue; + const source = stripComments(readFileSync(join(SRC, rel), "utf8")); + const pattern = + /apiPut(?:<[^>]*>)?\s*\(\s*["']\/api\/coach\/about-me["']/g; + for (const match of source.matchAll(pattern)) { + const open = (match.index ?? 0) + match[0].indexOf("("); + sites.push(rel); + if ( + !/signal:\s*AbortSignal\.timeout\(saveTimeoutMs\)/.test( + callText(source, open), + ) + ) { + missing.push(rel); + } + } + } + // Six saves today; a matcher that finds none must not read as clean. + expect(sites.length).toBeGreaterThanOrEqual(6); + expect(missing).toEqual([]); + }); +}); diff --git a/src/components/insights/__tests__/use-insights-advisor-force.test.ts b/src/components/insights/__tests__/use-insights-advisor-force.test.ts index 5348f35b5..880a3022d 100644 --- a/src/components/insights/__tests__/use-insights-advisor-force.test.ts +++ b/src/components/insights/__tests__/use-insights-advisor-force.test.ts @@ -7,7 +7,7 @@ * button reads as doing nothing (one half of the recurring * "no briefing today" chain). The read path stays a plain GET. */ -import { describe, it, expect, vi, beforeEach } from "vitest"; +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; const apiFetchRaw = vi.fn(); @@ -15,7 +15,7 @@ vi.mock("@/lib/api/api-fetch", () => ({ apiFetchRaw: (...a: unknown[]) => apiFetchRaw(...a), })); -import { fetchAdvisor } from "../use-insights-advisor"; +import { fetchAdvisor, forceAdvisorTimeoutMs } from "../use-insights-advisor"; function jsonResponse(data: unknown): Response { return { @@ -74,3 +74,56 @@ describe("fetchAdvisor — explicit regenerate forces a generation", () => { expect(result.payload?.dailyBriefing).toBeNull(); }); }); + +describe("fetchAdvisor — the regenerate waits as long as the response timeout allows (#1267)", () => { + afterEach(() => { + vi.useRealTimers(); + }); + + /** A POST that only ends when its signal aborts, like a slow model. */ + function hangUntilAborted() { + apiFetchRaw.mockImplementation( + (_url: string, init: { signal: AbortSignal }) => + new Promise((_resolve, reject) => { + init.signal.addEventListener("abort", () => + reject(new DOMException("aborted", "AbortError")), + ); + }), + ); + } + + it("is still waiting at 45 s and at 10 min with a 600 s setting", async () => { + vi.useFakeTimers(); + hangUntilAborted(); + let settled: unknown = null; + void fetchAdvisor({ force: true, responseTimeoutMs: 600_000 }).then( + (r) => (settled = r), + ); + await vi.advanceTimersByTimeAsync(45_001); + expect(settled).toBeNull(); + await vi.advanceTimersByTimeAsync(10 * 60_000); + expect(settled).toBeNull(); + await vi.advanceTimersByTimeAsync(forceAdvisorTimeoutMs(600_000)); + expect(settled).toEqual({ payload: null, outcome: "timeout" }); + }); + + it("never waits less than the briefing's own 180 s per call", () => { + // 60 s is what the payload publishes when nothing is set; the route still + // gives each briefing call 180 s. + expect(forceAdvisorTimeoutMs(60_000)).toBeGreaterThanOrEqual(2 * 180_000); + expect(forceAdvisorTimeoutMs(undefined)).toBe( + forceAdvisorTimeoutMs(60_000), + ); + }); + + it("keeps the read path at its fast 8 s bound", async () => { + vi.useFakeTimers(); + hangUntilAborted(); + let settled: unknown = null; + void fetchAdvisor({ responseTimeoutMs: 600_000 }).then( + (r) => (settled = r), + ); + await vi.advanceTimersByTimeAsync(8_001); + expect(settled).toEqual({ payload: null, outcome: "timeout" }); + }); +}); diff --git a/src/components/insights/insights-tab-strip.tsx b/src/components/insights/insights-tab-strip.tsx index 9c873a1c5..645ec9153 100644 --- a/src/components/insights/insights-tab-strip.tsx +++ b/src/components/insights/insights-tab-strip.tsx @@ -713,7 +713,7 @@ function InsightsTabStripImpl({ // // Refs #786 — the edge watches the COMBINED busy state (`regenerating || // regenerateSettling`): a timed-out regenerate flows straight into - // settling, so the spinner never falls at 45 s and NO error toast fires + // settling, so the spinner never falls at the client abort and NO error toast fires // while the server may still deliver. The edge fires once settling // resolves, with the proven outcome — `"fresh"` (success) or // `"settle-failed"` (its own honest error copy). The mapping is the pure diff --git a/src/components/insights/use-insights-advisor.ts b/src/components/insights/use-insights-advisor.ts index 9633a652e..10fa7bc60 100644 --- a/src/components/insights/use-insights-advisor.ts +++ b/src/components/insights/use-insights-advisor.ts @@ -17,7 +17,9 @@ import type { } from "@/lib/ai/schema"; import { queryKeys, refetchInactiveDailyReads } from "@/lib/query-keys"; import { apiFetchRaw } from "@/lib/api/api-fetch"; -import { useAiCapability } from "@/hooks/use-ai-capability"; +import { useAiCapability, useAiProviderState } from "@/hooks/use-ai-capability"; +import { AI_BUDGETS } from "@/lib/ai/ai-budgets"; +import { clientAbortMs } from "@/lib/ai/effective-timeout"; import type { BriefingFailureClass } from "@/lib/insights/briefing-failure-marker"; /** @@ -116,16 +118,29 @@ const ADVISOR_TIMEOUT_MS = 8_000; /** * v1.15.18 — the user-initiated regenerate (`force`) POSTs an INLINE - * generation: a ~1500-token warm completion that routinely runs longer - * than the 8 s read budget. Bounding the force branch at 8 s silently - * discarded a slow-but-successful generation — the abort returned a null - * payload, the mutation only invalidated (re-reading the OLD cache), yet - * the server had often already written the fresh briefing. Give the force - * branch a generous 45 s client budget so a slow success is kept, not - * dropped. The READ path stays fast at 8 s — only the explicit user tap - * may wait. Per `.planning/v1.15.18-daily-briefing-audit.md` Fix 1b. + * generation that routinely runs longer than the 8 s read budget; bounding it + * at 8 s discarded a slow-but-successful generation. + * + * #1267 — the budget follows the person's response-timeout setting instead of + * a constant 45 s, which cut every regenerate on a slow local model while the + * server still had minutes left. The route makes up to two sequential model + * calls (the generation and its grounding retry), and a reasoning model that + * answered nothing is asked once more inside the first, so three calls at the + * per-call timeout, at least the briefing's own 180 s default each. Never + * shorter than the server: the abort is only for a request nobody answers. */ -const FORCE_ADVISOR_TIMEOUT_MS = 45_000; +const FORCE_ADVISOR_MODEL_CALLS = 3; + +/** The client budget for a forced regenerate (exported for the test). */ +export function forceAdvisorTimeoutMs( + responseTimeoutMs: number | null | undefined, +): number { + return clientAbortMs( + responseTimeoutMs, + FORCE_ADVISOR_MODEL_CALLS, + AI_BUDGETS.comprehensive.timeoutMs, + ); +} /** * v1.16.7 — poll cadence + ceiling while the server reports @@ -146,7 +161,7 @@ export const ADVISOR_REVALIDATE_POLL_MAX_ATTEMPTS = 10; * * Async-truth contract, client end (the server end is documented at the head * of `src/app/api/insights/generate/route.ts`): the force POST generates - * INLINE and keeps running after the client aborts at 45 s — the server + * INLINE and keeps running after the client aborts — the server * usually writes the fresh cache moments later. A page that knows work is * running must keep saying so until it can prove an outcome, so the timeout * is NOT surfaced as an error. Instead the hook remembers the baseline it was @@ -276,12 +291,18 @@ interface AdvisorFetchResult { * no-briefing-today chain). The wire-contract test pins it. */ export async function fetchAdvisor( - options: { force?: boolean } = {}, + options: { + force?: boolean; + /** `ai.provider.responseTimeoutMs` from the account payload. */ + responseTimeoutMs?: number | null; + } = {}, ): Promise { const controller = new AbortController(); const timeoutHandle = setTimeout( () => controller.abort(), - options.force ? FORCE_ADVISOR_TIMEOUT_MS : ADVISOR_TIMEOUT_MS, + options.force + ? forceAdvisorTimeoutMs(options.responseTimeoutMs) + : ADVISOR_TIMEOUT_MS, ); let res: Response; try { @@ -437,6 +458,7 @@ export function useInsightsAdvisorQuery( enabled: boolean, ): UseInsightsAdvisorResult { const briefing = useAiCapability("briefing"); + const { responseTimeoutMs } = useAiProviderState(); const queryClient = useQueryClient(); // Refs #786 — settling state for a timed-out force regenerate (see the // contract comment above `AdvisorSettleState`). `settleOutcome` carries the @@ -500,7 +522,7 @@ export function useInsightsAdvisorQuery( } const mutation = useMutation({ - mutationFn: () => fetchAdvisor({ force: true }), + mutationFn: () => fetchAdvisor({ force: true, responseTimeoutMs }), onSuccess: (result) => { if (result.payload) { // A genuinely fresh generation landed — write it into the shared @@ -509,7 +531,7 @@ export function useInsightsAdvisorQuery( } else { // No fresh payload (timeout / no-provider / transient). The server's // inline POST may STILL have written a fresh briefing after the - // client gave up at 45 s, so re-read the GET to converge — but the + // client gave up, so re-read the GET to converge — but the // honest toast is gated on `regenerateOutcome` below, not on this // invalidate. // diff --git a/src/components/onboarding/baseline-form.tsx b/src/components/onboarding/baseline-form.tsx index ba06c061b..b73fdf595 100644 --- a/src/components/onboarding/baseline-form.tsx +++ b/src/components/onboarding/baseline-form.tsx @@ -15,6 +15,7 @@ import { } from "@/lib/profile/height-unit-display"; import { useTranslations } from "@/lib/i18n/context"; import { apiGet, apiPut } from "@/lib/api/api-fetch"; +import { useAboutMeSaveTimeoutMs } from "@/hooks/use-ai-capability"; import { localizedApiError } from "@/lib/api/localized-error"; import { AnamnesisCard, @@ -153,6 +154,7 @@ export function BaselineForm({ allergies: "", }); const [baseAboutMe, setBaseAboutMe] = useState(""); + const saveTimeoutMs = useAboutMeSaveTimeoutMs(); useEffect(() => { let active = true; @@ -229,7 +231,9 @@ export function BaselineForm({ // collapsed card never round-trips). const aboutMeBody = buildAnamnesisAboutMeBody(baseAboutMe, anamnesis); if (aboutMeBody) { - await apiPut("/api/coach/about-me", aboutMeBody); + await apiPut("/api/coach/about-me", aboutMeBody, { + signal: AbortSignal.timeout(saveTimeoutMs), + }); } } await onConfirmed(); diff --git a/src/components/profile/__tests__/height-entry-surfaces.test.tsx b/src/components/profile/__tests__/height-entry-surfaces.test.tsx index 86661e422..03c9310c2 100644 --- a/src/components/profile/__tests__/height-entry-surfaces.test.tsx +++ b/src/components/profile/__tests__/height-entry-surfaces.test.tsx @@ -28,6 +28,13 @@ vi.mock("@tanstack/react-query", () => ({ useMutation: () => ({ mutate: vi.fn(), isPending: false }), })); +// #1267 — the about-me save's abort reads the account payload; the +// react-query mock above has no client to read it from. +vi.mock("@/hooks/use-ai-capability", async (importOriginal) => ({ + ...(await importOriginal()), + useAboutMeSaveTimeoutMs: () => 90_000, +})); + vi.mock("sonner", () => ({ toast: { error: vi.fn(), success: vi.fn() }, })); diff --git a/src/components/records/__tests__/ai-profile-inclusion-manager.test.tsx b/src/components/records/__tests__/ai-profile-inclusion-manager.test.tsx index 8cdb6c73e..3bbfc0744 100644 --- a/src/components/records/__tests__/ai-profile-inclusion-manager.test.tsx +++ b/src/components/records/__tests__/ai-profile-inclusion-manager.test.tsx @@ -32,6 +32,14 @@ vi.mock("@tanstack/react-query", () => ({ }; }, })); + +// #1267 — the about-me save's abort reads the account payload; the +// react-query mock above has no client to read it from. +vi.mock("@/hooks/use-ai-capability", async (importOriginal) => ({ + ...(await importOriginal()), + useAboutMeSaveTimeoutMs: () => 90_000, +})); + vi.mock("@/lib/api/api-fetch", () => ({ apiGet: vi.fn(), apiPut: vi.fn(), diff --git a/src/components/records/about-me-note-manager.tsx b/src/components/records/about-me-note-manager.tsx index 422fd6001..72ffd8fd0 100644 --- a/src/components/records/about-me-note-manager.tsx +++ b/src/components/records/about-me-note-manager.tsx @@ -38,7 +38,10 @@ import { QueryErrorRow } from "@/components/ui/query-error-row"; import { SettingsInfoTile } from "@/components/settings/_info-tile"; import { Label } from "@/components/ui/label"; import { ConfirmButton } from "@/components/ui/confirm-button"; -import { useAiCapability } from "@/hooks/use-ai-capability"; +import { + useAboutMeSaveTimeoutMs, + useAiCapability, +} from "@/hooks/use-ai-capability"; import { useTranslations } from "@/lib/i18n/context"; import { queryKeys } from "@/lib/query-keys"; import { cn } from "@/lib/utils"; @@ -82,6 +85,7 @@ export function AboutMeNoteManager() { // there is a note to clear. const hasNote = savedNote.length > 0; + const saveTimeoutMs = useAboutMeSaveTimeoutMs(); const save = useMutation({ mutationKey: queryKeys.coachAboutMe(), // Send only the note. `conditions` / `coachFocus` / `allergies` are @@ -93,6 +97,7 @@ export function AboutMeNoteManager() { return apiPut( "/api/coach/about-me", withBaseToken(input, query.data?.updatedAt ?? undefined), + { signal: AbortSignal.timeout(saveTimeoutMs) }, ); }, onSuccess: (next) => { diff --git a/src/components/records/ai-profile-inclusion-manager.tsx b/src/components/records/ai-profile-inclusion-manager.tsx index 961faf4aa..e29f3bb62 100644 --- a/src/components/records/ai-profile-inclusion-manager.tsx +++ b/src/components/records/ai-profile-inclusion-manager.tsx @@ -11,6 +11,7 @@ import { toastWrittenOutcome } from "@/components/outcome/outcome-toast"; import { QueryErrorCard } from "@/components/ui/query-error-card"; import { Switch } from "@/components/ui/switch"; import { apiGet, apiPut } from "@/lib/api/api-fetch"; +import { useAboutMeSaveTimeoutMs } from "@/hooks/use-ai-capability"; import { isConflict, readUpdatedAtToken, @@ -72,6 +73,7 @@ export function AiProfileInclusionManager() { (section) => draft.includes(section) !== saved.includes(section), ); + const saveTimeoutMs = useAboutMeSaveTimeoutMs(); const save = useMutation({ mutationKey: queryKeys.coachAboutMe(), mutationFn: (sections: HealthProfileAiSection[]) => @@ -84,6 +86,7 @@ export function AiProfileInclusionManager() { readUpdatedAtToken(queryClient, queryKeys.coachAboutMe()) ?? undefined, ), + { signal: AbortSignal.timeout(saveTimeoutMs) }, ), onSuccess: () => { setDraft(null); diff --git a/src/components/records/allergy-free-text-note.tsx b/src/components/records/allergy-free-text-note.tsx index c974bdbef..aac23154c 100644 --- a/src/components/records/allergy-free-text-note.tsx +++ b/src/components/records/allergy-free-text-note.tsx @@ -36,6 +36,7 @@ import { useTranslations } from "@/lib/i18n/context"; import { queryKeys } from "@/lib/query-keys"; import { cn } from "@/lib/utils"; import { apiGet, apiPut } from "@/lib/api/api-fetch"; +import { useAboutMeSaveTimeoutMs } from "@/hooks/use-ai-capability"; import { withBaseToken, isConflict } from "@/lib/api/optimistic-token"; const FALLBACK_FIELD_MAX_CHARS = 500; @@ -75,6 +76,7 @@ export function AllergyFreeTextNote({ const value = draft ?? savedLine; const dirty = value !== savedLine; + const saveTimeoutMs = useAboutMeSaveTimeoutMs(); const save = useMutation({ mutationKey: queryKeys.coachAboutMe(), // Send only the allergies line — omitted fields are preserved server-side @@ -84,6 +86,7 @@ export function AllergyFreeTextNote({ return apiPut( "/api/coach/about-me", withBaseToken(input, query.data?.updatedAt ?? undefined), + { signal: AbortSignal.timeout(saveTimeoutMs) }, ); }, onSuccess: () => { diff --git a/src/components/records/conditions-manager.tsx b/src/components/records/conditions-manager.tsx index 42b2f2cdb..c876c1105 100644 --- a/src/components/records/conditions-manager.tsx +++ b/src/components/records/conditions-manager.tsx @@ -35,6 +35,7 @@ import { useTranslations } from "@/lib/i18n/context"; import { queryKeys } from "@/lib/query-keys"; import { cn } from "@/lib/utils"; import { apiGet, apiPut } from "@/lib/api/api-fetch"; +import { useAboutMeSaveTimeoutMs } from "@/hooks/use-ai-capability"; const FALLBACK_FIELD_MAX_CHARS = 500; @@ -76,13 +77,18 @@ export function ConditionsManager() { value.conditions !== saved.conditions || value.coachFocus !== saved.coachFocus; + const saveTimeoutMs = useAboutMeSaveTimeoutMs(); const save = useMutation({ mutationKey: queryKeys.coachAboutMe(), mutationFn: async (input: { conditions: string; coachFocus: string }) => - apiPut("/api/coach/about-me", { - conditions: input.conditions, - coachFocus: input.coachFocus, - }), + apiPut( + "/api/coach/about-me", + { + conditions: input.conditions, + coachFocus: input.coachFocus, + }, + { signal: AbortSignal.timeout(saveTimeoutMs) }, + ), onSuccess: () => { toast.success(t("records.conditions.savedToast")); setDrafts({ conditions: null, coachFocus: null }); diff --git a/src/components/records/free-text-take-over.tsx b/src/components/records/free-text-take-over.tsx index 400024b04..e52f2f608 100644 --- a/src/components/records/free-text-take-over.tsx +++ b/src/components/records/free-text-take-over.tsx @@ -26,6 +26,7 @@ import { Input } from "@/components/ui/input"; import { Label } from "@/components/ui/label"; import { ResponsiveSheet } from "@/components/ui/responsive-sheet"; import { apiGet, apiPost, apiPut } from "@/lib/api/api-fetch"; +import { useAboutMeSaveTimeoutMs } from "@/hooks/use-ai-capability"; import { withBaseToken } from "@/lib/api/optimistic-token"; import { showErrorToast } from "@/lib/errors/error-toast"; import { useTranslations } from "@/lib/i18n/context"; @@ -90,12 +91,14 @@ export function FreeTextTakeOver({ showErrorToast(t("records.life.takeOver.saveError"), { cause: error }), }); + const saveTimeoutMs = useAboutMeSaveTimeoutMs(); const clear = useMutation({ mutationKey: queryKeys.coachAboutMe(), mutationFn: () => apiPut( "/api/coach/about-me", withBaseToken({ [field]: "" }, aboutMe.data?.updatedAt ?? undefined), + { signal: AbortSignal.timeout(saveTimeoutMs) }, ), onSuccess: () => { toastWrittenOutcome("success", t("records.life.takeOver.clearedToast")); diff --git a/src/components/settings/__tests__/coach-links-without-coach.test.tsx b/src/components/settings/__tests__/coach-links-without-coach.test.tsx index 5025a769a..fb011f619 100644 --- a/src/components/settings/__tests__/coach-links-without-coach.test.tsx +++ b/src/components/settings/__tests__/coach-links-without-coach.test.tsx @@ -17,6 +17,8 @@ vi.mock("@/hooks/use-ai-capability", () => ({ }), // v1.41 — the thinking depth reads the resolved block; none here. useCoachReasoning: () => null, + // #1267 — the about-me save's abort, from the response-timeout setting. + useAboutMeSaveTimeoutMs: () => 90_000, })); vi.mock("next/navigation", () => ({ useRouter: () => ({ push: vi.fn(), replace: vi.fn() }), diff --git a/src/components/settings/__tests__/settings-read-error-family.test.tsx b/src/components/settings/__tests__/settings-read-error-family.test.tsx index 18f03becd..0acb1a061 100644 --- a/src/components/settings/__tests__/settings-read-error-family.test.tsx +++ b/src/components/settings/__tests__/settings-read-error-family.test.tsx @@ -33,6 +33,8 @@ vi.mock("@/hooks/use-ai-capability", () => ({ reason: null, onDeviceAllowed: true, }), + // #1267 — the about-me save's abort, from the response-timeout setting. + useAboutMeSaveTimeoutMs: () => 90_000, })); vi.mock("next/navigation", () => ({ diff --git a/src/hooks/use-ai-capability.ts b/src/hooks/use-ai-capability.ts index 2adddde62..f725aeda5 100644 --- a/src/hooks/use-ai-capability.ts +++ b/src/hooks/use-ai-capability.ts @@ -8,7 +8,10 @@ import type { AiConsentState, AiProviderState, } from "@/lib/ai/capabilities/types"; -import { PROVIDER_DEFAULT_TIMEOUT_MS } from "@/lib/ai/effective-timeout"; +import { + clientAbortMs, + PROVIDER_DEFAULT_TIMEOUT_MS, +} from "@/lib/ai/effective-timeout"; import type { CoachReasoningState } from "@/lib/ai/reasoning/resolve"; import type { AiUsageState } from "@/lib/ai/usage-level"; @@ -69,6 +72,18 @@ function useAiProviderStateInner(): AiProviderState { return user?.ai?.provider ?? NO_PROVIDER_STATE; } +/** + * #1267 — the timeout for a save of `PUT /api/coach/about-me`. A save that + * changes the text or the shared sections waits on one model call (the + * clarifying questions) before it answers, so the fixed 15 s request default + * reported a failed save while a slow model was still within the person's + * response timeout, and the server went on to commit it. + */ +export function useAboutMeSaveTimeoutMs(): number { + const { responseTimeoutMs } = useAiProviderState(); + return clientAbortMs(responseTimeoutMs, 1); +} + /** * v1.41 — the Coach's thinking depth, resolved on the server (`coachReasoning` * on `GET /api/auth/me`), or `null` while it is not known: `/me` loading, no diff --git a/src/lib/ai/__tests__/effective-timeout.test.ts b/src/lib/ai/__tests__/effective-timeout.test.ts index 96a95bf13..d93d2291a 100644 --- a/src/lib/ai/__tests__/effective-timeout.test.ts +++ b/src/lib/ai/__tests__/effective-timeout.test.ts @@ -1,5 +1,10 @@ import { describe, it, expect } from "vitest"; -import { resolveEffectiveTimeoutMs } from "../effective-timeout"; +import { + CLIENT_ABORT_MARGIN_MS, + clientAbortMs, + PROVIDER_DEFAULT_TIMEOUT_MS, + resolveEffectiveTimeoutMs, +} from "../effective-timeout"; const BUDGET = 120_000; @@ -24,3 +29,28 @@ describe("resolveEffectiveTimeoutMs", () => { expect(resolveEffectiveTimeoutMs(45, 60_000)).toBe(45_000); }); }); + +describe("clientAbortMs (#1267)", () => { + it("waits the published setting per call plus the margin", () => { + expect(clientAbortMs(600_000, 2)).toBe( + 2 * 600_000 + CLIENT_ABORT_MARGIN_MS, + ); + }); + + it("never waits less per call than the route's own longer default", () => { + // 60 s is what the payload publishes when nothing is set; the briefing + // route gives each call 180 s then. + expect(clientAbortMs(60_000, 2, 180_000)).toBe( + 2 * 180_000 + CLIENT_ABORT_MARGIN_MS, + ); + expect(clientAbortMs(600_000, 2, 180_000)).toBe( + 2 * 600_000 + CLIENT_ABORT_MARGIN_MS, + ); + }); + + it("reads a missing value as the provider default", () => { + expect(clientAbortMs(undefined, 1)).toBe( + PROVIDER_DEFAULT_TIMEOUT_MS + CLIENT_ABORT_MARGIN_MS, + ); + }); +}); diff --git a/src/lib/ai/effective-timeout.ts b/src/lib/ai/effective-timeout.ts index 9f97754bd..634f0c0ff 100644 --- a/src/lib/ai/effective-timeout.ts +++ b/src/lib/ai/effective-timeout.ts @@ -78,14 +78,22 @@ export const CLIENT_ABORT_MARGIN_MS = 30_000; * account payload published. Never recomputes the setting: an absent or * malformed value (a payload from before the field existed) reads as the * server's own default. + * + * `surfaceDefaultMs` is the route's own per-call default when it is longer + * than the provider default (the briefing's 180 s). The published value + * cannot say whether the person set it or it is the 60 s default, so the + * browser waits for the longer of the two: it may wait longer than the + * server needs, never shorter (#1267). */ export function clientAbortMs( responseTimeoutMs: number | null | undefined, modelCalls: number, + surfaceDefaultMs?: number, ): number { - const perCall = + const published = typeof responseTimeoutMs === "number" && responseTimeoutMs > 0 ? responseTimeoutMs : PROVIDER_DEFAULT_TIMEOUT_MS; + const perCall = Math.max(published, surfaceDefaultMs ?? 0); return perCall * modelCalls + CLIENT_ABORT_MARGIN_MS; }