feat(onboarding): Figma pass over the tenant arc (#12726)

Takes the four tenant onboarding steps to the design, and makes the model
choice explicit.

**Nothing is preselected on the connect step.** It arrived with a source
already chosen, which made the row read as a confirmation rather than a
question and let a customer pass the step having touched none of it. The
step now opens unanswered and cannot advance until a source is selected in
the visible row.

Two defects fell out of that, both found by Greptile in review:

- A saved draft can name an adapter the row does not show — one the registry
  dropped, or one in the advanced list. The gate asked `sourcePicked`, which
  only means "a draft named something", so Next stayed live on a step that
  had visibly asked nothing and would hire against the hidden name. It now
  asks `sourceSelected`, read off the visible row, and the snap that replaces
  an unofferable adapter clears the pick rather than presenting its
  replacement as chosen.
- Cmd+Enter bypassed that gate entirely, because the condition was written
  out twice and drifted. Both paths now ask one predicate.

Also: selection is a fill rather than a border; the input canvas opens only
for a chosen source; the close control is gone from every step (nothing
downstream of the connect step works until a model is connected); the arc
draws to the design's 424px column with a filled 44px name field; the CTA
reads "Next" through the arc and "Get started" at the end; the login spinner
reads "Preparing..."; the OAuth panels number their fields and show the URL
above the code.

Storybook: the arc stories waited for a button named "Connect" and had been
silently rendering the wrong step since the CTA was renamed. They now wait on
the destination heading. Three e2e specs had the same fault and now select a
source before advancing.

One test was removed rather than replaced — `hydrates again when the same
company comes back through onboarding` drove the close control, and three
substitutes each passed against a wizard with the behaviour deleted. The
reason is recorded where it stood.

Known and not fixed here: SidebarCompanyMenu opens the wizard at step 1 for
"create a new organization", and with no exit an existing user who changes
their mind is trapped.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Tonio 2026-09-02 12:19:15 -07:00 committed by GitHub
parent 8c3b8c432a
commit 141c5b1340
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
16 changed files with 532 additions and 251 deletions

View File

@ -82,10 +82,22 @@ async function runOnboardingWizard(page: Page, companyName: string) {
// name and hires under the neutral `general` role.
await page.waitForSelector("#onboarding-agent-name", { timeout: 30_000 });
await page.locator("#onboarding-agent-name").fill("Ada");
await page.getByRole("button", { name: /^Next/ }).click();
await page.getByRole("button", { name: /^Next$/ }).click();
// Step 4: adapter (claude_local default); heartbeat is intercepted.
await page.getByRole("button", { name: /^Connect$/ }).click();
// Step 4: pick a model source, then advance. Nothing is selected on arrival
// — the row is a question, not a confirmation — so the CTA is disabled until
// a tile is pressed. By role rather than by label: which adapters the tiles
// offer depends on the registry this environment reports.
const source = page.getByRole("radio").first();
await source.waitFor({ timeout: 30_000 });
await source.click();
// The forward button reads "Next" here too, so wait for it to enable rather
// than for it to appear — it is already on screen, disabled, and clicking a
// disabled button raises nothing and does nothing.
const connectNext = page.getByRole("button", { name: /^Next$/ });
await expect(connectNext).toBeEnabled({ timeout: 30_000 });
await connectNext.click();
// Step 5: review → Get started creates the first task and opens its
// detail page.

View File

@ -219,9 +219,17 @@ test.describe("Onboarding wizard", () => {
await page.locator("#onboarding-agent-name").fill("Ada");
await page.getByRole("button", { name: "Next" }).click();
// 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.
// Step 4 (Connect a model): pick a source. The step arrives with nothing
// selected — the tile row is a question, not a confirmation — and the login
// panel is what the answer opens, so there is nothing to assert until one
// is pressed. By role rather than by label, because which adapters the row
// offers depends on the registry this environment reports.
const source = page.getByRole("radio").first();
await source.waitFor({ timeout: 30_000 });
await source.click();
// The signal above reports no ready credential, so the login panel must now
// show, with no button to reuse a saved login.
//
// 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
@ -231,11 +239,13 @@ test.describe("Onboarding wizard", () => {
});
await expect(page.getByRole("button", { name: "Use saved login" })).toHaveCount(0);
// 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 CTA reads "Next" on this step as on the one before it. Waited on for
// *enabled* rather than for visible: it is already on screen and disabled
// until the environment probe settles, and clicking a disabled button
// raises nothing and does nothing.
const connectNext = page.getByRole("button", { name: "Next", exact: true });
await expect(connectNext).toBeEnabled({ timeout: 30_000 });
await connectNext.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({

View File

@ -66,8 +66,19 @@ test("captures planning mode UI for desktop and mobile", async ({ page }) => {
await page.waitForSelector("#onboarding-agent-name", { timeout: 30_000 });
await page.locator("#onboarding-agent-name").fill(AGENT_NAME);
await page.getByRole("button", { name: /^Next/ }).click();
await page.getByRole("button", { name: /^Connect$/ }).click();
await page.getByRole("button", { name: /^Next$/ }).click();
// The connect step arrives with no source selected — the tile row is a
// question, not a confirmation — so its CTA, which reads "Next" here too,
// stays disabled until one is pressed. Waited on for enabled rather than
// visible: it is already on screen, and clicking a disabled button raises
// nothing and does nothing.
const source = page.getByRole("radio").first();
await source.waitFor({ timeout: 30_000 });
await source.click();
const connectNext = page.getByRole("button", { name: /^Next$/ });
await expect(connectNext).toBeEnabled({ timeout: 30_000 });
await connectNext.click();
// The review step names the agent rather than the step.
await expect(

View File

@ -1424,7 +1424,7 @@ describe("AgentConfigForm environment selector", () => {
await runTest(result.container);
await startLogin(result.container);
expect(result.container.textContent).toContain("Preparing the login");
expect(result.container.textContent).toContain("Preparing...");
expect(result.container.textContent).not.toContain("https://");
});

View File

@ -2119,7 +2119,7 @@ function DisplayedCodeLoginPanel({
{isActive && !prompt && (
<div className="flex items-center gap-2 text-(length:--text-micro) text-muted-foreground">
<Loader2 className="size-3 animate-spin shrink-0" />
<span>Preparing the login</span>
<span>Preparing...</span>
</div>
)}
@ -2128,19 +2128,14 @@ function DisplayedCodeLoginPanel({
<div className="text-(length:--text-micro) text-muted-foreground">
Open the authentication page and enter the code.
</div>
{/* URL first, then the code. The instruction above says to open the
page and *then* enter the code, and the numbering now says the
same so the order the two rows appear in has to agree with both,
rather than handing over the code before the page it belongs to. */}
<div className="flex items-center justify-between gap-2">
<div className="min-w-0">
<div className="text-(length:--text-micro) uppercase tracking-wide text-muted-foreground">
Code
</div>
<span className="font-mono text-xs text-foreground break-all">{prompt.code}</span>
</div>
<AdapterLoginCopyButton value={prompt.code} label="Copy code" />
</div>
<div className="flex items-center justify-between gap-2">
<div className="min-w-0">
<div className="text-(length:--text-micro) uppercase tracking-wide text-muted-foreground">
Authentication URL
1. Authentication URL
</div>
<span className="font-mono text-xs text-foreground break-all">{prompt.url}</span>
</div>
@ -2161,6 +2156,15 @@ function DisplayedCodeLoginPanel({
</Button>
</div>
</div>
<div className="flex items-center justify-between gap-2">
<div className="min-w-0">
<div className="text-(length:--text-micro) uppercase tracking-wide text-muted-foreground">
2. Code
</div>
<span className="font-mono text-xs text-foreground break-all">{prompt.code}</span>
</div>
<AdapterLoginCopyButton value={prompt.code} label="Copy code" />
</div>
</div>
)}
@ -2607,7 +2611,7 @@ function SubmittedBrowserCodeLoginPanel({
{isActive && !authorizationUrl && !isCompleting && (
<div className="flex items-center gap-2 text-(length:--text-micro) text-muted-foreground">
<Loader2 className="size-3 animate-spin shrink-0" />
<span>Preparing the login</span>
<span>Preparing...</span>
</div>
)}
@ -2630,7 +2634,7 @@ function SubmittedBrowserCodeLoginPanel({
<div className="flex items-center justify-between gap-2">
<div className="min-w-0">
<div className="text-(length:--text-micro) uppercase tracking-wide text-muted-foreground">
Authorization URL
1. Authorization URL
</div>
<span className="font-mono text-xs text-foreground break-all">{authorizationUrl}</span>
</div>
@ -2653,7 +2657,7 @@ function SubmittedBrowserCodeLoginPanel({
</div>
<div className="space-y-1">
<div className="text-(length:--text-micro) uppercase tracking-wide text-muted-foreground">
Browser code
2. Browser code
</div>
<div className="flex items-center gap-2">
<input

View File

@ -890,6 +890,37 @@ describe("OnboardingWizard — which step it lands on", () => {
await settle();
}
async function press(button: Element) {
await act(async () => {
button.dispatchEvent(new MouseEvent("click", { bubbles: true }));
});
await settle();
}
/**
* The step's own CTA. By exact text, because "Back" sits beside it and both
* steps of the arc label their forward button the same way.
*/
function stepCta(): HTMLButtonElement {
const cta = [...document.body.querySelectorAll("button")].find(
(b) => b.textContent?.trim() === "Next",
);
expect(cta, "the step should render its Next button").toBeTruthy();
return cta as HTMLButtonElement;
}
/**
* Pick a model source. The connect step arrives with nothing chosen, so its
* CTA stays disabled until a tile is pressed found by `aria-checked`
* rather than by label, because this suite mocks the display registry and
* the tiles carry whatever it happens to return.
*/
async function pickModelSource() {
const tiles = [...document.body.querySelectorAll("button[aria-checked]")];
expect(tiles.length, "the connect step should offer a source").toBeGreaterThan(0);
await press(tiles[0]!);
}
it("seeds the lead agent's instructions with the mission it was never asked for", async () => {
// The regression this exists for. The agent step feeds
// `composeCeoInstructions` from the mission field, and a company entered
@ -898,22 +929,11 @@ describe("OnboardingWizard — which step it lands on", () => {
await openOnAgentStep();
await nameAgent();
const next = [...document.body.querySelectorAll("button")].find((b) =>
b.textContent?.includes("Next"),
)!;
await act(async () => {
next.dispatchEvent(new MouseEvent("click", { bubbles: true }));
});
await settle();
await press(stepCta());
const connect = [...document.body.querySelectorAll("button")].find((b) =>
b.textContent?.includes("Connect"),
)!;
expect(connect.hasAttribute("disabled")).toBe(false);
await act(async () => {
connect.dispatchEvent(new MouseEvent("click", { bubbles: true }));
});
await settle();
await pickModelSource();
expect(stepCta().hasAttribute("disabled")).toBe(false);
await press(stepCta());
expect(mockAgentsApi.saveInstructionsFile).toHaveBeenCalled();
const [, file] = mockAgentsApi.saveInstructionsFile.mock.calls[0];
@ -930,18 +950,9 @@ describe("OnboardingWizard — which step it lands on", () => {
await openOnAgentStep();
await nameAgent();
const next = [...document.body.querySelectorAll("button")].find((b) =>
b.textContent?.includes("Next"),
)!;
await act(async () => {
next.dispatchEvent(new MouseEvent("click", { bubbles: true }));
});
await settle();
expect(
[...document.body.querySelectorAll("button")]
.find((b) => b.textContent?.includes("Connect"))!
.hasAttribute("disabled"),
).toBe(false);
await press(stepCta());
await pickModelSource();
expect(stepCta().hasAttribute("disabled")).toBe(false);
mockGoalsApi.list.mockReturnValue(new Promise(() => {}));
await act(async () => {
@ -951,62 +962,28 @@ describe("OnboardingWizard — which step it lands on", () => {
});
await settle(2);
const connect = [...document.body.querySelectorAll("button")].find((b) =>
b.textContent?.includes("Connect"),
)!;
expect(connect.hasAttribute("disabled")).toBe(true);
expect(stepCta().hasAttribute("disabled")).toBe(true);
expect(mockAgentsApi.hire).not.toHaveBeenCalled();
});
it("hydrates again when the same company comes back through onboarding", async () => {
// The hydration marker is a ref, so it outlives the state it describes.
// `reset()` clears the mission field; leaving the marker set would make
// the second run believe a mission it no longer holds was already
// fetched — and hire the agent without it, exactly as before this fix.
await openOnAgentStep();
const close = [...document.body.querySelectorAll("button")].find((b) =>
b.querySelector(".sr-only")?.textContent?.includes("Close"),
);
expect(close).toBeDefined();
await act(async () => {
close!.dispatchEvent(new MouseEvent("click", { bubbles: true }));
});
await settle();
// `reset()` ran: the wizard is back at the front door with a cleared
// mission field, which is precisely the state the marker must not
// outlive.
expect(currentStep()).not.toBe("agent");
routerState.pathname = "/";
await rerender();
await settle();
routerState.pathname = "/PC1/onboarding";
dialogState.onboardingRouteDismissed = false;
await rerender();
await settle();
expect(currentStep()).toBe("agent");
await nameAgent();
const next = [...document.body.querySelectorAll("button")].find((b) =>
b.textContent?.includes("Next"),
)!;
await act(async () => {
next.dispatchEvent(new MouseEvent("click", { bubbles: true }));
});
await settle();
const connect = [...document.body.querySelectorAll("button")].find((b) =>
b.textContent?.includes("Connect"),
)!;
await act(async () => {
connect.dispatchEvent(new MouseEvent("click", { bubbles: true }));
});
await settle();
expect(mockAgentsApi.saveInstructionsFile).toHaveBeenCalled();
const [, file] = mockAgentsApi.saveInstructionsFile.mock.calls.at(-1)!;
expect(file.content).toContain("Scale the marketplace");
});
// Removed: "hydrates again when the same company comes back through
// onboarding".
//
// It closed the wizard with the X and re-opened it, which made `reset()`
// clear `hydratedMissionForRef` and the second pass hydrate again. The arc
// has no X any more — the connect step deliberately has no exit, because
// nothing downstream of it works until a model is connected — so `reset()`
// is now reachable only from a completed launch.
//
// Three substitutes were tried and all three were green against a wizard
// with the behaviour deleted, which is worse than no test: routing to "/"
// never withdraws the company; a swap to another company re-points the
// marker by itself, since it stores which company was hydrated rather than
// a bare flag; and either route dance remounts the inner wizard, so the ref
// does not survive to be tested. What the marker guards is still covered
// from the front by "seeds the lead agent's instructions with the mission it
// was never asked for". Restore a real version of this when the arc gains a
// way out.
it("hires under the neutral role, with the name the customer typed", async () => {
// The arc stopped asking for a role, so every onboarding hire is filed
@ -1016,20 +993,9 @@ describe("OnboardingWizard — which step it lands on", () => {
await openOnAgentStep();
await nameAgent("Ada");
const next = [...document.body.querySelectorAll("button")].find((b) =>
b.textContent?.includes("Next"),
)!;
await act(async () => {
next.dispatchEvent(new MouseEvent("click", { bubbles: true }));
});
await settle();
const connect = [...document.body.querySelectorAll("button")].find((b) =>
b.textContent?.includes("Connect"),
)!;
await act(async () => {
connect.dispatchEvent(new MouseEvent("click", { bubbles: true }));
});
await settle();
await press(stepCta());
await pickModelSource();
await press(stepCta());
expect(mockAgentsApi.hire).toHaveBeenCalled();
const [, payload] = mockAgentsApi.hire.mock.calls.at(-1)!;

View File

@ -227,6 +227,24 @@ function render() {
return { container, root, queryClient };
}
/**
* Press the first model-source tile.
*
* By `aria-checked` rather than by label: the display registry is mocked in this
* suite, so a tile reads "claude_localSubscription" rather than "Claude Code",
* and a test that selected on the visible name would be asserting the mock.
*
* The connect step arrives with nothing chosen, so this is what opens the input
* surface and lets the step advance.
*/
async function pickFirstSource(
click: (match: (text: string) => boolean) => Promise<void>,
): Promise<void> {
const tile = [...document.body.querySelectorAll("button[aria-checked]")][0];
const label = tile?.textContent?.trim() ?? "";
await click((text) => text === label);
}
describe("OnboardingWizard restore-gate (stale localStorage across accounts)", () => {
beforeEach(() => {
mockAuthApi.getSession.mockResolvedValue({
@ -301,6 +319,9 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", (
mockCompany.loading = false;
mockCompaniesApi.list.mockResolvedValue([]);
// The model step needs tiles to pick from, and this suite's default
// registry is empty.
mockAdapterRegistry.list = [{ type: "claude_local" }, { type: "codex_local" }];
const { root, queryClient } = render();
const renderTree = () =>
act(async () => {
@ -370,13 +391,14 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", (
await clickByText((t) => t.startsWith("Next"));
expect(document.body.textContent).toContain("Connect a model");
await pickFirstSource(clickByText);
expect(document.body.textContent).not.toContain("Adapter environment check");
expect(document.body.textContent).not.toContain("Test now");
// Through Connect to Review, so the Mission-row assertion runs against
// the checklist that actually renders it — stopping at the model step
// would let a Mission regression pass unseen.
await clickByText((t) => t.startsWith("Connect"));
await clickByText((t) => t.startsWith("Next"));
// The review step is the heading and the woken agent, nothing else: the
// checklist that restated the walk in three rows is gone, and with it
// the Mission row that could only render unchecked.
@ -405,6 +427,9 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", (
mockCompaniesApi.list.mockResolvedValue([]);
mockCompaniesApi.create.mockResolvedValue({ id: "company-new", issuePrefix: "INI" });
// The model step needs tiles to pick from, and this suite's default
// registry is empty.
mockAdapterRegistry.list = [{ type: "claude_local" }, { type: "codex_local" }];
const { root, queryClient } = render();
const renderTree = () =>
act(async () => {
@ -436,7 +461,8 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", (
});
await flushReact();
await clickText((t) => t.startsWith("Next"));
await clickText((t) => t.startsWith("Connect"));
await pickFirstSource(clickText);
await clickText((t) => t.startsWith("Next"));
expect(mockAgentsApi.hire).toHaveBeenCalled();
// The mock is declared with no parameters, so index the call rather than
@ -471,9 +497,10 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", (
await flushReact();
await clickByText((t) => t.startsWith("Next"));
expect(document.body.textContent).toContain("Connect a model");
await pickFirstSource(clickByText);
const connect = [...document.body.querySelectorAll("button")].find((b) =>
b.textContent?.trim().startsWith("Connect"),
b.textContent?.trim().startsWith("Next"),
)!;
await act(async () => {
connect.dispatchEvent(new MouseEvent("click", { bubbles: true }));
@ -569,6 +596,11 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", (
describe("hire gate: adapter authentication (claude_local, the default onboarding adapter)", () => {
/** Drives the wizard to the Connect step, agent name already filled in. */
async function openConnectStep() {
// The tile row is built from this registry, and the suite's default is
// empty. That was survivable while the step preselected a source; now that
// nothing is chosen until a tile is pressed, a step with no tiles is a step
// that can never advance.
mockAdapterRegistry.list = [{ type: "claude_local" }, { type: "codex_local" }];
mockCompaniesApi.create.mockResolvedValue({ id: "company-new", issuePrefix: "INI" });
window.localStorage.setItem(
ONBOARDING_STORAGE_KEY,
@ -610,6 +642,13 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", (
await clickByText((t) => t.startsWith("Next"));
expect(document.body.textContent).toContain("Connect a model");
// Pick a source. The step arrives with nothing chosen — `adapterType`
// carries a value for the hire, but that is not the same as the customer
// having answered — so the input surface stays closed and the step will
// not advance until a tile is pressed. Every case below is about what
// happens *after* that choice, so the helper makes it.
await pickFirstSource(clickByText);
return { root, clickByText };
}
@ -628,7 +667,7 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", (
});
const { root, clickByText } = await openConnectStep();
await clickByText((t) => t.startsWith("Connect"));
await clickByText((t) => t.startsWith("Next"));
expect(mockAgentsApi.hire).not.toHaveBeenCalled();
expect(document.body.textContent).toContain(
@ -653,7 +692,7 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", (
});
const { root, clickByText } = await openConnectStep();
await clickByText((t) => t.startsWith("Connect"));
await clickByText((t) => t.startsWith("Next"));
expect(mockAgentsApi.hire).toHaveBeenCalled();
@ -691,7 +730,7 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", (
async function connectWithApiKey() {
const handles = await openConnectStep();
await handles.clickByText((t) => t.startsWith("Use API keys"));
await handles.clickByText((t) => t.startsWith("Use API key"));
const field = document.body.querySelector(
'input[type="password"]',
) as HTMLInputElement;
@ -699,7 +738,7 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", (
setControlledValue(field, KEY);
});
await flushReact();
await handles.clickByText((t) => t.startsWith("Connect"));
await handles.clickByText((t) => t.startsWith("Next"));
return handles;
}
@ -782,7 +821,7 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", (
mockAgentsApi.hire.mockRejectedValueOnce(new Error("network went away"));
const { root, clickByText } = await connectWithApiKey();
await clickByText((t) => t.startsWith("Connect"));
await clickByText((t) => t.startsWith("Next"));
expect(mockSecretsApi.createMyUserSecret).toHaveBeenCalledTimes(1);
@ -802,12 +841,12 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", (
const { root, clickByText } = await openConnectStep();
await clickByText((t) => t.startsWith("Connect"));
await clickByText((t) => t.startsWith("Next"));
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"));
await clickByText((t) => t.startsWith("Use API key"));
await clickByText((t) => t.startsWith("Next"));
expect(mockAgentsApi.testEnvironment).toHaveBeenCalledTimes(2);
@ -829,13 +868,13 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", (
});
const { root, clickByText } = await openConnectStep();
await clickByText((t) => t.startsWith("Connect"));
await clickByText((t) => t.startsWith("Next"));
expect(mockAgentsApi.testEnvironment).toHaveBeenCalledTimes(1);
expect(mockAgentsApi.hire).not.toHaveBeenCalled();
// A second Connect must not treat the first (cached) blocking result as
// reusable — it re-probes, and the create path stays closed.
await clickByText((t) => t.startsWith("Connect"));
await clickByText((t) => t.startsWith("Next"));
expect(mockAgentsApi.testEnvironment).toHaveBeenCalledTimes(2);
expect(mockAgentsApi.hire).not.toHaveBeenCalled();
@ -850,7 +889,7 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", (
});
const { root, clickByText } = await openConnectStep();
await clickByText((t) => t.startsWith("Connect"));
await clickByText((t) => t.startsWith("Next"));
expect(mockAgentsApi.hire).toHaveBeenCalled();
const hireArgs = mockAgentsApi.hire.mock.calls.at(-1) as unknown[];
@ -875,7 +914,7 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", (
// scenario this test is named for.
const { root, clickByText } = await openConnectStep();
await clickByText((t) => t.startsWith("Connect"));
await clickByText((t) => t.startsWith("Next"));
expect(mockAgentsApi.hire).toHaveBeenCalled();
const hireArgs = mockAgentsApi.hire.mock.calls.at(-1) as unknown[];
@ -900,7 +939,7 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", (
});
const { root, clickByText } = await openConnectStep();
await clickByText((t) => t.startsWith("Connect"));
await clickByText((t) => t.startsWith("Next"));
expect(mockAgentsApi.hire).toHaveBeenCalled();
// The status route must not even be asked — the conflict is decided
@ -925,7 +964,7 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", (
});
const { root, clickByText } = await openConnectStep();
await clickByText((t) => t.startsWith("Connect"));
await clickByText((t) => t.startsWith("Next"));
expect(mockAgentsApi.hire).toHaveBeenCalled();
const hireArgs = mockAgentsApi.hire.mock.calls.at(-1) as unknown[];
@ -950,7 +989,7 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", (
});
const { root, clickByText } = await openConnectStep();
await clickByText((t) => t.startsWith("Connect"));
await clickByText((t) => t.startsWith("Next"));
expect(mockAgentsApi.testEnvironment).toHaveBeenCalled();
const testArgs = mockAgentsApi.testEnvironment.mock.calls.at(-1) as unknown[];
@ -969,7 +1008,7 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", (
// The default `beforeEach` mock already rejects with a 404 `ApiError`.
const { root, clickByText } = await openConnectStep();
await clickByText((t) => t.startsWith("Connect"));
await clickByText((t) => t.startsWith("Next"));
expect(mockAgentsApi.testEnvironment).toHaveBeenCalled();
const testArgs = mockAgentsApi.testEnvironment.mock.calls.at(-1) as unknown[];
@ -1023,7 +1062,7 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", (
);
const { root, clickByText } = await openConnectStep();
await clickByText((t) => t.startsWith("Connect"));
await clickByText((t) => t.startsWith("Next"));
expect(mockAgentsApi.hire).toHaveBeenCalled();
@ -1046,7 +1085,7 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", (
});
const { root, clickByText } = await openConnectStep();
await clickByText((t) => t.startsWith("Connect"));
await clickByText((t) => t.startsWith("Next"));
expect(mockAgentsApi.hire).not.toHaveBeenCalled();
expect(document.body.textContent).toContain(
@ -1067,10 +1106,10 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", (
mockAgentsApi.hire.mockRejectedValue(new Error("hire failed"));
const { root, clickByText } = await openConnectStep();
await clickByText((t) => t.startsWith("Connect"));
await clickByText((t) => t.startsWith("Next"));
expect(mockAgentsApi.getClaudeOAuthTokenStatus).toHaveBeenCalledTimes(1);
await clickByText((t) => t.startsWith("Connect"));
await clickByText((t) => t.startsWith("Next"));
expect(mockAgentsApi.getClaudeOAuthTokenStatus).toHaveBeenCalledTimes(2);
await act(async () => root.unmount());
@ -1228,7 +1267,7 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", (
// It mounts: the companies query sets `retry: false`, and with no
// companies the dashboard's "Get Started" button opens onboarding — a gate
// that rendered nothing here would make that button dead.
expect(document.body.textContent).not.toBe("");
expect(document.querySelector('[data-testid="onboarding-wizard"]')).not.toBeNull();
// The draft is not restored, because ownership cannot be verified...
const nameInput = document.body.querySelector(
"#onboarding-agent-name",
@ -1273,7 +1312,7 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", (
expect(localStorage).toHaveBeenCalled();
// It mounted: the wizard is open with no draft, rather than the render
// throwing on the way in.
expect(document.body.textContent).not.toBe("");
expect(document.querySelector('[data-testid="onboarding-wizard"]')).not.toBeNull();
localStorage.mockRestore();
await act(async () => {
@ -1314,7 +1353,7 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", (
await flushReact();
// Mounted rather than blank...
expect(document.body.textContent).not.toBe("");
expect(document.querySelector('[data-testid="onboarding-wizard"]')).not.toBeNull();
// ...but the draft was not restored, because the list cannot be trusted.
const nameInput = document.body.querySelector(
"#onboarding-agent-name",
@ -1325,44 +1364,14 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", (
root.unmount();
});
});
it("closes without throwing when the browser denies storage access", async () => {
// `reset()` clears the draft and `handleClose` calls it, so the close
// button is a fourth storage call site. Guarding the read, the cleanup and
// the persist effect one at a time is how this one stayed unguarded while
// the others looked fixed.
const deny = () => {
throw new DOMException("The operation is insecure.", "SecurityError");
};
const localStorage = vi.spyOn(window, "localStorage", "get").mockImplementation(deny);
mockCompany.companies = [{ id: "c1", name: "My Co", issuePrefix: "MC" }];
mockCompany.loading = false;
const { root, queryClient } = render();
await act(async () => {
root.render(
<QueryClientProvider client={queryClient}>
<OnboardingWizard />
</QueryClientProvider>,
);
});
await flushReact();
const close = [...document.body.querySelectorAll("button")].find((b) =>
b.textContent?.includes("Close"),
);
expect(close).toBeDefined();
await act(async () => {
close!.dispatchEvent(new MouseEvent("click", { bubbles: true }));
});
await flushReact();
expect(mockDialog.closeOnboarding).toHaveBeenCalled();
localStorage.mockRestore();
await act(async () => {
root.unmount();
});
});
// The close-on-storage-denial case is gone with the control that reached it.
// Onboarding is a gate now, not a dialog: there is no X, Escape does not
// dismiss it, and nothing in the wizard calls `handleClose`. A test that
// clicked Close was asserting a path a customer no longer has.
//
// The open side of the same hazard is still covered by "renders instead of
// throwing when the browser denies storage access" directly above, which is
// the half that can still happen.
it("leaves the draft untouched when the company query fails and onboarding is closed", async () => {
// Mounting is safe for the draft precisely because the persist effect is
// gated on the wizard being open. A closed wizard writes nothing, so the
@ -1490,7 +1499,7 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", (
await flushReact();
// Mounted — so this is a real observation, not an unmounted false pass.
expect(document.body.textContent).not.toBe("");
expect(document.querySelector('[data-testid="onboarding-wizard"]')).not.toBeNull();
expect(document.body.textContent).not.toContain("A's Lead");
const nameInput = document.body.querySelector(
"#onboarding-agent-name",
@ -1564,7 +1573,7 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", (
await flushReact();
// Mounted, so this observes the real thing rather than an empty document.
expect(document.body.textContent).not.toBe("");
expect(document.querySelector('[data-testid="onboarding-wizard"]')).not.toBeNull();
expect(document.body.textContent).not.toContain("A's Lead");
const nameInput = document.body.querySelector(
"#onboarding-agent-name",
@ -1668,6 +1677,12 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", (
mockInstanceSettingsApi.getExperimental.mockResolvedValue({ enableManagedSandboxOnly: false });
mockAgentsApi.getAdapterAuthSignal.mockReset();
mockAgentsApi.getAdapterAuthSignal.mockResolvedValue({ status: "present" });
// The row has to actually offer the adapter these drafts name. The login
// panel lives inside the input canvas, and the canvas opens for a chosen
// source — a saved adapter the row cannot show is not a chosen one, so
// without this the panel is absent for a reason that has nothing to do
// with the auth signal these tests are about.
mockAdapterRegistry.list = [{ type: "claude_local" }, { type: "codex_local" }];
});
/** Drives the wizard to the Connect step with a company already created. */
@ -1703,9 +1718,137 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", (
// enough to reach the end of that chain.
for (let i = 0; i < 5; i++) await flushReact();
expect(document.body.textContent).toContain("Connect a model");
// No pick needed here: the draft this helper restores already names an
// adapter, which is what a run returning to this step actually carries.
return { root, queryClient };
}
it("will not advance on a saved adapter the step no longer offers", async () => {
// A draft can name an adapter this registry does not carry — a cloud
// sandbox without claude_local, an adapter since disabled. The row hides
// it, so the step shows an unanswered question: no tile filled, no input
// canvas. The CTA has to agree with that.
//
// It did not. The gate asked `sourcePicked`, which only means "a draft
// named something", so Next stayed live on a step that had visibly asked
// nothing and would hire against the hidden name. Reported by Greptile on
// #12726.
mockAdapterRegistry.list = [{ type: "codex_local" }];
const { root } = await openStep4({ adapterType: "some_retired_adapter" });
const tiles = [...document.body.querySelectorAll("button[aria-checked]")];
expect(tiles.length, "the row should still offer what it has").toBeGreaterThan(0);
expect(
tiles.some((t) => t.getAttribute("aria-checked") === "true"),
"no tile should read as chosen",
).toBe(false);
const cta = [...document.body.querySelectorAll("button")].find(
(b) => b.textContent?.trim() === "Next",
);
expect(cta, "the step should render its Next button").toBeTruthy();
expect(
cta!.hasAttribute("disabled"),
"Next must not advance a question the step has not visibly asked",
).toBe(true);
// And it opens again the moment the customer answers it themselves.
await act(async () => {
tiles[0]!.dispatchEvent(new MouseEvent("click", { bubbles: true }));
});
for (let i = 0; i < 5; i++) await flushReact();
const ctaAfter = [...document.body.querySelectorAll("button")].find(
(b) => b.textContent?.trim() === "Next",
);
expect(ctaAfter!.hasAttribute("disabled")).toBe(false);
await act(async () => root.unmount());
});
it("will not advance on a saved adapter the row does not show", async () => {
// The other shape of the same defect, and the one the snap cannot cover.
//
// The tile row is `recommendedAdapters`; the snap's idea of "visible" is
// recommended *plus* the advanced list. An adapter in the second but not
// the first — a saved `opencode_local`, say — therefore satisfies the
// snap, which leaves it alone, while the row it is supposed to be chosen
// in never shows it. Nothing is highlighted, the canvas is shut, and with
// the gate on `sourcePicked` the CTA was live: one press hires against an
// adapter the customer has not seen on this screen.
mockAdapterRegistry.list = [{ type: "claude_local" }, { type: "opencode_local" }];
const { root } = await openStep4({ adapterType: "opencode_local" });
const tiles = [...document.body.querySelectorAll("button[aria-checked]")];
expect(
tiles.some((t) => t.getAttribute("aria-checked") === "true"),
"the saved adapter is not in this row, so nothing should read as chosen",
).toBe(false);
const cta = [...document.body.querySelectorAll("button")].find(
(b) => b.textContent?.trim() === "Next",
);
expect(
cta!.hasAttribute("disabled"),
"Next must not hire an adapter the row never offered",
).toBe(true);
await act(async () => root.unmount());
});
it("will not hire from the keyboard on a source nobody selected", async () => {
// The step has two ways forward, and gating only the visible one leaves
// the defect intact behind a keystroke. Cmd+Enter called
// `handleGiveHeartbeat` directly with its own, older list of conditions,
// so with the button correctly disabled the same screen still hired on
// Cmd+Enter. Reported by Greptile on #12726 after the button was fixed.
//
// The saved adapter is one the registry no longer carries, so the snap
// replaces it with `claude_local` and clears the pick: the row shows two
// tiles, neither chosen. A keystroke that gets through hires claude_local
// — a real, offerable adapter that nobody on this screen selected, which
// is what makes the bypass worth a test rather than a comment.
mockAdapterRegistry.list = [{ type: "claude_local" }, { type: "codex_local" }];
const { root } = await openStep4({ adapterType: "some_retired_adapter" });
const tiles = [...document.body.querySelectorAll("button[aria-checked]")];
expect(
tiles.some((t) => t.getAttribute("aria-checked") === "true"),
"the snap must not leave a tile reading as chosen",
).toBe(false);
const wizard = document.querySelector('[data-testid="onboarding-wizard"]');
expect(wizard, "the wizard should be mounted").not.toBeNull();
for (const modifier of [{ metaKey: true }, { ctrlKey: true }]) {
await act(async () => {
wizard!.dispatchEvent(
new KeyboardEvent("keydown", { key: "Enter", bubbles: true, ...modifier }),
);
});
for (let i = 0; i < 6; i++) await flushReact();
}
expect(
mockAgentsApi.hire,
"no keystroke may hire a source nobody selected",
).not.toHaveBeenCalled();
// And it works once the question is answered, so this is a gate rather
// than a dead shortcut.
await act(async () => {
tiles[0]!.dispatchEvent(new MouseEvent("click", { bubbles: true }));
});
for (let i = 0; i < 6; i++) await flushReact();
await act(async () => {
wizard!.dispatchEvent(
new KeyboardEvent("keydown", { key: "Enter", bubbles: true, metaKey: true }),
);
});
for (let i = 0; i < 6; i++) await flushReact();
expect(mockAgentsApi.hire).toHaveBeenCalled();
await act(async () => root.unmount());
});
it("starts no call to the test-environment route on adapter selection", async () => {
const { root } = await openStep4();
expect(mockAgentsApi.testEnvironment).not.toHaveBeenCalled();

View File

@ -112,7 +112,6 @@ import {
Check,
Loader2,
ChevronDown,
X
} from "lucide-react";
type Step = 0 | 1 | 2 | 3 | 4 | 5;
@ -549,6 +548,22 @@ function OnboardingWizardInner({
const [adapterType, setAdapterType] = useState<AdapterType>(() =>
restoreOnboardingAdapterType(saved?.adapterType),
);
/**
* Whether a model source has been chosen, as opposed to which one
* `adapterType` happens to hold.
*
* The two are not the same, and reading the second as the first is what made
* this step arrive with a tile already lit and its input already open: the
* hire needs an adapter, so `adapterType` always carries one, restored or
* defaulted. A customer who never touched the row could reach the end of the
* step having chosen nothing.
*
* Restored true when the draft names a source. Someone returning here has
* already answered, and asking again would throw that answer away.
*/
const [sourcePicked, setSourcePicked] = useState<boolean>(
() => typeof saved?.adapterType === "string" && saved.adapterType.length > 0,
);
const savedNativeRunnerDraft = saved?.adapterType === "paperclip_runner";
const [cwd, setCwd] = useState((saved?.cwd as string) ?? "");
// Native drafts may carry provider-specific configuration that is invalid
@ -1027,19 +1042,46 @@ function OnboardingWizardInner({
* `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);
const sourceSelected =
sourcePicked && recommendedAdapters.some((opt) => opt.type === adapterType);
/**
* When the input canvas is open.
* Whether the connect step may advance.
*
* 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.
* One predicate, because there are two ways to advance and they drifted. The
* button's condition and Cmd+Enter's were written out separately, so when the
* button gained `sourceSelected` and `adapterEnvLoading` the keyboard kept the
* older, shorter list and hired against a source the row had never shown.
* The same defect the button was just fixed for, one path over.
*
* `loading` is deliberately not here. The keyboard handler returns on it
* before reaching any step, for a reason particular to keystrokes: a second
* Enter re-enters a handler whose guard is state the first has not written
* yet. That check belongs at the top of the handler, not per-step.
*
* Anything that gates this step belongs in here, so the next one is added
* once rather than twice.
*/
const canvasOpen = sourceSelected || showAdapterLoginPanel;
const connectStepReady =
sourceSelected && !adapterEnvLoading && !missionUnresolvedForHire;
/**
* When the input canvas is open: exactly when a source has been chosen.
*
* The card is the answer to the tile that was just pressed, so an untouched
* row leaves nothing under it a sign-in bar offered before the question was
* answered says the step already knows which provider is meant, which it does
* not.
*
* This used to open for a pending sign-in as well, `|| showAdapterLoginPanel`,
* so that a restored draft naming an adapter this step no longer offers still
* had something to press. That reasoning came from when the row arrived with a
* selection already made and an invisible one was a dead end. It is not one
* now: the row is a question, an unofferable saved adapter simply leaves it
* unanswered, and the visible tiles are the thing to press. `showAdapterLoginPanel`
* still decides what goes *inside* the canvas only not whether it exists.
*/
const canvasOpen = sourceSelected;
// 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
@ -1058,6 +1100,13 @@ function OnboardingWizardInner({
if (visible.some((a) => a.type === adapterType)) return;
const next = visible[0].type as AdapterType;
setAdapterType(next);
// The snap is not a choice. It replaces a name the customer can no longer
// see with the first one they can, which is the right thing to hold — but
// holding it *as chosen* would put a filled tile and an open sign-in panel
// on a step nobody has answered, and re-create by the back door exactly the
// preselection this step was changed to stop doing. The saved answer was
// unofferable, so the question is open again.
setSourcePicked(false);
if (next === "codex_local") return;
if (next === "opencode_local") {
setModel(DEFAULT_OPENCODE_LOCAL_MODEL);
@ -1977,7 +2026,11 @@ function OnboardingWizardInner({
}
else if (step === 2 && companyName.trim() && companyGoal.trim()) handleConfirmMission();
else if (step === 3 && agentName.trim()) setStep(4);
else if (step === 4 && agentName.trim() && !missionUnresolvedForHire)
// `connectStepReady`, the same predicate the step's button uses. Spelling
// the condition out here again is what let this path hire against a
// source the tile row had never shown, after the button was gated and
// this was not.
else if (step === 4 && agentName.trim() && connectStepReady)
handleGiveHeartbeat();
else if (step === 5) handleLaunchToDashboard();
}
@ -2040,16 +2093,19 @@ function OnboardingWizardInner({
RemoveScroll which blocks wheel events on our custom (non-DialogContent)
scroll container. A plain div preserves the background without scroll-locking. */}
<div className="fixed inset-0 z-50 bg-background" />
<div className="fixed inset-0 z-50 flex" onKeyDown={handleKeyDown}>
{/* Close button */}
<button
onClick={handleClose}
className="absolute top-4 left-4 z-10 rounded-sm p-1.5 text-muted-foreground/60 hover:text-foreground transition-colors"
>
<X className="h-5 w-5" />
<span className="sr-only">Close</span>
</button>
{/* A deliberate hook for "the wizard mounted".
The tests that assert it opens used to prove it by finding any text
in the document which, with the front door mocked to null in that
suite, was only ever the close button's screen-reader label. Removing
the button took the proof with it, and those tests would have gone on
passing indefinitely if it had stayed. A named anchor says what they
mean rather than depending on whatever happens to render. */}
<div
data-testid="onboarding-wizard"
className="fixed inset-0 z-50 flex"
onKeyDown={handleKeyDown}
>
{/* Step 0: Front Door — full-screen choice */}
{step === 0 && (
<div className="w-full flex flex-col overflow-y-auto">
@ -2086,8 +2142,13 @@ function OnboardingWizardInner({
// narrower than the next screen's makes the whole frame jump on
// Continue — which is the thing that read as "off" to begin
// with, and is more obvious once the buttons match.
// 68px sides, so the column inside the 560px frame is 424px —
// the measure the design draws every arc step to. It was 40px
// (a 480px column), which is wide enough that the two model
// tiles stretch and the name field sits under a question far
// narrower than itself.
isAgentArcStep || step === 1
? "w-(--sz-560px) max-w-full px-8 py-10 sm:px-10 sm:py-11"
? "w-(--sz-560px) max-w-full px-8 py-10 sm:px-(--sz-68px) sm:py-11"
: "w-full max-w-md px-8 py-12",
)}
>
@ -2195,7 +2256,7 @@ function OnboardingWizardInner({
// sentence restating it only pushes the fields down.
lede={
step === 3 ? undefined : step === 4 ? (
<>Paperclip works with your existing subscription or API keys.</>
<>Paperclip works with your subscription or API keys.</>
) : (
<>{agentName.trim() || "Your first agent"} is ready to work!</>
)
@ -2541,11 +2602,21 @@ function OnboardingWizardInner({
`general` role; a specific one can be set later, where there
is context to choose it in. */}
{step === 3 && (
<div className="mx-auto flex w-full max-w-(--sz-320px) flex-col gap-9">
<div className="mx-auto flex w-full flex-col gap-9">
<div className="flex flex-col gap-2">
<Label htmlFor="onboarding-agent-name">Name</Label>
<Label htmlFor="onboarding-agent-name">Agent name</Label>
{/*
Filled, not outlined, and the column's full width the
same field the naming step before the hand-off draws.
`bg-muted` is the design's field surface; the default
Input is a hairline border over `bg-input/30`, which on
this ground reads as an empty outline rather than a place
to type. The border is kept but made transparent so the
focus ring, which colours the border, still has one.
*/}
<Input
id="onboarding-agent-name"
className="h-(--sz-44px) rounded-lg border-transparent bg-muted shadow-none dark:bg-muted"
placeholder="e.g. Chief of staff, Designer, Ron..."
value={agentName}
onChange={(e) => setAgentName(e.target.value)}
@ -2582,11 +2653,13 @@ function OnboardingWizardInner({
}))}
mode={credentialMode}
selectedId={
sourcePicked &&
recommendedAdapters.some((opt) => opt.type === adapterType)
? adapterType
: null
}
onSelect={(id) => {
setSourcePicked(true);
setAdapterType(id);
if (id === "codex_local") return;
if (id === "opencode_local") {
@ -2873,11 +2946,9 @@ function OnboardingWizardInner({
primaryLabel={
step === 1
? "Continue"
: step === 3
? "Next"
: step === 4
? "Connect"
: "Get started"
: step === 5
? "Get started"
: "Next"
}
loadingLabel={
step === 1
@ -2893,7 +2964,13 @@ function OnboardingWizardInner({
: step === 3
? !agentName.trim()
: step === 4
? loading || adapterEnvLoading || missionUnresolvedForHire
? // Nothing is chosen on arrival, so the step cannot
// advance until something is. Without this a customer
// could pass the model step having touched none of
// it, and be hired against whatever the draft
// happened to carry. See `connectStepReady`, which
// Cmd+Enter asks as well.
!connectStepReady || loading
: loading || launchStateIncomplete
}
onPrimary={() => {

View File

@ -4,6 +4,7 @@ import { AnimatePresence, motion } from "motion/react";
import { cn } from "../../lib/utils";
import {
CANVAS_CONTENT_ENTER,
CANVAS_ENTER_TRAVEL,
CANVAS_CONTENT_EXIT,
CANVAS_CONTENT_TRAVEL,
} from "./onboarding-motion";
@ -53,17 +54,24 @@ export function ConnectInputCanvas({
Which leaves the padding to the contents as well: theirs is already sized
for what they hold, and a second inset would push it off the step's measure.
Nothing animates on this wrapper, deliberately. It carried an enter/exit
three times height, then opacity and stalled every time, once leaving the
login card rendered inside a two-pixel box and once at four percent opacity
while `open` was true the whole while. The casualty each time was the OAuth
URL a customer has to click. The swap inside still animates; the container
holding it does not need to, and cannot be trusted to.
The wrapper animates its arrival and nothing else. Picking a source is what
brings this into being, so it descends into place rather than appearing
already there the movement is what ties it to the tile just pressed.
Opacity and transform only. An earlier version animated *height* here with
`overflow: hidden`, and stalled three separate times once leaving the login
card rendered inside a two-pixel box, once at four percent opacity while
`open` was true throughout. The casualty each time was the OAuth URL a
customer has to click. A height that is measured once cannot hold a panel
that grows when a login starts; these two properties can, because neither
clips and neither is measured.
*/
return (
<div
<motion.div
className="mt-5 flex items-center"
style={{ minHeight: MIN_CONTENT_HEIGHT }}
initial={{ opacity: 0, y: -CANVAS_ENTER_TRAVEL }}
animate={{ opacity: 1, y: 0, transition: CANVAS_CONTENT_ENTER }}
>
{/*
`popLayout`, so the leaving input is taken out of flow while it animates
@ -89,7 +97,7 @@ export function ConnectInputCanvas({
{children}
</motion.div>
</AnimatePresence>
</div>
</motion.div>
);
}

View File

@ -18,7 +18,7 @@ import { LINK_LABEL_FADE_IN, LINK_LABEL_FADE_OUT } from "./onboarding-motion";
*/
const LINK_LABEL: Record<CredentialMode, string> = {
subscription: "Use API keys instead",
subscription: "Use API key instead",
api: "Use subscription instead",
};

View File

@ -82,12 +82,17 @@ function ModelSourceTile({
// and lending it to focus as well would mean tabbing across the row
// looked like picking every tile in turn.
"outline-none focus-visible:ring-ring/50 focus-visible:ring-(length:--rad-3)",
// Hover brings the surface up to the same half-strength ground the
// selected tile already sits on, and stops there. Pointing at a tile
// should say "this one is live", not "this one is chosen" — so the
// bright stroke stays reserved for the choice, and the only thing
// separating hover from selection is the border.
selected ? "border-foreground bg-accent/50" : "border-border hover:bg-accent/50",
// Selection is a lighter surface, not a brighter edge. Both states keep
// the same border — it draws the tile, not the choice — and the fill
// carries the state. A bright stroke on one tile made the row read as
// one outlined object beside one plain one, rather than two of a kind
// with one of them picked.
//
// Hover stops short of the selected fill, so pointing at a tile says
// "this one is live" rather than "this one is chosen".
selected
? "border-border bg-accent"
: "border-border bg-card hover:bg-accent/40",
)}
>
<span className="flex size-(--sz-30px) shrink-0 items-center justify-center">

View File

@ -147,6 +147,17 @@ export const CANVAS_EASE = TAG_SWAP_EASE;
* 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.
*/
/**
* How far the input canvas descends into place when a source is picked.
*
* Larger than the swap's travel, and in the opposite direction. A swap trades
* one input for another in a space that already exists, so it barely moves; this
* is a surface arriving where there was none, and it comes down from above so
* the movement reads as the tile above it opening out.
*/
export const CANVAS_ENTER_TRAVEL = 10;
export const CANVAS_CONTENT_ENTER = TAG_SWAP_ENTER;
export const CANVAS_CONTENT_EXIT = TAG_SWAP_EXIT;
export const CANVAS_CONTENT_TRAVEL = TAG_SWAP_TRAVEL;

View File

@ -2286,6 +2286,7 @@ span.paperclip-mention-chip[data-mention-kind="external-object"] {
--sz-calc-13: calc(0.75rem + 0.5rem); /* Extracted from ui/src/components/IssueRow.tsx (ml-[calc(theme(spacing.3)+theme(spacing.2))]). */
--sz-140px: 140px; /* Extracted from ui/src/components/JsonSchemaForm.tsx (min-h-[140px]). */
--sz-52px: 52px; /* Extracted from ui/src/components/KanbanBoard.tsx (w-[52px]). */
--sz-68px: 68px; /* The onboarding arc's side inset — 560px frame, 424px column. */
--sz-48px: 48px; /* Extracted from ui/src/components/KanbanBoard.tsx (min-w-[48px]). */
--sz-260px: 260px; /* Extracted from ui/src/components/KanbanBoard.tsx (min-w-[260px]). */
--sz-120px: 120px; /* Extracted from ui/src/components/KanbanBoard.tsx (min-h-[120px]). */

View File

@ -45,12 +45,15 @@ describe("storybook onboarding draft", () => {
expect(readOnboardingDraft()?.createdAgentId).toBe("");
});
// 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", () => {
// A run standing on step 3 has not reached the connect step, so it cannot
// have chosen a source there. Seeding one is not a harmless head start: the
// step reads a saved `adapterType` as "already picked", and every arc story
// opened with Claude Code selected and its sign-in panel already showing —
// the preselection the step was changed to stop doing, restored by the
// fixture. Stories that need a source click one, the way a customer does.
it("does not claim a model source was chosen before the connect step", () => {
seedOnboardingDraft();
expect(readOnboardingDraft()?.adapterType).toBe("claude_local");
expect(readOnboardingDraft()).not.toHaveProperty("adapterType");
});
// `restoreOnboardingState` treats restoring as an authorization decision and

View File

@ -40,10 +40,15 @@ export function seedOnboardingDraft(): void {
companyName: "Paperclip Storybook",
agentName: "Darnold",
agentRole: "general",
// 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",
// No `adapterType`. This draft describes a run standing on step 3, and a
// run that has not reached the connect step cannot have chosen a source
// there — seeding one made every arc story arrive with Claude Code already
// picked, which is precisely the preselection the step was changed to stop
// doing. Stories that need a source pick one, the way a customer does.
//
// (It read `claude_code` before that, which is no adapter at all: the step
// recovered by falling back, and the hire would have posted a type the
// server does not know.)
createdCompanyId: STORYBOOK_COMPANY_ID,
createdCompanyPrefix: "PAP",
createdAgentId: "",

View File

@ -128,14 +128,38 @@ const STEP_TIMEOUT_MS = 15_000;
* 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.
*
* The arrival is waited on by *heading*, not by the next button's label. The arc
* labels its forward button "Next" on every step it has one, so a story that
* waited for a button name would be satisfied by the button it just clicked and
* report arriving somewhere it never left. This is not hypothetical: these
* stories waited for a button named "Connect" until the label changed, at which
* point Review sat on the connect step looking like a story written to open
* there the exact failure `STEP_TIMEOUT_MS` is commented against.
*/
async function advance(from: string, to: string) {
async function advance(to: string) {
await waitFor(
() => expect(screen.getByRole("button", { name: from })).toBeEnabled(),
() => expect(screen.getByRole("button", { name: PRIMARY })).toBeEnabled(),
{ timeout: STEP_TIMEOUT_MS },
);
await userEvent.click(screen.getByRole("button", { name: from }));
await screen.findByRole("button", { name: to }, { timeout: STEP_TIMEOUT_MS });
await userEvent.click(screen.getByRole("button", { name: PRIMARY }));
await screen.findByText(to, { selector: "h2, h1" }, { timeout: STEP_TIMEOUT_MS });
}
/** The arc's forward button, which reads the same on every step but the last. */
const PRIMARY = "Next";
/**
* Pick a model source, which the connect step needs before it will go forward.
*
* Nothing is selected on arrival deliberately, so the row reads as a question
* rather than a confirmation and the CTA stays disabled until one is pressed.
* Found by role rather than by label so the choice does not depend on which
* adapters the fixture registry happens to offer.
*/
async function pickFirstSource() {
const tiles = await screen.findAllByRole("radio", {}, { timeout: STEP_TIMEOUT_MS });
await userEvent.click(tiles[0]!);
}
/**
@ -191,7 +215,7 @@ export const ConnectAModel: StoryObj = {
return resetOnboardingFixtureState;
},
render: () => <WizardArc />,
play: () => advance("Next", "Connect"),
play: () => advance("Connect a model"),
};
/**
@ -208,7 +232,7 @@ export const ConnectAModelAlreadySignedIn: StoryObj = {
return resetOnboardingFixtureState;
},
render: () => <WizardArc />,
play: () => advance("Next", "Connect"),
play: () => advance("Connect a model"),
};
/**
@ -224,7 +248,7 @@ export const ConnectAModelNoSandbox: StoryObj = {
return resetOnboardingFixtureState;
},
render: () => <WizardArc />,
play: () => advance("Next", "Connect"),
play: () => advance("Connect a model"),
};
/**
@ -245,8 +269,9 @@ export const Review: StoryObj = {
},
render: () => <WizardArc />,
play: async () => {
await advance("Next", "Connect");
await advance("Connect", "Get started");
await advance("Connect a model");
await pickFirstSource();
await advance("Let's get started...");
},
};