From ae5f400acc943019a0931c194960659a7701659f Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Mon, 24 Aug 2026 07:17:17 -0700 Subject: [PATCH] Retry onboarding credential probe instead of pretending no key Closes CL-6868 --- apps/web/src/onboarding.ts | 32 ++++- apps/web/src/pages/home-page.tsx | 7 +- apps/web/src/pages/onboarding-page.tsx | 31 +++-- apps/web/test/onboarding.test.tsx | 155 +++++++++++++++++++++++++ 4 files changed, 211 insertions(+), 14 deletions(-) diff --git a/apps/web/src/onboarding.ts b/apps/web/src/onboarding.ts index 22af8b30f..63c28f207 100644 --- a/apps/web/src/onboarding.ts +++ b/apps/web/src/onboarding.ts @@ -24,6 +24,17 @@ const CredentialsPage = type({ data: type({ status: "string" }).array(), }); +/** + * Outcome of the cheap credentials read used before trusting a + * `seeded: true` hard-skip. A probe that cannot complete is never + * collapsed into `none` (CL-6868): that would open paste-a-key as if no + * key exists when one may already be connected. + */ +export type ActiveCredentialProbe = + | { readonly kind: "active" } + | { readonly kind: "none" } + | { readonly kind: "error" }; + /** * Whether `tenantId` has at least one credential in the `active` * status. A cheap, single read — no provider/catalog chain resolution — @@ -32,16 +43,20 @@ const CredentialsPage = type({ * without adding a second round trip's worth of catalog/provider * plumbing to the browser bundle. */ -export async function hasActiveCredential(tenantId: string): Promise { +export async function hasActiveCredential( + tenantId: string, +): Promise { try { const response = await fetch(`/api/tenants/${tenantId}/credentials`); - if (!response.ok) return false; + if (!response.ok) return { kind: "error" }; const body: unknown = await response.json().catch(() => null); const parsed = CredentialsPage(body); - if (parsed instanceof type.errors) return false; - return parsed.data.some((c) => c.status === "active"); + if (parsed instanceof type.errors) return { kind: "error" }; + return parsed.data.some((c) => c.status === "active") + ? { kind: "active" } + : { kind: "none" }; } catch { - return false; + return { kind: "error" }; } } @@ -56,6 +71,12 @@ const ErrorEnvelope = type({ const FALLBACK_ERROR_MESSAGE = "Setting up your workbench hit a snag — we're on it. Try again in a moment."; +/** Consumer sentence when the pre-skip credentials probe cannot complete + * (CL-6868). Paired with Retry on the current setup step — never opens + * paste-a-key as if no key exists. */ +export const CREDENTIAL_PROBE_FAILURE_MESSAGE = + "Checking for a connected key hit a snag. Try again in a moment."; + export type ProvisionOutcome = | { readonly kind: "existing-member"; @@ -72,6 +93,7 @@ export type ProvisionOutcome = * personal bench. Lets the onboarding page independently confirm * (`hasActiveCredential`) a working credential exists before * trusting `seeded: true` enough to skip the credential step. + * A probe `error` must not be treated as absent (CL-6868). */ readonly tenantId?: string; } diff --git a/apps/web/src/pages/home-page.tsx b/apps/web/src/pages/home-page.tsx index 766832f1b..a156db062 100644 --- a/apps/web/src/pages/home-page.tsx +++ b/apps/web/src/pages/home-page.tsx @@ -116,9 +116,12 @@ export function HomeRoute({ navigate(NEW_WORKBENCH_PATH); return; } - void hasActiveCredential(selectedTenantId).then((hasCredential) => { + void hasActiveCredential(selectedTenantId).then((probe) => { if (cancelled) return; - if (!hasCredential) { + // Only a confirmed miss means "connect a provider". A probe + // failure must not pretend no key exists (CL-6868) — keep + // waiting and retry; the key may already be connected. + if (probe.kind === "none") { setState({ kind: "needs-provider" }); return; } diff --git a/apps/web/src/pages/onboarding-page.tsx b/apps/web/src/pages/onboarding-page.tsx index bb2ff382c..a3c3a0646 100644 --- a/apps/web/src/pages/onboarding-page.tsx +++ b/apps/web/src/pages/onboarding-page.tsx @@ -37,6 +37,7 @@ import { OLLAMA_PLACEHOLDER_SECRET } from "@workbench/hub-client/credential-test import { useNavigate } from "../navigation"; import { completeSetup, + CREDENTIAL_PROBE_FAILURE_MESSAGE, CREDENTIAL_PROVIDERS, hasActiveCredential, HUGGINGFACE_CONNECT_START_PATH, @@ -224,11 +225,21 @@ export function OnboardingPage({ user }: { readonly user: SessionUser }) { // Confirm one independently (a cheap credentials read) before // handing off; no tenantId (should not happen alongside // seeded: true) falls through to the credential step too. - const confirmed = - result.tenantId !== undefined && - (await hasActiveCredential(result.tenantId)); - if (confirmed) { + // A probe that cannot complete stays on this setup step with + // Retry (CL-6868) — never opens paste-a-key as if none exists. + if (result.tenantId === undefined) { + setResumingUnseeded(false); + setState({ phase: "credential", error: null }); + return; + } + const probe = await hasActiveCredential(result.tenantId); + if (probe.kind === "active") { navigate("/"); + } else if (probe.kind === "error") { + setState({ + phase: "provisioning-error", + message: CREDENTIAL_PROBE_FAILURE_MESSAGE, + }); } else { setResumingUnseeded(false); setState({ phase: "credential", error: null }); @@ -245,11 +256,17 @@ export function OnboardingPage({ user }: { readonly user: SessionUser }) { } else if (result.kind === "provisioned" && result.seeded) { // The seed run's validation trigger only proves a workflow run // started, never that it succeeded against a real credential. - // Confirm one independently before handing off. - const confirmed = await hasActiveCredential(result.tenantId); - if (confirmed) { + // Confirm one independently before handing off. Probe failure + // stays here with Retry (CL-6868) — never paste-a-key as none. + const probe = await hasActiveCredential(result.tenantId); + if (probe.kind === "active") { setResumingUnseeded(false); navigate("/"); + } else if (probe.kind === "error") { + setState({ + phase: "provisioning-error", + message: CREDENTIAL_PROBE_FAILURE_MESSAGE, + }); } else { setResumingUnseeded(false); setState({ phase: "credential", error: null }); diff --git a/apps/web/test/onboarding.test.tsx b/apps/web/test/onboarding.test.tsx index c6e79985e..ab7ebb80c 100644 --- a/apps/web/test/onboarding.test.tsx +++ b/apps/web/test/onboarding.test.tsx @@ -19,6 +19,7 @@ import { App } from "../src/app"; import { NavigationProvider } from "../src/navigation"; import { CREDENTIAL_PROVIDERS, + hasActiveCredential, PRIMARY_CREDENTIAL_PROVIDERS, readHuggingFaceConnectReturn, readOpenRouterConnectReturn, @@ -253,6 +254,50 @@ describe("triggerFirstLoginProvisioning", () => { }); }); +describe("hasActiveCredential", () => { + // CL-6868: a transient credentials read must never coerce to "no key" — + // that path opens paste-a-key as if none exists when one may already be + // connected. Probe failures are a distinct outcome from a confirmed miss. + test("an active credential is reported as active", async () => { + globalThis.fetch = (async () => + json({ + data: [{ id: "cred_1", status: "active" }], + nextCursor: null, + })) as unknown as typeof fetch; + + expect(await hasActiveCredential("ten_1")).toEqual({ kind: "active" }); + }); + + test("an empty credentials page is a confirmed none, not a probe failure", async () => { + globalThis.fetch = (async () => + json({ data: [], nextCursor: null })) as unknown as typeof fetch; + + expect(await hasActiveCredential("ten_1")).toEqual({ kind: "none" }); + }); + + test("a network failure is a probe error, never coerced to none", async () => { + globalThis.fetch = (async () => { + throw new Error("connection refused"); + }) as unknown as typeof fetch; + + expect(await hasActiveCredential("ten_1")).toEqual({ kind: "error" }); + }); + + test("a non-OK credentials response is a probe error, never coerced to none", async () => { + globalThis.fetch = (async () => + json({ error: "boom" }, 500)) as unknown as typeof fetch; + + expect(await hasActiveCredential("ten_1")).toEqual({ kind: "error" }); + }); + + test("an unparseable credentials body is a probe error, never coerced to none", async () => { + globalThis.fetch = (async () => + json({ not: "credentials" })) as unknown as typeof fetch; + + expect(await hasActiveCredential("ten_1")).toEqual({ kind: "error" }); + }); +}); + describe("submitCredential", () => { test("a rejected key comes back as a rejected outcome with the hub's own reason", async () => { globalThis.fetch = (async () => @@ -569,6 +614,116 @@ describe("App landing fresh on a hub-seeded workbench", () => { } }); + test("provisioned, seeded: true stays on setup with Retry when the credential probe fails (CL-6868)", async () => { + // Transient credentials-read failure must not open paste-a-key as if + // none exists — the key may already be connected. Stay on the setup + // step with one consumer sentence and Retry. + globalThis.fetch = (async (url: string) => { + if (url === "/api/onboarding/provision") { + return json({ + kind: "provisioned", + tenantId: "ten_1", + tenantSlug: "ada-user1", + seeded: true, + }); + } + if (url === "/api/tenants/ten_1/credentials") { + throw new Error("connection refused"); + } + throw new Error(`unexpected fetch: ${url}`); + }) as unknown as typeof fetch; + + const container = document.createElement("div"); + document.body.appendChild(container); + const root = createRoot(container); + const { navigate, calls } = trackedNavigate(); + try { + act(() => { + root.render( + , + ); + }); + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 10)); + }); + + const text = container.textContent ?? ""; + expect(text).not.toContain("Bring your own AI"); + expect(text).not.toContain("or paste a provider API key"); + expect(text).toContain( + "Checking for a connected key hit a snag. Try again in a moment.", + ); + const retry = Array.from(container.querySelectorAll("button")).find( + (button) => button.textContent === "Try again", + ); + expect(retry).not.toBeUndefined(); + expect(calls).toEqual([]); + } finally { + act(() => root.unmount()); + container.remove(); + } + }); + + test("existing-member, seeded: true stays on setup with Retry when the credential probe fails (CL-6868)", async () => { + globalThis.fetch = (async (url: string) => { + if (url === "/api/onboarding/provision") { + return json({ + kind: "existing-member", + seeded: true, + tenantId: "ten_1", + }); + } + if (url === "/api/tenants/ten_1/credentials") { + return json({ error: "boom" }, 500); + } + throw new Error(`unexpected fetch: ${url}`); + }) as unknown as typeof fetch; + + const container = document.createElement("div"); + document.body.appendChild(container); + const root = createRoot(container); + const { navigate, calls } = trackedNavigate(); + try { + act(() => { + root.render( + , + ); + }); + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 10)); + }); + + const text = container.textContent ?? ""; + expect(text).not.toContain("Bring your own AI"); + expect(text).not.toContain("or paste a provider API key"); + expect(text).toContain( + "Checking for a connected key hit a snag. Try again in a moment.", + ); + const retry = Array.from(container.querySelectorAll("button")).find( + (button) => button.textContent === "Try again", + ); + expect(retry).not.toBeUndefined(); + expect(calls).toEqual([]); + } finally { + act(() => root.unmount()); + container.remove(); + } + }); + test("a freshly provisioned bench with no working credential still shows the credential step", async () => { globalThis.fetch = (async (url: string) => { if (url === "/api/onboarding/provision") {