From e1e1b0e9853d7f66b3a338bc966428e460d42358 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Fri, 26 Jun 2026 09:26:31 +0200 Subject: [PATCH 1/3] fix(notifications): treat channel config decrypt/parse failure as a hard reject MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A channel whose stored config cannot be decrypted or JSON-parsed is a structurally-permanent failure, not a transient blip: the ciphertext/key pairing and the malformed body decode the same way on every retry. Routing it through the transient path burned the 5-failure backoff budget and ended on the misleading `give_up_after_5_failures` reason — during a key-rotation gap every channel hit this and auto-disabled for the wrong reason. Classify decrypt and parse failures as hard rejects so the channel auto-disables on first occurrence with the precise reason (`_config_decrypt_failed` / `_config_parse_failed`). Genuine transient failures (network, timeout, 5xx) keep the existing backoff path. --- .../__tests__/dispatcher.test.ts | 90 +++++++++++++++++++ src/lib/notifications/dispatcher.ts | 21 +++-- 2 files changed, 106 insertions(+), 5 deletions(-) diff --git a/src/lib/notifications/__tests__/dispatcher.test.ts b/src/lib/notifications/__tests__/dispatcher.test.ts index 1795de825..f350ec7b5 100644 --- a/src/lib/notifications/__tests__/dispatcher.test.ts +++ b/src/lib/notifications/__tests__/dispatcher.test.ts @@ -502,6 +502,96 @@ describe("dispatchNotification — Telegram hard reject", () => { }); }); +describe("dispatchNotification — config parse/decrypt failure is a hard reject", () => { + it("malformed channel config auto-disables on first occurrence (not after 5 transient ticks)", async () => { + const channel = makeChannel({ + type: "NTFY", + // Not valid JSON → JSON.parse throws in sendToChannel. + config: "{ this is not json", + }); + vi.mocked(prisma.notificationChannel.findMany).mockResolvedValueOnce([ + channel, + ] as never); + + await dispatchNotification({ + eventType: "SYSTEM_ALERT", + userId: "u-1", + title: "t", + message: "m", + }); + + // The sender is never reached on a parse failure. + expect(sendViaNtfyMock).not.toHaveBeenCalled(); + + // Channel auto-disabled immediately with the precise parse reason — + // NOT the misleading `give_up_after_5_failures`. + const updateCalls = ( + prisma.notificationChannel.update as unknown as ReturnType + ).mock.calls; + const disablePayload = updateCalls.find( + (c) => (c[0] as { data: { enabled?: boolean } }).data?.enabled === false, + ); + expect(disablePayload).toBeTruthy(); + expect( + (disablePayload?.[0] as { data: { disabledReason?: string } }).data + .disabledReason, + ).toBe("ntfy_config_parse_failed"); + + // It is logged as a hard reject, not a transient give-up. + expect(auditLog).toHaveBeenCalledWith( + "notification.channel.auto_disabled", + expect.objectContaining({ + details: expect.objectContaining({ + reason: "ntfy_config_parse_failed", + kind: "hard_reject", + }), + }), + ); + }); + + it("a genuine transient failure still takes the transient path (stays enabled, backoff set)", async () => { + const channel = makeChannel({ + type: "NTFY", + config: JSON.stringify({ serverUrl: "https://ntfy.example", topic: "t" }), + consecutiveFailures: 0, + }); + vi.mocked(prisma.notificationChannel.findMany).mockResolvedValueOnce([ + channel, + ] as never); + sendViaNtfyMock.mockResolvedValueOnce({ + ok: false, + hardReject: false, + statusCode: 503, + reason: "ntfy_503", + }); + (prisma.notificationChannel.update as unknown as ReturnType) + .mockResolvedValueOnce({ consecutiveFailures: 1 }) + .mockResolvedValue({}); + + await dispatchNotification({ + eventType: "SYSTEM_ALERT", + userId: "u-1", + title: "t", + message: "m", + }); + + const updateCalls = ( + prisma.notificationChannel.update as unknown as ReturnType + ).mock.calls; + // No enabled=false update → channel stays enabled, backoff scheduled. + const disablePayload = updateCalls.find( + (c) => (c[0] as { data: { enabled?: boolean } }).data?.enabled === false, + ); + expect(disablePayload).toBeUndefined(); + const cooldownPayload = updateCalls.find( + (c) => + (c[0] as { data: { nextRetryAt?: Date } }).data?.nextRetryAt instanceof + Date, + ); + expect(cooldownPayload).toBeTruthy(); + }); +}); + describe("dispatchNotification — reminders reach Web Push without APNs", () => { it("a PWA-only self-hoster (WEB_PUSH channel, no APNS) still gets MEDICATION_REMINDER", async () => { const channel = makeChannel({ type: "WEB_PUSH" }); diff --git a/src/lib/notifications/dispatcher.ts b/src/lib/notifications/dispatcher.ts index 9bbeadc27..cab12442e 100644 --- a/src/lib/notifications/dispatcher.ts +++ b/src/lib/notifications/dispatcher.ts @@ -263,9 +263,16 @@ async function sendToChannel( decrypted = decrypt(encryptedConfig); } catch { getEvent()?.addWarning(`Failed to decrypt ${type} channel config`); + // A config that can't be decrypted is a structurally-permanent failure, + // not a transient blip: the ciphertext / key pairing won't fix itself on + // a retry. Hard-reject so the channel auto-disables on first occurrence + // with an accurate reason, rather than burning the 5-failure transient + // budget and ending on the misleading `give_up_after_5_failures`. During + // a key-rotation gap every channel would otherwise auto-disable for the + // wrong reason. return { ok: false, - hardReject: false, + hardReject: true, reason: `${type.toLowerCase()}_config_decrypt_failed`, }; } @@ -277,9 +284,10 @@ async function sendToChannel( config = JSON.parse(decrypted) as TelegramChannelConfig; } catch { getEvent()?.addWarning("Failed to parse Telegram channel config"); + // Malformed config decodes the same on every retry — permanent. return { ok: false, - hardReject: false, + hardReject: true, reason: "telegram_config_parse_failed", }; } @@ -291,9 +299,10 @@ async function sendToChannel( config = JSON.parse(decrypted) as NtfyChannelConfig; } catch { getEvent()?.addWarning("Failed to parse ntfy channel config"); + // Malformed config decodes the same on every retry — permanent. return { ok: false, - hardReject: false, + hardReject: true, reason: "ntfy_config_parse_failed", }; } @@ -305,9 +314,10 @@ async function sendToChannel( config = JSON.parse(decrypted) as WebhookChannelConfig; } catch { getEvent()?.addWarning("Failed to parse webhook channel config"); + // Malformed config decodes the same on every retry — permanent. return { ok: false, - hardReject: false, + hardReject: true, reason: "webhook_config_parse_failed", }; } @@ -319,9 +329,10 @@ async function sendToChannel( config = JSON.parse(decrypted) as EmailChannelConfig; } catch { getEvent()?.addWarning("Failed to parse email channel config"); + // Malformed config decodes the same on every retry — permanent. return { ok: false, - hardReject: false, + hardReject: true, reason: "email_config_parse_failed", }; } From 4c571b0e0419b05a4ac9059680449adad2d38fa2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Fri, 26 Jun 2026 09:27:29 +0200 Subject: [PATCH 2/3] fix(insights): cancel the Coach upstream provider call on client disconnect The Coach SSE turn ran the provider call to completion before the stream existed and never wired it to the request's abort signal, so a mid-generation client disconnect still paid the full upstream token cost. Thread an optional `signal` through CompletionParams and singleUserTurn into each client's safeFetch (composed with the per-call timeout via AbortSignal.any), and pass `request.signal` from the chat route's no-tools path. An early disconnect now tears the upstream request down instead of generating into a closed connection. Omitting the signal leaves behaviour unchanged (timeout-only). --- src/app/api/insights/chat/route.ts | 4 ++++ src/lib/ai/anthropic-client.ts | 4 +++- src/lib/ai/codex-client.ts | 4 +++- src/lib/ai/local-client.ts | 8 +++++++- src/lib/ai/openai-client.ts | 4 +++- src/lib/ai/types.ts | 12 ++++++++++++ 6 files changed, 32 insertions(+), 4 deletions(-) diff --git a/src/app/api/insights/chat/route.ts b/src/app/api/insights/chat/route.ts index aedc94be7..f418a1c7e 100644 --- a/src/app/api/insights/chat/route.ts +++ b/src/app/api/insights/chat/route.ts @@ -567,6 +567,10 @@ Reply now as the assistant, in ${locale === "de" ? "German" : "English"}. Fetch user: userPrompt, temperature: AI_BUDGETS.coach.temperature, maxTokens: AI_BUDGETS.coach.maxTokens, + // v1.20.1 — thread the request's abort signal so a mid-generation + // client disconnect tears the upstream provider call down instead of + // paying the full token cost into a closed connection. + signal: request.signal, }), }); result = fallback.result; diff --git a/src/lib/ai/anthropic-client.ts b/src/lib/ai/anthropic-client.ts index 97162de03..00c704701 100644 --- a/src/lib/ai/anthropic-client.ts +++ b/src/lib/ai/anthropic-client.ts @@ -259,7 +259,9 @@ export class AnthropicClient implements AIProvider { // 60 s ceiling — see openai-client.ts for the rationale. // v1.11.2 — base URL is user/admin-overridable; pin the connect-time DNS // check so a private/metadata address is rejected (SSRF/rebinding). - { timeoutMs: 60_000, requirePublicHost: true }, + // v1.20.1 — compose the caller's cancel signal (Coach SSE disconnect) so + // a mid-generation abort tears the upstream call down early. + { timeoutMs: 60_000, requirePublicHost: true, signal: params.signal }, ); if (!res.ok) { diff --git a/src/lib/ai/codex-client.ts b/src/lib/ai/codex-client.ts index 2be9fcf8c..dcae2020a 100644 --- a/src/lib/ai/codex-client.ts +++ b/src/lib/ai/codex-client.ts @@ -467,7 +467,9 @@ export class CodexClient implements AIProvider { // clients use so a long generation is not clipped by the 15 s // default while still bounding a tar-pit upstream. }, - { timeoutMs: 60_000 }, + // v1.20.1 — compose the caller's cancel signal (Coach SSE disconnect) so + // a mid-generation abort tears the upstream call down early. + { timeoutMs: 60_000, signal: params.signal }, ); } diff --git a/src/lib/ai/local-client.ts b/src/lib/ai/local-client.ts index 477d9bebc..fa41db7d7 100644 --- a/src/lib/ai/local-client.ts +++ b/src/lib/ai/local-client.ts @@ -125,7 +125,13 @@ export class LocalOpenAICompatibleClient implements AIProvider { ...(params.responseFormat === "json" ? { format: "json" } : {}), }), }, - { timeoutMs: 60_000, requirePublicHost: !allowPrivate }, + // v1.20.1 — compose the caller's cancel signal (Coach SSE disconnect) so + // a mid-generation abort tears the upstream call down early. + { + timeoutMs: 60_000, + requirePublicHost: !allowPrivate, + signal: params.signal, + }, ); if (!res.ok) { diff --git a/src/lib/ai/openai-client.ts b/src/lib/ai/openai-client.ts index e4a8c808d..f84cda5f4 100644 --- a/src/lib/ai/openai-client.ts +++ b/src/lib/ai/openai-client.ts @@ -90,7 +90,9 @@ export class OpenAIClient implements AIProvider { // v1.11.2 — the base URL is user/admin-overridable (BYO gateway), so pin // the connect-time DNS check: a base URL resolving to a private/metadata // address is rejected, closing the SSRF/rebinding surface. - { timeoutMs: 60_000, requirePublicHost: true }, + // v1.20.1 — compose the caller's cancel signal (Coach SSE disconnect) so + // a mid-generation abort tears the upstream call down early. + { timeoutMs: 60_000, requirePublicHost: true, signal: params.signal }, ); if (!res.ok) { diff --git a/src/lib/ai/types.ts b/src/lib/ai/types.ts index 0e1cc8028..318becc18 100644 --- a/src/lib/ai/types.ts +++ b/src/lib/ai/types.ts @@ -271,6 +271,15 @@ export interface CompletionParams { * Mapped per provider; omitted from the wire when unset. */ toolChoice?: "auto" | "none"; + /** + * v1.20.1 — optional caller-owned cancellation signal. Threaded onto the + * client's `safeFetch` so a mid-generation client disconnect (the Coach SSE + * route wires `request.signal` here) tears the upstream provider request + * down instead of running it to completion and paying the full token cost + * into a closed connection. Composed with the per-client timeout inside + * `safeFetch`. Omitted → behaviour is unchanged (timeout-only). + */ + signal?: AbortSignal; } export interface CompletionResult { @@ -313,6 +322,8 @@ export function singleUserTurn(p: { documents?: CompletionDocument[]; tools?: AiToolDef[]; toolChoice?: "auto" | "none"; + /** v1.20.1 — caller-owned cancel signal threaded to the client fetch. */ + signal?: AbortSignal; }): CompletionParams { const images = p.images ?? []; const documents = p.documents ?? []; @@ -349,6 +360,7 @@ export function singleUserTurn(p: { responseFormat: p.responseFormat, tools: p.tools, toolChoice: p.toolChoice, + signal: p.signal, }; } From c304268a9ea2405f328b708acc0d2180e0f28814 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Fri, 26 Jun 2026 09:27:42 +0200 Subject: [PATCH 3/3] fix(ai): tag empty-reply and stream-failed provider throws with a sentinel status MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The four AI clients threw bare Errors on an empty reply (and, for Codex, on a stream-level failure) with no httpStatus, so the provider-chain classifier saw the same null-status shape as a genuine transport failure (ECONNRESET, DNS, timeout) and could not tell them apart in observability. Attach a sentinel `httpStatus: 0` plus a `kind` discriminator (`empty_response` / `stream_failed`) on those throws. The cascade is unchanged: `isHardProviderFailure` already treats a status <= 0 as a hard failure and walks to the next provider — this only sharpens the logs. --- src/lib/ai/__tests__/openai-client.test.ts | 68 ++++++++++++++++++++++ src/lib/ai/anthropic-client.ts | 11 +++- src/lib/ai/codex-client.ts | 41 +++++++++++-- src/lib/ai/local-client.ts | 11 +++- src/lib/ai/openai-client.ts | 14 ++++- 5 files changed, 138 insertions(+), 7 deletions(-) diff --git a/src/lib/ai/__tests__/openai-client.test.ts b/src/lib/ai/__tests__/openai-client.test.ts index 170223801..2a94eed33 100644 --- a/src/lib/ai/__tests__/openai-client.test.ts +++ b/src/lib/ai/__tests__/openai-client.test.ts @@ -290,4 +290,72 @@ describe("OpenAIClient", () => { image_url: { url: "data:image/png;base64,AAAA" }, }); }); + + it("tags an empty-content reply with sentinel httpStatus 0 + kind for the chain classifier", async () => { + vi.stubGlobal( + "fetch", + vi.fn().mockResolvedValue({ + ok: true, + json: () => + Promise.resolve({ + choices: [{ message: { content: "" } }], + usage: { total_tokens: 5 }, + }), + }), + ); + + const client = new OpenAIClient({ + apiKey: "sk-test", + model: "gpt-4o-mini", + baseUrl: "https://api.openai.com/v1", + }); + + await client + .generateCompletion(singleUserTurn({ system: "s", user: "u" })) + .then( + () => { + throw new Error("expected an empty-content throw"); + }, + (err: unknown) => { + // The throw distinguishes an empty 200-OK reply from ECONNRESET: + // sentinel httpStatus 0 (still classified as a hard failure, so the + // cascade is unchanged) + a `kind` discriminator for observability. + expect((err as Error).message).toContain("empty content"); + expect((err as { httpStatus?: number }).httpStatus).toBe(0); + expect((err as { kind?: string }).kind).toBe("empty_response"); + }, + ); + }); + + it("threads the caller's abort signal onto the upstream fetch", async () => { + const mockFetch = vi.fn().mockResolvedValue({ + ok: true, + json: () => + Promise.resolve({ + choices: [{ message: { content: "ok" } }], + usage: { total_tokens: 1 }, + }), + }); + vi.stubGlobal("fetch", mockFetch); + + const client = new OpenAIClient({ + apiKey: "sk-test", + model: "gpt-4o-mini", + baseUrl: "https://api.openai.com/v1", + }); + + const ctrl = new AbortController(); + await client.generateCompletion( + singleUserTurn({ system: "s", user: "u", signal: ctrl.signal }), + ); + + // safeFetch composes the caller signal with the timeout via AbortSignal.any, + // so a signal lands on the dispatched fetch init. + const init = mockFetch.mock.calls[0][1]; + expect(init.signal).toBeInstanceOf(AbortSignal); + + // Aborting the caller's controller propagates to the composed signal. + ctrl.abort(); + expect(init.signal.aborted).toBe(true); + }); }); diff --git a/src/lib/ai/anthropic-client.ts b/src/lib/ai/anthropic-client.ts index 00c704701..349db83d2 100644 --- a/src/lib/ai/anthropic-client.ts +++ b/src/lib/ai/anthropic-client.ts @@ -320,7 +320,16 @@ export class AnthropicClient implements AIProvider { // A tool_use-only reply carries no text — that is valid (F1). Only an empty // reply with neither text NOR a tool call is an error. if (!rawText && !toolCalls) { - throw new Error("Anthropic returned empty content"); + // v1.20.1 — sentinel httpStatus + kind so the chain classifier can tell + // an empty 200-OK reply apart from a transport failure. Cascade unchanged + // (`status <= 0` is already a hard failure). + const err = new Error("Anthropic returned empty content"); + Object.assign(err, { + httpStatus: 0, + kind: "empty_response", + upstream: "anthropic", + }); + throw err; } // When we prefilled the assistant turn with `{`, the model continues diff --git a/src/lib/ai/codex-client.ts b/src/lib/ai/codex-client.ts index dcae2020a..4bc717814 100644 --- a/src/lib/ai/codex-client.ts +++ b/src/lib/ai/codex-client.ts @@ -494,7 +494,15 @@ export class CodexClient implements AIProvider { requestedSlug: string, ): Promise { if (!res.body) { - throw new Error("Codex returned no response body"); + // v1.20.1 — sentinel httpStatus + kind so the chain classifier can tell + // a stream-level failure apart from a connect-time transport error. + const err = new Error("Codex returned no response body"); + Object.assign(err, { + httpStatus: 0, + kind: "stream_failed", + upstream: "codex", + }); + throw err; } const reader = res.body.getReader(); @@ -597,6 +605,10 @@ export class CodexClient implements AIProvider { const e = new Error(message); Object.assign(e, { upstream: "codex", + // v1.20.1 — sentinel httpStatus + kind so the chain classifier + // distinguishes a stream-level failure from a transport error. + httpStatus: 0, + kind: "stream_failed", errorCode: err?.code ?? null, model: serverModel ?? requestedSlug, }); @@ -606,13 +618,25 @@ export class CodexClient implements AIProvider { const reason = parsed.response?.incomplete_details?.reason ?? "incomplete response"; - throw new Error(`Codex stream incomplete: ${reason}`); + const e = new Error(`Codex stream incomplete: ${reason}`); + Object.assign(e, { + httpStatus: 0, + kind: "stream_failed", + upstream: "codex", + }); + throw e; } case "error": case "response.error": { const message = parsed.error?.message ?? "Codex stream returned an error event"; - throw new Error(message); + const e = new Error(message); + Object.assign(e, { + httpStatus: 0, + kind: "stream_failed", + upstream: "codex", + }); + throw e; } default: // Unknown event types are tolerated and ignored, same @@ -629,7 +653,16 @@ export class CodexClient implements AIProvider { // A function-call-only reply carries no text — valid (F1). Only an empty // reply with neither text NOR a tool call is an error. if (!content && toolCalls.length === 0) { - throw new Error("Codex returned empty content"); + // v1.20.1 — sentinel httpStatus + kind so the chain classifier can tell + // an empty reply apart from a transport failure. Cascade unchanged + // (`status <= 0` is already a hard failure). + const err = new Error("Codex returned empty content"); + Object.assign(err, { + httpStatus: 0, + kind: "empty_response", + upstream: "codex", + }); + throw err; } return { diff --git a/src/lib/ai/local-client.ts b/src/lib/ai/local-client.ts index fa41db7d7..eaf30ceff 100644 --- a/src/lib/ai/local-client.ts +++ b/src/lib/ai/local-client.ts @@ -161,7 +161,16 @@ export class LocalOpenAICompatibleClient implements AIProvider { const content = json.choices?.[0]?.message?.content; if (!content) { - throw new Error("Local AI returned empty content"); + // v1.20.1 — sentinel httpStatus + kind so the chain classifier can tell + // an empty 200-OK reply apart from a transport failure. Cascade unchanged + // (`status <= 0` is already a hard failure). + const err = new Error("Local AI returned empty content"); + Object.assign(err, { + httpStatus: 0, + kind: "empty_response", + upstream: "local", + }); + throw err; } return { diff --git a/src/lib/ai/openai-client.ts b/src/lib/ai/openai-client.ts index f84cda5f4..26b291296 100644 --- a/src/lib/ai/openai-client.ts +++ b/src/lib/ai/openai-client.ts @@ -126,7 +126,19 @@ export class OpenAIClient implements AIProvider { // A reply with tool calls and no prose is valid (F1 tool loop); only an // empty reply with neither content NOR tool calls is an error. if (!content && !toolCalls) { - throw new Error("OpenAI returned empty content"); + // v1.20.1 — tag the empty-reply throw with a sentinel `httpStatus: 0` + + // `kind` so the provider-chain classifier can tell an empty 200-OK reply + // apart from a genuine transport failure (ECONNRESET, DNS, timeout) in + // observability. The cascade is unchanged: `isHardProviderFailure` + // already treats `status <= 0` as a hard failure and walks to the next + // provider. + const err = new Error("OpenAI returned empty content"); + Object.assign(err, { + httpStatus: 0, + kind: "empty_response", + upstream: "openai", + }); + throw err; } return {