From 65a62350ea61be3404d28d9216484982a70c6790 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Fri, 11 Sep 2026 10:53:41 +0200 Subject: [PATCH] fix(onboarding): the AI panel claims an outage only when one was seen MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The done screen folded the `unknown` provider verdict into `unhealthy` and painted "This instance's provider is not answering right now." `unknown` is an empty `provider_health` table, which is the state of every instance until its first AI call — so the most common path through the screen made a claim about something nobody had measured. It now falls to the neutral panel, which says nothing either way; the offer is withheld in both cases as before. The projection itself drew the same unearned conclusion from staleness alone: a quiet instance whose last success was two days old read `unhealthy` without any failure behind it. A verdict now needs an observed one — a row whose last outcome was a failure — and a merely old success reads `unknown`. --- docs/api/openapi.yaml | 12 +++-- .../onboarding/__tests__/done-screen.test.tsx | 53 ++++++++++++------- src/components/onboarding/done-screen.tsx | 23 ++++---- .../__tests__/server-provider-health.test.ts | 17 +++++- src/lib/ai/server-provider-health.ts | 26 ++++++--- src/lib/openapi/routes/profile.ts | 2 +- 6 files changed, 92 insertions(+), 41 deletions(-) diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index 2c9848fc6..bc6ab4873 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -44493,11 +44493,13 @@ components: - unknown description: "Instance-wide health of the operator's shared providers (`admin-openai` / `admin-codex`), folded across all accounts and reduced to a tri-state: `healthy` when one of them demonstrably served somebody within the - last 24 h and no credential sits in its auth-failure cooldown, `unhealthy` when they have been tried and are - not working, `unknown` when nothing has been tried yet on this instance or the read failed. Deliberately - carries no count, no timestamp and no account identity — 'the provider of this instance last worked' is a - statement about the instance, not about other people. Fails closed: treat `unknown` exactly like - `unhealthy`. Reading this never probes a provider." + last 24 h and no credential sits in its auth-failure cooldown, `unhealthy` when the last outcome somebody + observed was a failure, `unknown` when nothing has been tried lately on this instance (including a brand-new + instance whose ledger is still empty) or the read failed. Deliberately carries no count, no timestamp and no + account identity — 'the provider of this instance last worked' is a statement about the instance, not about + other people. Fails closed for the offer: neither `unhealthy` nor `unknown` may be offered. They are not + interchangeable for copy, though — `unhealthy` is a verdict a client may state, `unknown` is the absence of + one and must not be reported as an outage. Reading this never probes a provider." serverProviderOffer: type: boolean description: "Whether the shared provider may honestly be offered to this caller in one tap. True only when all of: the diff --git a/src/components/onboarding/__tests__/done-screen.test.tsx b/src/components/onboarding/__tests__/done-screen.test.tsx index eaa9ad780..a373502c3 100644 --- a/src/components/onboarding/__tests__/done-screen.test.tsx +++ b/src/components/onboarding/__tests__/done-screen.test.tsx @@ -186,24 +186,41 @@ describe(" shared-provider offer", () => { expect(html).not.toContain('data-slot="onboarding-ai-unavailable"'); }); - it.each(["unhealthy", "unknown"] as const)( - "says the provider is not answering on %s, and offers no button", - (health) => { - aiProviderState.data = { - managedBy: "server", - serverProviderHealth: health, - serverProviderOffer: false, - serverProviderConsent: false, - }; - const html = render(); - expect(html).toContain('data-slot="onboarding-ai-unavailable"'); - expect(html).not.toContain('data-slot="onboarding-ai-offer-grant"'); - expect(html).not.toContain('data-slot="onboarding-ai-shared-key"'); - // The rest of the panel is untouched in every branch. - expect(html).toContain('data-slot="onboarding-ai-keyless"'); - expect(html).toContain('href="/settings/ai"'); - }, - ); + it("says the provider is not answering only when a failure earned it", () => { + aiProviderState.data = { + managedBy: "server", + serverProviderHealth: "unhealthy", + serverProviderOffer: false, + serverProviderConsent: false, + }; + const html = render(); + expect(html).toContain('data-slot="onboarding-ai-unavailable"'); + expect(html).not.toContain('data-slot="onboarding-ai-offer-grant"'); + expect(html).not.toContain('data-slot="onboarding-ai-shared-key"'); + // The rest of the panel is untouched in every branch. + expect(html).toContain('data-slot="onboarding-ai-keyless"'); + expect(html).toContain('href="/settings/ai"'); + }); + + it("claims nothing either way when nobody has measured the provider", () => { + // `unknown` is an empty `provider_health` table — the state of every + // instance until its first AI call, and the most common way through this + // screen. Saying "not answering" there is a claim about something nobody + // measured, which is the one thing this panel exists not to do. + aiProviderState.data = { + managedBy: "server", + serverProviderHealth: "unknown", + serverProviderOffer: false, + serverProviderConsent: false, + }; + const html = render(); + expect(html).not.toContain('data-slot="onboarding-ai-unavailable"'); + expect(html).not.toContain('data-slot="onboarding-ai-offer-grant"'); + expect(html).not.toContain('data-slot="onboarding-ai-shared-key"'); + // The neutral panel is the whole panel, minus any claim about a provider. + expect(html).toContain('data-slot="onboarding-ai-keyless"'); + expect(html).toContain('href="/settings/ai"'); + }); it("keeps the shared-key note for the one state it was ever true of", () => { aiProviderState.data = { diff --git a/src/components/onboarding/done-screen.tsx b/src/components/onboarding/done-screen.tsx index 799a06daf..e1f22a9e1 100644 --- a/src/components/onboarding/done-screen.tsx +++ b/src/components/onboarding/done-screen.tsx @@ -35,8 +35,9 @@ interface AiProviderStatus { /** Origin of the provider that would serve this user, if any. */ managedBy?: "user" | "local" | "server" | null; /** - * v1.38.19 (wave D) — the instance-wide tri-state. Fail closed: `unknown` - * is treated exactly like `unhealthy`, here and everywhere. + * v1.38.19 (wave D) — the instance-wide tri-state. Fail closed for the + * offer: neither `unhealthy` nor `unknown` earns one. Only `unhealthy` is + * a verdict this screen may state out loud. */ serverProviderHealth?: "healthy" | "unhealthy" | "unknown"; /** Whether the shared provider may honestly be offered in one tap. */ @@ -174,17 +175,21 @@ export function DoneScreen({ state }: { state: OnboardingStateDto }) { operatorProvides && !consentOnFile && aiProvider?.serverProviderOffer === true; - // Only a health verdict paints the "not answering" card. When the offer is - // refused for a reason that is not health — the operator switched the - // assistant surfaces off, or this is a managed profile whose provider is - // the operator's by definition — the panel falls back to its neutral form - // and claims nothing either way. + // Only a health verdict somebody EARNED paints the "not answering" card — + // `unhealthy`, which means the provider was tried and the last thing that + // happened was a failure. `unknown` is not that verdict: it is an empty + // ledger, which is the state of every instance until its first AI call and + // therefore the most common way through this screen. It falls to the + // neutral panel, as does every other non-health reason the offer is + // withheld (the assistant surfaces switched off, a managed profile whose + // provider is the operator's by definition). The neutral panel claims + // nothing either way, which is the only honest thing to say about a state + // nobody measured. const sharedUnavailable = operatorProvides && !consentOnFile && !offerShared && - (aiProvider?.serverProviderHealth === "unhealthy" || - aiProvider?.serverProviderHealth === "unknown"); + aiProvider?.serverProviderHealth === "unhealthy"; return (
{ await expect(readServerProviderHealth(NOW)).resolves.toBe("healthy"); }); - it("is unhealthy when the last success is older than the window", async () => { + it("is unknown when the last outcome was a success, just not a recent one", async () => { + // A quiet single-person instance nobody asked anything for two days has + // not observed a failure. "It is not working" would be a claim about + // something that was never measured; the offer is withheld either way. rows({ lastResult: "ok", lastOkAt: ago(SERVER_PROVIDER_FRESH_WINDOW_MS + 60_000), }); - await expect(readServerProviderHealth(NOW)).resolves.toBe("unhealthy"); + await expect(readServerProviderHealth(NOW)).resolves.toBe("unknown"); }); it("is unhealthy when rows exist but none ever succeeded", async () => { @@ -80,6 +83,16 @@ describe("readServerProviderHealth", () => { await expect(readServerProviderHealth(NOW)).resolves.toBe("unhealthy"); }); + it("is unhealthy when a stale success is the only thing beside a failure", async () => { + // Somebody tried and it failed, and no success has contradicted that + // since. That verdict was earned and the card may state it. + rows( + { lastResult: "ok", lastOkAt: ago(SERVER_PROVIDER_FRESH_WINDOW_MS * 2) }, + { lastResult: "hard_failed", lastOkAt: null }, + ); + await expect(readServerProviderHealth(NOW)).resolves.toBe("unhealthy"); + }); + it("lets a 401 in cooldown beat a fresh ok — a dead key means the same for everyone", async () => { rows( { lastResult: "ok", lastOkAt: ago(60 * 1000) }, diff --git a/src/lib/ai/server-provider-health.ts b/src/lib/ai/server-provider-health.ts index aea613a71..3748028ab 100644 --- a/src/lib/ai/server-provider-health.ts +++ b/src/lib/ai/server-provider-health.ts @@ -27,8 +27,10 @@ * ledger fails open (`provider-health-ledger.ts`) because there it protects * a generation already under way — contributing nothing is the safe outcome. * Here the output is a promise made to a person, so a read error, a timeout - * or an empty table is `unknown`, and `unknown` buys nothing: the caller - * treats it exactly like `unhealthy`. + * or an empty table is `unknown`, and `unknown` never buys an offer. It does + * NOT buy the opposite claim either: an empty table is the state of every + * instance until its first AI call, and telling that operator "the provider + * is not answering" is the same unmeasured claim the offer itself avoids. * * No probe. `POST /api/ai/test` exists and would be the direct answer, but it * is a real completion billed to whoever the chain resolves to — usually the @@ -44,9 +46,11 @@ import { annotate } from "@/lib/logging/context"; * * healthy — the instance's shared provider demonstrably served somebody * inside the freshness window, and no credential is benched. - * unhealthy — it has been tried and it is not working. - * unknown — nobody has tried it yet on this instance, or the read - * failed. Never an offer; see the module note on failing closed. + * unhealthy — somebody tried it and the last thing that happened was a + * failure. A verdict a screen is allowed to state out loud. + * unknown — nobody has tried it lately on this instance, or the read + * failed. Never an offer either, but it is also not a verdict: + * a caller may withhold the offer and must not claim an outage. */ export type ServerProviderHealth = "healthy" | "unhealthy" | "unknown"; @@ -108,5 +112,15 @@ export async function readServerProviderHealth( const workedRecently = rows.some( (row) => row.lastOkAt !== null && row.lastOkAt.getTime() >= floor, ); - return workedRecently ? "healthy" : "unhealthy"; + if (workedRecently) return "healthy"; + + // Nothing succeeded inside the window, and that alone is not a failure. + // `unhealthy` is a statement the screen puts to a person — "it is not + // answering" — so it has to be earned by an outcome somebody actually + // observed: a row whose LAST outcome was a failure, uncontradicted by a + // recent success. When every row's last outcome was a success and it is + // merely old, nobody has asked this provider for anything lately, and the + // honest answer is that we do not know. Either way there is no offer. + const lastOutcomeFailed = rows.some((row) => row.lastResult !== "ok"); + return lastOutcomeFailed ? "unhealthy" : "unknown"; } diff --git a/src/lib/openapi/routes/profile.ts b/src/lib/openapi/routes/profile.ts index e8b805e81..246e215cf 100644 --- a/src/lib/openapi/routes/profile.ts +++ b/src/lib/openapi/routes/profile.ts @@ -567,7 +567,7 @@ const aiProviderResponse = z serverProviderHealth: z .enum(["healthy", "unhealthy", "unknown"]) .describe( - "Instance-wide health of the operator's shared providers (`admin-openai` / `admin-codex`), folded across all accounts and reduced to a tri-state: `healthy` when one of them demonstrably served somebody within the last 24 h and no credential sits in its auth-failure cooldown, `unhealthy` when they have been tried and are not working, `unknown` when nothing has been tried yet on this instance or the read failed. Deliberately carries no count, no timestamp and no account identity — 'the provider of this instance last worked' is a statement about the instance, not about other people. Fails closed: treat `unknown` exactly like `unhealthy`. Reading this never probes a provider.", + "Instance-wide health of the operator's shared providers (`admin-openai` / `admin-codex`), folded across all accounts and reduced to a tri-state: `healthy` when one of them demonstrably served somebody within the last 24 h and no credential sits in its auth-failure cooldown, `unhealthy` when the last outcome somebody observed was a failure, `unknown` when nothing has been tried lately on this instance (including a brand-new instance whose ledger is still empty) or the read failed. Deliberately carries no count, no timestamp and no account identity — 'the provider of this instance last worked' is a statement about the instance, not about other people. Fails closed for the offer: neither `unhealthy` nor `unknown` may be offered. They are not interchangeable for copy, though — `unhealthy` is a verdict a client may state, `unknown` is the absence of one and must not be reported as an outage. Reading this never probes a provider.", ), serverProviderOffer: z .boolean()