Skip to content

Retry onboarding credential probe instead of pretending no key #389

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
TheGreatAxios merged 1 commit into from
Aug 24, 2026
Merged

Retry onboarding credential probe instead of pretending no key #389

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 27 additions & 5 deletions apps/web/src/onboarding.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 —
Expand All @@ -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<boolean> {
export async function hasActiveCredential(
tenantId: string,
): Promise<ActiveCredentialProbe> {
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" };
}
}

Expand All @@ -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";
Expand All @@ -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;
}
Expand Down
7 changes: 5 additions & 2 deletions apps/web/src/pages/home-page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
31 changes: 24 additions & 7 deletions apps/web/src/pages/onboarding-page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 });
Expand All @@ -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 });
Expand Down
155 changes: 155 additions & 0 deletions apps/web/test/onboarding.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { App } from "../src/app";
import { NavigationProvider } from "../src/navigation";
import {
CREDENTIAL_PROVIDERS,
hasActiveCredential,
PRIMARY_CREDENTIAL_PROVIDERS,
readHuggingFaceConnectReturn,
readOpenRouterConnectReturn,
Expand Down Expand Up @@ -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 () =>
Expand Down Expand Up @@ -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(
<App
path={ONBOARDING_PATH}
navigate={navigate}
session={signedIn}
onSignedIn={noop}
onSignOut={noop}
onRetry={noop}
/>,
);
});
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(
<App
path={ONBOARDING_PATH}
navigate={navigate}
session={signedIn}
onSignedIn={noop}
onSignOut={noop}
onRetry={noop}
/>,
);
});
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") {
Expand Down
Loading