diff --git a/.gitignore b/.gitignore index 5abd1374e1..8f54df5a15 100644 --- a/.gitignore +++ b/.gitignore @@ -3,6 +3,7 @@ node_modules/ **/node_modules **/node_modules/ dist/ +dist-preview/ packages/paperclip-runner/runner/target/ ui/storybook-static/ .env diff --git a/tests/e2e/nux-phase4-screenshots.spec.ts b/tests/e2e/nux-phase4-screenshots.spec.ts index e18b5a5b8f..6b6f5ec521 100644 --- a/tests/e2e/nux-phase4-screenshots.spec.ts +++ b/tests/e2e/nux-phase4-screenshots.spec.ts @@ -93,9 +93,17 @@ test.describe("NUX Phase 4 visual QA", () => { await page.evaluate(() => window.localStorage.clear()); await openWizard(page); // Reach the full-screen front door (step 0): either it shows directly or - // "← Back to start" returns to it from the create step. + // the naming step's Back returns to it. + // + // That control used to be a "← Back to start" text link. The naming step now + // wears the same footer pair as the steps after it, so its Back is labelled + // like theirs — it still lands on the front door, because the front door is + // what sits behind step 1. + // + // Exact, because the progress strip's segments are buttons with their own + // labels and an unanchored /Back/ would match more than one. if (!(await page.getByRole("heading", { name: "Welcome to Paperclip" }).count())) { - await page.getByRole("button", { name: /Back to start/ }).click(); + await page.getByRole("button", { name: "Back", exact: true }).click(); } await expect( page.getByRole("heading", { name: "Welcome to Paperclip" }), diff --git a/tests/e2e/onboarding.spec.ts b/tests/e2e/onboarding.spec.ts index 724322003c..c53e5d99b2 100644 --- a/tests/e2e/onboarding.spec.ts +++ b/tests/e2e/onboarding.spec.ts @@ -222,12 +222,20 @@ test.describe("Onboarding wizard", () => { // Step 4 (Connect a model): the default adapter is claude_local, and the // signal above reports no ready credential, so the login panel must show // with no button to reuse a saved login. - await expect(page.getByText("Sign in to the environment")).toBeVisible({ + // + // The panel names the provider rather than the plumbing it runs on, so this + // title is per-adapter. "Sign in to the environment" is now only the fallback + // for an adapter with no known provider name, which claude_local is not. + await expect(page.getByText("Sign in to Anthropic")).toBeVisible({ timeout: 15_000, }); await expect(page.getByRole("button", { name: "Use saved login" })).toHaveCount(0); - await page.getByRole("button", { name: /^Connect/ }).click(); + // Exact, because the progress strip's segments are buttons too and one of + // them is labelled "Connect a model" for assistive tech. An unanchored + // /^Connect/ matches both it and this CTA, which is a strict-mode violation + // rather than a wrong click — Playwright refuses instead of guessing. + await page.getByRole("button", { name: "Connect", exact: true }).click(); // The failed test blocks the hire and shows its own checks. await expect(page.getByText("The claude CLI was not found on this host.")).toBeVisible({ diff --git a/ui/connect-model-preview.html b/ui/connect-model-preview.html new file mode 100644 index 0000000000..aa01f44dc4 --- /dev/null +++ b/ui/connect-model-preview.html @@ -0,0 +1,40 @@ + + + + + + + Connect a model — Paperclip onboarding + + + + + +
+ + + diff --git a/ui/package.json b/ui/package.json index 38ccf73e10..20ea33f78c 100644 --- a/ui/package.json +++ b/ui/package.json @@ -16,11 +16,12 @@ "scripts": { "dev": "vite", "build": "vite build", + "build:preview": "vite build --config vite.preview.config.mjs", "storybook": "storybook dev -p 6006 -c storybook/.storybook", "build-storybook": "storybook build -c storybook/.storybook -o storybook-static", "preview": "vite preview", "typecheck": "tsc -b", - "clean": "rm -rf dist storybook-static tsconfig.tsbuildinfo", + "clean": "rm -rf dist dist-preview storybook-static tsconfig.tsbuildinfo", "prepack": "rm -f package.dev.json && cp package.json package.dev.json && node ../scripts/generate-ui-package-json.mjs", "postpack": "if [ -f package.dev.json ]; then mv package.dev.json package.json; fi" }, diff --git a/ui/public/brands/claude-color.svg b/ui/public/brands/claude-color.svg new file mode 100644 index 0000000000..f54cffccf7 --- /dev/null +++ b/ui/public/brands/claude-color.svg @@ -0,0 +1,17 @@ + + + + + + + + + + + + + + + + + diff --git a/ui/public/brands/codex-color.svg b/ui/public/brands/codex-color.svg new file mode 100644 index 0000000000..1981b63e95 --- /dev/null +++ b/ui/public/brands/codex-color.svg @@ -0,0 +1,17 @@ + + + + + + + + + + + + + + + + + diff --git a/ui/src/components/AgentConfigForm.render.test.tsx b/ui/src/components/AgentConfigForm.render.test.tsx index fb9f40d158..e6191c4cb5 100644 --- a/ui/src/components/AgentConfigForm.render.test.tsx +++ b/ui/src/components/AgentConfigForm.render.test.tsx @@ -611,7 +611,7 @@ async function runTest(container: HTMLElement) { } async function startLogin(container: HTMLElement) { - await clickByText(container, "Log in"); + await clickByText(container, "Sign in"); await flushReact(); } @@ -1089,11 +1089,11 @@ describe("AgentConfigForm environment selector", () => { const result = await renderCodexSandbox(); roots.push(result.root); - expect(findButton(result.container, "Log in")).toBeFalsy(); + expect(findButton(result.container, "Sign in")).toBeFalsy(); await runTest(result.container); - expect(findButton(result.container, "Log in")).toBeTruthy(); + expect(findButton(result.container, "Sign in")).toBeTruthy(); }); it("hides the Codex login for a provider without the login pseudo-terminal capability", async () => { @@ -1118,7 +1118,7 @@ describe("AgentConfigForm environment selector", () => { await runTest(result.container); - expect(findButton(result.container, "Log in")).toBeFalsy(); + expect(findButton(result.container, "Sign in")).toBeFalsy(); }); it("shows the login affordance and the displayed-code panel for a third adapter with a projected login capability", async () => { @@ -1129,13 +1129,13 @@ describe("AgentConfigForm environment selector", () => { const result = await renderVendorSandbox(); roots.push(result.root); - expect(findButton(result.container, "Log in")).toBeFalsy(); + expect(findButton(result.container, "Sign in")).toBeFalsy(); await runTest(result.container); // The projected capability gates the login affordance on for the third // adapter. - expect(findButton(result.container, "Log in")).toBeTruthy(); + expect(findButton(result.container, "Sign in")).toBeTruthy(); await startLogin(result.container); @@ -1154,11 +1154,11 @@ describe("AgentConfigForm environment selector", () => { const result = await renderGrokSandbox(); roots.push(result.root); - expect(findButton(result.container, "Log in")).toBeFalsy(); + expect(findButton(result.container, "Sign in")).toBeFalsy(); await runTest(result.container); - expect(findButton(result.container, "Log in")).toBeTruthy(); + expect(findButton(result.container, "Sign in")).toBeTruthy(); await startLogin(result.container); @@ -1171,11 +1171,11 @@ describe("AgentConfigForm environment selector", () => { const result = await renderClaudeSandbox(); roots.push(result.root); - expect(findButton(result.container, "Log in")).toBeFalsy(); + expect(findButton(result.container, "Sign in")).toBeFalsy(); await runTest(result.container); - expect(findButton(result.container, "Log in")).toBeTruthy(); + expect(findButton(result.container, "Sign in")).toBeTruthy(); }); it("hides the Login button for a Claude sandbox whose provider lacks the setup-token login capability", async () => { @@ -1199,7 +1199,7 @@ describe("AgentConfigForm environment selector", () => { // E2B does not advertise the setup-token login capability, so the panel // stays hidden even after the auth-missing check. - expect(findButton(result.container, "Log in")).toBeFalsy(); + expect(findButton(result.container, "Sign in")).toBeFalsy(); }); it("hides the Login button for a Daytona sandbox while the capabilities report no setup-token support", async () => { @@ -1234,7 +1234,7 @@ describe("AgentConfigForm environment selector", () => { await runTest(result.container); - expect(findButton(result.container, "Log in")).toBeFalsy(); + expect(findButton(result.container, "Sign in")).toBeFalsy(); }); it("gates a pseudo-terminal login on the provider pty capability for a non-Claude adapter", async () => { @@ -1260,7 +1260,7 @@ describe("AgentConfigForm environment selector", () => { await runTest(result.container); - expect(findButton(result.container, "Log in")).toBeFalsy(); + expect(findButton(result.container, "Sign in")).toBeFalsy(); }); it("shows a pseudo-terminal login for a non-Claude adapter when the provider advertises pty support", async () => { @@ -1285,7 +1285,7 @@ describe("AgentConfigForm environment selector", () => { await runTest(result.container); - expect(findButton(result.container, "Log in")).toBeTruthy(); + expect(findButton(result.container, "Sign in")).toBeTruthy(); }); it("shows the Login button when a parent lifts the test feedback and renders the panel from the descriptor", async () => { @@ -1353,11 +1353,11 @@ describe("AgentConfigForm environment selector", () => { }); await flushReact(); - expect(findButton(container, "Log in")).toBeFalsy(); + expect(findButton(container, "Sign in")).toBeFalsy(); await runTest(container); - expect(findButton(container, "Log in")).toBeTruthy(); + expect(findButton(container, "Sign in")).toBeTruthy(); }); it("does not show the Login button when the Test result has no adapter_auth_missing check", async () => { @@ -1366,7 +1366,7 @@ describe("AgentConfigForm environment selector", () => { await runTest(result.container); - expect(findButton(result.container, "Log in")).toBeFalsy(); + expect(findButton(result.container, "Sign in")).toBeFalsy(); }); it("does not show the Login button when the effective environment is Local", async () => { @@ -1380,7 +1380,7 @@ describe("AgentConfigForm environment selector", () => { await runTest(result.container); - expect(findButton(result.container, "Log in")).toBeFalsy(); + expect(findButton(result.container, "Sign in")).toBeFalsy(); }); it("shows the Login button for an agent with no own environment under the managed-sandbox-only policy", async () => { @@ -1416,11 +1416,11 @@ describe("AgentConfigForm environment selector", () => { ); roots.push(result.root); - expect(findButton(result.container, "Log in")).toBeFalsy(); + expect(findButton(result.container, "Sign in")).toBeFalsy(); await runTest(result.container); - expect(findButton(result.container, "Log in")).toBeTruthy(); + expect(findButton(result.container, "Sign in")).toBeTruthy(); }); it("keeps the Login button hidden under the managed-sandbox-only policy when no managed sandbox is available", async () => { @@ -1447,7 +1447,7 @@ describe("AgentConfigForm environment selector", () => { ); roots.push(result.root); - expect(findButton(result.container, "Log in")).toBeFalsy(); + expect(findButton(result.container, "Sign in")).toBeFalsy(); }); it("starts a login session for the effective sandbox and shows the code and the authentication URL", async () => { @@ -1561,8 +1561,8 @@ describe("AgentConfigForm environment selector", () => { "codex_local", "session-1", ); - // The panel resets: the Log in button is available again and the code is gone. - const login = findButton(result.container, "Log in"); + // The panel resets: the Sign in button is available again and the code is gone. + const login = findButton(result.container, "Sign in"); expect(login?.disabled).toBe(false); expect(findButton(result.container, "Cancel")).toBeFalsy(); expect(result.container.textContent).not.toContain("WXYZ-1234"); @@ -1603,7 +1603,7 @@ describe("AgentConfigForm environment selector", () => { await runTest(result.container); await startLogin(result.container); - const startButton = findButton(result.container, "Log in"); + const startButton = findButton(result.container, "Sign in"); expect(startButton).toBeTruthy(); expect(startButton?.disabled).toBe(true); expect(mockAgentsApi.startAdapterAuthLogin).toHaveBeenCalledTimes(1); @@ -1673,7 +1673,7 @@ describe("AgentConfigForm environment selector", () => { roots.push(result.root); await runTest(result.container); - expect(findButton(result.container, "Log in")).toBeTruthy(); + expect(findButton(result.container, "Sign in")).toBeTruthy(); const select = result.container.querySelector("select"); await act(async () => { @@ -1685,7 +1685,7 @@ describe("AgentConfigForm environment selector", () => { }); await flushReact(); - expect(findButton(result.container, "Log in")).toBeFalsy(); + expect(findButton(result.container, "Sign in")).toBeFalsy(); }); it("shows the authorization URL and a browser-code input for a Claude sandbox", async () => { @@ -1821,7 +1821,7 @@ describe("AgentConfigForm environment selector", () => { ]); roots.push(result.root); - // Log in on the first sandbox. The stored state adds the fixed + // Sign in on the first sandbox. The stored state adds the fixed // `CLAUDE_CODE_OAUTH_TOKEN` binding and the non-secret claim to the form. await runTest(result.container); await startLogin(result.container); @@ -2036,7 +2036,7 @@ describe("AgentConfigForm environment selector", () => { , ); }); - await flushUntil(() => Boolean(findButton(container, "Log in"))); + await flushUntil(() => Boolean(findButton(container, "Sign in"))); expect(findButton(container, "Use saved login")).toBeUndefined(); expect(onApplyStored).not.toHaveBeenCalled(); @@ -2063,7 +2063,7 @@ describe("AgentConfigForm environment selector", () => { // The panel shows a fixed message and returns to its start state. The Log in // button is available again. expect(result.container.textContent).toContain("The login did not finish"); - expect(findButton(result.container, "Log in")?.disabled).toBe(false); + expect(findButton(result.container, "Sign in")?.disabled).toBe(false); // The panel never shows the provider failure message, which could carry a // secret. expect(result.container.textContent).not.toContain("the provider rejected the browser code"); @@ -2088,7 +2088,7 @@ describe("AgentConfigForm environment selector", () => { ); expect(result.container.textContent).toContain("The login did not finish"); - expect(findButton(result.container, "Log in")?.disabled).toBe(false); + expect(findButton(result.container, "Sign in")?.disabled).toBe(false); }); it("shows a terminal failure and stops polling on a status 404 from server cleanup", async () => { @@ -2119,7 +2119,7 @@ describe("AgentConfigForm environment selector", () => { // The panel shows the fixed failure message and returns to its start state. expect(result.container.textContent).toContain("The login did not finish"); - expect(findButton(result.container, "Log in")?.disabled).toBe(false); + expect(findButton(result.container, "Sign in")?.disabled).toBe(false); // The panel shows no credential material: no authorization URL and no // browser-code input. @@ -2156,9 +2156,9 @@ describe("AgentConfigForm environment selector", () => { "company-1", "claude-session-1", ); - // The panel resets: the Log in button is available again, and the URL and the + // The panel resets: the Sign in button is available again, and the URL and the // browser-code input are gone. - expect(findButton(result.container, "Log in")?.disabled).toBe(false); + expect(findButton(result.container, "Sign in")?.disabled).toBe(false); expect(findButton(result.container, "Cancel")).toBeFalsy(); expect(result.container.textContent).not.toContain("https://claude.example.test/authorize"); expect(result.container.querySelector('input[aria-label="Browser code"]')).toBeFalsy(); @@ -2190,10 +2190,10 @@ describe("AgentConfigForm environment selector", () => { "company-1", "claude-session-1", ); - // The panel reset even though the cancel returned a 404: the Log in button is + // The panel reset even though the cancel returned a 404: the Sign in button is // available again, and the URL and the browser-code input are gone. No error // message remains. - expect(findButton(result.container, "Log in")?.disabled).toBe(false); + expect(findButton(result.container, "Sign in")?.disabled).toBe(false); expect(findButton(result.container, "Cancel")).toBeFalsy(); expect(result.container.textContent).not.toContain("https://claude.example.test/authorize"); expect(result.container.querySelector('input[aria-label="Browser code"]')).toBeFalsy(); @@ -2229,9 +2229,9 @@ describe("AgentConfigForm environment selector", () => { const result = await renderClaudeSandbox(); await runTest(result.container); - // The panel shows the Log in button but no session started, so no active + // The panel shows the Sign in button but no session started, so no active // session exists to cancel. - expect(findButton(result.container, "Log in")).toBeTruthy(); + expect(findButton(result.container, "Sign in")).toBeTruthy(); await act(async () => { result.root.unmount(); @@ -2335,7 +2335,7 @@ describe("AgentConfigForm environment selector", () => { await flushFake(); await clickFake(container, "Test"); - await clickFake(container, "Log in"); + await clickFake(container, "Sign in"); // The login is active: both polls have run at least once. const statusCallsAtStart = mockAgentsApi.getClaudeSetupTokenLoginStatus.mock.calls.length; @@ -2374,7 +2374,7 @@ describe("AgentConfigForm environment selector", () => { "claude-session-1", ); // The Log in button is available again, and the Cancel button is gone. - expect(findButton(container, "Log in")?.disabled).toBe(false); + expect(findButton(container, "Sign in")?.disabled).toBe(false); expect(findButton(container, "Cancel")).toBeFalsy(); // Both polls stopped. A further ten seconds adds no new poll call. @@ -2597,7 +2597,7 @@ describe("AgentConfigForm create-mode Claude OAuth binding", () => { // The panel shows a fixed, non-secret message and returns to its start state. expect(result.container.textContent).toContain("The login did not finish"); - expect(findButton(result.container, "Log in")?.disabled).toBe(false); + expect(findButton(result.container, "Sign in")?.disabled).toBe(false); expect(result.container.textContent).not.toContain( "the provider rejected the stored-session claim", ); diff --git a/ui/src/components/AgentConfigForm.tsx b/ui/src/components/AgentConfigForm.tsx index e7065aa35d..b244f06839 100644 --- a/ui/src/components/AgentConfigForm.tsx +++ b/ui/src/components/AgentConfigForm.tsx @@ -2192,6 +2192,26 @@ export type AdapterLoginPanelProps = AdapterLoginDescriptor & { // The login panel dispatcher. It picks the panel from the projected panel mode, // not from the adapter name. The `submitted_browser_code` mode shows the // submitted-browser-code panel; every other mode shows the displayed-code panel. +/** + * The account a source signs in to, named where one is known. + * + * "Sign in to the environment" describes the plumbing — a login performed inside + * a sandbox — and is the honest label when the provider is unknown. But for the + * two sources onboarding offers, the customer is signing in to Anthropic or to + * OpenAI, and naming that is what tells them which password manager entry to + * reach for. The generic wording stays for anything not listed, where a guess + * would be worse than a description. + */ +const ADAPTER_LOGIN_PROVIDER: Record = { + claude_local: "Anthropic", + codex_local: "OpenAI", +}; + +function adapterLoginTitle(adapterType: string): string { + const provider = ADAPTER_LOGIN_PROVIDER[adapterType]; + return provider ? `Sign in to ${provider}` : "Sign in to the environment"; +} + export function AdapterLoginPanel(props: AdapterLoginPanelProps) { const getCapabilities = useAdapterCapabilities(); const panelMode = getCapabilities(props.adapterType).login?.panelMode; @@ -2266,9 +2286,14 @@ function DisplayedCodeLoginPanel({ const startDisabled = startLogin.isPending || isActive; return ( -
+
+ {/* `gap`, not `space-y`: the live region below collapses to + `display: none` whenever it has nothing to announce, and + `space-y` would still put its 8px on the row above — dead space + inside the card that pushes the row off centre. A gap only + applies between children that render. */}
- Sign in to the environment + {adapterLoginTitle(adapterType)}
{isActive && (
@@ -2397,6 +2422,7 @@ const CLAUDE_LOGIN_TIMED_OUT_MESSAGE = "The login timed out. Start the login aga // only the server `stored` state as success, and it never shows the OAuth token. function SubmittedBrowserCodeLoginPanel({ companyId, + adapterType, environmentId, onStored, onApplyStored, @@ -2714,9 +2740,14 @@ function SubmittedBrowserCodeLoginPanel({ }; return ( -
+
+ {/* `gap`, not `space-y`: the live region below collapses to + `display: none` whenever it has nothing to announce, and + `space-y` would still put its 8px on the row above — dead space + inside the card that pushes the row off centre. A gap only + applies between children that render. */}
- Sign in to the environment + {adapterLoginTitle(adapterType)}
{isActive && (
diff --git a/ui/src/components/OnboardingWizard.test.tsx b/ui/src/components/OnboardingWizard.test.tsx index c83b1c24eb..f6067ac83a 100644 --- a/ui/src/components/OnboardingWizard.test.tsx +++ b/ui/src/components/OnboardingWizard.test.tsx @@ -91,6 +91,12 @@ const mockInstanceSettingsApi = vi.hoisted(() => ({ const mockApprovalsApi = vi.hoisted(() => ({ create: vi.fn(), })); +const mockSecretsApi = vi.hoisted(() => ({ + listMyUserSecrets: vi.fn(), + createUserSecretDefinition: vi.fn(), + createMyUserSecret: vi.fn(), + rotateMyUserSecret: vi.fn(), +})); const mockIssuesApi = vi.hoisted(() => ({ create: vi.fn(), })); @@ -122,6 +128,7 @@ vi.mock("../api/companies", () => ({ companiesApi: mockCompaniesApi })); vi.mock("../api/goals", () => ({ goalsApi: mockGoalsApi })); vi.mock("../api/agents", () => ({ agentsApi: mockAgentsApi })); vi.mock("../api/approvals", () => ({ approvalsApi: mockApprovalsApi })); +vi.mock("../api/secrets", () => ({ secretsApi: mockSecretsApi })); vi.mock("../api/issues", () => ({ issuesApi: mockIssuesApi })); vi.mock("../api/projects", () => ({ projectsApi: mockProjectsApi })); vi.mock("../api/environments", () => ({ environmentsApi: mockEnvironmentsApi })); @@ -134,7 +141,12 @@ vi.mock("../adapters/metadata", () => ({ isVisualAdapterChoice: () => true })); vi.mock("../adapters/adapter-display-registry", () => ({ getAdapterDisplay: (type: string) => ({ type, - recommended: false, + // Mirrors the real registry, where these two and only these two are + // `recommended`. A blanket `false` used to be harmless because every adapter + // then sat in the "Advanced settings" disclosure and was reachable anyway; + // with the step down to a tile row built from this flag, it made that row + // empty in every test and hid the surface under it. + recommended: type === "claude_local" || type === "codex_local", label: type, description: "", icon: () => null, @@ -649,6 +661,160 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", ( await act(async () => root.unmount()); }); + // The Connect handler reuses a passing probe instead of re-running it, so the + // effect that clears the cache has to name every input to the configuration + // the probe tested. `credentialMode` and `apiKey` were missing from it, and + // the gap is reachable: the probe and the hire share one try/catch, so a hire + // that throws leaves the pass in state. Switching to a key and pressing + // Connect again then hired against a key nothing had tested. + /** + * Typing a key into this step must not put the key into the agent's stored + * configuration. That configuration is persisted and revisioned, so a plain + * value there is a live credential at rest in every copy of it — which is + * what this step did before, and what the Claude token path has always + * avoided by holding a `user_secret_ref` instead. + */ + describe("an API key typed on the step", () => { + const KEY = "sk-ant-typed-by-the-customer"; + + // The canvas holding the key field only opens once a source is selected, + // and the tile row that selects one is built from this registry. The + // suite's default is empty, which leaves the step with no tiles, no + // canvas, and no field to type into. + beforeEach(() => { + mockAdapterRegistry.list = [{ type: "claude_local" }, { type: "codex_local" }]; + // No definition and no stored value yet: the first customer to type a key. + mockSecretsApi.listMyUserSecrets.mockResolvedValue([]); + mockSecretsApi.createUserSecretDefinition.mockResolvedValue({ id: "def-1" }); + mockSecretsApi.createMyUserSecret.mockResolvedValue({ id: "secret-abc" }); + mockSecretsApi.rotateMyUserSecret.mockResolvedValue({ id: "secret-existing" }); + }); + + async function connectWithApiKey() { + const handles = await openConnectStep(); + await handles.clickByText((t) => t.startsWith("Use API keys")); + const field = document.body.querySelector( + 'input[type="password"]', + ) as HTMLInputElement; + await act(async () => { + setControlledValue(field, KEY); + }); + await flushReact(); + await handles.clickByText((t) => t.startsWith("Connect")); + return handles; + } + + it("is stored as the user's own secret and referenced, never carried in the hire", async () => { + const { root } = await connectWithApiKey(); + + expect(mockSecretsApi.createMyUserSecret).toHaveBeenCalledTimes(1); + const [, createBody] = mockSecretsApi.createMyUserSecret.mock.calls.at(-1) as [ + string, + { definitionKey: string; value: string }, + ]; + expect(createBody.definitionKey).toBe("ANTHROPIC_API_KEY"); + expect(createBody.value).toBe(KEY); + + const hireBody = (mockAgentsApi.hire.mock.calls.at(-1) as unknown[])[1] as { + adapterConfig: { env?: Record }; + }; + // The same binding kind the subscription half of this step produces. + expect(hireBody.adapterConfig.env?.ANTHROPIC_API_KEY).toEqual({ + type: "user_secret_ref", + key: "ANTHROPIC_API_KEY", + version: "latest", + }); + // The whole payload, not just that one field: the point is that the key + // is nowhere in what gets persisted, however it might be nested. + expect(JSON.stringify(hireBody)).not.toContain(KEY); + + await act(async () => root.unmount()); + }); + + // Onboarding is the first thing to need this definition, so it creates it. + it("creates the definition once, then reuses it", async () => { + await connectWithApiKey(); + expect(mockSecretsApi.createUserSecretDefinition).toHaveBeenCalledTimes(1); + + mockSecretsApi.listMyUserSecrets.mockResolvedValue([ + { definition: { id: "def-1", key: "ANTHROPIC_API_KEY" }, secret: null }, + ]); + const { root } = await connectWithApiKey(); + + expect(mockSecretsApi.createUserSecretDefinition).toHaveBeenCalledTimes(1); + + await act(async () => root.unmount()); + }); + + // A second value against one definition is what the server refuses, so a + // customer who already has a key stored must rotate rather than add. + it("rotates an existing value instead of storing a second one", async () => { + mockSecretsApi.listMyUserSecrets.mockResolvedValue([ + { + definition: { id: "def-1", key: "ANTHROPIC_API_KEY" }, + secret: { id: "secret-existing" }, + }, + ]); + const { root } = await connectWithApiKey(); + + expect(mockSecretsApi.rotateMyUserSecret).toHaveBeenCalledWith( + expect.any(String), + "secret-existing", + { value: KEY }, + ); + expect(mockSecretsApi.createMyUserSecret).not.toHaveBeenCalled(); + + await act(async () => root.unmount()); + }); + + // The one outcome that must never happen is a hire that falls back to + // embedding the key because storing it failed. + it("blocks the hire when the key cannot be stored", async () => { + mockSecretsApi.createMyUserSecret.mockRejectedValue(new Error("vault unreachable")); + const { root } = await connectWithApiKey(); + + expect(mockAgentsApi.hire).not.toHaveBeenCalled(); + expect(document.body.textContent).toContain("Could not store the API key"); + + await act(async () => root.unmount()); + }); + + it("stores one secret when Connect is pressed twice with the same key", async () => { + mockAgentsApi.hire.mockRejectedValueOnce(new Error("network went away")); + const { root, clickByText } = await connectWithApiKey(); + + await clickByText((t) => t.startsWith("Connect")); + + expect(mockSecretsApi.createMyUserSecret).toHaveBeenCalledTimes(1); + + await act(async () => root.unmount()); + }); + }); + + it("re-probes rather than reusing a pass when the credential mode changes", async () => { + mockAgentsApi.testEnvironment.mockResolvedValue({ + adapterType: "claude_local", + status: "pass" as const, + checks: [], + testedAt: new Date().toISOString(), + }); + // The hire fails, which is what leaves the passing probe behind. + mockAgentsApi.hire.mockRejectedValueOnce(new Error("network went away")); + + const { root, clickByText } = await openConnectStep(); + + await clickByText((t) => t.startsWith("Connect")); + expect(mockAgentsApi.testEnvironment).toHaveBeenCalledTimes(1); + + // Switch to API keys, which changes the configuration the hire will send. + await clickByText((t) => t.startsWith("Use API keys")); + await clickByText((t) => t.startsWith("Connect")); + + expect(mockAgentsApi.testEnvironment).toHaveBeenCalledTimes(2); + + await act(async () => root.unmount()); + }); + it("does not open the create path on a cached warn result that holds adapter_auth_missing", async () => { mockAgentsApi.testEnvironment.mockResolvedValue({ adapterType: "claude_local", @@ -1550,21 +1716,21 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", ( it("shows the login panel for claude_local when the signal reports no ready credential", async () => { mockAgentsApi.getAdapterAuthSignal.mockResolvedValue({ status: "absent" }); const { root } = await openStep4({ adapterType: "claude_local" }); - expect(document.body.textContent).toContain("Sign in to the environment"); + expect(document.body.textContent).toContain("Sign in to Anthropic"); await act(async () => root.unmount()); }); it("shows the login panel for codex_local when the signal cannot decide", async () => { mockAgentsApi.getAdapterAuthSignal.mockResolvedValue({ status: "unknown" }); const { root } = await openStep4({ adapterType: "codex_local" }); - expect(document.body.textContent).toContain("Sign in to the environment"); + expect(document.body.textContent).toContain("Sign in to OpenAI"); await act(async () => root.unmount()); }); it("hides the login panel when the signal reports a ready credential", async () => { mockAgentsApi.getAdapterAuthSignal.mockResolvedValue({ status: "present" }); const { root } = await openStep4({ adapterType: "claude_local" }); - expect(document.body.textContent).not.toContain("Sign in to the environment"); + expect(document.body.textContent).not.toContain("Sign in to Anthropic"); await act(async () => root.unmount()); }); @@ -1596,8 +1762,13 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", ( await flushReact(); }; - await clickByText((t) => t.startsWith("Advanced settings")); - await clickByText((t) => t === "codex_local"); + // Straight to the tile. The adapter change used to be reached through an + // "Advanced settings" disclosure listing every non-recommended adapter; + // the step now offers Claude and Codex as tiles and spends that line on + // the credential switch instead. What is asserted below is unchanged — + // changing the source re-reads the signal — only the route there is. + // The tile's text is the label plus its credential tag, hence the prefix. + await clickByText((t) => t.startsWith("codex_local")); expect(mockAgentsApi.getAdapterAuthSignal).toHaveBeenCalledWith( "company-new", @@ -1613,7 +1784,7 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", ( mockInstanceSettingsApi.get.mockResolvedValue({ defaultEnvironmentId: null }); mockAgentsApi.getAdapterAuthSignal.mockResolvedValue({ status: "absent" }); const { root } = await openStep4({ adapterType: "claude_local" }); - expect(document.body.textContent).not.toContain("Sign in to the environment"); + expect(document.body.textContent).not.toContain("Sign in to Anthropic"); expect(mockAgentsApi.getAdapterAuthSignal).not.toHaveBeenCalled(); await act(async () => root.unmount()); }); diff --git a/ui/src/components/OnboardingWizard.tsx b/ui/src/components/OnboardingWizard.tsx index e960043176..ea13f62506 100644 --- a/ui/src/components/OnboardingWizard.tsx +++ b/ui/src/components/OnboardingWizard.tsx @@ -1,5 +1,5 @@ import { useEffect, useState, useMemo, useRef } from "react"; -import type { CSSProperties } from "react"; +import type { ComponentType, CSSProperties } from "react"; import { useQuery, useQueryClient } from "@tanstack/react-query"; import { MotionConfig, motion } from "motion/react"; import type { @@ -11,6 +11,7 @@ import type { } from "@paperclipai/shared"; import { AGENT_ROLES, AGENT_ROLE_LABELS, ADAPTER_AUTH_MISSING_CHECK_CODE } from "@paperclipai/shared"; import { AdapterLoginPanel } from "./AgentConfigForm"; +import { secretsApi } from "../api/secrets"; import { Label } from "./ui/label"; import { Input } from "./ui/input"; import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "./ui/select"; @@ -83,8 +84,19 @@ import { import { AsciiArtAnimation } from "./AsciiArtAnimation"; import { FrontDoor } from "./FrontDoor"; import { PillGuy } from "./onboarding/PillGuy"; -import { AGENT_ARC_WIZARD_STEPS, Stepper, agentArcStepFor } from "./onboarding/Stepper"; +import { SleepingZs } from "./onboarding/SleepingZs"; +import { + AGENT_ARC_WIZARD_STEPS, + ONBOARDING_STEP_LABELS, + ONBOARDING_WIZARD_STEPS, + Stepper, + agentArcStepFor, + onboardingStepPositionFor, +} from "./onboarding/Stepper"; import { AgentPreview } from "./onboarding/AgentPreview"; +import { ModelSourceTiles, type CredentialMode } from "./onboarding/ModelSourceTiles"; +import { CredentialModeLink } from "./onboarding/CredentialModeLink"; +import { ApiKeyField, ConnectInputCanvas } from "./onboarding/ConnectInputCanvas"; import { FooterNav } from "./onboarding/FooterNav"; import { OnboardingHeading } from "./onboarding/OnboardingPrimitives"; import { DEFAULT_AGENT_ROLE } from "../lib/onboarding-agent-role"; @@ -176,6 +188,52 @@ function adapterConfigHasAnthropicApiKey(config: Record): boole return binding.type === "secret_ref" || binding.type === "user_secret_ref"; } +/** + * Full-colour brand marks for the sources this step offers. + * + * The registry's own icons are monochrome, drawn to sit in dense config UI + * where a row of saturated logos would be noise. This step is the opposite + * case: two large tiles carrying the whole choice, where the brand is the + * fastest thing to recognise. + * + * Keyed by adapter type with a fallback, so the row stays registry-driven. An + * adapter with no brand file here still renders — with its registry icon — + * rather than a gap where a tile should be. + */ +const MODEL_SOURCE_BRAND_MARKS: Record = { + claude_local: "/brands/claude-color.svg", + codex_local: "/brands/codex-color.svg", +}; + +/** + * The environment variable each source reads its key from. + * + * Named rather than described in the field above it, because the customer knows + * which key they are holding and does not know where this step will put it. The + * mapping already existed in this file as prose inside the environment-check + * hint; this is the same knowledge, in a form the key field can use. + */ +const API_KEY_ENV_KEYS: Record = { + claude_local: ANTHROPIC_API_KEY_ENV_KEY, + codex_local: "OPENAI_API_KEY", +}; + +function apiKeyEnvKeyFor(adapterType: string): string { + return API_KEY_ENV_KEYS[adapterType] ?? "API_KEY"; +} + +function ModelSourceMark({ + type, + Fallback, +}: { + type: string; + Fallback: ComponentType<{ className?: string }>; +}) { + const brand = MODEL_SOURCE_BRAND_MARKS[type]; + if (!brand) return ; + return ; +} + // Exported so tests write/read the exact key the component uses, instead of // duplicating the literal and silently drifting from it if it's ever renamed. export const ONBOARDING_STORAGE_KEY = "paperclip-onboarding-state"; @@ -516,6 +574,22 @@ function OnboardingWizardInner({ useState(false); const [unsetAnthropicLoading, setUnsetAnthropicLoading] = useState(false); const [showMoreAdapters, setShowMoreAdapters] = useState(false); + /** + * Whether the connect step is asking for a subscription sign-in or an API key. + * + * Restored from the draft like everything else on this step: someone who + * picked keys, left, and came back should not be handed a sign-in panel they + * already said no to. + */ + const [credentialMode, setCredentialMode] = useState( + (saved?.credentialMode as CredentialMode) ?? "subscription", + ); + /** + * The key itself, held only for as long as the wizard is open. It is written + * into the adapter config at hire time and never into the draft — a draft is + * `localStorage`, and a provider key does not belong there. + */ + const [apiKey, setApiKey] = useState(""); // The owner's stored Claude subscription login, read right before the hire // (see handleGiveHeartbeat). Onboarding applies it with no extra control, // so nothing else reads this state yet. @@ -565,6 +639,13 @@ function OnboardingWizardInner({ // the binding cannot answer for a config that now does — see the reuse // check in `handleGiveHeartbeat`. const adapterEnvResultAppliedStoredLoginRef = useRef(false); + /** + * The secret a key typed on this step was stored as, remembered for the key it + * holds. Connect can be pressed more than once — a hire that fails leaves the + * customer on the step to try again — and without this each press would store + * another copy of the same credential. + */ + const apiKeySecretRef = useRef<{ key: string } | null>(null); createdCompanyIdRef.current = createdCompanyId; // The mission of the company actually in hand, which is not always the one @@ -757,6 +838,8 @@ function OnboardingWizardInner({ const state = { step, companyName, companyGoal, missionPath, missionConfirmed, q1, q2, q3, q4, agentName, agentRole, adapterType, cwd, model, command, args, url, + // The mode, never the key: this blob is localStorage. + credentialMode, createdCompanyId, createdCompanyPrefix, createdAgentId, createdCompanyGoalId, createdProjectId, createdIssueRef, onboardingPath, growWorkflows, growPainPoints, growAutomate, @@ -765,6 +848,7 @@ function OnboardingWizardInner({ }, [ effectiveOnboardingOpen, step, companyName, companyGoal, missionPath, missionConfirmed, q1, q2, q3, q4, agentName, agentRole, adapterType, cwd, model, command, args, url, + credentialMode, createdCompanyId, createdCompanyPrefix, createdAgentId, createdCompanyGoalId, createdProjectId, createdIssueRef, onboardingPath, growWorkflows, growPainPoints, growAutomate, @@ -892,6 +976,17 @@ function OnboardingWizardInner({ const authSignalStatus = authSignalQuery.data?.status ?? null; const showAdapterLoginPanel = canShowAdapterLogin && (authSignalStatus === "absent" || authSignalStatus === "unknown"); + /** + * The signal is being fetched and has not answered yet. + * + * Worth its own state rather than folding into "no panel to show". Until it + * answers, `authSignalStatus` is null and every not-signed-in customer looks + * momentarily identical to a signed-in one — so the card would assert that + * they are already signed in, for exactly as long as the request takes, and + * then replace it with a sign-in prompt. A reassurance that is wrong and then + * withdrawn is worse than saying nothing for a beat. + */ + const authSignalUndecided = canShowAdapterLogin && authSignalStatus === null; const isLocalAdapterCaps = adapterCaps.supportsInstructionsBundle || @@ -924,6 +1019,25 @@ function OnboardingWizardInner({ }; }, [disabledTypes]); + /** + * A source chosen from the visible row. Read off the row rather than off + * `adapterType` alone, because a restored draft can name an adapter this step + * no longer offers — a selection the customer cannot see. + */ + const sourceSelected = recommendedAdapters.some((opt) => opt.type === adapterType); + + /** + * When the input canvas is open. + * + * A selected source is the ordinary reason — the card is the answer to the + * tile that was just pressed, so an untouched row leaves nothing under it. But + * it opens for a pending sign-in regardless of the row, because the adapter + * needing credentials does not depend on it having a tile: a restored draft + * naming an adapter this step no longer offers still cannot hire without one, + * and hiding the panel would leave that dead end with nothing to press. + */ + const canvasOpen = sourceSelected || showAdapterLoginPanel; + // The default (or a saved) adapterType can name an adapter the server has // since disabled — e.g. a cloud sandbox registry without claude_local. The // grid hides it, so without this snap the wizard would silently keep an @@ -970,12 +1084,22 @@ function OnboardingWizardInner({ command.trim() || (COMMAND_PLACEHOLDERS[adapterType] ?? adapterType.replace(/_local$/, "")); + // Throw the cached probe away whenever the thing it probed changes. Every + // input to `buildAdapterConfig` belongs in this list, `credentialMode` and + // `apiKey` included: the Connect handler reuses a passing result instead of + // re-probing, so a dependency missing here is a hire that skips the check. + // + // That is reachable rather than theoretical. The hire runs after the probe + // inside one try/catch, so a hire that fails — a network error, a server + // error — leaves the pass sitting in state. Switch to an API key, paste one, + // press Connect again, and without these two the wizard would hire against a + // key nothing ever tested. useEffect(() => { if (step !== 4) return; setAdapterEnvResult(null); adapterEnvResultAppliedStoredLoginRef.current = false; setAdapterEnvError(null); - }, [step, adapterType, model, command, args, url]); + }, [step, adapterType, model, command, args, url, credentialMode, apiKey]); const selectedModel = (adapterModels ?? []).find((m) => m.id === model); const hasAnthropicApiKeyOverrideCheck = @@ -1192,7 +1316,67 @@ function OnboardingWizardInner({ } } - function buildAdapterConfig(): Record { + /** + * Store the typed key as the customer's own user secret, and report whether it + * is in place. + * + * A user secret rather than a company one, to match the subscription half of + * this very step: signing in stores the Claude token as a user secret and + * binds a `user_secret_ref`. Two credential modes on one step that scoped + * their secrets differently would be hard to justify and easy to get wrong + * later. It also keeps the key to the person who typed it instead of exposing + * it to everyone with company secret access, and agent runs still resolve it + * through the company's responsible user. + * + * A user secret needs a definition to hang off. The Claude token's is fixed + * and server-owned; there is no such definition for API keys, so onboarding + * creates one on first use. That needs company owner or admin rights, which + * whoever just created this company in onboarding has. + * + * Returns false on failure, having set the error. Callers must treat false as + * a stop: there is deliberately no path that hands the raw key back, because + * the only thing left to do with it would be to embed it. + */ + async function storeApiKeyUserSecret(companyId: string): Promise { + const key = apiKey.trim(); + const envKey = apiKeyEnvKeyFor(adapterType); + if (apiKeySecretRef.current?.key === key) return true; + try { + const entries = await secretsApi.listMyUserSecrets(companyId); + const existing = entries.find((entry) => entry.definition.key === envKey); + const definitionId = + existing?.definition.id ?? + ( + await secretsApi.createUserSecretDefinition(companyId, { + key: envKey, + name: `${envKey} for onboarding`, + description: "Created while connecting a model during onboarding.", + }) + ).id; + // Rotate rather than create when a value is already stored, because + // creating a second value for one definition is what the server refuses. + if (existing?.secret) { + await secretsApi.rotateMyUserSecret(companyId, existing.secret.id, { value: key }); + } else { + await secretsApi.createMyUserSecret(companyId, { + definitionId, + definitionKey: envKey, + value: key, + }); + } + apiKeySecretRef.current = { key }; + return true; + } catch (err) { + setError( + err instanceof Error + ? `Could not store the API key: ${err.message}` + : "Could not store the API key.", + ); + return false; + } + } + + function buildAdapterConfig(bindApiKey = false): Record { const adapter = getUIAdapter(adapterType); const config = adapter.buildAdapterConfig({ ...defaultCreateValues, @@ -1227,6 +1411,35 @@ function OnboardingWizardInner({ env.ANTHROPIC_API_KEY = { type: "plain", value: "" }; config.env = env; } + // A key typed on this step is the credential the agent is being hired with, + // so it has to reach the configuration the hire sends — and the same one the + // environment test probes, or the test would pass on a config the hire does + // not use. Only when the mode asks for it: leaving a stale reference in the + // config after switching back to a subscription is what the server rejects + // alongside the Claude OAuth binding. + // + // A reference, never the key itself. The adapter configuration is + // persisted and revisioned, so a `{ type: "plain", value }` here would leave + // a live credential at rest in every copy of it. This mirrors + // `buildFixedClaudeOAuthBinding`, which holds a reference to the stored + // Claude token for the same reason. + // + // Guarded on the caller having stored the secret, not on the key being + // present. If storing failed this stays false, and the right outcome is a + // configuration with no credential — which the hire then blocks on — rather + // than one that quietly falls back to embedding the value. + if (credentialMode === "api" && bindApiKey) { + const env = + typeof config.env === "object" && config.env !== null && !Array.isArray(config.env) + ? { ...(config.env as Record) } + : {}; + env[apiKeyEnvKeyFor(adapterType)] = { + type: "user_secret_ref", + key: apiKeyEnvKeyFor(adapterType), + version: "latest", + }; + config.env = env; + } return config; } @@ -1552,7 +1765,15 @@ function OnboardingWizardInner({ // configuration the hire sends — a config without the binding can // report missing authentication for a user the binding would have // covered. - const baseAdapterConfig = buildAdapterConfig(); + // Store the key before anything is built from it, so both the probe and the + // hire describe it the same way — as a reference. A failure here stops the + // hire rather than falling through to a configuration with no credential. + let apiKeyStored = false; + if (credentialMode === "api" && apiKey.trim()) { + apiKeyStored = await storeApiKeyUserSecret(createdCompanyId); + if (!apiKeyStored) return; + } + const baseAdapterConfig = buildAdapterConfig(apiKeyStored); let storedClaudeLogin: ClaudeOAuthTokenStatusResponse | null = null; if ( adapterType === "claude_local" && @@ -1832,13 +2053,24 @@ function OnboardingWizardInner({ >
@@ -1853,31 +2085,21 @@ function OnboardingWizardInner({ a segment for it would be one the run can never fill, and the count would visibly skip from 1 to 3. */} {!showsAgentArcStepper && ( -
- {([1, 3, 4, 5] as const).map((s) => { - const filled = step >= s; - const canJump = canJumpToOnboardingStep({ - targetStep: s, - currentStep: step, - entryStep, - }); - return ( -
+ + canJumpToOnboardingStep({ + targetStep: ONBOARDING_WIZARD_STEPS[target - 1]!, + currentStep: step, + entryStep, + }) + } + onJumpToStep={(target) => + setStep(ONBOARDING_WIZARD_STEPS[target - 1]! as Step) + } + /> )} {/* The agent arc's progress strip. Numbered 1–3 over the wizard's @@ -1909,7 +2131,14 @@ function OnboardingWizardInner({ {/* mb-6 continues the prototype's single rhythm past this block: it groups the hero and heading, and the step's own controls sit a step below on the same spacing. */} -
+ {/* The gap under the agent — its name to the step's title — + is tighter than the step's other rows on purpose. The name + labels the character directly above it, so the two read as + one object; at the full row rhythm the name floated between + the character and the title and belonged to neither. 24px + against the 36px used elsewhere, a little over a third + less. `mb-9` still holds the block off the step content. */} +
+ {/* `relative` is load-bearing: the sleep marks anchor + to this box and travel out past its top-right + corner. */} +
+ + {/* Only while it is actually asleep. A still grey + silhouette reads as a placeholder that failed to + load rather than as something waiting its turn. */} + {step < 5 && } +
@@ -1952,7 +2190,7 @@ function OnboardingWizardInner({ {/* Step content */} {step === 2 && onboardingPath === "grow" && ( -
+
@@ -2037,18 +2275,29 @@ function OnboardingWizardInner({
)} - {/* Step 1: name the organization (both paths). One question, one - design: this mirrors the funnel's naming screen — same - question, same sub, same left-aligned heading in a centered - column — so a customer creating their second organization - in-app is asked exactly what their first one asked them. */} + {/* Step 1: name the organization (both paths). + Dressed as the arc steps that follow it — centred heading, no + lede, and the same footer pair — because a customer walks + straight from here into them, and one screen reading as a + different product is more jarring than this one no longer + matching the funnel's naming screen exactly. The question + itself is still the funnel's, so the ask has not changed. + + The lede went because it said what the field already says: a + labelled "Name" under "What is the name of your organization?" + does not need a sentence explaining that it names the + organization. */} {step === 1 && ( -
+
-
+ {/* The field takes the agent step's measure rather than the + column's, so the two questions the wizard asks — name the + organization, name the agent — present the same target. + The heading stays full width above it, as it does there. */} +
-
)} {/* Step 2: Define your mission */} {step === 2 && onboardingPath !== "grow" && ( -
+
@@ -2281,12 +2524,12 @@ function OnboardingWizardInner({ `general` role; a specific one can be set later, where there is context to choose it in. */} {step === 3 && ( -
+
setAgentName(e.target.value)} autoFocus @@ -2297,132 +2540,130 @@ function OnboardingWizardInner({ {/* Step 4: Connect a model — adapter + model + env check (capsule above) */} {step === 4 && ( -
+
{/* The two cards are self-describing; an "Adapter type" eyebrow above them named the mechanism rather than the choice. */}
-
- {recommendedAdapters.map((opt) => ( - - ))} -
+ {/* The row is `ModelSourceTiles`, the same component the + connect-step prototype is drawn with, so the shipped step + and the design under review cannot drift apart. - - - {showMoreAdapters && ( -
- {moreAdapters.map((opt) => ( - - ))} -
- )} -
- - {/* Shows as soon as the cheap auth signal reports no ready - credential, well before any adapter environment test - runs. Reuses the same panel the agent configuration form - shows after a test — see AdapterLoginPanel in - AgentConfigForm.tsx. No "Use saved login" control: the - hire step already applies a stored login on its own. */} - {showAdapterLoginPanel && createdCompanyId && resolvedLoginEnvironmentId && ( - { - queryClient.invalidateQueries({ - queryKey: queryKeys.agents.authSignal( - createdCompanyId, - adapterType, - resolvedLoginEnvironmentId, - ), - }); + Sources come from `recommendedAdapters`, not a list + written here. That filter is `recommended` in the display + registry, which today means Claude Code and Codex and + nothing else — so the row stays two tiles because the + registry says so, and a third would appear here the day + someone marks one rather than the day someone remembers + to edit this file. */} + ({ + id: opt.type, + label: opt.label, + icon: , + }))} + mode={credentialMode} + selectedId={ + recommendedAdapters.some((opt) => opt.type === adapterType) + ? adapterType + : null + } + onSelect={(id) => { + setAdapterType(id); + if (id === "codex_local") return; + if (id === "opencode_local") { + setModel(DEFAULT_OPENCODE_LOCAL_MODEL); + return; + } + setModel(""); }} /> - )} + + {/* The credential switch stands where the adapter + disclosure used to. That disclosure existed to reach the + adapters this step does not offer, and with the row down + to the two that are supported it was a control whose + whole contents were out of scope. The question actually + left on this step is how the two are authenticated, so + that is what the line asks. + + It names the destination rather than the state, which is + what a sentence has to do where a checkbox does not — + and it is only readable because the tiles' own tags, + directly above, say where you are. */} +
+ +
+ +
+ + {/* One canvas under the tiles, holding whatever the current + choice needs: a browser-code login for Claude, a + displayed-code login for Codex, or a key field for either + when the mode is keys. Four inputs, one place — so the + Connect button below does not move every time the answer + changes. + + Closed until a source is picked. `contentKey` is the + source and the mode together, because either one changing + means a different input, and that is what the canvas + swaps on. */} + + {credentialMode === "api" ? ( + + ) : showAdapterLoginPanel && + createdCompanyId && + resolvedLoginEnvironmentId ? ( + /* Shows as soon as the cheap auth signal reports no ready + credential, well before any adapter environment test + runs. Reuses the same panel the agent configuration + form shows after a test — see AdapterLoginPanel in + AgentConfigForm.tsx. No "Use saved login" control: the + hire step already applies a stored login on its own. */ + { + queryClient.invalidateQueries({ + queryKey: queryKeys.agents.authSignal( + createdCompanyId, + adapterType, + resolvedLoginEnvironmentId, + ), + }); + }} + /> + ) : ( + /* No panel to show, and the two reasons for that are not + the same news. Saying either is better than an empty + card — the canvas is open because a source is selected, + and a blank one reads as something that failed to load — + but they must not be conflated: telling someone with no + sandbox that they are "already signed in" on it is + false, and it hides the one thing actually blocking + them. */ +

+ {authSignalUndecided + ? "Checking this source's credentials…" + : canShowAdapterLogin + ? "This source is already signed in on the managed sandbox." + : "No managed sandbox is available to sign in against yet."} +

+ )} +
{/* Conditional adapter fields */} {/* No model picker. Every adapter this step offers resolves @@ -2590,37 +2831,67 @@ function OnboardingWizardInner({
)} - {isAgentArcStep && ( + {/* Step 1 shares the arc's footer so the pair keeps its shape and + position from the first screen onward. Its Back is the only one + that leaves the wizard's steps rather than walking them: step 1 + is where a company is named, and behind it is the path chooser, + so `canGoBackFromOnboardingStep` — which bounds a run to the + steps it entered on — does not decide this one. */} + {(isAgentArcStep || step === 1) && ( setStep(backStepFrom(step)) - : undefined + step === 1 + ? () => { + setOnboardingPath(null); + setStep(0); + } + : canGoBackFromOnboardingStep({ currentStep: step, entryStep }) + ? () => setStep(backStepFrom(step)) + : undefined } // The prototype's cloud flow hires on this step and calls the // action "Create". Here the model step sits between, so this // one advances — which is exactly the distinction the // prototype's own local flow draws with "Next". - primaryLabel={step === 3 ? "Next" : step === 4 ? "Connect" : "Get started"} - loadingLabel={step === 4 ? "Connecting..." : "Launching..."} + primaryLabel={ + step === 1 + ? "Continue" + : step === 3 + ? "Next" + : step === 4 + ? "Connect" + : "Get started" + } + loadingLabel={ + step === 1 + ? "Creating..." + : step === 4 + ? "Connecting..." + : "Launching..." + } loading={step === 3 ? false : loading} primaryDisabled={ - step === 3 - ? !agentName.trim() - : step === 4 - ? loading || adapterEnvLoading || missionUnresolvedForHire - : loading || launchStateIncomplete + step === 1 + ? !companyName.trim() || loading + : step === 3 + ? !agentName.trim() + : step === 4 + ? loading || adapterEnvLoading || missionUnresolvedForHire + : loading || launchStateIncomplete } onPrimary={() => { - if (step === 3) setStep(4); + if (step === 1) { + if (skipsMissionStep) void handleCreateCompany(); + else setStep(2); + } else if (step === 3) setStep(4); else if (step === 4) handleGiveHeartbeat(); else handleLaunchToDashboard(); }} /> )} - {/* Footer navigation */} - {!isAgentArcStep && ( + {/* Footer navigation for the steps that still use the old pair. */} + {!isAgentArcStep && step !== 1 && (
{canGoBackFromOnboardingStep({ currentStep: step, entryStep }) && ( @@ -2636,22 +2907,6 @@ function OnboardingWizardInner({ )}
- {step === 1 && ( - - )} {step === 2 && (
+ ); +} + +/** + * The API key field, for when the credential mode is keys rather than a + * subscription. + * + * Built to the login panel's shape on purpose: same card, same padding, same + * label-left / control-right row, same 28px control height. These two are + * alternatives to each other — one canvas shows one or the other, and the + * credential switch above trades between them — so they should read as two + * answers to one question rather than as two different kinds of thing. Before + * this the key field was a stacked label over a full-width input with no card + * at all, and flipping the mode changed the shape of the step rather than its + * content. + * + * The variable name is the label rather than a sentence about it. Someone + * pasting a key knows which one they are holding; what they cannot know is where + * this step will put it, and the name answers that in the place it is asked — + * while staying short enough to sit opposite the field the way "Sign in to the + * environment" sits opposite its button. + */ +export function ApiKeyField({ + envKey, + value, + onChange, +}: { + envKey: string; + value: string; + onChange: (next: string) => void; +}) { + const inputRef = useRef(null); + + // Focus on mount, because the canvas only opens when this is the thing that + // was asked for. Layout effect so it happens before paint rather than as a + // visible jump after it. + useLayoutEffect(() => { + inputRef.current?.focus(); + }, []); + + return ( +
+ +
+ ); +} diff --git a/ui/src/components/onboarding/ConnectModelPreview.tsx b/ui/src/components/onboarding/ConnectModelPreview.tsx new file mode 100644 index 0000000000..fdd6f2acb2 --- /dev/null +++ b/ui/src/components/onboarding/ConnectModelPreview.tsx @@ -0,0 +1,152 @@ +import { useState } from "react"; +import { MotionConfig } from "motion/react"; + +import { Checkbox } from "../ui/checkbox"; +import { AgentPreview } from "./AgentPreview"; +import { CredentialModeLink } from "./CredentialModeLink"; +import { FooterNav } from "./FooterNav"; +import { + ModelSourceTiles, + type CredentialMode, + type ModelSource, +} from "./ModelSourceTiles"; +import { OnboardingHeading } from "./OnboardingPrimitives"; +import { PillGuy } from "./PillGuy"; +import { SleepingZs } from "./SleepingZs"; +import { Stepper } from "./Stepper"; + +/** + * A prototype of the connect step, from the PCLP-Onboarding file (nodes + * 2941:8291 and 2933:4592). + * + * A mock, not the shipped step. The wizard's real step 4 puts two adapter cards + * over an advanced-settings disclosure and probes the environment before + * hiring; none of that is wired up here. What is here is the part the design is + * actually asking a question about — how the row of sources reads as you point + * at it, pick one, and flip the whole row between subscription and API + * credentials — so it can be judged before any of that machinery is moved. + * + * It lives in `components/` rather than beside a story because two surfaces + * render it: the Storybook stories, and the standalone + * `connect-model-preview.html` entry that gets deployed for review. A copy in + * each would have drifted the moment one was tweaked. + * + * Nothing here reaches a backend, and it needs none of the app's providers — + * every piece it composes is presentational. + */ + +/** + * The two sources the step offers, matching the shipped step's own list. + * + * Claude Code and Codex are the only adapters the display registry marks + * `recommended`, and the real step builds its row from exactly that filter — so + * a third tile here would be a design the wizard could never render. OpenCode + * was drawn at one point and is deliberately gone. + */ +const MODEL_SOURCES: ModelSource[] = [ + { + id: "claude_local", + label: "Claude Code", + icon: , + }, + { + id: "codex_local", + label: "Codex", + icon: , + }, +]; + +/** + * Which control flips the credential mode. Two alternates of the same + * behaviour, kept side by side so they can be compared rather than argued + * about: + * + * `checkbox` is the Figma frames — a ticked box reading "Use API keys instead", + * which shows the current state plainly and costs a row of chrome. + * + * `link` is a line of text that renames itself on press. Lighter, and it turns + * the row into a single sentence, but it can only ever name the destination — + * so where you are now is left entirely to the tiles' tags. + */ +export type CredentialControl = "checkbox" | "link"; + +export function ConnectModelPreview({ + initialSourceId = null, + initialUseApiKeys = false, + control = "checkbox", +}: { + initialSourceId?: string | null; + initialUseApiKeys?: boolean; + control?: CredentialControl; +}) { + const [selectedId, setSelectedId] = useState(initialSourceId); + const [useApiKeys, setUseApiKeys] = useState(initialUseApiKeys); + const mode: CredentialMode = useApiKeys ? "api" : "subscription"; + + return ( + // The arc's own convention: OS-level reduced motion neutralises the + // movement, and every piece below still arrives in its final state. + +
+ {/* Connect is the arc's second step. `Stepper` carries its own bottom + margin, which is the gap the frame wants under the dots. */} + + +
+ {/* `relative` is load-bearing: the sleep marks anchor to this box and + travel out past its top-right corner. */} +
+ + +
+ +
+ +
+ +
+ +
+ + + {control === "link" ? ( + setUseApiKeys(next === "api")} + /> + ) : ( + + )} +
+ + {/* The CTA has nothing to connect until a source is picked, so it stays + disabled rather than failing on press. */} + {}} + primaryLabel="Connect" + primaryDisabled={selectedId === null} + onPrimary={() => {}} + /> +
+
+ ); +} diff --git a/ui/src/components/onboarding/CredentialModeLink.tsx b/ui/src/components/onboarding/CredentialModeLink.tsx new file mode 100644 index 0000000000..cfd85d7d15 --- /dev/null +++ b/ui/src/components/onboarding/CredentialModeLink.tsx @@ -0,0 +1,89 @@ +import { AnimatePresence, motion } from "motion/react"; + +import { cn } from "../../lib/utils"; +import type { CredentialMode } from "./ModelSourceTiles"; +import { LINK_LABEL_FADE_IN, LINK_LABEL_FADE_OUT } from "./onboarding-motion"; + +/** + * The credential-mode switch as a line of text instead of a checkbox — an + * alternate for the connect step, not a replacement. + * + * The label names the destination rather than the state: "Use API keys + * instead" while on the subscription, "Use subscription instead" once on API + * keys. That is what makes a link work where a checkbox does not — a checkbox + * can be ticked or not and reads the same either way, whereas a bare sentence + * has to say what pressing it does. The consequence is that this control never + * shows you where you are; the tiles' tags do that, and this alternate only + * holds up because they are right above it. + */ + +const LINK_LABEL: Record = { + subscription: "Use API keys instead", + api: "Use subscription instead", +}; + +const OTHER_MODE: Record = { + subscription: "api", + api: "subscription", +}; + +export function CredentialModeLink({ + mode, + onChange, +}: { + mode: CredentialMode; + onChange: (next: CredentialMode) => void; +}) { + return ( + + ); +} diff --git a/ui/src/components/onboarding/FooterNav.tsx b/ui/src/components/onboarding/FooterNav.tsx index 2636d714e3..5159d6a838 100644 --- a/ui/src/components/onboarding/FooterNav.tsx +++ b/ui/src/components/onboarding/FooterNav.tsx @@ -25,14 +25,19 @@ export function FooterNav({ onPrimary: () => void; }) { return ( -
+
{onBack ? ( - // has-[>svg]:pr-4 gives "Back" room from the pill's right edge, - // overriding size="sm"'s symmetric padding on that side only. + // Same size as the primary, not a tier down. Back is ghost until you + // point at it, and a shorter pill made the hover surface read as a + // different kind of control sitting slightly low in the row rather than + // the other half of a pair. + // + // The padding stays asymmetric against size="lg"'s symmetric px-4: the + // arrow needs less room on its side than the word does on its own. + ); +} + +export function ModelSourceTiles({ + sources, + mode, + selectedId, + onSelect, + label, +}: { + sources: ModelSource[]; + mode: CredentialMode; + /** `null` before anything has been picked — the step opens this way. */ + selectedId: string | null; + onSelect: (id: string) => void; + label: string; +}) { + const tiles = useRef(new Map()); + + /** + * Arrow keys move the selection and the focus together, which is what a + * radio group is expected to do — without it the role would be announced and + * then not behave, which is worse than plain buttons. Selection wraps at both + * ends; three tiles is short enough that stopping at the edges just reads as + * the key having failed. + */ + const moveSelection = (delta: number) => { + if (sources.length === 0) return; + const current = sources.findIndex((source) => source.id === selectedId); + // Nothing picked yet: either arrow enters the row from the near end. + const from = current === -1 ? (delta > 0 ? -1 : 0) : current; + const next = (from + delta + sources.length) % sources.length; + const target = sources[next]!; + onSelect(target.id); + tiles.current.get(target.id)?.focus(); + }; + + return ( +
{ + if (event.key === "ArrowRight" || event.key === "ArrowDown") { + event.preventDefault(); + moveSelection(1); + } else if (event.key === "ArrowLeft" || event.key === "ArrowUp") { + event.preventDefault(); + moveSelection(-1); + } + }} + > + {sources.map((source) => ( + onSelect(source.id)} + buttonRef={(node) => { + if (node) tiles.current.set(source.id, node); + else tiles.current.delete(source.id); + }} + /> + ))} +
+ ); +} diff --git a/ui/src/components/onboarding/SleepingZs.tsx b/ui/src/components/onboarding/SleepingZs.tsx new file mode 100644 index 0000000000..46bb32401e --- /dev/null +++ b/ui/src/components/onboarding/SleepingZs.tsx @@ -0,0 +1,177 @@ +import { useState } from "react"; +import { motion, useReducedMotion } from "motion/react"; + +import { cn } from "../../lib/utils"; + +/** + * Sleep marks drifting off the dormant agent. + * + * The capsule is grey and closed-eyed for the whole of the connect step, which + * is accurate — nothing has been hired yet — but a still silhouette reads as a + * placeholder that failed to load rather than as something waiting. Three small + * z's rising off its shoulder say "asleep, not broken" without adding a second + * thing to look at. + * + * Decorative and announced to nobody: the state it depicts is already carried + * by the step's own copy. + */ + +/** + * The glyphs, smallest first. Each rises further and ends larger than the one + * below it, so the three together read as one plume with depth rather than as + * three identical marks on different timers — the "zzZZ" shape of the thing + * written down. + */ +const Z_TIERS = [ + { glyph: "z", sizeClass: "text-(length:--text-nano)", scaleTo: 0.95, reach: 1 }, + { glyph: "z", sizeClass: "text-xs", scaleTo: 1.1, reach: 1.25 }, + { glyph: "Z", sizeClass: "text-sm", scaleTo: 1.25, reach: 1.5 }, +] as const; + +/** + * Applied to both ends of every glyph's scale, so the marks read larger without + * the plume changing shape — a bump to the tiers' own `scaleTo` values alone + * would have grown the three by different amounts and flattened the depth + * between them. + */ +const Z_SCALE = 1.1; + +/** + * Where a glyph is born, measured down from the anchor at the dome's crown. + * + * Low enough that the marks read as rising off the head rather than hovering + * above it, but back off the silhouette: further down, the first frames of each + * glyph landed on the dome's own grey and the fade-in was lost against it. + */ +const ORIGIN_DROP = 17; + +const Z_SCALE_FROM = 0.55 * Z_SCALE; + +type ZTier = (typeof Z_TIERS)[number]; + +type ZFlight = { + launchX: number; + launchY: number; + driftX: number; + driftY: number; + rotate: number; + duration: number; + delay: number; +}; + +function randomBetween(min: number, max: number) { + return min + Math.random() * (max - min); +} + +/** + * A fresh flight for one glyph. + * + * Re-rolled every cycle rather than fixed at mount. Three fixed loops of + * different lengths do drift apart, but they still repeat exactly, and at this + * size the eye picks the period up within a few passes — which is the one thing + * an idle animation must not do. + */ +function nextFlight(tier: ZTier, index: number, first: boolean): ZFlight { + // Each mark leaves from a slightly different point rather than all three from + // one. Without this the tiers launch stacked and the first moment of a cycle + // is a smudge of overlapping glyphs instead of a plume. + const launchX = randomBetween(-4, 4); + // Every glyph starts `ORIGIN_DROP` below the anchor and climbs from there. + // The drift is measured off the launch point rather than the anchor, so + // moving the origin slides the whole plume without shortening its travel. + const launchY = ORIGIN_DROP + randomBetween(-3, 3); + return { + launchX, + launchY, + driftX: launchX + randomBetween(9, 18) * tier.reach, + driftY: launchY - randomBetween(20, 30) * tier.reach, + rotate: randomBetween(-14, 16), + duration: randomBetween(1.9, 2.6), + // A gap between cycles, so the plume puffs rather than streams. Kept short + // enough that all three are never idle together for long: an animation + // whose whole point is "still running, just asleep" cannot afford stretches + // where there is nothing on screen at all. The first delay is staggered by + // tier so they do not launch as one on the step's first frame. + delay: first ? index * 0.4 : randomBetween(0.3, 1.1), + }; +} + +function SleepyZ({ tier, index }: { tier: ZTier; index: number }) { + // `cycle` is a remount key, not a counter anyone reads: changing it replaces + // the span so the next flight starts from `initial` again. Re-running + // `animate` alone would tween from wherever the last one ended, and the glyph + // would wander off instead of restarting at the shoulder. + const [cycle, setCycle] = useState(0); + const [flight, setFlight] = useState(() => nextFlight(tier, index, true)); + + return ( + { + setFlight(nextFlight(tier, index, false)); + setCycle((previous) => previous + 1); + }} + > + {tier.glyph} + + ); +} + +/** + * Positioned absolutely over the caller's capsule, which must be `relative`. + * The marks are anchored to the dome's upper-right shoulder and travel out + * past the box, so nothing between here and the step's own frame may clip. + * + * Rendered as nothing at all when the OS asks for reduced motion. This is the + * one animation on the step with no end — the usual token-level treatment + * shortens durations, which for an endless loop just means it repeats faster. + */ +export function SleepingZs({ className }: { className?: string }) { + const reducedMotion = useReducedMotion(); + if (reducedMotion) return null; + + return ( + + + {Z_TIERS.map((tier, index) => ( + + ))} + + + ); +} diff --git a/ui/src/components/onboarding/Stepper.test.ts b/ui/src/components/onboarding/Stepper.test.ts index 990e512ef9..6dc9e2e260 100644 --- a/ui/src/components/onboarding/Stepper.test.ts +++ b/ui/src/components/onboarding/Stepper.test.ts @@ -1,5 +1,10 @@ import { describe, expect, it } from "vitest"; -import { AGENT_ARC_TOTAL_STEPS, agentArcStepFor } from "./Stepper"; +import { + AGENT_ARC_TOTAL_STEPS, + ONBOARDING_WIZARD_STEPS, + agentArcStepFor, + onboardingStepPositionFor, +} from "./Stepper"; describe("agentArcStepFor", () => { it("numbers the arc from the agent step, not from the wizard's first step", () => { @@ -30,3 +35,35 @@ describe("agentArcStepFor", () => { } }); }); + +describe("onboardingStepPositionFor", () => { + it("counts the full walk's own steps, not the wizard's", () => { + // The strip draws steps 1, 3, 4, 5 — four segments over a wizard that + // numbers to five. + expect(onboardingStepPositionFor(1)).toBe(1); + expect(onboardingStepPositionFor(3)).toBe(2); + expect(onboardingStepPositionFor(4)).toBe(3); + expect(onboardingStepPositionFor(5)).toBe(4); + }); + + it("holds the last completed segment on a step it does not draw", () => { + // The mission step is passed through on the "grow" path but has no segment. + // Counting keeps the strip on step 1's segment; an index lookup would find + // nothing and report no progress at all from a screen the customer reached + // by making progress. + expect(onboardingStepPositionFor(2)).toBe(1); + }); + + it("reports nothing before the walk starts", () => { + // The front door is not part of the count. + expect(onboardingStepPositionFor(0)).toBe(0); + }); + + it("never counts past the segments it advertises", () => { + for (const wizardStep of [-1, 0, 1, 2, 3, 4, 5, 6, 99]) { + const position = onboardingStepPositionFor(wizardStep); + expect(position).toBeGreaterThanOrEqual(0); + expect(position).toBeLessThanOrEqual(ONBOARDING_WIZARD_STEPS.length); + } + }); +}); diff --git a/ui/src/components/onboarding/Stepper.tsx b/ui/src/components/onboarding/Stepper.tsx index 335b9e960f..bb309a78ca 100644 --- a/ui/src/components/onboarding/Stepper.tsx +++ b/ui/src/components/onboarding/Stepper.tsx @@ -11,7 +11,7 @@ export const AGENT_ARC_TOTAL_STEPS = 3; * What each segment goes to. These are the labels assistive tech reads, in * place of a bare number: the wizard has its own step numbering, and two * controls both announcing "Step 1" while meaning different steps is worse - * than no number at all. The visible "Step N of 3" line carries the count. + * than no number at all. The strip's own "Step N of 3" line carries the count. */ export const AGENT_ARC_STEP_LABELS = [ "Create your first agent", @@ -22,6 +22,38 @@ export const AGENT_ARC_STEP_LABELS = [ /** Wizard step numbers that make up the arc, in order. */ export const AGENT_ARC_WIZARD_STEPS = [3, 4, 5] as const; +/** + * The full walk, for a run that started at the front door rather than partway + * into the arc. + * + * Step 2 is absent: onboarding no longer asks for the mission, so a segment for + * it would be one the run can never fill. The run still passes *through* step 2 + * on the "grow" path, which is why position is counted rather than looked up — + * see `onboardingStepPositionFor`. + */ +export const ONBOARDING_WIZARD_STEPS = [1, 3, 4, 5] as const; + +/** Destinations for the full walk, in the same order. */ +export const ONBOARDING_STEP_LABELS = [ + "Name your organization", + "Create your first agent", + "Connect a model", + "Review", +] as const; + +/** + * Position in the full walk: how many of its steps are at or behind `step`. + * + * Counted rather than indexed because the wizard visits steps the strip does + * not draw. On the mission step there is no segment to be "on", and an index + * lookup would return nothing and light none of them — reporting no progress + * from a screen the customer reached by making progress. Counting keeps the + * strip on the last segment actually completed. + */ +export function onboardingStepPositionFor(step: number): number { + return ONBOARDING_WIZARD_STEPS.filter((entry) => entry <= step).length; +} + /** * Map a wizard step onto its position in the arc, or `null` when the step is * outside it. @@ -40,7 +72,14 @@ export function agentArcStepFor(wizardStep: number): number | null { } /** - * Segmented progress strip with a "Step N of M" label. + * Segmented progress strip: three dots, centred over the step's own centred + * hero and heading. + * + * The "Step N of M" count is announced but not drawn. Three dots at this size + * are read in a glance — there is no counting to help with — so the line was + * spending a whole row, and the only left-aligned element on an otherwise + * centred step, to restate what the dots already say. Assistive tech has no + * glance, so the sentence stays in the accessibility tree. * * Segments double as the way back to a step already completed, which is the * affordance the wizard's full-length bar provides outside the arc. A segment @@ -52,41 +91,49 @@ export function agentArcStepFor(wizardStep: number): number | null { export function Stepper({ step, total = AGENT_ARC_TOTAL_STEPS, + labels = AGENT_ARC_STEP_LABELS, canJumpToStep, onJumpToStep, }: { step: number; total?: number; + /** + * What each segment goes to. Defaults to the arc's three; the full walk from + * the front door passes its own four, since the same strip serves both and a + * segment announcing "Create your first agent" on the organization step would + * be worse than a bare number. + */ + labels?: readonly string[]; canJumpToStep?: (target: number) => boolean; onJumpToStep?: (target: number) => void; }) { return ( -
-
- {Array.from({ length: total }, (_, index) => index + 1).map((segment) => { - const jumpable = Boolean(canJumpToStep?.(segment) && onJumpToStep); - return ( -
- +
+ {Array.from({ length: total }, (_, index) => index + 1).map((segment) => { + const jumpable = Boolean(canJumpToStep?.(segment) && onJumpToStep); + return ( +
diff --git a/ui/src/components/onboarding/onboarding-motion.ts b/ui/src/components/onboarding/onboarding-motion.ts index ca28408883..5bdebcf319 100644 --- a/ui/src/components/onboarding/onboarding-motion.ts +++ b/ui/src/components/onboarding/onboarding-motion.ts @@ -24,7 +24,11 @@ export const CAPSULE_ENTER_DURATION = 1.0; export const capsuleMotion = { initial: { opacity: 0, scale: 0.5 }, animate: { opacity: 1, scale: 1 }, - transition: { type: "spring" as const, duration: CAPSULE_ENTER_DURATION, bounce: 0.4 }, + transition: { + type: "spring" as const, + duration: CAPSULE_ENTER_DURATION, + bounce: 0.4, + }, }; /** The name/role reveal: the label fade is staggered by 25% of this. */ @@ -52,3 +56,97 @@ export const capsuleHeroMotion = { opacity: { duration: 0.55, ease: STEP_EASE }, }, }; + +/** + * The credential tag's swap between "Subscription" and "API" on the connect + * step's source tiles. + * + * Both labels share one clipped slot and cross inside it: the outgoing one + * always falls out of frame while the incoming one rises into place. Fixing the + * direction is the point — deriving it from which way the toggle moved would + * make one control produce two different animations, and at 10px the tag is far + * too small for that to read as anything but a flicker. + * + * The exit stays 80ms shorter than the enter so the slot has mostly cleared by + * the time the arriving label reaches the middle of it, rather than the two + * words being legible on top of each other. Both moved together when the swap + * was lengthened, which is what keeps that relationship: stretching only the + * enter would have opened the gap instead, and the swap would read as one label + * leaving and a separate one arriving. + * + * Eased in and out — the house material curve, mirroring + * `--motion-ease-standard` — rather than the arc's expo-out. Expo-out leaves at + * full speed from the first frame, which suits something arriving from + * offscreen; over 7px it just looked like the label snapped and then settled. + * Easing into the movement gives the swap a beginning. + */ +export const TAG_SWAP_TRAVEL = 7; +export const TAG_SWAP_EASE = [0.4, 0, 0.2, 1] as const; +export const TAG_SWAP_ENTER = { duration: 0.34, ease: TAG_SWAP_EASE } as const; +export const TAG_SWAP_EXIT = { duration: 0.26, ease: TAG_SWAP_EASE } as const; + +/** + * The credential-mode link's own label swap, when that control is a line of + * text rather than a checkbox. + * + * A plain crossfade, with no travel — deliberately unlike the tag it triggers. + * The tag slides because it is being replaced inside a slot it shares with the + * label before it; the link is not replaced, it is one control renaming itself, + * and giving it the same movement would read as a second thing changing rather + * than the cause of the first. + * + * The old label leaves quickly and the new one starts once it is nearly gone, + * so the two are never both readable — two near-identical sentences at half + * opacity are unreadable in a way two single words are not. Even with the + * stagger it settles just inside the tag swap, so the sentence and the tags + * finish together. + */ +export const LINK_LABEL_FADE_OUT = { + duration: 0.12, + ease: TAG_SWAP_EASE, +} as const; +export const LINK_LABEL_FADE_IN = { + duration: 0.22, + delay: 0.08, + ease: TAG_SWAP_EASE, +} as const; + +/** + * The connect step's input canvas: the card that opens under the tiles once a + * source is picked, and re-fills itself when the choice changes. + * + * Everything here is the tag swap's vocabulary, reused deliberately. The canvas + * is downstream of that control — picking a source or flipping the credential + * mode is what fills it — so a second easing or a second rhythm would read as a + * separate thing reacting rather than the same gesture continuing. + * + * The canvas container itself does not animate at all. It carried an open/close + * three times — height, then opacity — and stalled every time, once leaving the + * login card rendered inside a two-pixel box. The content swap below is where + * the motion lives, and it is enough. + */ +export const CANVAS_EASE = TAG_SWAP_EASE; + +/** + * The content swap inside the canvas, when the source or the credential mode + * changes while it is already open. + * + * Shorter than the canvas opening, and with the same enter/exit asymmetry as the + * tag: the outgoing input is mostly gone before the incoming one arrives, so two + * different forms are never legible on top of each other. + * + * The swap itself is the tag's, not a variation on it: one input falls out of + * the card while the next rises into place, on the same travel and the same + * curve. Flipping the credential mode moves the tag and re-fills the canvas in + * one gesture, and giving the two ends of that gesture different motion would + * make them read as separate events. + * + * There is no spinner and no hold. An earlier version had both, on the reasoning + * that the panels behind the canvas fetch — but they are components, available + * the moment the choice changes, and the 400ms floor needed to make a spinner + * legible was time added to a swap that had nothing to wait for. A spinner + * standing in for no work is a slower screen that also says something untrue. + */ +export const CANVAS_CONTENT_ENTER = TAG_SWAP_ENTER; +export const CANVAS_CONTENT_EXIT = TAG_SWAP_EXIT; +export const CANVAS_CONTENT_TRAVEL = TAG_SWAP_TRAVEL; diff --git a/ui/src/connect-model-preview-main.tsx b/ui/src/connect-model-preview-main.tsx new file mode 100644 index 0000000000..c5b6ca3a69 --- /dev/null +++ b/ui/src/connect-model-preview-main.tsx @@ -0,0 +1,86 @@ +import { StrictMode } from "react"; +import { createRoot } from "react-dom/client"; + +import { + ConnectModelPreview, + type CredentialControl, +} from "./components/onboarding/ConnectModelPreview"; +import "./index.css"; + +/** + * Harness for the standalone `connect-model-preview.html` entry — the build + * that gets deployed so the connect-step mock can be reviewed from a link + * rather than a checkout. + * + * Deliberately bare. `ConnectModelPreview` composes only presentational pieces + * and never reaches a backend, so there is no provider stack, no query client + * and no router here; adding them would mean the deployed page was exercising + * different code from the Storybook one. + * + * `dark` is set on in the entry document rather than mounted through + * ThemeProvider, for the same reason: the design is dark and the class variant + * is all the tokens need. + */ + +/** + * `?state=` picks which frame the page opens on, mirroring the `?step=` + * convention the onboarding-flow preview uses. Everything stays clickable + * afterwards — the parameter chooses a starting point, not a locked state — so + * a reviewer sent straight to one frame can still reach the others. + */ +const STATES = { + default: {}, + subscription: { initialSourceId: "claude_local" }, + api: { initialSourceId: "claude_local", initialUseApiKeys: true }, +} as const; + +type StateName = keyof typeof STATES; + +function isStateName(value: string | null): value is StateName { + return value !== null && value in STATES; +} + +/** + * Which mode switch the page opens with. Orthogonal to `?state=`, so either + * control can be opened on any of the frames. + * + * The text link is the default because it is the direction that was chosen; the + * Figma checkbox stays reachable at `?control=checkbox` for comparison. Sharing + * a bare link and landing on the option nobody picked is a worse failure than + * having to type a parameter to see the runner-up. + */ +const DEFAULT_CONTROL: CredentialControl = "link"; + +function isControl(value: string | null): value is CredentialControl { + return value === "checkbox" || value === "link"; +} + +const params = new URLSearchParams(window.location.search); +const requested = params.get("state"); +const control = params.get("control"); + +createRoot(document.getElementById("root")!).render( + + {/* + Centred against the viewport, not against whatever the page happens to be + tall. `min-h-dvh` measures the viewport itself — a percentage min-height + needs an ancestor with a definite height to resolve against, and this one + has none, so it silently resolved to nothing and the step sat at the top. + + The vertical centring is `my-auto` on the child rather than `items-center` + on the row. They look identical until the step is taller than the window: + align-items overflows a centred item equally in both directions and the + top half becomes unreachable, since scrolling cannot reach above the + container's start. Auto margins collapse to zero when there is no free + space, so a short window falls back to top-aligned and scrolls. + */} +
+
+ +
+
+
, +); diff --git a/ui/src/index.css b/ui/src/index.css index 7455a5e4d9..4c60ccc6a3 100644 --- a/ui/src/index.css +++ b/ui/src/index.css @@ -2266,6 +2266,7 @@ span.paperclip-mention-chip[data-mention-kind="external-object"] { --sz-44rem: 44rem; --sz-36rem: 36rem; --sz-neg-1_25rem: -1.25rem; + --sz-85pct: 85%; --sz-30pct: 30%; --sz-24pct: 24%; --sz-18pct: 18%; diff --git a/ui/src/pages/NewAgent.test.tsx b/ui/src/pages/NewAgent.test.tsx index c3cd15d10f..42e500f38c 100644 --- a/ui/src/pages/NewAgent.test.tsx +++ b/ui/src/pages/NewAgent.test.tsx @@ -232,8 +232,8 @@ async function renderNewAgent() { // start the login, and let the panel reach the server `stored` state. async function completeClaudeLogin(container: HTMLElement) { await clickByText(container, "Test Agent"); - await flushUntil(() => Boolean(findButton(container, "Log in"))); - await clickByText(container, "Log in"); + await flushUntil(() => Boolean(findButton(container, "Sign in"))); + await clickByText(container, "Sign in"); await flushUntil(() => (container.textContent ?? "").includes("Authenticated")); } @@ -328,12 +328,12 @@ describe("NewAgent Claude subscription login", () => { roots.push(result.root); // Before the test the page shows no login affordance. - expect(findButton(result.container, "Log in")).toBeFalsy(); + expect(findButton(result.container, "Sign in")).toBeFalsy(); await clickByText(result.container, "Test Agent"); - await flushUntil(() => Boolean(findButton(result.container, "Log in"))); + await flushUntil(() => Boolean(findButton(result.container, "Sign in"))); - const loginButton = findButton(result.container, "Log in"); + const loginButton = findButton(result.container, "Sign in"); const createButton = findButton(result.container, "Create agent"); expect(loginButton).toBeTruthy(); expect(createButton).toBeTruthy(); @@ -401,8 +401,8 @@ describe("NewAgent Claude subscription login", () => { await clickByText(result.container, "Test Agent"); // The panel shows the replace action only after it reads the stored-token // status, so the button label proves the panel captured the version. - await flushUntil(() => Boolean(findButton(result.container, "Log in to replace"))); - await clickByText(result.container, "Log in to replace"); + await flushUntil(() => Boolean(findButton(result.container, "Sign in to replace"))); + await clickByText(result.container, "Sign in to replace"); await flushUntil(() => mockAgentsApi.startClaudeSetupTokenLogin.mock.calls.length > 0); expect(mockAgentsApi.startClaudeSetupTokenLogin).toHaveBeenCalledWith("company-1", { diff --git a/ui/storybook/.storybook/preview.tsx b/ui/storybook/.storybook/preview.tsx index 537b1d6328..9a816d8965 100644 --- a/ui/storybook/.storybook/preview.tsx +++ b/ui/storybook/.storybook/preview.tsx @@ -7,6 +7,14 @@ import { } from "@paperclipai/shared"; import { MemoryRouter } from "@/lib/router"; import { ONBOARDING_STORAGE_KEY } from "@/components/OnboardingWizard"; +import { STORYBOOK_COMPANY_ID } from "../fixtures/onboardingDraft"; +import { + STORYBOOK_SANDBOX_ENVIRONMENT_ID, + storybookAuthSignal, + storybookEnvironmentCapabilities, + storybookEnvironmentTest, + storybookEnvironments, +} from "../fixtures/onboardingEnvironment"; import { BreadcrumbProvider } from "@/context/BreadcrumbContext"; import { CompanyProvider } from "@/context/CompanyContext"; import { DialogProvider } from "@/context/DialogContext"; @@ -22,6 +30,7 @@ import { storybookAuthSession, storybookCompanies, storybookDashboardSummary, + storybookHiredAgent, storybookIssues, storybookLiveRuns, storybookProjects, @@ -137,6 +146,10 @@ function installStorybookApiFixtures() { return Response.json({ enableIsolatedWorkspaces: true, autoRestartDevServerWhenIdle: false, + // The cloud-tenant shape, and what the onboarding connect step resolves + // its login environment through: without it the step looks for a local + // default and never finds the managed sandbox. + enableManagedSandboxOnly: true, }); } @@ -144,17 +157,192 @@ function installStorybookApiFixtures() { return Response.json({}); } - // The onboarding wizard's connect step reads these. An empty environment - // list is the cloud-tenant shape — agents run in a managed sandbox rather - // than a configured environment — and it is also the state that produces - // the "no managed sandbox environment is available" notice, which is worth - // being able to look at rather than only meeting it on a live stack. + // The connect step's provider sign-in is gated on a *sandbox* environment + // resolving, its provider supporting a login PTY, and the auth signal coming + // back absent. These three answers decide whether that panel renders at all, + // so a story picks them through `onboardingFixtureState` rather than getting + // one hard-coded shape — an earlier version returned an empty environment + // list here and made the panel invisible everywhere. if (/^\/api\/companies\/[^/]+\/environments$/.test(url.pathname)) { + return Response.json(storybookEnvironments()); + } + + if ( + /^\/api\/companies\/[^/]+\/environments\/capabilities$/.test(url.pathname) + ) { + return Response.json(storybookEnvironmentCapabilities()); + } + + if ( + /^\/api\/companies\/[^/]+\/adapters\/[^/]+\/auth-signal/.test( + url.pathname, + ) + ) { + return Response.json(storybookAuthSignal()); + } + + if ( + /^\/api\/companies\/[^/]+\/adapters\/[^/]+\/models$/.test(url.pathname) + ) { return Response.json([]); } - if (/^\/api\/companies\/[^/]+\/adapters\/[^/]+\/models$/.test(url.pathname)) { - return Response.json([]); + // Codex's login, which is a different flow on different routes. + // + // Claude signs in through the setup-token routes below; every other adapter + // uses these generic per-adapter ones. Only the Claude half was stubbed at + // first, so pressing Sign in on the Codex tile fell through to the dev + // server and came back 404 — which reads as a broken product rather than as + // a missing fixture, and the two are not distinguishable from the panel. + // + // Its panel mode is `displayed_code`, not Claude's `submitted_browser_code`: + // the server shows a URL *and* a code to type into it, and nothing is typed + // back here. So this is a genuinely different card, and the canvas holding it + // has to size to it too. + const adapterLoginMatch = url.pathname.match( + /^\/api\/companies\/[^/]+\/adapters\/([^/]+)\/login-sessions(?:\/([^/]+))?(\/cancel)?$/, + ); + if (adapterLoginMatch) { + const session = { + sessionId: "adapter-login-storybook", + environmentId: STORYBOOK_SANDBOX_ENVIRONMENT_ID, + // `waiting_for_user` is the state this panel is worth looking at in: the + // session is live and the customer is being asked for something. + status: "waiting_for_user", + expiresAt: null, + failure: null, + }; + if (adapterLoginMatch[3]) return Response.json({ ...session, status: "cancelled" }); + // The prompt rides the owner read of the session rather than a route of + // its own — the shape that differs from Claude's, where it is guarded + // separately. Returning it only on the read with a session id keeps that + // distinction rather than flattening the two flows into one. + if (adapterLoginMatch[2]) { + return Response.json({ + ...session, + prompt: { + url: "https://auth.openai.com/device", + code: "STORY-BOOK", + }, + }); + } + return Response.json(session); + } + + // Claude's setup-token login, enough of it to watch the panel expand. + // + // The point is not the login — it is what the panel does to the card around + // it. Starting a login turns a single row into a row plus an authorization + // URL plus a code field, and the onboarding canvas that holds it animates + // its own height and clips its overflow. A canvas that measured itself once + // would cut that expansion off, and nothing short of driving the flow would + // show it. + if ( + /^\/api\/companies\/[^/]+\/setup-token-login-sessions$/.test(url.pathname) + ) { + return Response.json({ + sessionId: "setup-token-storybook", + environmentId: STORYBOOK_SANDBOX_ENVIRONMENT_ID, + status: "awaiting_browser_code", + expiresAt: null, + failure: null, + }); + } + if ( + /^\/api\/companies\/[^/]+\/setup-token-login-sessions\/[^/]+$/.test( + url.pathname, + ) + ) { + return Response.json({ + sessionId: "setup-token-storybook", + environmentId: STORYBOOK_SANDBOX_ENVIRONMENT_ID, + status: "awaiting_browser_code", + expiresAt: null, + failure: null, + }); + } + // The authorization URL is its own route, and deliberately so: the status + // read above is public and carries no secret, while the URL is an owner-only + // read. The panel polls this one separately and stays on "Preparing the + // login…" until it answers — so a fixture without it looks like a hung login + // rather than a missing route, which is exactly how it was misread once. + if ( + /^\/api\/companies\/[^/]+\/setup-token-login-sessions\/[^/]+\/prompt$/.test( + url.pathname, + ) + ) { + return Response.json({ + authorizationUrl: + "https://claude.ai/oauth/authorize?client_id=storybook&response_type=code&state=storybook", + transportAdvisory: null, + }); + } + // Submitting the browser code. The panel hands the pasted code here and then + // completes; both are stubbed so the last stage of the flow — the one where + // the card is at its tallest — can actually be reached. + if ( + /^\/api\/companies\/[^/]+\/setup-token-login-sessions\/[^/]+\/code$/.test( + url.pathname, + ) + ) { + return Response.json({ + sessionId: "setup-token-storybook", + environmentId: STORYBOOK_SANDBOX_ENVIRONMENT_ID, + status: "awaiting_completion", + expiresAt: null, + failure: null, + transportAdvisory: null, + }); + } + if ( + /^\/api\/companies\/[^/]+\/claude-oauth-token-status$/.test(url.pathname) + ) { + return new Response(null, { status: 404 }); + } + + // The hire, and the three calls either side of it. + // + // These exist so the review step can be reached the way a customer reaches + // it — by pressing Connect — rather than by seeding a draft that claims the + // hire already happened. The difference is not pedantry: the wizard only + // offers Back on a step it walked *forward* into, so a story that starts on + // the review step renders it without the control it is supposed to have. + // + // The environment test, which is the hire's gate. It answers from the story's + // auth state rather than always passing — see `storybookEnvironmentTest`. + const testEnvMatch = url.pathname.match( + /^\/api\/companies\/[^/]+\/adapters\/([^/]+)\/test-environment$/, + ); + if (testEnvMatch) { + return Response.json(storybookEnvironmentTest(testEnvMatch[1])); + } + if (/^\/api\/companies\/[^/]+\/agent-hires$/.test(url.pathname)) { + // `approval: null` on purpose. A hire that returns one sends the wizard + // through the approvals API before it advances, and this story is about + // the step it lands on rather than the path it took. + return Response.json({ agent: storybookHiredAgent, approval: null }); + } + const instructionsBundleMatch = url.pathname.match( + /^\/api\/agents\/([^/]+)\/instructions-bundle(\/file)?$/, + ); + if (instructionsBundleMatch) { + // The wizard seeds the lead's instructions here and swallows a failure — + // so an unstubbed route costs nothing but a console warning on every run, + // which is the kind of noise that trains people to ignore the console. + if (instructionsBundleMatch[2]) { + return Response.json({ path: "AGENTS.md", content: "" }); + } + return Response.json({ + agentId: instructionsBundleMatch[1], + companyId: STORYBOOK_COMPANY_ID, + mode: "managed", + rootPath: null, + managedRootPath: `/managed/agents/${instructionsBundleMatch[1]}`, + entryFile: "AGENTS.md", + resolvedEntryPath: `/managed/agents/${instructionsBundleMatch[1]}/AGENTS.md`, + editable: true, + warnings: [], + }); } if ( @@ -232,6 +420,16 @@ function installStorybookApiFixtures() { supportsLocalAgentJwt: true, requiresMaterializedRuntimeSkills: false, supportsModelProfiles: true, + // `useAdapterCapabilities` prefers this listing over its own static + // defaults, so an omission here is not a smaller fixture — it is a + // capability the adapter loses. Without `login` the onboarding + // connect step's provider sign-in silently never renders, which is + // indistinguishable from it having been removed. Mirrors + // `KNOWN_DEFAULTS` in `use-adapter-capabilities.ts`. + login: { + panelMode: "submitted_browser_code", + timeoutPolicy: "fixed", + }, }, }, { @@ -247,6 +445,10 @@ function installStorybookApiFixtures() { supportsLocalAgentJwt: true, requiresMaterializedRuntimeSkills: false, supportsModelProfiles: true, + login: { + panelMode: "displayed_code", + timeoutPolicy: "caller_bounded", + }, }, }, ]); diff --git a/ui/storybook/fixtures/onboardingDraft.test.ts b/ui/storybook/fixtures/onboardingDraft.test.ts index d086011d7f..bb8a322305 100644 --- a/ui/storybook/fixtures/onboardingDraft.test.ts +++ b/ui/storybook/fixtures/onboardingDraft.test.ts @@ -4,7 +4,7 @@ import { afterEach, describe, expect, it } from "vitest"; import { ONBOARDING_STORAGE_KEY } from "@/components/OnboardingWizard"; import { - STORYBOOK_AGENT_ID, + ONBOARDING_ARC_ENTRY_STEP, STORYBOOK_COMPANY_ID, clearOnboardingDraft, readOnboardingDraft, @@ -21,7 +21,7 @@ describe("storybook onboarding draft", () => { // story restore a saved step instead of the one it asked for. The reviewer // then sees a screen they did not click on, which reads as a wizard bug. it("leaves nothing behind once cleared", () => { - seedOnboardingDraft(5); + seedOnboardingDraft(); expect(readOnboardingDraft()).not.toBeNull(); clearOnboardingDraft(); @@ -29,25 +29,28 @@ describe("storybook onboarding draft", () => { expect(window.localStorage.getItem(ONBOARDING_STORAGE_KEY)).toBeNull(); }); - it("writes the step the story asked for", () => { - for (const step of [3, 4, 5] as const) { - seedOnboardingDraft(step); - expect(readOnboardingDraft()?.step).toBe(step); - } + // The wizard captures `entryStep` from this draft once, at mount, and offers + // Back only while `currentStep > entryStep`. Seeding a later step is therefore + // not a shortcut to it — it is a step that can never show its Back button. + it("enters the arc at its first step, so later steps can be walked into", () => { + seedOnboardingDraft(); + expect(readOnboardingDraft()?.step).toBe(ONBOARDING_ARC_ENTRY_STEP); }); // `createdAgentId` is what `launchStateIncomplete` checks. Filling it in // before the hire would paint over the guard step 5 is supposed to show when - // it is reached without an agent, so the earlier steps must leave it empty. - it("only claims an agent exists from the review step onward", () => { - seedOnboardingDraft(3); + // it is reached without an agent. + it("does not claim an agent exists before the hire", () => { + seedOnboardingDraft(); expect(readOnboardingDraft()?.createdAgentId).toBe(""); + }); - seedOnboardingDraft(4); - expect(readOnboardingDraft()?.createdAgentId).toBe(""); - - seedOnboardingDraft(5); - expect(readOnboardingDraft()?.createdAgentId).toBe(STORYBOOK_AGENT_ID); + // The label, not the type, was seeded here once. Nothing failed loudly: the + // connect step fell back to a real adapter, and the mismatch would only have + // surfaced as a hire posting a type the server does not know. + it("names the adapter by its type", () => { + seedOnboardingDraft(); + expect(readOnboardingDraft()?.adapterType).toBe("claude_local"); }); // `restoreOnboardingState` treats restoring as an authorization decision and @@ -55,7 +58,7 @@ describe("storybook onboarding draft", () => { // owns. Seeding a company the fixtures do not report would silently restore // nothing, and every story would quietly fall back to its `initialStep`. it("names the company the fixtures report as owned", () => { - seedOnboardingDraft(5); + seedOnboardingDraft(); expect(readOnboardingDraft()?.createdCompanyId).toBe(STORYBOOK_COMPANY_ID); }); diff --git a/ui/storybook/fixtures/onboardingDraft.ts b/ui/storybook/fixtures/onboardingDraft.ts index 2af977c54c..88bdf2aa33 100644 --- a/ui/storybook/fixtures/onboardingDraft.ts +++ b/ui/storybook/fixtures/onboardingDraft.ts @@ -16,21 +16,37 @@ import { ONBOARDING_STORAGE_KEY } from "@/components/OnboardingWizard"; export const STORYBOOK_COMPANY_ID = "company-storybook"; export const STORYBOOK_AGENT_ID = "agent-storybook"; -export function seedOnboardingDraft(step: 3 | 4 | 5): void { +/** Where the agent arc begins. Every story in it enters here — see below. */ +export const ONBOARDING_ARC_ENTRY_STEP = 3; + +/** + * The draft a run holds when it arrives at the agent arc. + * + * It seeds the *entry* step and nothing further on purpose. The wizard offers + * Back only on a step it walked forward into — `currentStep > entryStep`, and + * `entryStep` is captured once at mount from this very draft — so a story that + * seeds step 4 or 5 directly renders those steps permanently without their Back + * button. Stories that want a later step click their way to it instead. + * + * `createdAgentId` is therefore absent rather than seeded: the hire happens for + * real, through the fixtured route, which is also what keeps step 5's + * `launchStateIncomplete` guard honest instead of painted over. + */ +export function seedOnboardingDraft(): void { window.localStorage.setItem( ONBOARDING_STORAGE_KEY, JSON.stringify({ - step, + step: ONBOARDING_ARC_ENTRY_STEP, companyName: "Paperclip Storybook", agentName: "Darnold", agentRole: "general", - adapterType: "claude_code", + // The adapter's real type, not its label. This read `claude_code`, which + // is no adapter at all — the connect step recovered by falling back, and + // the hire would have posted a type the server does not know. + adapterType: "claude_local", createdCompanyId: STORYBOOK_COMPANY_ID, createdCompanyPrefix: "PAP", - // Only from the review step onward. Before the hire there is no agent, and - // filling this in earlier would hide the incomplete-state guard step 5 - // shows when it is reached without one. - createdAgentId: step >= 5 ? STORYBOOK_AGENT_ID : "", + createdAgentId: "", }), ); } diff --git a/ui/storybook/fixtures/onboardingEnvironment.ts b/ui/storybook/fixtures/onboardingEnvironment.ts new file mode 100644 index 0000000000..a7240d0ec9 --- /dev/null +++ b/ui/storybook/fixtures/onboardingEnvironment.ts @@ -0,0 +1,121 @@ +import { ADAPTER_AUTH_MISSING_CHECK_CODE } from "@paperclipai/shared"; +import type { AdapterAuthSignal } from "@paperclipai/shared"; + +/** + * The environment and auth state the connect step reads, as something a story + * can choose. + * + * The step's provider sign-in panel is gated on four separate things — the + * adapter declaring a login capability, a *sandbox* environment resolving, that + * environment's provider supporting a login PTY, and the auth signal coming back + * absent. Miss any one and the panel silently does not render, which looks + * exactly like it having been deleted. + * + * That is not hypothetical: the first version of these fixtures returned an + * empty environment list, and the sign-in panel was invisible in every story + * because of it. So the states are named here and selected per story rather than + * left implicit in a single hard-coded response. + */ + +export type OnboardingEnvironmentState = + /** A cloud tenant as it should be: one managed sandbox, sign-in reachable. */ + | "managed-sandbox" + /** The broken shape seen on staging — the step can offer no place to test. */ + | "none"; + +export const STORYBOOK_SANDBOX_PROVIDER = "daytona"; +export const STORYBOOK_SANDBOX_ENVIRONMENT_ID = "environment-storybook-sandbox"; + +interface FixtureState { + environments: OnboardingEnvironmentState; + authSignal: AdapterAuthSignal; +} + +/** + * Mutable on purpose. The fetch fixtures are installed once, before any story + * renders, so a story cannot swap the handler — it sets what the handler reads. + */ +export const onboardingFixtureState: FixtureState = { + environments: "managed-sandbox", + authSignal: "absent", +}; + +export function setOnboardingFixtureState(next: Partial): void { + Object.assign(onboardingFixtureState, next); +} + +export function resetOnboardingFixtureState(): void { + onboardingFixtureState.environments = "managed-sandbox"; + onboardingFixtureState.authSignal = "absent"; +} + +/** + * `managedByPaperclip` and a non-local driver are what `resolveManagedSandbox + * EnvironmentId` looks for; `config.provider` is what the capability lookup keys + * on. All three have to line up or the environment resolves and the panel still + * does not appear. + */ +export function storybookEnvironments(): unknown[] { + if (onboardingFixtureState.environments === "none") return []; + return [ + { + id: STORYBOOK_SANDBOX_ENVIRONMENT_ID, + companyId: "company-storybook", + name: "Managed sandbox", + driver: "sandbox", + status: "active", + config: { provider: STORYBOOK_SANDBOX_PROVIDER }, + metadata: { managedByPaperclip: true }, + }, + ]; +} + +export function storybookEnvironmentCapabilities(): unknown { + return { + sandboxProviders: { + [STORYBOOK_SANDBOX_PROVIDER]: { supportsLoginPty: true }, + }, + }; +} + +export function storybookAuthSignal(): { status: AdapterAuthSignal } { + return { status: onboardingFixtureState.authSignal }; +} + +/** + * The environment test, answering from the same auth state the sign-in panel + * reads. + * + * This is the hire's gate, not decoration. `blocksAgentCreate` stops the hire on + * a `fail`, and on any result — `pass` included — carrying a check whose code is + * `adapter_auth_missing`. Both shipped adapters emit that code when a sandbox + * target has no ready authentication, so a customer who has not signed in cannot + * reach the review step. + * + * An earlier version of this fixture returned `pass` with an empty check list + * whatever the auth state, which let Connect through with no model connected — + * the exact defect this step exists to prevent, reproduced in the one place + * built for catching it. A fixture that always passes cannot show a gate. + */ +export function storybookEnvironmentTest(adapterType: string): unknown { + const authenticated = onboardingFixtureState.authSignal === "present"; + return { + adapterType, + // `warn` rather than `fail`: the gate is the check code, and the wizard is + // explicit that a warn with no missing-auth check still hires. Using `fail` + // would pass this story for the wrong reason and hide a regression in that + // rule. + status: authenticated ? "pass" : "warn", + checks: authenticated + ? [] + : [ + { + code: ADAPTER_AUTH_MISSING_CHECK_CODE, + status: "warn", + title: "No working authentication", + detail: "Sign in to the provider before hiring this agent.", + }, + ], + testedAt: new Date(0).toISOString(), + }; +} diff --git a/ui/storybook/fixtures/paperclipData.ts b/ui/storybook/fixtures/paperclipData.ts index d698938df9..ea3b9054f5 100644 --- a/ui/storybook/fixtures/paperclipData.ts +++ b/ui/storybook/fixtures/paperclipData.ts @@ -193,6 +193,39 @@ export const storybookAgents: Agent[] = [ export const storybookAgentMap = new Map(storybookAgents.map((agent) => [agent.id, agent])); +/** + * The agent the onboarding hire returns. + * + * Kept out of `storybookAgents` deliberately: that list is what the company + * already has, and this one does not exist until the wizard's Connect step + * creates it. Putting it in the list would give the review step an agent it had + * not yet hired. + */ +export const storybookHiredAgent: Agent = { + id: "agent-storybook", + companyId: "company-storybook", + name: "Darnold", + urlKey: "darnold", + role: "general", + title: "Chief of Staff", + icon: "sparkles", + status: "idle", + reportsTo: null, + capabilities: "Runs the company's first workflows and hires the team behind them.", + adapterType: "claude_local", + adapterConfig: {}, + runtimeConfig: {}, + budgetMonthlyCents: 100_000, + spentMonthlyCents: 0, + pauseReason: null, + pausedAt: null, + permissions: { canCreateAgents: true }, + lastHeartbeatAt: null, + metadata: null, + createdAt: recent(0), + updatedAt: recent(0), +}; + export const storybookIssueLabels: IssueLabel[] = [ { id: "label-ui", diff --git a/ui/storybook/stories/onboarding-agent-arc.stories.tsx b/ui/storybook/stories/onboarding-agent-arc.stories.tsx index d4ec67077c..28b5ad7bec 100644 --- a/ui/storybook/stories/onboarding-agent-arc.stories.tsx +++ b/ui/storybook/stories/onboarding-agent-arc.stories.tsx @@ -1,4 +1,5 @@ import type { Meta, StoryObj } from "@storybook/react-vite"; +import { expect, screen, userEvent, waitFor } from "storybook/test"; import { useEffect, useState } from "react"; import { OnboardingWizard } from "@/components/OnboardingWizard"; @@ -7,10 +8,15 @@ import { Stepper } from "@/components/onboarding/Stepper"; import { useCompanyListQuery } from "@/api/companies-query"; import { useDialog } from "@/context/DialogContext"; import { + ONBOARDING_ARC_ENTRY_STEP, STORYBOOK_COMPANY_ID, clearOnboardingDraft, seedOnboardingDraft, } from "../fixtures/onboardingDraft"; +import { + resetOnboardingFixtureState, + setOnboardingFixtureState, +} from "../fixtures/onboardingEnvironment"; /** * The onboarding wizard's agent arc: create the agent, connect a model, review. @@ -33,8 +39,8 @@ const meta = { export default meta; /** - * Seeds the draft the wizard restores from, opens it, and takes the draft back - * out again on the way past. + * Seeds the draft the wizard restores from, opens it at the arc's first step, + * and takes the draft back out again on the way past. * * Three details the wizard's own design forces: * @@ -49,18 +55,22 @@ export default meta; * since localStorage is per-origin and would otherwise hand one account's draft * to another. * - * Step 5 is seeded rather than requested: `openOnboarding({ initialStep })` - * accepts 1–4 only, because the review step is somewhere the wizard arrives - * rather than somewhere it starts. + * Every story enters here, at step 3, and the later ones walk forward. Opening + * directly on a later step is the obvious shortcut and it is wrong: `entryStep` + * is captured once at mount from exactly this draft, `initialStep` sets both it + * and the current step together, and Back is offered only while + * `currentStep > entryStep`. A story opened on step 4 is a step 4 that can never + * show its Back button — which is not a preview of the step, it is a preview of + * a state no customer is ever in. * * And the cleanup is not housekeeping. That same per-origin storage is shared * with every other story in the session: a draft left behind makes the next * story restore a saved step ahead of the one it asked for, so the reviewer * lands on a screen they did not click on and reads it as a wizard bug. */ -function WizardAtStep({ step }: { step: 3 | 4 | 5 }) { +function WizardArc() { const [seeded] = useState(() => { - seedOnboardingDraft(step); + seedOnboardingDraft(); return true; }); @@ -80,33 +90,164 @@ function WizardAtStep({ step }: { step: 3 | 4 | 5 }) { const { openOnboarding } = useDialog(); useEffect(() => { if (!seeded || !ready) return; - // `initialStep` is deliberately omitted for the review step. An explicit - // option overrides the restored draft — "options take precedence over saved - // state" is the wizard's rule, not an accident — so passing one here would - // clamp 5 to 4 and land on Connect. Steps 3 and 4 pass it because being - // explicit is better when the option can express the step; step 5 cannot be - // expressed that way, so the draft carries it alone. - openOnboarding( - step <= 4 - ? { initialStep: step as 3 | 4, companyId: STORYBOOK_COMPANY_ID } - : { companyId: STORYBOOK_COMPANY_ID }, - ); - }, [seeded, ready, openOnboarding, step]); + openOnboarding({ + initialStep: ONBOARDING_ARC_ENTRY_STEP, + companyId: STORYBOOK_COMPANY_ID, + }); + }, [seeded, ready, openOnboarding]); if (!ready) return null; return ; } +/** + * Every wait here is given an explicit timeout because the library's default is + * one second, and every wait in this file outlasts it: the wizard does not mount + * until the companies query settles, the hire runs four requests end to end. A + * default-timeout wait gives up, the play function fails, and the story renders + * the step it started on — which looks exactly like a story that was written to + * open there. That is the failure this whole file exists to avoid, so it is + * worth naming rather than inlining. + */ +const STEP_TIMEOUT_MS = 15_000; + +/** + * Presses the wizard's primary button once it is enabled, and waits for the + * step it opens. + * + * The dialog is portalled to `document.body`, so the queries are scoped to the + * body rather than to `canvasElement` — a canvas-scoped query finds an empty + * mount point and times out. + * + * Waiting for `toBeEnabled` is not defensive padding. Connect stays disabled + * through `adapterEnvLoading` and `missionUnresolvedForHire`, both of which + * resolve from queries, so clicking on first paint clicks a dead button and the + * story silently stops one step short of where it says it is. + * + * The button is queried again immediately before the click rather than reused + * from the wait above. The wizard re-renders as those queries land, and a node + * captured a moment earlier can be detached by the time it is clicked — a click + * that raises no error and does nothing. + */ +async function advance(from: string, to: string) { + await waitFor( + () => expect(screen.getByRole("button", { name: from })).toBeEnabled(), + { timeout: STEP_TIMEOUT_MS }, + ); + await userEvent.click(screen.getByRole("button", { name: from })); + await screen.findByRole("button", { name: to }, { timeout: STEP_TIMEOUT_MS }); +} + +/** + * Naming the organization — the step before the arc, and the one a self-hosted + * run starts on. It carries no draft and no company: this is where a company is + * created, so seeding either would be describing a run that had already been + * here. + * + * Worth a story because it is dressed as the arc steps that follow it, and that + * only holds if the three are looked at together. Its Back leaves the wizard's + * steps for the front door rather than walking back through them, so it is the + * one Back on the flow that `canGoBackFromOnboardingStep` does not decide. + */ +function NamingStep() { + useEffect(() => clearOnboardingDraft, []); + const companies = useCompanyListQuery(); + const ready = companies.isSuccess && companies.data !== undefined; + const { openOnboarding } = useDialog(); + useEffect(() => { + if (!ready) return; + // No `companyId`: this step is where one is created, and naming the run's + // company here would be handing it the thing it exists to ask for. + openOnboarding({ initialStep: 1 }); + }, [ready, openOnboarding]); + if (!ready) return null; + return ; +} + +export const NameYourOrganization: StoryObj = { + render: () => , +}; + +/** + * The arc's first step, and the one place Back is correctly absent: a run + * entering here has nowhere behind it that belongs to it — step 1 creates a + * company, and this run already holds one. + */ export const CreateYourAgent: StoryObj = { - render: () => , + render: () => , }; +/** + * The connect step as a signed-out cloud tenant meets it: a managed sandbox + * resolves, and the provider sign-in panel is offered because the auth signal + * comes back absent. + */ export const ConnectAModel: StoryObj = { - render: () => , + beforeEach: () => { + setOnboardingFixtureState({ + environments: "managed-sandbox", + authSignal: "absent", + }); + return resetOnboardingFixtureState; + }, + render: () => , + play: () => advance("Next", "Connect"), }; +/** + * The same step once the provider is already authenticated. The sign-in panel + * is gone — this is the only difference, and it is worth a story because the + * panel's absence is otherwise indistinguishable from it being broken. + */ +export const ConnectAModelAlreadySignedIn: StoryObj = { + beforeEach: () => { + setOnboardingFixtureState({ + environments: "managed-sandbox", + authSignal: "present", + }); + return resetOnboardingFixtureState; + }, + render: () => , + play: () => advance("Next", "Connect"), +}; + +/** + * No managed sandbox to test against. + * + * This is the state a walker actually hit on staging, and the step is honest + * about it rather than passing and stranding them later. Worth being able to + * look at without breaking a stack to get there. + */ +export const ConnectAModelNoSandbox: StoryObj = { + beforeEach: () => { + setOnboardingFixtureState({ environments: "none", authSignal: "unknown" }); + return resetOnboardingFixtureState; + }, + render: () => , + play: () => advance("Next", "Connect"), +}; + +/** + * The review step, reached by hiring rather than by claiming a hire happened. + * + * Walking the whole arc is what makes this an honest preview of the step: the + * Back button is offered because the run genuinely walked forward into it, and + * `launchStateIncomplete` is satisfied because an agent genuinely exists. A + * seeded `createdAgentId` would paint over that guard rather than clear it. + */ export const Review: StoryObj = { - render: () => , + beforeEach: () => { + setOnboardingFixtureState({ + environments: "managed-sandbox", + authSignal: "present", + }); + return resetOnboardingFixtureState; + }, + render: () => , + play: async () => { + await advance("Next", "Connect"); + await advance("Connect", "Get started"); + }, }; export const ProgressStrip: StoryObj = { @@ -156,7 +297,10 @@ export const PillMorph: StoryObj = { }, []); return (
- +