From 4436cf00a2e2b665a4d0e93684c6a72dff085b44 Mon Sep 17 00:00:00 2001 From: Tonio Date: Thu, 27 Aug 2026 16:53:12 -0700 Subject: [PATCH] Render the onboarding agent arc's real steps in Storybook (#12369) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - New tenants meet it through an onboarding wizard: create your agent, connect a model, review > - Those three screens only render for a signed-in account that owns a provisioned stack, so the only way to look at one was to walk a real signup > - Round 4 redesigned all three and shipped them without anyone seeing them render; when the connect step then failed on a live stack, the review step behind it could not be reached at all > - Storybook already mounts the app's provider stack and stubs `/api`, and already has an `Onboarding/Agent arc` story — but that story previews `AgentCapsule`, which the wizard stopped using in round 4 > - This pull request mounts the real wizard in Storybook at each step, and replaces the stale capsule stories with the component the arc actually renders > - The benefit is that these screens can be reviewed, and regressions seen, without provisioning anything ## Linked Issues or Issue Description No existing issue. Describing it inline, following `.github/ISSUE_TEMPLATE/enhancement.yml`. **What existing behavior does this improve?** Reviewing the tenant onboarding wizard. Today it can only be seen by signing up for a real account and provisioning a real stack. **Subsystem affected** `ui` — the onboarding wizard and its Storybook coverage. **Current behavior** `OnboardingWizard` renders only for a signed-in account that owns a company. There is no route, harness, or story that mounts it, so no screen in the agent arc can be looked at in isolation. The existing `Onboarding/Agent arc` story previews `AgentCapsule` in `slot`/`configured`/`online` and describes it as what "the wizard holds in one tree slot" — round 4 replaced that with `PillGuy` and `dormant`/`alive`, so the story documents a component the arc no longer renders. **Proposed behavior** Stories that mount the real `OnboardingWizard` at each of the three steps against the existing Storybook API fixtures, plus stories for `PillGuy` and its transition. **Reason and benefit** Round 4 shipped three redesigned screens that nobody could see. The connect step then failed on staging, which made the review step unreachable even with an account — reviewing it required hand-editing `localStorage`. Stories remove that whole class of problem. **Breaking changes** None. Storybook-only; no product code is touched. ## What Changed - `CreateYourAgent`, `ConnectAModel`, `Review` — the real wizard, per step. - `PillStates` and `PillMorph` — the two states, and the transition on a loop with a toggle. The morph is the arc's payoff and the hardest thing to judge from a still. - Replaces the `AgentCapsule` stories in this file with `PillGuy`. `AgentCapsule` is still used by `DesignGuide` and keeps its coverage there. - Four routes added to the Storybook fetch fixtures: `/api/instance/settings`, `…/environments`, `…/adapters/:type/models`. The empty environment list is the cloud-tenant shape, and also the state that produces the "no managed sandbox environment is available" notice — worth being able to look at rather than only meeting it on a live stack. ## Three properties of the wizard the stories had to respect Each of these cost a debugging cycle, so they are documented at the call site: 1. **The draft is seeded during render, not in an effect.** Roughly twenty `useState(saved?.x ?? default)` initializers read the restored blob exactly once, so a draft written after mount arrives too late. 2. **Nothing mounts until the companies list settles.** The wizard's own mount gate waits on `isFetching`, but that query is *disabled* until the account settles, and a disabled query is not fetching. Mounting straight away gets an inner wizard that reads a null draft, falls back to `initialStep`, and then persists that back over the seed. A real session never hits this because the dashboard has already loaded the list. 3. **The review step opens with no `initialStep`.** An explicit option takes precedence over saved state by design, so passing one clamps 5 to 4 and lands on Connect. ## Verification - `npx vitest run src/components/OnboardingWizard.test.tsx src/components/OnboardingWizard.step.test.tsx` — 44 tests, all passing. - `npx tsc -p tsconfig.json --noEmit` — no new errors. (`src/lib/sentry.*` reports pre-existing missing-type errors for `@sentry/browser` on master.) - Each of the three step stories loaded in a running Storybook and read back: - `create-your-agent` → "STEP 1 OF 3 / Create your first agent / Name / Next" - `connect-a-model` → "STEP 2 OF 3 / Connect a model / Paperclip works with your existing subscription or API keys. / Claude Code / Codex / Advanced settings / Connect" - `review` → "STEP 3 OF 3 / Let's get started... / Darnold is ready to work! / Get started" One difference from a real walk, worth knowing before treating a story as ground truth: the review story has no Back button, because `entryStep` is 5 there and back-navigation is bounded by where the run entered. ## Risks Low. Storybook-only — no product code, routes, or bundles change. The four added fetch fixtures are inside the Storybook mock and cannot affect the app. The one thing to watch: these stories mount the real wizard, so a future change to how it restores drafts or decides its initial step can break them. That is arguably the point — the stories would be the first place it shows — but it does mean they are coupled to internals rather than to a prop surface, and the three notes above are what a maintainer needs. ## Model Used Claude Fable 5 (`claude-fable-5`), extended thinking, with tool use: file editing, shell, and browser automation for loading and reading back each story. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [ ] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Claude Fable 5 --- ui/storybook/.storybook/preview.tsx | 40 +++++ ui/storybook/fixtures/onboardingDraft.test.ts | 66 ++++++++ ui/storybook/fixtures/onboardingDraft.ts | 50 ++++++ .../stories/onboarding-agent-arc.stories.tsx | 156 +++++++++++++++--- 4 files changed, 287 insertions(+), 25 deletions(-) create mode 100644 ui/storybook/fixtures/onboardingDraft.test.ts create mode 100644 ui/storybook/fixtures/onboardingDraft.ts diff --git a/ui/storybook/.storybook/preview.tsx b/ui/storybook/.storybook/preview.tsx index 5643e371d8..b4a1b93276 100644 --- a/ui/storybook/.storybook/preview.tsx +++ b/ui/storybook/.storybook/preview.tsx @@ -3,6 +3,7 @@ import type { Preview } from "@storybook/react-vite"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import type { WorkTimelineResult } from "@paperclipai/shared"; import { MemoryRouter } from "@/lib/router"; +import { ONBOARDING_STORAGE_KEY } from "@/components/OnboardingWizard"; import { BreadcrumbProvider } from "@/context/BreadcrumbContext"; import { CompanyProvider } from "@/context/CompanyContext"; import { DialogProvider } from "@/context/DialogContext"; @@ -130,6 +131,23 @@ function installStorybookApiFixtures() { }); } + if (url.pathname === "/api/instance/settings") { + 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. + if (/^\/api\/companies\/[^/]+\/environments$/.test(url.pathname)) { + return Response.json([]); + } + + if (/^\/api\/companies\/[^/]+\/adapters\/[^/]+\/models$/.test(url.pathname)) { + return Response.json([]); + } + if (url.pathname === "/api/adapters") { return Response.json([ { @@ -449,6 +467,28 @@ const preview: Preview = { }, }, }, + + /** + * Every story starts without an onboarding draft. + * + * `localStorage` is per-origin and the preview frame keeps one for the whole + * session, so a story that seeds a draft would otherwise hand it to whatever a + * reviewer opens next: the wizard restores that saved step ahead of the step + * the new story asked for, and the reviewer lands on a screen they never + * clicked on. + * + * Cleared here rather than on the seeding story's unmount, deliberately. + * Switching stories navigates the preview iframe, so the page is torn down + * rather than React-unmounted and an unmount cleanup never runs — which is + * exactly how the first attempt at this leaked anyway. + */ + beforeEach: () => { + try { + window.localStorage.removeItem(ONBOARDING_STORAGE_KEY); + } catch { + // Storage access throws in some privacy modes; nothing to clean up there. + } + }, }; export default preview; diff --git a/ui/storybook/fixtures/onboardingDraft.test.ts b/ui/storybook/fixtures/onboardingDraft.test.ts new file mode 100644 index 0000000000..d086011d7f --- /dev/null +++ b/ui/storybook/fixtures/onboardingDraft.test.ts @@ -0,0 +1,66 @@ +// @vitest-environment jsdom + +import { afterEach, describe, expect, it } from "vitest"; + +import { ONBOARDING_STORAGE_KEY } from "@/components/OnboardingWizard"; +import { + STORYBOOK_AGENT_ID, + STORYBOOK_COMPANY_ID, + clearOnboardingDraft, + readOnboardingDraft, + seedOnboardingDraft, +} from "./onboardingDraft"; + +afterEach(() => { + window.localStorage.clear(); +}); + +describe("storybook onboarding draft", () => { + // The leak this exists to stop: `localStorage` is per-origin and shared by + // every story in a session, so a seeded draft left behind makes the *next* + // 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); + expect(readOnboardingDraft()).not.toBeNull(); + + clearOnboardingDraft(); + expect(readOnboardingDraft()).toBeNull(); + 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); + } + }); + + // `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); + expect(readOnboardingDraft()?.createdAgentId).toBe(""); + + seedOnboardingDraft(4); + expect(readOnboardingDraft()?.createdAgentId).toBe(""); + + seedOnboardingDraft(5); + expect(readOnboardingDraft()?.createdAgentId).toBe(STORYBOOK_AGENT_ID); + }); + + // `restoreOnboardingState` treats restoring as an authorization decision and + // throws the whole blob away when the saved company is not one the account + // 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); + expect(readOnboardingDraft()?.createdCompanyId).toBe(STORYBOOK_COMPANY_ID); + }); + + it("survives a malformed value without throwing", () => { + window.localStorage.setItem(ONBOARDING_STORAGE_KEY, "{not json"); + expect(readOnboardingDraft()).toBeNull(); + }); +}); diff --git a/ui/storybook/fixtures/onboardingDraft.ts b/ui/storybook/fixtures/onboardingDraft.ts new file mode 100644 index 0000000000..2af977c54c --- /dev/null +++ b/ui/storybook/fixtures/onboardingDraft.ts @@ -0,0 +1,50 @@ +import { ONBOARDING_STORAGE_KEY } from "@/components/OnboardingWizard"; + +/** + * The onboarding draft, as the wizard stories need to write it. + * + * `localStorage` is per-origin, so every story in a Storybook session shares + * one. A story that seeds a draft and walks away leaves it for the next one: + * the wizard restores a saved step ahead of whatever step that story asked for, + * and the reviewer gets a screen they did not click on. So seeding and clearing + * are a pair, and they live here rather than inline so the pairing is testable. + * + * The key is imported rather than restated. It is the wizard's, and a second + * copy of a storage key is a bug waiting for the first one to be renamed. + */ + +export const STORYBOOK_COMPANY_ID = "company-storybook"; +export const STORYBOOK_AGENT_ID = "agent-storybook"; + +export function seedOnboardingDraft(step: 3 | 4 | 5): void { + window.localStorage.setItem( + ONBOARDING_STORAGE_KEY, + JSON.stringify({ + step, + companyName: "Paperclip Storybook", + agentName: "Darnold", + agentRole: "general", + adapterType: "claude_code", + 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 : "", + }), + ); +} + +export function clearOnboardingDraft(): void { + window.localStorage.removeItem(ONBOARDING_STORAGE_KEY); +} + +export function readOnboardingDraft(): Record | null { + const raw = window.localStorage.getItem(ONBOARDING_STORAGE_KEY); + if (raw === null) return null; + try { + return JSON.parse(raw) as Record; + } catch { + return null; + } +} diff --git a/ui/storybook/stories/onboarding-agent-arc.stories.tsx b/ui/storybook/stories/onboarding-agent-arc.stories.tsx index 710827d1a8..d4ec67077c 100644 --- a/ui/storybook/stories/onboarding-agent-arc.stories.tsx +++ b/ui/storybook/stories/onboarding-agent-arc.stories.tsx @@ -1,12 +1,29 @@ import type { Meta, StoryObj } from "@storybook/react-vite"; -import { AgentCapsule } from "@/components/AgentCapsule"; +import { useEffect, useState } from "react"; + +import { OnboardingWizard } from "@/components/OnboardingWizard"; +import { PillGuy } from "@/components/onboarding/PillGuy"; import { Stepper } from "@/components/onboarding/Stepper"; +import { useCompanyListQuery } from "@/api/companies-query"; +import { useDialog } from "@/context/DialogContext"; +import { + STORYBOOK_COMPANY_ID, + clearOnboardingDraft, + seedOnboardingDraft, +} from "../fixtures/onboardingDraft"; /** - * The onboarding wizard's agent arc: create the agent, connect it, review. - * These are the three steps a customer walks inside the tenant — company - * creation happens in Cloud before they arrive, which is why the strip counts - * to three rather than to the wizard's own step numbers. + * The onboarding wizard's agent arc: create the agent, connect a model, review. + * These are the three steps a customer walks inside the tenant — the + * organization is named in Cloud before they arrive, which is why the strip + * counts to three rather than to the wizard's own step numbers. + * + * The step stories below mount the real wizard against the Storybook API + * fixtures. That matters more here than in most stories: these screens only + * render for a signed-in account that owns a provisioned stack, so before this + * existed the only way to see them was to walk a real signup — and when the + * connect step failed on a live stack, the review step behind it could not be + * reached at all. */ const meta = { title: "Onboarding/Agent arc", @@ -15,6 +32,83 @@ 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. + * + * Three details the wizard's own design forces: + * + * The draft is written during render, not in an effect. Roughly twenty + * `useState(saved?.x ?? default)` initializers read the restored blob exactly + * once, on first render, so a draft written after mount arrives too late to + * matter. + * + * `createdCompanyId` has to be a company the fixtures report as owned. + * `restoreOnboardingState` treats restoring as an authorization decision and + * discards the whole blob when the saved company is not in the list — correctly, + * 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. + * + * 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 }) { + const [seeded] = useState(() => { + seedOnboardingDraft(step); + return true; + }); + + useEffect(() => clearOnboardingDraft, []); + + // Nothing is mounted until the companies list has settled, and that ordering + // is load-bearing rather than tidiness. The wizard's own mount gate waits on + // `isFetching`, but this query is *disabled* until the account settles, and a + // disabled query is not fetching — so mounting immediately gets an inner + // wizard whose ~20 one-shot initializers read a null draft, take `initialStep` + // instead, and then persist that back over the seed. A real session does not + // hit this because the dashboard has already loaded the list by the time + // anyone opens onboarding. + const companies = useCompanyListQuery(); + const ready = companies.isSuccess && companies.data !== undefined; + + 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]); + + if (!ready) return null; + return ; +} + +export const CreateYourAgent: StoryObj = { + render: () => , +}; + +export const ConnectAModel: StoryObj = { + render: () => , +}; + +export const Review: StoryObj = { + render: () => , +}; + export const ProgressStrip: StoryObj = { render: () => (
@@ -26,15 +120,15 @@ export const ProgressStrip: StoryObj = { }; /** - * The capsule's three states, which the wizard holds in one tree slot so the - * morph reads as a single object coming to life rather than three renders. + * The agent's two states, side by side. `dormant` waits to be configured; + * `alive` is the hired agent on the review step. */ -export const CapsuleStates: StoryObj = { +export const PillStates: StoryObj = { render: () => (
- {(["slot", "configured", "online"] as const).map((state) => ( + {(["dormant", "alive"] as const).map((state) => (
- + {state} @@ -45,20 +139,32 @@ export const CapsuleStates: StoryObj = { }; /** - * The same three states rendered with the default cross-fade, for comparison - * with the traced outline above. + * The transition on its own, on a loop. + * + * Worth a story of its own because it is the arc's payoff and the hardest part + * to judge from a still. The two states share a silhouette but differ in fill, + * eye shape, and a tuft the dormant state does not have at all, so they + * cross-fade rather than path-morph — there is no honest interpolation between + * them, and a faked one warps the eyes through shapes the design never draws. */ -export const CapsuleStatesCrossfade: StoryObj = { - render: () => ( -
- {(["slot", "configured", "online"] as const).map((state) => ( -
- - - {state} - -
- ))} -
- ), +export const PillMorph: StoryObj = { + render: function PillMorphStory() { + const [alive, setAlive] = useState(false); + useEffect(() => { + const id = setInterval(() => setAlive((v) => !v), 1800); + return () => clearInterval(id); + }, []); + return ( +
+ + +
+ ); + }, };