From c457036fd9a9c74812e4efa123832002b95e4bab Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sun, 9 Aug 2026 12:56:40 -0700 Subject: [PATCH 1/4] CL-5782: Add tests for named provision and no silent mint --- apps/web/test/onboarding.test.tsx | 65 +++++++++++++++++++++- packages/onboarding/test/provision.test.ts | 56 +++++++++++++++++-- 2 files changed, 114 insertions(+), 7 deletions(-) diff --git a/apps/web/test/onboarding.test.tsx b/apps/web/test/onboarding.test.tsx index de2bcf984..d15bf7dee 100644 --- a/apps/web/test/onboarding.test.tsx +++ b/apps/web/test/onboarding.test.tsx @@ -47,7 +47,7 @@ describe("triggerFirstLoginProvisioning", () => { 500, )) as unknown as typeof fetch; - const result = await triggerFirstLoginProvisioning(); + const result = await triggerFirstLoginProvisioning("Ada's bench"); expect(result).toEqual({ kind: "error", message: "Could not provision a workbench for this account.", @@ -59,7 +59,7 @@ describe("triggerFirstLoginProvisioning", () => { throw new Error("connection refused"); }) as unknown as typeof fetch; - const result = await triggerFirstLoginProvisioning(); + const result = await triggerFirstLoginProvisioning("Ada's bench"); expect(result.kind).toBe("error"); if (result.kind !== "error") throw new Error("unreachable"); expect(result.message).toContain("connection refused"); @@ -69,9 +69,68 @@ describe("triggerFirstLoginProvisioning", () => { globalThis.fetch = (async () => json({ kind: "existing-member" })) as unknown as typeof fetch; - const result = await triggerFirstLoginProvisioning(); + const result = await triggerFirstLoginProvisioning("Ada's bench"); expect(result).toEqual({ kind: "existing-member" }); }); + + test("sends the workbench name in the provision request body", async () => { + let requestBody: unknown = undefined; + let requestInit: RequestInit | undefined; + globalThis.fetch = (async (_url: string, init: RequestInit) => { + requestInit = init; + requestBody = + init.body === undefined ? undefined : JSON.parse(init.body as string); + return json({ kind: "existing-member" }); + }) as unknown as typeof fetch; + + await triggerFirstLoginProvisioning("Research bench"); + expect(requestBody).toEqual({ name: "Research bench" }); + expect(requestInit?.method).toBe("POST"); + }); + + test("omits a body when no name is given (the shell routing probe)", async () => { + let sentBody: unknown = "__sentinel__"; + globalThis.fetch = (async (_url: string, init: RequestInit) => { + sentBody = init.body; + return json({ kind: "existing-member" }); + }) as unknown as typeof fetch; + + await triggerFirstLoginProvisioning(); + expect(sentBody).toBeUndefined(); + }); + + test("a provisioned bench with a server seed stays a 'provisioned' outcome — the credential step is not skipped", async () => { + // Regression guard: a server-side seed (operator-configured key) must + // not collapse the outcome into `existing-member` or otherwise hide + // that the bench was just provisioned. The wizard relies on this + // distinction to render the credential step as pre-satisfied (with a + // skip option) rather than branching past it entirely. + let requestBody: unknown = undefined; + globalThis.fetch = (async (_url: string, init: RequestInit) => { + requestBody = + init.body === undefined ? undefined : JSON.parse(init.body as string); + return json({ + kind: "provisioned", + tenantId: "ten_1", + tenantSlug: "ada-user1", + seeded: true, + seedSkipReason: "operator_seed_key", + }); + }) as unknown as typeof fetch; + + const result = await triggerFirstLoginProvisioning("Ada's bench"); + expect(requestBody).toEqual({ name: "Ada's bench" }); + expect(result).toEqual({ + kind: "provisioned", + tenantId: "ten_1", + tenantSlug: "ada-user1", + seeded: true, + seedSkipReason: "operator_seed_key", + }); + // The credential step stays in the flow precisely because seeded is + // reported faithfully, not folded away. + if (result.kind === "provisioned") expect(result.seeded).toBe(true); + }); }); describe("testCredential", () => { diff --git a/packages/onboarding/test/provision.test.ts b/packages/onboarding/test/provision.test.ts index de06cb195..66a376ba1 100644 --- a/packages/onboarding/test/provision.test.ts +++ b/packages/onboarding/test/provision.test.ts @@ -131,6 +131,7 @@ describe("provisionPersonalTenantIfNeeded", () => { hubUrl: "http://localhost:3000", userId: "user_1", userEmail: "alice@example.com", + displayName: "Alice's Lab", pushWorkflow: noopPush, log: collector().log, }); @@ -164,6 +165,7 @@ describe("provisionPersonalTenantIfNeeded", () => { hubUrl: "http://localhost:3000", userId: "user_1", userEmail: "alice@example.com", + displayName: "Alice's Lab", pushWorkflow: noopPush, log: collector().log, }), @@ -203,13 +205,18 @@ describe("provisionPersonalTenantIfNeeded", () => { }; } if (method === "POST" && path === "/api/tenants") { - const parsed = body as { parentId?: string; slug: string }; + const parsed = body as { + parentId?: string; + slug: string; + name: string; + }; expect(parsed.parentId).toBeUndefined(); + expect(parsed.name).toBe("Alice's Lab"); return { status: 201, data: { id: TENANT_ID, - name: "alice's workbench", + name: parsed.name, slug: parsed.slug, domain: `${parsed.slug}.localhost`, createdAt: "2026-01-01T00:00:00.000Z", @@ -227,6 +234,7 @@ describe("provisionPersonalTenantIfNeeded", () => { hubUrl: "http://localhost:3000", userId: "user_1", userEmail: "alice@example.com", + displayName: "Alice's Lab", pushWorkflow: noopPush, log, }); @@ -238,6 +246,39 @@ describe("provisionPersonalTenantIfNeeded", () => { expect(lines.some((line) => line.includes("ANTHROPIC_API_KEY"))).toBe(true); }); + test("zero principals without a display name: returns needs-onboarding and creates nothing", async () => { + const lines: string[] = []; + const log = (line: string) => lines.push(line); + let tenantsPosted = 0; + const api: ApiCall = async (method, path) => { + if (method === "GET" && path === "/api/me/principals") { + return { + status: 200, + data: { data: [], nextCursor: null }, + cookies: [], + }; + } + if (method === "POST" && path === "/api/tenants") { + tenantsPosted += 1; + throw new Error("must not create without a display name"); + } + throw new Error(`unexpected call: ${method} ${path}`); + }; + + const result = await provisionPersonalTenantIfNeeded({ + api, + cookies: ["session=abc"], + hubUrl: "http://localhost:3000", + userId: "user_1", + userEmail: "alice@example.com", + pushWorkflow: noopPush, + log, + }); + + expect(result).toEqual({ kind: "needs-onboarding" }); + expect(tenantsPosted).toBe(0); + }); + test("zero principals with a seed model configured: provisions under the operator tenant and seeds the default workflow", async () => { let principalsCalls = 0; const startedRuns: string[] = []; @@ -271,13 +312,18 @@ describe("provisionPersonalTenantIfNeeded", () => { }; } if (method === "POST" && path === "/api/tenants") { - const parsed = body as { parentId?: string; slug: string }; + const parsed = body as { + parentId?: string; + slug: string; + name: string; + }; expect(parsed.parentId).toBe("ten_operator"); + expect(parsed.name).toBe("Alice's Lab"); return { status: 201, data: { id: TENANT_ID, - name: "alice's workbench", + name: parsed.name, slug: parsed.slug, domain: `${parsed.slug}.localhost`, parentId: "ten_operator", @@ -384,6 +430,7 @@ describe("provisionPersonalTenantIfNeeded", () => { hubUrl: "http://localhost:3000", userId: "user_1", userEmail: "alice@example.com", + displayName: "Alice's Lab", operatorTenantId: "ten_operator", seedModel: MODEL, pushWorkflow: noopPush, @@ -571,6 +618,7 @@ describe("provisionPersonalTenantIfNeeded", () => { hubUrl: "http://localhost:3000", userId: "user_1", userEmail: "alice@example.com", + displayName: "Alice's Lab", seedModel: MODEL, pushWorkflow: noopPush, log: collector().log, From 8848bb8079351244a2e39798b0f6b60b52cb71c9 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sun, 9 Aug 2026 12:56:40 -0700 Subject: [PATCH 2/4] CL-5782: Require workbench name; stop silent personal-bench mint Provision accepts an optional display name and returns needs-onboarding when a first-login probe has no name, so the shell routes into the naming wizard instead of minting an email-derived bench. The onboarding page sends the chosen name and keeps a single credential step (pre-satisfied when the server already seeded). --- apps/web/src/main.tsx | 14 ++- apps/web/src/onboarding.ts | 23 ++-- apps/web/src/pages/onboarding-page.tsx | 155 +++++++++++++++++++------ packages/onboarding/src/provision.ts | 14 ++- packages/onboarding/src/routes.ts | 16 +++ 5 files changed, 174 insertions(+), 48 deletions(-) diff --git a/apps/web/src/main.tsx b/apps/web/src/main.tsx index a67a2a9fc..f9fe8bb08 100644 --- a/apps/web/src/main.tsx +++ b/apps/web/src/main.tsx @@ -41,10 +41,9 @@ function Root() { // The first-login hook: once per session that reaches signed-in, ask // the hub whether this is a session with zero principals anywhere. - // Idempotent on the hub side, so re-running it on a page reload for - // an existing member costs one read and nothing else. A failure here - // blocks the shell entirely — a signed-in user with no bench and a - // failed provisioning attempt has nothing useful to do in the app. + // Without a display name the hub does not mint a bench — it returns + // needs-onboarding so we route into the naming wizard. Existing members + // cost one read. A failure blocks the shell entirely. const [provisioningError, setProvisioningError] = useState( null, ); @@ -56,8 +55,11 @@ function Root() { setProvisioningError(null); void triggerFirstLoginProvisioning().then((result) => { if (cancelled) return; - if (result.kind === "provisioned") navigate(ONBOARDING_PATH); - else if (result.kind === "error") setProvisioningError(result.message); + if (result.kind === "needs-onboarding" || result.kind === "provisioned") { + navigate(ONBOARDING_PATH); + } else if (result.kind === "error") { + setProvisioningError(result.message); + } }); return () => { cancelled = true; diff --git a/apps/web/src/onboarding.ts b/apps/web/src/onboarding.ts index 977d244dc..34de0f0a5 100644 --- a/apps/web/src/onboarding.ts +++ b/apps/web/src/onboarding.ts @@ -1,15 +1,14 @@ // The browser side of the first-login hook: one call against the // hub's native onboarding route, made once per session. A session with -// zero principals anywhere gets a personal bench provisioned server-side -// (see @workbench/onboarding); this just learns whether that happened -// so the interface can route into the onboarding placeholder — and -// distinguishes a real failure from "nothing to do", so a broken -// provisioning call never leaves the user silently benchless. +// zero principals and no display name is reported as needs-onboarding so +// the UI can route into the naming wizard; only an explicit name creates +// the personal bench. Distinguishes real failures from "nothing to do", +// so a broken provisioning call never leaves the user silently benchless. import { type } from "arktype"; const ProvisionResult = type({ - kind: "'existing-member' | 'provisioned'", + kind: "'existing-member' | 'provisioned' | 'needs-onboarding'", "tenantId?": "string", "tenantSlug?": "string", "seeded?": "boolean", @@ -22,6 +21,7 @@ const ErrorEnvelope = type({ export type ProvisionOutcome = | { readonly kind: "existing-member" } + | { readonly kind: "needs-onboarding" } | { readonly kind: "provisioned"; readonly tenantId: string; @@ -31,10 +31,18 @@ export type ProvisionOutcome = } | { readonly kind: "error"; readonly message: string }; -export async function triggerFirstLoginProvisioning(): Promise { +export async function triggerFirstLoginProvisioning( + displayName?: string, +): Promise { try { const response = await fetch("/api/onboarding/provision", { method: "POST", + ...(displayName !== undefined + ? { + headers: { "content-type": "application/json" }, + body: JSON.stringify({ name: displayName }), + } + : {}), }); const body: unknown = await response.json().catch(() => null); if (!response.ok) { @@ -55,6 +63,7 @@ export async function triggerFirstLoginProvisioning(): Promise }; } if (parsed.kind === "existing-member") return { kind: "existing-member" }; + if (parsed.kind === "needs-onboarding") return { kind: "needs-onboarding" }; if ( parsed.tenantId === undefined || parsed.tenantSlug === undefined || diff --git a/apps/web/src/pages/onboarding-page.tsx b/apps/web/src/pages/onboarding-page.tsx index fb19c90fe..685d271be 100644 --- a/apps/web/src/pages/onboarding-page.tsx +++ b/apps/web/src/pages/onboarding-page.tsx @@ -1,11 +1,12 @@ -// First-run, end to end: land here fresh with no usable inference -// credential, add one for real, and watch the default routines fire. -// The heavy lifting — proving the key with a real call, seeding the -// bench, deploying and confirming every default workflow — all happens -// server-side in `@workbench/onboarding`; this page is the guided -// wizard around it. A session that already has a seeded bench (an -// operator-configured seed key, or a returning member) skips straight -// to the orientation cards this screen always ended with. +// First-run wizard in three steps: name your workbench, add an +// inference credential, then get oriented. The heavy lifting — proving +// the key with a real call, seeding the bench, deploying and confirming +// every default workflow — happens server-side in `@workbench/onboarding`; +// this page is the guided shell around it. The credential step is always +// part of the flow: when the server already has a usable seed (an +// operator-configured key, or a returning member) it renders +// pre-satisfied with a skip option rather than branching into a +// different tree that hides the step entirely. import { Button, @@ -26,11 +27,11 @@ import { AtSign, Bot, CircleAlert, + CircleCheck, KeyRound, - Library, MessageSquare, } from "lucide-react"; -import { useCallback, useEffect, useState } from "react"; +import { useCallback, useState } from "react"; import type { FormEvent } from "react"; import { Link, useNavigate } from "../navigation"; @@ -46,19 +47,13 @@ const GUIDANCE_CARDS = [ icon: , title: "Channels", description: - "Conversations with your team and your agents live in channels, the same way threads do — a starter channel is ready for you below.", + "Conversations with your team and your agents live in channels. Your starter channel is ready — head there to send your first message.", }, { icon: , title: "Routines", description: - "A routine is a workflow an agent runs on your behalf — scheduled, triggered, or kicked off right from chat. Runs show up under Runs as they execute.", - }, - { - icon: , - title: "Library", - description: - "Every workflow definition running anywhere in your benches is browsable in the Library, so you can see what a routine actually does before trusting it.", + "A routine is a workflow an agent runs on your behalf — scheduled, triggered, or kicked off right from a channel. Your bench ships with a couple of starter routines already running.", }, { icon: , @@ -78,7 +73,8 @@ function routineLabel(assetName: string): string { } type WizardState = - | { readonly phase: "loading" } + | { readonly phase: "naming" } + | { readonly phase: "provisioning" } | { readonly phase: "provisioning-error"; readonly message: string } | { readonly phase: "credential"; readonly error: string | null } | { readonly phase: "submitting" } @@ -106,11 +102,20 @@ function GuidanceCards() { } function wizardSteps(phase: WizardState["phase"]): WorkflowStep[] { + const nameDone = phase !== "naming"; const credentialDone = phase === "seeded" || phase === "guidance"; - const credentialCurrent = phase === "credential" || phase === "submitting"; + const credentialCurrent = + phase === "credential" || + phase === "submitting" || + phase === "provisioning"; return [ { number: 1, + label: "Name your workbench", + status: nameDone ? "completed" : "current", + }, + { + number: 2, label: "Add a credential", status: credentialDone ? "completed" @@ -119,13 +124,13 @@ function wizardSteps(phase: WizardState["phase"]): WorkflowStep[] { : "pending", }, { - number: 2, + number: 3, label: "Run your first routine", status: phase === "seeded" ? "completed" - : credentialCurrent - ? "pending" + : phase === "guidance" + ? "completed" : "pending", }, ]; @@ -166,25 +171,52 @@ function ProviderPicker({ export function OnboardingPage() { const navigate = useNavigate(); - const [state, setState] = useState({ phase: "loading" }); + const [state, setState] = useState({ phase: "naming" }); + const [workbenchName, setWorkbenchName] = useState(""); const [provider, setProvider] = useState("anthropic"); const [apiKey, setApiKey] = useState(""); + // Whether the server already had a usable seed when we provisioned. + // Kept out of WizardState so a failed own-key submit doesn't wipe the + // skip option — the credential step stays in place either way. + const [preSatisfied, setPreSatisfied] = useState(false); + const [skipReason, setSkipReason] = useState(null); - const runProvisioning = useCallback(() => { - setState({ phase: "loading" }); - void triggerFirstLoginProvisioning().then((result) => { + const runProvisioning = useCallback((name: string) => { + setState({ phase: "provisioning" }); + void triggerFirstLoginProvisioning(name).then((result) => { if (result.kind === "error") { setState({ phase: "provisioning-error", message: result.message }); } else if (result.kind === "existing-member") { - setState({ phase: "guidance" }); - } else if (result.seeded) { - setState({ phase: "guidance" }); - } else { + setPreSatisfied(true); + setSkipReason(null); setState({ phase: "credential", error: null }); + } else if (result.kind === "provisioned" && result.seeded) { + setPreSatisfied(true); + setSkipReason(result.seedSkipReason ?? null); + setState({ phase: "credential", error: null }); + } else if (result.kind === "provisioned") { + setPreSatisfied(false); + setSkipReason(null); + setState({ phase: "credential", error: null }); + } else { + // needs-onboarding after an explicit name should not happen; treat + // as a soft error so the user can retry naming. + setState({ + phase: "provisioning-error", + message: + "The hub did not create your workbench. Try a different name.", + }); } }); }, []); - useEffect(runProvisioning, [runProvisioning]); + + const handleNameSubmit = useCallback( + (event: FormEvent) => { + event.preventDefault(); + runProvisioning(workbenchName); + }, + [runProvisioning, workbenchName], + ); const handleSubmitCredential = useCallback( (event: FormEvent) => { @@ -198,6 +230,8 @@ export function OnboardingPage() { workflows: outcome.workflows, }); } else { + // preSatisfied is intentionally preserved: a bad own-key + // submit must not remove the skip path the server seed gave. setState({ phase: "credential", error: outcome.message }); } }); @@ -205,7 +239,39 @@ export function OnboardingPage() { [provider, apiKey], ); - if (state.phase === "loading") { + if (state.phase === "naming") { + return ( + +
+ +
+ + setWorkbenchName(event.target.value)} + required + aria-describedby="onboarding-workbench-name-help" + autoFocus + /> +

+ Used as the display name for your bench. +

+ +
+
+ + ); + } + + if (state.phase === "provisioning") { return (
@@ -223,7 +289,10 @@ export function OnboardingPage() { title="Couldn't set up your workbench" description={state.message} action={ - } @@ -282,6 +351,24 @@ export function OnboardingPage() { description="Your workbench needs an inference credential before any agent or routine can run. Pick a provider and paste your own key — it's used only for this bench." > + {preSatisfied && ( + } + title="A working key is already in place" + description={ + skipReason ?? + "An operator-configured credential is set, so agents and routines can run right away. Add your own key below to use it instead, or skip ahead to your channel." + } + action={ + + } + /> + )}
's workbench"` so callers that only probe membership + * (no naming step yet) still get a sensible default if they create. */ + displayName?: string; operatorTenantId?: string; seedModel?: ModelSource; pushWorkflow: WorkflowPusher; @@ -222,8 +227,15 @@ export async function provisionPersonalTenantIfNeeded( return { kind: "existing-member", seeded: true }; } + // No membership yet. Creation requires an explicit display name from the + // onboarding naming step — a shell membership probe (no name) must not + // silently mint a personal bench. + if (args.displayName === undefined || args.displayName.trim().length === 0) { + return { kind: "needs-onboarding" }; + } + const tenantCreateBody: { name: string; slug: string; parentId?: string } = { - name: `${args.userEmail.split("@")[0] ?? args.userEmail}'s workbench`, + name: args.displayName.trim(), slug: expectedSlug, }; if (args.operatorTenantId !== undefined) diff --git a/packages/onboarding/src/routes.ts b/packages/onboarding/src/routes.ts index 3144d22e6..e93e74914 100644 --- a/packages/onboarding/src/routes.ts +++ b/packages/onboarding/src/routes.ts @@ -29,6 +29,10 @@ const SubmitCredential = type({ apiKey: "string > 0", }); +const ProvisionBody = type({ + "name?": "string > 0", +}); + export type CreateOnboardingRoutesDeps = { hubUrl: string; operatorTenantId?: string; @@ -90,6 +94,17 @@ export function createOnboardingRoutes( const cookies = cookiesFromHeader(c.req.header("cookie")); try { + // Optional body: the naming wizard sends `{ name }`; the shell's + // membership probe may POST with no body and only wants the read path. + const rawBody: unknown = await c.req.json().catch(() => null); + const body = + rawBody === null + ? undefined + : (() => { + const parsed = ProvisionBody(rawBody); + return parsed instanceof type.errors ? undefined : parsed; + })(); + const provisionArgs: Parameters< typeof provisionPersonalTenantIfNeeded >[0] = { @@ -105,6 +120,7 @@ export function createOnboardingRoutes( provisionArgs.operatorTenantId = deps.operatorTenantId; if (deps.seedModel !== undefined) provisionArgs.seedModel = deps.seedModel; + if (body?.name !== undefined) provisionArgs.displayName = body.name; const result = await provisionPersonalTenantIfNeeded(provisionArgs); From 603bf2a53e2db30e058098560dac8b8fe5f1c0de Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sun, 9 Aug 2026 13:15:44 -0700 Subject: [PATCH 3/4] CL-5782: Align provision tests with required display name Permanent slug-conflict coverage must POST a name so the create path runs; without a name the route returns needs-onboarding. Document that displayName is required to mint, not a silent fallback. --- packages/onboarding/src/provision.ts | 5 ++-- packages/onboarding/test/routes.test.ts | 38 ++++++++++++++++++++++++- 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/packages/onboarding/src/provision.ts b/packages/onboarding/src/provision.ts index f8f337cf4..2ba62ceef 100644 --- a/packages/onboarding/src/provision.ts +++ b/packages/onboarding/src/provision.ts @@ -60,9 +60,8 @@ export type ProvisionArgs = { hubUrl: string; userId: string; userEmail: string; - /** Display name for the personal bench. When omitted, falls back to - * `"'s workbench"` so callers that only probe membership - * (no naming step yet) still get a sensible default if they create. */ + /** Display name for the personal bench. Required to mint: when omitted + * (shell membership probe), returns `needs-onboarding` and creates nothing. */ displayName?: string; operatorTenantId?: string; seedModel?: ModelSource; diff --git a/packages/onboarding/test/routes.test.ts b/packages/onboarding/test/routes.test.ts index 6f54fe6b9..546a42e33 100644 --- a/packages/onboarding/test/routes.test.ts +++ b/packages/onboarding/test/routes.test.ts @@ -52,6 +52,8 @@ describe("POST /provision", () => { // A slug-conflict where the caller still has no principal anywhere is a // dead end the client cannot retry out of — it must surface as a // permanent error so the UI offers "contact support", not "try again". + // Body must include a name so provision enters the create path; without + // a name the route returns needs-onboarding and never hits the hub. const hub = new Hono(); hub.get("/api/me/principals", (c) => c.json({ data: [], nextCursor: null }), @@ -68,7 +70,11 @@ describe("POST /provision", () => { }); const app = mountAuthenticated(routes); - const response = await app.request("/provision", { method: "POST" }); + const response = await app.request("/provision", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ name: "Alice's Lab" }), + }); expect(response.status).toBe(500); const body = (await response.json()) as { @@ -81,6 +87,36 @@ describe("POST /provision", () => { } }); + test("a nameless membership probe returns needs-onboarding without creating", async () => { + const creates: unknown[] = []; + const hub = new Hono(); + hub.get("/api/me/principals", (c) => + c.json({ data: [], nextCursor: null }), + ); + hub.post("/api/tenants", async (c) => { + creates.push(await c.req.json()); + return c.json({ id: "tnt_x" }, 201); + }); + const server = Bun.serve({ port: 0, fetch: hub.fetch }); + try { + const routes = createOnboardingRoutes({ + hubUrl: `http://localhost:${server.port}`, + pushWorkflow: async () => "pushed", + log: () => undefined, + }); + const app = mountAuthenticated(routes); + + const response = await app.request("/provision", { method: "POST" }); + + expect(response.status).toBe(200); + const body = (await response.json()) as { kind: string }; + expect(body.kind).toBe("needs-onboarding"); + expect(creates).toEqual([]); + } finally { + server.stop(true); + } + }); + test("rapid retries from the same user are rate-limited (429)", async () => { const routes = createOnboardingRoutes({ hubUrl: "http://127.0.0.1:0", From 6c39e3fe22b5aef851dacfd98bd2d67d4f75397a Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sun, 9 Aug 2026 13:25:20 -0700 Subject: [PATCH 4/4] CL-5782: Rate-limit only named provision creates The two-step first-login flow is a nameless membership probe followed by a named create. Gating both requests on the same 10s window 429s anyone who submits a name within the probe window. Parse the body first and only rate-limit (and record) create attempts that carry a name. --- packages/onboarding/src/routes.ts | 63 ++++++++++++++----------- packages/onboarding/test/routes.test.ts | 50 ++++++++++++++++++-- 2 files changed, 82 insertions(+), 31 deletions(-) diff --git a/packages/onboarding/src/routes.ts b/packages/onboarding/src/routes.ts index e93e74914..ba929f01c 100644 --- a/packages/onboarding/src/routes.ts +++ b/packages/onboarding/src/routes.ts @@ -72,39 +72,46 @@ export function createOnboardingRoutes( ); } - const now = Date.now(); - const lastAttempt = lastProvisionByUser.get(user.id); - if ( - lastAttempt !== undefined && - now - lastAttempt < PROVISION_RATE_LIMIT_MS - ) { - return c.json( - { - error: { - code: "rate_limited", - kind: "transient" as const, - message: - "Too many provisioning attempts. Please wait a moment and try again.", + // Optional body: the naming wizard sends `{ name }`; the shell's + // membership probe may POST with no body and only wants the read path. + // Parse before rate-limiting so the read probe never burns a create slot. + const rawBody: unknown = await c.req.json().catch(() => null); + const body = + rawBody === null + ? undefined + : (() => { + const parsed = ProvisionBody(rawBody); + return parsed instanceof type.errors ? undefined : parsed; + })(); + const isCreateAttempt = body?.name !== undefined; + + // Rate-limit only named creates. The two-step first-login flow is + // probe (no name) → naming submit (with name); gating both would 429 + // anyone who types a name within the window of their membership probe. + if (isCreateAttempt) { + const now = Date.now(); + const lastAttempt = lastProvisionByUser.get(user.id); + if ( + lastAttempt !== undefined && + now - lastAttempt < PROVISION_RATE_LIMIT_MS + ) { + return c.json( + { + error: { + code: "rate_limited", + kind: "transient" as const, + message: + "Too many provisioning attempts. Please wait a moment and try again.", + }, }, - }, - 429, - ); + 429, + ); + } + lastProvisionByUser.set(user.id, now); } - lastProvisionByUser.set(user.id, now); const cookies = cookiesFromHeader(c.req.header("cookie")); try { - // Optional body: the naming wizard sends `{ name }`; the shell's - // membership probe may POST with no body and only wants the read path. - const rawBody: unknown = await c.req.json().catch(() => null); - const body = - rawBody === null - ? undefined - : (() => { - const parsed = ProvisionBody(rawBody); - return parsed instanceof type.errors ? undefined : parsed; - })(); - const provisionArgs: Parameters< typeof provisionPersonalTenantIfNeeded >[0] = { diff --git a/packages/onboarding/test/routes.test.ts b/packages/onboarding/test/routes.test.ts index 546a42e33..f7c1565f3 100644 --- a/packages/onboarding/test/routes.test.ts +++ b/packages/onboarding/test/routes.test.ts @@ -117,16 +117,24 @@ describe("POST /provision", () => { } }); - test("rapid retries from the same user are rate-limited (429)", async () => { + test("rapid named create retries from the same user are rate-limited (429)", async () => { + // Rate limit applies only to named creates (the membership probe must not + // burn a slot — otherwise the naming wizard always 429s within 10s of + // first login). const routes = createOnboardingRoutes({ hubUrl: "http://127.0.0.1:0", pushWorkflow: async () => "pushed", log: () => undefined, }); const app = mountAuthenticated(routes); + const named = { + method: "POST" as const, + headers: { "content-type": "application/json" }, + body: JSON.stringify({ name: "Alice's Lab" }), + }; - const first = await app.request("/provision", { method: "POST" }); - const second = await app.request("/provision", { method: "POST" }); + const first = await app.request("/provision", named); + const second = await app.request("/provision", named); // The first call runs (and fails transiently against the dead hub). expect(first.status).toBe(503); @@ -139,6 +147,42 @@ describe("POST /provision", () => { expect(body.error.kind).toBe("transient"); }); + test("a membership probe does not rate-limit the following named create", async () => { + // Two-step first-login: shell probe (no name) then naming submit (with name). + // The probe must not consume the create rate-limit slot. + const hub = new Hono(); + hub.get("/api/me/principals", (c) => + c.json({ data: [], nextCursor: null }), + ); + const server = Bun.serve({ port: 0, fetch: hub.fetch }); + try { + const routes = createOnboardingRoutes({ + hubUrl: `http://localhost:${server.port}`, + pushWorkflow: async () => "pushed", + log: () => undefined, + }); + const app = mountAuthenticated(routes); + + const probe = await app.request("/provision", { method: "POST" }); + expect(probe.status).toBe(200); + expect(((await probe.json()) as { kind: string }).kind).toBe( + "needs-onboarding", + ); + + // Named create reaches the hub (503/500 from incomplete mock is fine); + // the only failure mode this test forbids is 429 from the probe. + const create = await app.request("/provision", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ name: "Alice's Lab" }), + }); + expect(create.status).not.toBe(429); + expect([500, 503]).toContain(create.status); + } finally { + server.stop(true); + } + }); + test("an anonymous request is rejected before provisioning runs", async () => { const routes = createOnboardingRoutes({ hubUrl: "http://127.0.0.1:0",