From 5ee1b3e28fc470a008797a1e156457512471cd92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Fri, 2 Oct 2026 23:01:36 +0200 Subject: [PATCH] fix(ai): run every model call under the response-timeout setting The response-timeout setting reached the briefing, the status notes and the Coach, which threaded it by hand. Document reads, document extraction, lab OCR and lab staging called the provider without a timeout, so every client fell back to its own 60 s literal and a slow self-hosted model was cut off however high the setting stood. The setting now rides on the provider instance. resolveProvider, resolveProviderChain and resolveProviderForTest stamp the record's aiResponseTimeoutSeconds on everything they return, and each wire client reads its ceiling through callTimeoutMs: the setting, else the surface's own timeoutMs, else 60 s. A call site no longer has to pass it, so a new one cannot forget it. The connection test runs under the same ceiling a read does. The Coach nudge tick and the reaction line keep their short ceilings (timeoutPolicy: "surface-ceiling"): the tick runs accounts one after another under a shared budget, the reaction line's claim lease is shorter than the largest setting, and both have deterministic copy that stands in. /api/auth/me publishes the effective value as ai.provider.responseTimeoutMs, so a client can size its own request timeout from it instead of guessing. Refs #1090 --- docs/api/openapi.yaml | 9 + .../getting-started-checklist.test.tsx | 15 +- .../settings/ai/response-timeout-card.tsx | 12 +- .../__tests__/use-ai-capability.test.tsx | 1 + src/hooks/use-ai-capability.ts | 2 + .../provider-response-timeout.test.ts | 85 +++++++ .../response-timeout-binding.test.ts | 223 ++++++++++++++++++ src/lib/ai/anthropic-client.ts | 7 +- .../ai/capabilities/__tests__/resolve.test.ts | 10 + src/lib/ai/capabilities/resolve.ts | 21 +- src/lib/ai/capabilities/types.ts | 9 + src/lib/ai/codex-client.ts | 12 +- src/lib/ai/effective-timeout.ts | 86 ++++++- src/lib/ai/local-client.ts | 9 +- src/lib/ai/openai-client.ts | 7 +- src/lib/ai/provider.ts | 62 ++++- src/lib/ai/types.ts | 24 ++ src/lib/jobs/coach-nudge-ai.ts | 4 + src/lib/jobs/reaction-line.ts | 4 + src/lib/openapi/routes/profile.ts | 7 + .../auth-me-ai-capabilities.test.ts | 15 ++ 21 files changed, 588 insertions(+), 36 deletions(-) create mode 100644 src/lib/ai/__tests__/provider-response-timeout.test.ts create mode 100644 src/lib/ai/__tests__/response-timeout-binding.test.ts diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index 4a1bd8893..94a77538c 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -72185,10 +72185,19 @@ components: type: boolean description: "Whether the person in front of the screen may set up a provider for this record: false inside somebody else's record and while the operator's master switch is off. A setup hint is shown only when this is true." + responseTimeoutMs: + type: integer + exclusiveMinimum: 0 + maximum: 9007199254740991 + description: "How long one model call on this record may take before the server gives up, in milliseconds: the record's + response-timeout setting (Settings → AI), else 60000. A client that waits on a model call (a document read, + lab OCR) sizes its own request timeout from this plus a margin, so it never gives up before the server + does." required: - configured - managedBy - canConfigure + - responseTimeoutMs additionalProperties: false description: The account-level companion to the capability map. RecordSessionState: diff --git a/src/components/onboarding/__tests__/getting-started-checklist.test.tsx b/src/components/onboarding/__tests__/getting-started-checklist.test.tsx index 5d6f65c2f..13d7733dc 100644 --- a/src/components/onboarding/__tests__/getting-started-checklist.test.tsx +++ b/src/components/onboarding/__tests__/getting-started-checklist.test.tsx @@ -39,6 +39,7 @@ let providerState: AiProviderState = { configured: true, managedBy: "user", canConfigure: true, + responseTimeoutMs: 60_000, }; vi.mock("@/hooks/use-ai-capability", () => ({ useAiProviderState: () => providerState, @@ -135,11 +136,21 @@ describe(" — rows for an established account", () => it("leaves the AI row out when AI cannot be set up here", () => { // The operator switched AI off, or the record belongs to somebody else: // the list never carries a to-do nobody on this screen can finish. - providerState = { configured: false, managedBy: null, canConfigure: false }; + providerState = { + configured: false, + managedBy: null, + canConfigure: false, + responseTimeoutMs: 60_000, + }; currentUser = user(); const html = render(); expect(html).toContain("4 of 5 done"); - providerState = { configured: true, managedBy: "user", canConfigure: true }; + providerState = { + configured: true, + managedBy: "user", + canConfigure: true, + responseTimeoutMs: 60_000, + }; }); it("keeps the supporting queries enabled while the card can render", () => { diff --git a/src/components/settings/ai/response-timeout-card.tsx b/src/components/settings/ai/response-timeout-card.tsx index c471694e3..386522ec8 100644 --- a/src/components/settings/ai/response-timeout-card.tsx +++ b/src/components/settings/ai/response-timeout-card.tsx @@ -3,12 +3,12 @@ /* ──────────────────────────────────────────────────────────────── * v1.22 (#89) — Response timeout (seconds). * - * A per-user upstream timeout for AI generation, threaded onto the provider - * call (`CompletionParams.timeoutMs`). Surfaced here mainly for local / - * self-hosted backends: an MLX/exo server can take >60 s on the first request - * while it loads the model, which the legacy 60 s default timed out before the - * first token landed. Empty = the built-in comprehensive-briefing default - * (~120 s, `AI_BUDGETS.comprehensive.timeoutMs`). + * A per-user upstream timeout for every model call on the record: the + * resolver binds it to the provider and each client reads its ceiling through + * `callTimeoutMs` (only the nudge tick and the reaction line keep their own + * short ceilings). Surfaced here mainly for local / self-hosted backends: an + * MLX/exo server can take >60 s on the first request while it loads the model. + * Empty = each surface's own default (60 s for most, longer for the briefing). * ──────────────────────────────────────────────────────────────── */ import { useRef, useState, type FormEvent } from "react"; diff --git a/src/hooks/__tests__/use-ai-capability.test.tsx b/src/hooks/__tests__/use-ai-capability.test.tsx index fbc7e9bf8..38a57a787 100644 --- a/src/hooks/__tests__/use-ai-capability.test.tsx +++ b/src/hooks/__tests__/use-ai-capability.test.tsx @@ -76,6 +76,7 @@ describe("useAiCapability", () => { configured: false, managedBy: null, canConfigure: false, + responseTimeoutMs: 60_000, }); }); diff --git a/src/hooks/use-ai-capability.ts b/src/hooks/use-ai-capability.ts index fa8721c30..6a8abd503 100644 --- a/src/hooks/use-ai-capability.ts +++ b/src/hooks/use-ai-capability.ts @@ -7,6 +7,7 @@ import type { AiCapabilityState, AiProviderState, } from "@/lib/ai/capabilities/types"; +import { PROVIDER_DEFAULT_TIMEOUT_MS } from "@/lib/ai/effective-timeout"; /** * The web reader for the `ai` block on `GET /api/auth/me`. @@ -34,6 +35,7 @@ const NO_PROVIDER_STATE: AiProviderState = Object.freeze({ configured: false, managedBy: null, canConfigure: false, + responseTimeoutMs: PROVIDER_DEFAULT_TIMEOUT_MS, }); /** One capability, for the record this browser is inside. */ diff --git a/src/lib/ai/__tests__/provider-response-timeout.test.ts b/src/lib/ai/__tests__/provider-response-timeout.test.ts new file mode 100644 index 000000000..006585c72 --- /dev/null +++ b/src/lib/ai/__tests__/provider-response-timeout.test.ts @@ -0,0 +1,85 @@ +/** + * Every exported resolver hands out providers carrying the record owner's + * response-timeout setting. This is the end of the pipe that makes the + * setting unforgettable at the call site: the surface functions never see it, + * the provider they are given already does. + */ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +vi.mock("@/lib/db", () => ({ + prisma: { + user: { findUnique: vi.fn(), update: vi.fn() }, + appSettings: { findUnique: vi.fn() }, + }, +})); +vi.mock("@/lib/crypto", () => ({ + decrypt: vi.fn((v: string) => `decrypted:${v}`), + encrypt: vi.fn((v: string) => `encrypted:${v}`), +})); + +import { prisma } from "@/lib/db"; +import { + resolveProvider, + resolveProviderChain, + resolveProviderForTest, +} from "../provider"; + +const LOCAL_ROW = { + aiProvider: "LOCAL", + aiModel: "llama", + aiBaseUrl: "https://llm.example.org/v1", + aiAnthropicKeyEncrypted: null, + aiLocalKeyEncrypted: null, + aiOpenaiKeyEncrypted: null, + aiCompatBaseUrl: null, + aiCompatKeyEncrypted: null, + aiCompatModel: null, + role: "USER", + aiProviderChain: [{ providerType: "local", priority: 1, enabled: true }], + useCentralCodex: false, + managedProfileAt: null, +}; + +function withSetting(seconds: number | null) { + vi.mocked(prisma.user.findUnique).mockResolvedValue({ + ...LOCAL_ROW, + aiResponseTimeoutSeconds: seconds, + } as never); + vi.mocked(prisma.appSettings.findUnique).mockResolvedValue(null as never); +} + +beforeEach(() => { + vi.resetAllMocks(); +}); + +describe("the resolvers bind the record's response-timeout setting", () => { + it("resolveProvider", async () => { + withSetting(300); + const provider = await resolveProvider("user-1"); + expect(provider.type).toBe("local"); + expect(provider.responseTimeoutSeconds).toBe(300); + }); + + it("resolveProviderChain, every entry", async () => { + withSetting(300); + const chain = await resolveProviderChain("user-1"); + expect(chain.length).toBeGreaterThan(0); + for (const entry of chain) { + expect(entry.instance.responseTimeoutSeconds).toBe(300); + } + }); + + it("resolveProviderForTest, so the connection test waits as long as a read", async () => { + withSetting(300); + const provider = await resolveProviderForTest("user-1", { + provider: "LOCAL", + }); + expect(provider.responseTimeoutSeconds).toBe(300); + }); + + it("binds nothing when the setting is unset", async () => { + withSetting(null); + const provider = await resolveProvider("user-1"); + expect(provider.responseTimeoutSeconds).toBeNull(); + }); +}); diff --git a/src/lib/ai/__tests__/response-timeout-binding.test.ts b/src/lib/ai/__tests__/response-timeout-binding.test.ts new file mode 100644 index 000000000..5da896b28 --- /dev/null +++ b/src/lib/ai/__tests__/response-timeout-binding.test.ts @@ -0,0 +1,223 @@ +/** + * The person's response-timeout setting reaches every model call. + * + * A slow self-hosted model could need minutes for a five-page lab report, and + * raising Settings → AI → response timeout did nothing for it: the document + * read, the document extraction and the lab OCR called the provider without a + * `timeoutMs`, so every client fell back to its own 60 s. The setting now rides + * on the provider instance (stamped by the resolver) and every client reads its + * ceiling through `callTimeoutMs`, so these surfaces get it without passing it. + * + * These cases drive the real clients and the real surface functions down to + * the `safeFetch` call and read the timeout off it, so they hold the pipe and + * not just the helper. + */ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const safeFetch = vi.fn(); +vi.mock("@/lib/safe-fetch", () => ({ + safeFetch: (...a: unknown[]) => safeFetch(...a), +})); +vi.mock("../local-host-allowlist", () => ({ + aiEgressPolicyFor: () => ({}), + isLocalAiHostAllowed: () => true, +})); +vi.mock("@/lib/db", () => ({ prisma: {} })); + +import { AnthropicClient } from "../anthropic-client"; +import { CodexClient } from "../codex-client"; +import { + bindResponseTimeout, + callTimeoutMs, + clientAbortMs, + PROVIDER_DEFAULT_TIMEOUT_MS, +} from "../effective-timeout"; +import { LocalOpenAICompatibleClient } from "../local-client"; +import { OpenAIClient } from "../openai-client"; +import { singleUserTurn, type AIProvider } from "../types"; +import { runDocumentAssist } from "@/lib/documents/assist"; +import { + runDocumentSummary, + transcribeDocument, +} from "@/lib/documents/describe"; +import { runInboundExtraction } from "@/lib/documents/extract"; +import { runOcrExtraction } from "@/lib/labs/ocr-extract"; + +const STOP = new Error("stop after the fetch was dialled"); + +beforeEach(() => { + safeFetch.mockReset(); + safeFetch.mockRejectedValue(STOP); +}); + +/** The `timeoutMs` the most recent `safeFetch` call was dialled with. */ +function dialledTimeout(): number { + const call = safeFetch.mock.calls.at(-1); + expect(call, "safeFetch was never called").toBeDefined(); + return (call![2] as { timeoutMs: number }).timeoutMs; +} + +function local(): LocalOpenAICompatibleClient { + return new LocalOpenAICompatibleClient({ + apiKey: null, + model: "llama", + baseUrl: "https://llm.example.org/v1", + }); +} + +const CLIENTS: Record AIProvider> = { + local, + openai: () => + new OpenAIClient({ + apiKey: "k", + model: "gpt-4o", + baseUrl: "https://api.openai.com/v1", + }), + anthropic: () => + new AnthropicClient({ apiKey: "k", model: "claude-sonnet-4-6" }), + codex: () => + new CodexClient({ + accessToken: "t", + accountId: "a", + onTokenRefresh: vi.fn(), + slugChain: ["gpt-5.5"], + }), +}; + +describe("callTimeoutMs", () => { + it("lets the setting win over the surface value and the default", () => { + expect(callTimeoutMs({}, null)).toBe(PROVIDER_DEFAULT_TIMEOUT_MS); + expect(callTimeoutMs({ timeoutMs: 180_000 }, null)).toBe(180_000); + expect(callTimeoutMs({}, 300)).toBe(300_000); + expect(callTimeoutMs({ timeoutMs: 180_000 }, 30)).toBe(30_000); + }); + + it("keeps a surface ceiling where the surface says so", () => { + expect( + callTimeoutMs( + { timeoutMs: 9_000, timeoutPolicy: "surface-ceiling" }, + 600, + ), + ).toBe(9_000); + }); +}); + +describe("bindResponseTimeout", () => { + it("stamps a positive setting and clears an unset one", () => { + const p = local(); + expect(bindResponseTimeout(p, 240).responseTimeoutSeconds).toBe(240); + expect(bindResponseTimeout(p, null).responseTimeoutSeconds).toBeNull(); + expect(bindResponseTimeout(p, 0).responseTimeoutSeconds).toBeNull(); + }); +}); + +describe("clientAbortMs", () => { + it("waits for every model call plus the margin", () => { + expect(clientAbortMs(300_000, 1)).toBe(330_000); + expect(clientAbortMs(300_000, 3)).toBe(930_000); + }); + + it("reads a missing or malformed value as the server default", () => { + expect(clientAbortMs(undefined, 2)).toBe(2 * 60_000 + 30_000); + expect(clientAbortMs(0, 1)).toBe(90_000); + }); +}); + +describe.each(Object.entries(CLIENTS))("the %s client", (_name, make) => { + it("dials with the bound setting, over the surface value", async () => { + const provider = bindResponseTimeout(make(), 420); + await expect( + provider.generateCompletion( + singleUserTurn({ system: "s", user: "u", timeoutMs: 120_000 }), + ), + ).rejects.toBeDefined(); + expect(dialledTimeout()).toBe(420_000); + }); + + it("dials with the default when nothing is set", async () => { + await expect( + make().generateCompletion(singleUserTurn({ system: "s", user: "u" })), + ).rejects.toBeDefined(); + expect(dialledTimeout()).toBe(60_000); + }); + + it("keeps a surface ceiling", async () => { + const provider = bindResponseTimeout(make(), 420); + await expect( + provider.generateCompletion( + singleUserTurn({ + system: "s", + user: "u", + timeoutMs: 9_000, + timeoutPolicy: "surface-ceiling", + }), + ), + ).rejects.toBeDefined(); + expect(dialledTimeout()).toBe(9_000); + }); +}); + +describe("the surfaces that used to fall back to 60 s", () => { + const image = [{ mediaType: "image/png" as const, dataBase64: "AAAA" }]; + + function bound(): AIProvider { + return bindResponseTimeout(local(), 300); + } + + it("document summary", async () => { + await expect( + runDocumentSummary({ + provider: bound(), + providerType: "local", + images: image, + locale: "en", + }), + ).rejects.toBe(STOP); + expect(dialledTimeout()).toBe(300_000); + }); + + it("document transcription (Read with AI)", async () => { + await expect( + transcribeDocument({ + provider: bound(), + providerType: "local", + images: image, + }), + ).rejects.toBe(STOP); + expect(dialledTimeout()).toBe(300_000); + }); + + it("document filing suggestion", async () => { + await expect( + runDocumentAssist({ + provider: bound(), + providerType: "local", + images: image, + }), + ).rejects.toBe(STOP); + expect(dialledTimeout()).toBe(300_000); + }); + + it("document extraction, which lab staging runs", async () => { + await expect( + runInboundExtraction({ + provider: bound(), + providerType: "local", + images: image, + }), + ).rejects.toBe(STOP); + expect(dialledTimeout()).toBe(300_000); + }); + + it("lab OCR", async () => { + await expect( + runOcrExtraction({ + userId: "u1", + provider: bound(), + providerType: "local", + images: image, + }), + ).rejects.toBe(STOP); + expect(dialledTimeout()).toBe(300_000); + }); +}); diff --git a/src/lib/ai/anthropic-client.ts b/src/lib/ai/anthropic-client.ts index c26903f90..6f4b460fe 100644 --- a/src/lib/ai/anthropic-client.ts +++ b/src/lib/ai/anthropic-client.ts @@ -1,4 +1,5 @@ import { safeFetch } from "@/lib/safe-fetch"; +import { callTimeoutMs } from "./effective-timeout"; import { extractJsonObject } from "./json-extract"; import type { AIProvider, @@ -157,6 +158,8 @@ function buildAnthropicTools(tools: AiToolDef[]) { export class AnthropicClient implements AIProvider { readonly type = "anthropic" as const; + /** See `AIProvider.responseTimeoutSeconds`; stamped by the resolver. */ + responseTimeoutSeconds: number | null = null; private config: AnthropicClientConfig; constructor(config: AnthropicClientConfig) { @@ -259,9 +262,9 @@ export class AnthropicClient implements AIProvider { // check so a private/metadata address is rejected (SSRF/rebinding). // v1.20.1 — compose the caller's cancel signal (Coach SSE disconnect) so // a mid-generation abort tears the upstream call down early. - // v1.21.5 — honour the caller's per-request timeout override; default 60 s. + // The record owner's response-timeout setting, else the surface value, else 60 s. { - timeoutMs: params.timeoutMs ?? 60_000, + timeoutMs: callTimeoutMs(params, this.responseTimeoutSeconds), requirePublicHost: true, signal: params.signal, }, diff --git a/src/lib/ai/capabilities/__tests__/resolve.test.ts b/src/lib/ai/capabilities/__tests__/resolve.test.ts index d6f1dba90..e631014bd 100644 --- a/src/lib/ai/capabilities/__tests__/resolve.test.ts +++ b/src/lib/ai/capabilities/__tests__/resolve.test.ts @@ -452,9 +452,18 @@ describe("the provider block", () => { configured: true, managedBy: "local", canConfigure: true, + responseTimeoutMs: 60_000, }); }); + it("publishes the record's response-timeout setting as the effective timeout", () => { + const inputs = baseline(); + inputs.provider = { ...inputs.provider, responseTimeoutSeconds: 420 }; + expect(resolveAiProviderState(inputs).responseTimeoutMs).toBe(420_000); + inputs.provider = { ...inputs.provider, responseTimeoutSeconds: null }; + expect(resolveAiProviderState(inputs).responseTimeoutMs).toBe(60_000); + }); + it("offers setup only on one's own record, and only with the master on", () => { for (const recordKind of ["shared", "managed"] as const) { const inputs = baseline(); @@ -473,6 +482,7 @@ describe("the provider block", () => { configured: false, managedBy: null, canConfigure: false, + responseTimeoutMs: 60_000, }); }); }); diff --git a/src/lib/ai/capabilities/resolve.ts b/src/lib/ai/capabilities/resolve.ts index 60c48f119..5b97aee38 100644 --- a/src/lib/ai/capabilities/resolve.ts +++ b/src/lib/ai/capabilities/resolve.ts @@ -13,6 +13,10 @@ * a client acted on it. They stay a per-call refusal with their own codes. */ import { documentProviderRank } from "@/lib/documents/provider-rank"; +import { + PROVIDER_DEFAULT_TIMEOUT_MS, + resolveEffectiveTimeoutMs, +} from "@/lib/ai/effective-timeout"; import { isExternalProvider, isOperatorHeldProvider, @@ -56,6 +60,12 @@ export interface ProviderPresence { /** The person's in-browser OCR opt-in, which lets a text-only provider read a document. */ localOcrEnabled: boolean; managedBy: AiProviderManagedBy | null; + /** + * The record's `aiResponseTimeoutSeconds`, read off the same row. Not a + * presence fact, but the account payload publishes the effective timeout + * beside the provider and this row is already in hand. Absent → unset. + */ + responseTimeoutSeconds?: number | null; } /** Whose record this is, as far as configuring a provider for it goes. */ @@ -332,12 +342,21 @@ export function resolveAiProviderState( inputs: AiCapabilityInputs | null, ): AiProviderState { if (inputs === null) { - return { configured: false, managedBy: null, canConfigure: false }; + return { + configured: false, + managedBy: null, + canConfigure: false, + responseTimeoutMs: PROVIDER_DEFAULT_TIMEOUT_MS, + }; } return { configured: inputs.provider.entries.length > 0, managedBy: inputs.provider.managedBy, canConfigure: inputs.recordKind === "self" && inputs.switches.enabled, + responseTimeoutMs: resolveEffectiveTimeoutMs( + inputs.provider.responseTimeoutSeconds, + PROVIDER_DEFAULT_TIMEOUT_MS, + ), }; } diff --git a/src/lib/ai/capabilities/types.ts b/src/lib/ai/capabilities/types.ts index 13adfceca..1dc6274df 100644 --- a/src/lib/ai/capabilities/types.ts +++ b/src/lib/ai/capabilities/types.ts @@ -313,6 +313,15 @@ export interface AiProviderState { * and when the operator's master switch is off. */ canConfigure: boolean; + /** + * How long one model call on this record may take before the server gives + * up, in milliseconds: the record's response-timeout setting, else the 60 s + * default. A surface that waits on a model call (document reads, lab OCR) + * sizes its own request abort from this plus a margin, so the browser never + * gives up before the server does. Surfaces with a larger budget of their + * own (the briefing) may run longer when the setting is unset. + */ + responseTimeoutMs: number; } /** The `ai` block the account payload publishes. */ diff --git a/src/lib/ai/codex-client.ts b/src/lib/ai/codex-client.ts index 9a2232e2b..5cba79686 100644 --- a/src/lib/ai/codex-client.ts +++ b/src/lib/ai/codex-client.ts @@ -1,4 +1,5 @@ import { randomUUID } from "node:crypto"; +import { callTimeoutMs } from "./effective-timeout"; import { safeFetch } from "@/lib/safe-fetch"; import type { AIProvider, @@ -253,6 +254,8 @@ export interface CodexAttemptDiagnostics { export class CodexClient implements AIProvider { readonly type = "codex" as const; + /** See `AIProvider.responseTimeoutSeconds`; stamped by the resolver. */ + responseTimeoutSeconds: number | null = null; private accessToken: string; private accountId: string; private onTokenRefresh: () => Promise<{ @@ -509,9 +512,12 @@ export class CodexClient implements AIProvider { }, // v1.20.1 — compose the caller's cancel signal (Coach SSE disconnect) so // a mid-generation abort tears the upstream call down early. - // v1.21.5 — honour the caller's per-request timeout override (the - // comprehensive briefing needs >60 s); default unchanged at 60 s. - { timeoutMs: params.timeoutMs ?? 60_000, signal: params.signal }, + // The record owner's response-timeout setting, else the surface value + // (the comprehensive briefing needs >60 s), else 60 s. + { + timeoutMs: callTimeoutMs(params, this.responseTimeoutSeconds), + signal: params.signal, + }, ); } diff --git a/src/lib/ai/effective-timeout.ts b/src/lib/ai/effective-timeout.ts index 1055c558e..9f97754bd 100644 --- a/src/lib/ai/effective-timeout.ts +++ b/src/lib/ai/effective-timeout.ts @@ -1,19 +1,23 @@ +import type { AIProvider, CompletionParams } from "./types"; + +/** + * The provider timeout a call gets when neither the surface nor the person set + * one. Every client used to carry its own copy of this literal. + */ +export const PROVIDER_DEFAULT_TIMEOUT_MS = 60_000; + /** * Resolve the upstream provider timeout (ms) for a non-chat generation surface. * * The per-user `User.aiResponseTimeoutSeconds` setting (Settings → AI) is the * operator's lever for slow self-hosted / local backends whose first request - * loads the model. Coach already threads it onto its provider call; the - * generated-narrative surfaces (daily briefing, status cards, period - * narratives, …) historically hardcoded their per-surface `AI_BUDGETS.*` - * default and ignored the setting, so raising the timeout had no effect on - * them. This helper is the one place that converts the stored seconds into the - * milliseconds a `CompletionParams.timeoutMs` expects, falling back to the - * surface's budget default when the user has not set a value. + * loads the model. This helper is the one place that converts the stored + * seconds into milliseconds, falling back to the surface's budget default when + * the user has not set a value. * - * Semantics mirror Coach: a positive stored value wins (seconds → ms), and any - * unset / non-positive value yields the surface default. The setting is bounded - * to 10–600 s at write-time, so no read-time clamp is needed here. + * Semantics: a positive stored value wins (seconds → ms), and any unset / + * non-positive value yields the surface default. The setting is bounded to + * 10–600 s at write-time, so no read-time clamp is needed here. */ export function resolveEffectiveTimeoutMs( aiResponseTimeoutSeconds: number | null | undefined, @@ -23,3 +27,65 @@ export function resolveEffectiveTimeoutMs( ? aiResponseTimeoutSeconds * 1000 : budgetDefaultMs; } + +/** + * The timeout one provider call actually runs under. Every client reads its + * `safeFetch` ceiling from here and nowhere else. + * + * The person's setting rides on the provider instance (`responseTimeoutSeconds`, + * stamped by `bindResponseTimeout` when the provider is resolved for a record), + * so a call site cannot forget it: whatever a surface passes as `timeoutMs` is + * only the default the setting overrides. A surface that is deliberately + * latency-bounded and has a deterministic fallback opts out with + * `timeoutPolicy: "surface-ceiling"`, and the structural guard keeps that set + * frozen. + */ +export function callTimeoutMs( + params: Pick, + responseTimeoutSeconds: number | null | undefined, +): number { + const surfaceMs = params.timeoutMs ?? PROVIDER_DEFAULT_TIMEOUT_MS; + if (params.timeoutPolicy === "surface-ceiling") return surfaceMs; + return resolveEffectiveTimeoutMs(responseTimeoutSeconds, surfaceMs); +} + +/** + * Stamp the record owner's response-timeout setting onto a resolved provider. + * Called by every exported resolver in `provider.ts` on its way out, which is + * the only way a provider reaches a call site. + */ +export function bindResponseTimeout

( + provider: P, + aiResponseTimeoutSeconds: number | null | undefined, +): P { + provider.responseTimeoutSeconds = + aiResponseTimeoutSeconds != null && aiResponseTimeoutSeconds > 0 + ? aiResponseTimeoutSeconds + : null; + return provider; +} + +/** + * Room a browser leaves on top of the server's model-call budget for what + * surrounds the calls: the upload, decrypting and rasterising the original, + * writing the result, the response itself. + */ +export const CLIENT_ABORT_MARGIN_MS = 30_000; + +/** + * The request timeout a browser uses for a route that waits on `modelCalls` + * sequential model calls, given the `ai.provider.responseTimeoutMs` the + * 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. + */ +export function clientAbortMs( + responseTimeoutMs: number | null | undefined, + modelCalls: number, +): number { + const perCall = + typeof responseTimeoutMs === "number" && responseTimeoutMs > 0 + ? responseTimeoutMs + : PROVIDER_DEFAULT_TIMEOUT_MS; + return perCall * modelCalls + CLIENT_ABORT_MARGIN_MS; +} diff --git a/src/lib/ai/local-client.ts b/src/lib/ai/local-client.ts index aec80be1e..27c96df89 100644 --- a/src/lib/ai/local-client.ts +++ b/src/lib/ai/local-client.ts @@ -1,4 +1,5 @@ import { safeFetch } from "@/lib/safe-fetch"; +import { callTimeoutMs } from "./effective-timeout"; import { annotate } from "@/lib/logging/context"; import { aiEgressPolicyFor } from "./local-host-allowlist"; import type { @@ -105,6 +106,8 @@ function withStrictJsonPrefix(messages: AiMessage[]): AiMessage[] { */ export class LocalOpenAICompatibleClient implements AIProvider { readonly type = "local" as const; + /** See `AIProvider.responseTimeoutSeconds`; stamped by the resolver. */ + responseTimeoutSeconds: number | null = null; /** Local servers commonly reject an unknown `tools` field — never send it. */ readonly supportsTools = false; private config: LocalClientConfig; @@ -218,8 +221,8 @@ export class LocalOpenAICompatibleClient implements AIProvider { // v1.20.1 — compose the caller's cancel signal (Coach SSE disconnect) so // a mid-generation abort tears the upstream call down early. { - // v1.22 (#89) — honour the caller's per-request timeout override; default 60 s. - timeoutMs: params.timeoutMs ?? 60_000, + // The record owner's response-timeout setting, else the surface value, else 60 s. + timeoutMs: callTimeoutMs(params, this.responseTimeoutSeconds), ...egress, signal: params.signal, }, @@ -353,7 +356,7 @@ export class LocalOpenAICompatibleClient implements AIProvider { // per-idle window. Composed with the caller's cancel signal so a client // disconnect still tears the call down immediately. const idle = new AbortController(); - const idleMs = params.timeoutMs ?? 60_000; + const idleMs = callTimeoutMs(params, this.responseTimeoutSeconds); const signal = params.signal ? AbortSignal.any([params.signal, idle.signal]) : idle.signal; diff --git a/src/lib/ai/openai-client.ts b/src/lib/ai/openai-client.ts index 0bf8b7c42..63464563c 100644 --- a/src/lib/ai/openai-client.ts +++ b/src/lib/ai/openai-client.ts @@ -1,4 +1,5 @@ import { safeFetch } from "@/lib/safe-fetch"; +import { callTimeoutMs } from "./effective-timeout"; import { annotate } from "@/lib/logging/context"; import type { AIProvider, CompletionParams, CompletionResult } from "./types"; import { selectOpenAIChatCompletionsCapabilities } from "./openai-capabilities"; @@ -59,6 +60,8 @@ type OpenAIProviderType = "admin-key" | "codex" | "openai-compatible"; export class OpenAIClient implements AIProvider { readonly type: OpenAIProviderType; + /** See `AIProvider.responseTimeoutSeconds`; stamped by the resolver. */ + responseTimeoutSeconds: number | null = null; private config: OpenAIClientConfig; constructor(config: OpenAIClientConfig) { @@ -156,7 +159,7 @@ export class OpenAIClient implements AIProvider { // address is rejected, closing the SSRF/rebinding surface. // v1.20.1 — compose the caller's cancel signal (Coach SSE disconnect) so // a mid-generation abort tears the upstream call down early. - // v1.21.5 — honour the caller's per-request timeout override; default 60 s. + // The record owner's response-timeout setting, else the surface value, else 60 s. // v1.33.1 (#470) — the gateway tag reuses the Local provider's host // policy verbatim: public hosts always; a private host only when the // operator granted its origin (`AI_PRIVATE_ORIGINS` or the legacy host @@ -174,7 +177,7 @@ export class OpenAIClient implements AIProvider { // pin: its base URL is a repository constant, there is nothing an // operator legitimately redirects. { - timeoutMs: params.timeoutMs ?? 60_000, + timeoutMs: callTimeoutMs(params, this.responseTimeoutSeconds), ...(this.isGateway || this.type === "admin-key" ? aiEgressPolicyFor(url, { operatorTrusted: this.config.operatorTrusted, diff --git a/src/lib/ai/provider.ts b/src/lib/ai/provider.ts index 885ee43de..89bc856a0 100644 --- a/src/lib/ai/provider.ts +++ b/src/lib/ai/provider.ts @@ -1,4 +1,5 @@ import type { AIProvider, CompletionResult } from "./types"; +import { bindResponseTimeout } from "./effective-timeout"; import { prisma } from "@/lib/db"; import { decrypt } from "@/lib/crypto"; import { CodexClient, resolveCodexVisionSlug } from "./codex-client"; @@ -499,18 +500,24 @@ export async function resolveProvider(userId: string): Promise { aiCompatModel: true, role: true, managedProfileAt: true, + aiResponseTimeoutSeconds: true, }, }); + // The record's response-timeout setting rides on whatever this returns, on + // every branch, the operator-key fallback for a managed profile included: + // the person waiting on the record is who the call serves. + const bind = (provider: AIProvider): AIProvider => + bindResponseTimeout(provider, userRow?.aiResponseTimeoutSeconds); const policy = providerCredentialPolicy( authority, userRow?.managedProfileAt ?? null, ); - if (policy === "operator-default") return resolveAdminProvider(); + if (policy === "operator-default") return bind(await resolveAdminProvider()); // 1. Per-user Anthropic / Local if (userRow) { const userProvider = buildUserProvider(userRow); - if (userProvider) return userProvider; + if (userProvider) return bind(userProvider); } // 2. Codex OAuth (either explicitly selected or implicit fallback) @@ -518,11 +525,26 @@ export async function resolveProvider(userId: string): Promise { const tryCodex = explicitChoice === "CHATGPT_OAUTH" || !explicitChoice; if (tryCodex) { const codex = await resolveCodexProvider(userId); - if (codex) return codex; + if (codex) return bind(codex); } // 3. Admin OpenAI key (also acts as fallback for user-OPENAI selection) - return resolveAdminProvider(); + return bind(await resolveAdminProvider()); +} + +/** + * The record owner's `aiResponseTimeoutSeconds`, for `resolveProviderForTest`, + * whose many branches (saved config, unsaved override, the chain) are bound in + * one place on the way out. The other two resolvers read it with their row. + */ +async function readResponseTimeoutSeconds( + userId: string, +): Promise { + const row = await prisma.user.findUnique({ + where: { id: userId }, + select: { aiResponseTimeoutSeconds: true }, + }); + return row?.aiResponseTimeoutSeconds ?? null; } /** @@ -570,8 +592,12 @@ export async function resolveProviderChain( aiProviderChain: true, useCentralCodex: true, managedProfileAt: true, + aiResponseTimeoutSeconds: true, }, }); + // See `resolveProvider`: every instance carries the record's setting. + const bind = (provider: AIProvider): AIProvider => + bindResponseTimeout(provider, userRow?.aiResponseTimeoutSeconds); const policy = providerCredentialPolicy( authority, userRow?.managedProfileAt ?? null, @@ -580,7 +606,7 @@ export async function resolveProviderChain( const provider = await resolveAdminProvider(); return provider.type === "none" ? [] - : [{ providerType: "admin-openai", instance: provider }]; + : [{ providerType: "admin-openai", instance: bind(provider) }]; } const rawChain = userRow?.aiProviderChain ?? null; @@ -594,7 +620,10 @@ export async function resolveProviderChain( policy, }); if (instance) { - resolved.push({ providerType: entry.providerType, instance }); + resolved.push({ + providerType: entry.providerType, + instance: bind(instance), + }); } } @@ -607,7 +636,10 @@ export async function resolveProviderChain( if (userRow?.useCentralCodex) { const centralCodex = await resolveAdminCodexProvider(); if (centralCodex) { - resolved.push({ providerType: "admin-codex", instance: centralCodex }); + resolved.push({ + providerType: "admin-codex", + instance: bind(centralCodex), + }); } } @@ -854,6 +886,7 @@ const PRESENCE_USER_SELECT = { useCentralCodex: true, managedProfileAt: true, labsLocalOcrEnabled: true, + aiResponseTimeoutSeconds: true, } as const; const PRESENCE_SETTINGS_SELECT = { @@ -947,6 +980,7 @@ export function probeProviderChain( // A guardian's in-browser OCR is not the record's opt-in to use. localOcrEnabled: false, managedBy: adminKey ? "server" : null, + responseTimeoutSeconds: row.aiResponseTimeoutSeconds, }; } @@ -1002,6 +1036,7 @@ export function probeProviderChain( entries, localOcrEnabled: row.labsLocalOcrEnabled, managedBy: entries.length > 0 ? managedBy : null, + responseTimeoutSeconds: row.aiResponseTimeoutSeconds, }; }, { freshInBackground: true }, @@ -1146,6 +1181,19 @@ export class AITestConfigError extends Error { export async function resolveProviderForTest( userId: string, override: AITestOverride = {}, +): Promise { + // The test runs under the same timeout generation would, so a slow local + // model that needs a raised setting is not reported as dead by the test. + const provider = await resolveProviderForTestUnbound(userId, override); + return bindResponseTimeout( + provider, + await readResponseTimeoutSeconds(userId), + ); +} + +async function resolveProviderForTestUnbound( + userId: string, + override: AITestOverride, ): Promise { const stored = await prisma.user.findUnique({ where: { id: userId }, diff --git a/src/lib/ai/types.ts b/src/lib/ai/types.ts index 898edb62c..541dcbb24 100644 --- a/src/lib/ai/types.ts +++ b/src/lib/ai/types.ts @@ -115,6 +115,14 @@ export interface AIProvider { * available" and answer from base context. */ supportsTools?: boolean; + /** + * The record owner's `aiResponseTimeoutSeconds`, stamped by the resolver + * that handed this provider out (`bindResponseTimeout`). Each client reads + * its upstream ceiling through `callTimeoutMs(params, this.responseTimeoutSeconds)`, + * so the setting reaches every call without the call site passing it. + * `null` / absent → the surface's `timeoutMs`, else the 60 s default. + */ + responseTimeoutSeconds?: number | null; generateCompletion(params: CompletionParams): Promise; /** * v1.22 (#89) — optional true-streaming variant. A provider that can emit @@ -303,8 +311,21 @@ export interface CompletionParams { * aborted mid-stream ("operation aborted due to timeout"), which left the * briefing — and the insights trend narrative that reads the same cached * block — permanently blank. Sized in `AI_BUDGETS.comprehensive.timeoutMs`. + * + * This is the SURFACE default. The record owner's response-timeout setting, + * bound to the provider instance at resolution, overrides it (see + * `callTimeoutMs` in `effective-timeout.ts`) unless `timeoutPolicy` says the + * value is a hard ceiling. */ timeoutMs?: number; + /** + * `"surface-ceiling"` makes `timeoutMs` a hard ceiling the person's + * response-timeout setting does not lift. Reserved for unattended surfaces + * that are latency-bounded by design and fall back to deterministic copy + * (the Coach nudge tick, the reaction line). The set of callers is frozen by + * `ai-call-timeout-guard.test.ts`. + */ + timeoutPolicy?: "surface-ceiling"; } export interface CompletionResult { @@ -351,6 +372,8 @@ export function singleUserTurn(p: { signal?: AbortSignal; /** v1.22 (#89) — per-request upstream timeout override (ms); see CompletionParams. */ timeoutMs?: number; + /** See `CompletionParams.timeoutPolicy`. */ + timeoutPolicy?: "surface-ceiling"; }): CompletionParams { const images = p.images ?? []; const documents = p.documents ?? []; @@ -385,6 +408,7 @@ export function singleUserTurn(p: { toolChoice: p.toolChoice, signal: p.signal, timeoutMs: p.timeoutMs, + ...(p.timeoutPolicy ? { timeoutPolicy: p.timeoutPolicy } : {}), }; } diff --git a/src/lib/jobs/coach-nudge-ai.ts b/src/lib/jobs/coach-nudge-ai.ts index ed3c3a374..a7056d7d4 100644 --- a/src/lib/jobs/coach-nudge-ai.ts +++ b/src/lib/jobs/coach-nudge-ai.ts @@ -239,6 +239,10 @@ export const composeNudgeWithAI: ComposeNudgeWithAI = async (params) => { temperature: budget.temperature, maxTokens, timeoutMs: COACH_NUDGE_AI_CALL_TIMEOUT_MS, + // The tick is sequential across every account and bounded by + // `COACH_NUDGE_AI_TICK_BUDGET_MS`; the template is always ready. A + // raised response-timeout setting must not hold the tick for others. + timeoutPolicy: "surface-ceiling", signal: AbortSignal.timeout(COACH_NUDGE_AI_CALL_TIMEOUT_MS + 1_000), }), ); diff --git a/src/lib/jobs/reaction-line.ts b/src/lib/jobs/reaction-line.ts index 3c32cf7d6..e5b646c1a 100644 --- a/src/lib/jobs/reaction-line.ts +++ b/src/lib/jobs/reaction-line.ts @@ -673,6 +673,10 @@ export async function runReactionLine( temperature: budget.temperature, maxTokens, timeoutMs: REACTION_LINE_TIMEOUT_MS, + // The claim lease (`REACTION_LINE_CLAIM_LEASE_MS`) is shorter than the + // largest response-timeout setting, and the deterministic lead stands + // in when the line does not arrive, so this ceiling is not lifted. + timeoutPolicy: "surface-ceiling", signal: AbortSignal.timeout(REACTION_LINE_TIMEOUT_MS + 1_000), }), ); diff --git a/src/lib/openapi/routes/profile.ts b/src/lib/openapi/routes/profile.ts index 56ab7a1ad..257f01428 100644 --- a/src/lib/openapi/routes/profile.ts +++ b/src/lib/openapi/routes/profile.ts @@ -181,6 +181,13 @@ export const aiProviderState = z .describe( "Whether the person in front of the screen may set up a provider for this record: false inside somebody else's record and while the operator's master switch is off. A setup hint is shown only when this is true.", ), + responseTimeoutMs: z + .number() + .int() + .positive() + .describe( + "How long one model call on this record may take before the server gives up, in milliseconds: the record's response-timeout setting (Settings → AI), else 60000. A client that waits on a model call (a document read, lab OCR) sizes its own request timeout from this plus a margin, so it never gives up before the server does.", + ), }) .meta({ id: "AiProviderState", diff --git a/tests/integration/auth-me-ai-capabilities.test.ts b/tests/integration/auth-me-ai-capabilities.test.ts index 63bd98a91..fdabc33a0 100644 --- a/tests/integration/auth-me-ai-capabilities.test.ts +++ b/tests/integration/auth-me-ai-capabilities.test.ts @@ -73,6 +73,7 @@ interface MePayload { configured: boolean; managedBy: string | null; canConfigure: boolean; + responseTimeoutMs: number; }; }; } @@ -171,9 +172,20 @@ describe("GET /api/auth/me — ai", () => { configured: true, managedBy: "user", canConfigure: true, + responseTimeoutMs: 60_000, }); }); + it("publishes the record's response-timeout setting as the effective timeout", async () => { + const user = await makeUser("slow-local", { + aiResponseTimeoutSeconds: 420, + }); + await grantConsent(user.id, "ai_full"); + await signIn(user.id); + const me = await readMe(); + expect(me.ai.provider.responseTimeoutMs).toBe(420_000); + }); + it("closes everything when the operator's master switch is off", async () => { const user = await makeUser("master-off"); await grantConsent(user.id, "ai_full"); @@ -216,6 +228,7 @@ describe("GET /api/auth/me — ai", () => { configured: false, managedBy: null, canConfigure: true, + responseTimeoutMs: 60_000, }); }); @@ -318,6 +331,7 @@ describe("GET /api/auth/me — ai", () => { configured: false, managedBy: null, canConfigure: false, + responseTimeoutMs: 60_000, }); }); @@ -352,6 +366,7 @@ describe("GET /api/auth/me — ai", () => { configured: true, managedBy: "server", canConfigure: false, + responseTimeoutMs: 60_000, }); }); });