From 88a0f885e800b533e5820231a6c03bf4a047fedb Mon Sep 17 00:00:00 2001 From: Tonio Date: Mon, 24 Aug 2026 01:35:15 -0700 Subject: [PATCH] Brand lockup, no idle env-check card, no Mission row (#12074) 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 customers meet the product through an onboarding arc that ends in the tenant wizard > - A staging walk of that arc found three rough edges: an ad-hoc icon standing in for the brand, an environment-check card that narrates a probe the flow already runs on its own, and a review checklist that still lists a Mission the arc stopped asking for > - Each one makes the product look less finished than it is at the exact moment a customer decides what it is > - This pull request renders the brand lockup, hides the idle environment-check card while keeping the probe and its failure surface, and drops the Mission row > - The benefit is a first-session arc that reads as one product, with no controls for questions nobody was asked ## Linked Issues or Issue Description No public issue exists. The changes come from walking the sign-up arc on a staging fleet. **What happened:** The model step shows an "Adapter environment check" card with a "Test now" button even though pressing Connect runs the same probe and blocks a failing hire. The review step lists "Mission" in its checklist although onboarding no longer asks for one. The auth page renders a sparkles icon beside the word "Paperclip" instead of the brand lockup. **Expected behavior:** The model step shows the check only when a probe has found something to fix. The review checklist lists only what onboarding set up. The brand renders as the lockup asset used across surfaces. **Actual behavior:** An idle card narrates a probe that runs regardless. A permanent unchecked row marks a question nobody was asked. The brand is a generic icon plus text. **Steps to reproduce:** 1. Sign up on a staging fleet and enter the tenant wizard. 2. On "Create your first agent", choose a role and press Next: the model step shows the "Adapter environment check" card before anything has been probed. 3. Continue to Review: the checklist lists "Mission" as a permanently unchecked row. **Additional context:** The Mission row outlived the removal of the mission step (#11935). The environment probe itself still runs on Connect and blocks a failing hire; only its idle card is at issue. The brand lockup lands across all three surfaces in the same round — paperclip-cloud#270 and paperclip-id#58 carry the other halves. ## What Changed - `PaperclipLockup` renders the brand asset (mark + wordmark, one geometry, `fill="currentColor"`); the auth page uses it in place of the sparkles icon. - The adapter environment check's idle card (explainer + "Test now") no longer renders. The probe still runs on Connect and still blocks a failing hire. - The check's failure content still renders when a probe has found something — the blocking error points the customer at "the reported checks", so they stay visible. - Connect retries a cached failed probe instead of reusing it. With "Test now" gone, Connect is the only retry, and a stale fail would lock out a machine the customer has since fixed. - The review checklist drops its "Mission" row. ## Verification Run the tenant suite: ``` cd ui && npx vitest run ``` - 4365 tests pass across 471 files; `npx tsc --noEmit` is clean. - New test drives the wizard to the model step and asserts the environment-check card is absent, anchored on "Connect a model" so an unrendered step cannot pass as an absence. - The review assertions anchor on the remaining rows ("Organization name", "Agent created", "Model connected"). ## Risks - **Behavioral change:** a cached *failed* probe is re-run on Connect instead of reused. Pass and warn results are still cached. This only affects the retry path that "Test now" used to serve. - **Hidden, not removed:** the environment check machinery is intact; only the idle card is gone. A failing probe still blocks the hire and still shows its checks. - **Brand:** the wordmark now ships inside an SVG; its accessible name carries the text. Screen readers announce "Paperclip" as before. ## Model Used Claude Fable 5 (`claude-fable-5`) via Claude Code, with tool use and code execution; earlier rounds on this branch's predecessor used Claude Opus 5 (`claude-opus-5`). ## 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 - [x] My branch name describes the change and contains no internal Paperclip ticket id - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [ ] 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 - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Claude Opus 5 --- ui/src/components/OnboardingWizard.test.tsx | 102 ++++++++++++++++++++ ui/src/components/OnboardingWizard.tsx | 50 +++++----- ui/src/components/PaperclipLockup.tsx | 47 +++++++++ ui/src/pages/Auth.tsx | 7 +- 4 files changed, 178 insertions(+), 28 deletions(-) create mode 100644 ui/src/components/PaperclipLockup.tsx diff --git a/ui/src/components/OnboardingWizard.test.tsx b/ui/src/components/OnboardingWizard.test.tsx index 1159bf65f6..67b1a72ffe 100644 --- a/ui/src/components/OnboardingWizard.test.tsx +++ b/ui/src/components/OnboardingWizard.test.tsx @@ -57,6 +57,16 @@ const mockAgentsApi = vi.hoisted(() => ({ instructionsBundle: vi.fn(async () => ({ entryFile: "AGENTS.md" })), saveInstructionsFile: vi.fn(async () => ({})), })); +// The Connect path loads environment settings before probing; without these +// the probe dies on "Could not load environment settings" and the hire never +// runs — which reads as a mysterious 0-call assertion, not an error. +const mockEnvironmentsApi = vi.hoisted(() => ({ + list: vi.fn(async () => []), +})); +const mockInstanceSettingsApi = vi.hoisted(() => ({ + get: vi.fn(async () => ({ defaultEnvironmentId: null })), + getExperimental: vi.fn(async () => ({ enableManagedSandboxOnly: false })), +})); const mockApprovalsApi = vi.hoisted(() => ({ create: vi.fn(), })); @@ -93,6 +103,8 @@ vi.mock("../api/agents", () => ({ agentsApi: mockAgentsApi })); vi.mock("../api/approvals", () => ({ approvalsApi: mockApprovalsApi })); vi.mock("../api/issues", () => ({ issuesApi: mockIssuesApi })); vi.mock("../api/projects", () => ({ projectsApi: mockProjectsApi })); +vi.mock("../api/environments", () => ({ environmentsApi: mockEnvironmentsApi })); +vi.mock("../api/instanceSettings", () => ({ instanceSettingsApi: mockInstanceSettingsApi })); vi.mock("../adapters", () => ({ listUIAdapters: () => mockAdapterRegistry.list, getUIAdapter: () => ({ buildAdapterConfig: () => ({}) }), @@ -257,6 +269,96 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", ( await act(async () => root.unmount()); }); + it("shows no environment-check card on the model step, and no Mission row on review", async () => { + // Round-3 walk feedback: the adapter environment check still runs — + // Connect probes before hiring and blocks on a fail — but its idle card + // (explainer plus "Test now") is gone. And the review checklist lost its + // "Mission" row: onboarding stopped asking, so the row could only ever + // render unchecked. Both asserted against positive anchors so an + // unrendered step cannot pass as an absence. + mockCompaniesApi.create.mockResolvedValue({ id: "company-new", issuePrefix: "INI" }); + const { root } = await openStepOne("create"); + await clickByText((t) => t.startsWith("Next")); + expect(document.body.textContent).toContain("Create your first agent"); + + // Step 3 → 4 needs an agent name; choosing a role fills it. + const roleTrigger = document.body.querySelector("#onboarding-agent-role") as HTMLElement; + await act(async () => { + roleTrigger.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + const ceo = [...document.body.querySelectorAll('[role="option"]')].find( + (o) => o.textContent?.trim() === "CEO", + ) as HTMLElement; + await act(async () => { + ceo.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + await clickByText((t) => t.startsWith("Next")); + + expect(document.body.textContent).toContain("Connect a model"); + 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")); + expect(document.body.textContent).toContain("Review"); + expect(document.body.textContent).toContain("Organization name"); + expect(document.body.textContent).toContain("Agent created"); + expect(document.body.textContent).toContain("Model connected"); + expect(document.body.textContent).not.toContain("Mission"); + + await act(async () => root.unmount()); + }); + + it("hires one agent when Connect fires twice in one breath", async () => { + // The Connect handler re-runs a cached failed probe now that "Test now" + // is gone — so two overlapping submissions could both pass the fresh + // probe and both hire. `loading` cannot stop the second caller: it is + // state, unwritten while the first call is still awaiting. The ref + // guard must make the second submission a no-op. + let resolveHire: (v: { agent: { id: string }; approval: null }) => void = () => {}; + mockAgentsApi.hire.mockReturnValue( + new Promise((resolve) => { + resolveHire = resolve; + }), + ); + mockCompaniesApi.create.mockResolvedValue({ id: "company-new", issuePrefix: "INI" }); + const { root } = await openStepOne("create"); + await clickByText((t) => t.startsWith("Next")); + const roleTrigger = document.body.querySelector("#onboarding-agent-role") as HTMLElement; + await act(async () => { + roleTrigger.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + const ceo = [...document.body.querySelectorAll('[role="option"]')].find( + (o) => o.textContent?.trim() === "CEO", + ) as HTMLElement; + await act(async () => { + ceo.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + await clickByText((t) => t.startsWith("Next")); + expect(document.body.textContent).toContain("Connect a model"); + + const connect = [...document.body.querySelectorAll("button")].find((b) => + b.textContent?.trim().startsWith("Connect"), + )!; + await act(async () => { + connect.dispatchEvent(new MouseEvent("click", { bubbles: true })); + connect.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + await act(async () => resolveHire({ agent: { id: "agent-1" }, approval: null })); + await flushReact(); + + expect(mockAgentsApi.hire).toHaveBeenCalledTimes(1); + + await act(async () => root.unmount()); + }); + it("creates one company for one keystroke, modifier or not", async () => { // The name field handles Enter itself and does not check for a modifier, // so Cmd+Enter in that field reaches the field's handler *and* the diff --git a/ui/src/components/OnboardingWizard.tsx b/ui/src/components/OnboardingWizard.tsx index 146fcf5cb7..1f06aec913 100644 --- a/ui/src/components/OnboardingWizard.tsx +++ b/ui/src/components/OnboardingWizard.tsx @@ -461,6 +461,11 @@ function OnboardingWizardInner({ // either, because it is not set until the request it guards has resolved. A // ref is written before the request goes out, so the second caller sees it. const creatingCompanyRef = useRef(false); + // Same shape for the hire. Greptile (round-3 PR): with "Test now" gone the + // Connect handler re-runs a cached failed probe — and two overlapping + // submissions could then both pass the fresh probe and both hire. `loading` + // cannot stop the second caller for the same reason as above. + const hiringAgentRef = useRef(false); createdCompanyIdRef.current = createdCompanyId; // The mission of the company actually in hand, which is not always the one @@ -1256,6 +1261,8 @@ function OnboardingWizardInner({ setStep(5); return; } + if (hiringAgentRef.current) return; + hiringAgentRef.current = true; setLoading(true); setError(null); try { @@ -1293,7 +1300,13 @@ function OnboardingWizardInner({ } if (isLocalAdapter) { - const result = adapterEnvResult ?? (await runAdapterEnvironmentTest()); + // A cached pass or warn is still good; a cached fail is retried. With + // the "Test now" card gone, this button is the only way to re-probe, + // and reusing a stale fail would lock a customer out of a machine + // they have since fixed. + const cachedUsable = + adapterEnvResult && adapterEnvResult.status !== "fail" ? adapterEnvResult : null; + const result = cachedUsable ?? (await runAdapterEnvironmentTest()); if (!result) return; // Block the hire on a failed environment test. A pass or a warn may // proceed; a fail means the agent cannot run as configured. @@ -1366,6 +1379,7 @@ function OnboardingWizardInner({ } catch (err) { setError(err instanceof Error ? err.message : "Failed to create agent"); } finally { + hiringAgentRef.current = false; setLoading(false); } } @@ -2232,29 +2246,15 @@ function OnboardingWizardInner({ )} - {isLocalAdapter && ( + {/* The environment check runs without being shown: Connect + probes the adapter before hiring (see handleGiveHeartbeat) + and blocks the hire on a fail. The idle card — probe + explainer plus a "Test now" button — is gone from this + step, so this block renders only when a probe has actually + found something: the checks the blocking error tells the + customer to fix have to be visible somewhere. */} + {isLocalAdapter && (adapterEnvError || (adapterEnvResult && adapterEnvResult.status !== "pass")) && (
-
-
-

- Adapter environment check -

-

- Runs a live probe that asks the adapter CLI to - respond with hello. -

-
- -
- {adapterEnvError && (
{adapterEnvError} @@ -2397,9 +2397,11 @@ function OnboardingWizardInner({
{/* Review checklist — everything that's now set up */}
+ {/* No "Mission" row: onboarding stopped asking for one, so a + checklist item for it could only ever render unchecked — + a permanent red mark for a question nobody was asked. */} {[ { label: "Organization name", done: Boolean(companyName.trim()) }, - { label: "Mission", done: Boolean(companyGoal.trim()) }, { label: "Agent created", done: Boolean(createdAgentId) }, { label: "Model connected", done: Boolean(createdAgentId) }, ].map(({ label, done }) => ( diff --git a/ui/src/components/PaperclipLockup.tsx b/ui/src/components/PaperclipLockup.tsx new file mode 100644 index 0000000000..80b5d0689a --- /dev/null +++ b/ui/src/components/PaperclipLockup.tsx @@ -0,0 +1,47 @@ +import type { SVGProps } from "react"; + +interface PaperclipLockupProps extends Omit, "children"> { + decorative?: boolean; + title?: string; +} + +/** + * The full Paperclip lockup — mark plus wordmark — from the brand asset + * (`paperclip_lockup_dark_mode alt.svg`). The export is white-filled for dark + * chrome; here every path fills `currentColor`, so the one geometry follows + * the theme the way the brand system expects ("one vocabulary, two surfaces"). + * The viewBox is cropped to the artwork's content bounds — the export carries + * stage padding that would render the lockup at a third of its container. + * + * Size it with a height class (`h-5 w-auto`); width follows the aspect. + */ +export function PaperclipLockup({ + decorative = false, + title = "Paperclip", + className, + ...rest +}: PaperclipLockupProps) { + return ( + + + + + + + + + + + + + ); +} diff --git a/ui/src/pages/Auth.tsx b/ui/src/pages/Auth.tsx index bbd4707a4a..d16592949d 100644 --- a/ui/src/pages/Auth.tsx +++ b/ui/src/pages/Auth.tsx @@ -8,7 +8,7 @@ import { Button } from "@/components/ui/button"; import { AsciiArtAnimation } from "@/components/AsciiArtAnimation"; import { PaperclipLoading } from "@/components/AnimatedPaperclipIcon"; import { ThemeToggle } from "@/components/ThemeToggle"; -import { Sparkles } from "lucide-react"; +import { PaperclipLockup } from "../components/PaperclipLockup"; type AuthMode = "sign_in" | "sign_up"; @@ -88,9 +88,8 @@ export function AuthPage() { {/* Left half — form */}
-
- - Paperclip +
+