From 8c89340444cfcf7438ac81c211d243f189cedd1d Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Wed, 2 Sep 2026 16:50:15 -0500 Subject: [PATCH] fix(onboarding): preserve draft through company refetch (#12735) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Onboarding creates an organization in the browser. > - The browser keeps onboarding drafts for the same origin. > - A new data directory does not clear that browser data. > - The organization create request refreshes the company list. > - The old gate unmounted the live wizard during that refresh. > - This pull request keeps the wizard mounted after its first draft check. > - The customer can continue to the agent step after the organization is created. ## Linked Issues or Issue Description No matching public issue was found. Related earlier fix: Refs #12667. **What happened?** A local canary install could create an organization through the API and then return the browser to an empty organization-name screen. **Expected behavior** The wizard must continue to the agent step after it creates the organization. **Steps to reproduce** 1. Keep a Paperclip onboarding draft in the browser. 2. Run npx paperclipai@canary onboard with a new data directory. 3. Open /onboarding. 4. Enter an organization name and select Continue. **Paperclip version or commit** 2026.902.0-canary.7. The fix is based on current master. **Deployment mode** Local trusted mode through the Paperclip CLI. **Install method** npx package install. **Agent adapter(s) involved** Not adapter-specific. **Database mode** Embedded PostgreSQL. ## What Changed - Keep the onboarding wizard mounted after its first successful draft ownership check. - Keep a failed ownership check retryable, so a later verified fetch restores the saved draft. - Add component, source E2E, and published-canary coverage for the retained-draft refetch case. ## Verification - Confirmed that the new canary scenario fails against 2026.902.0-canary.7 before this fix. - pnpm exec vitest run ui/src/components/OnboardingWizard.test.tsx - PAPERCLIP_E2E_PORT=3245 pnpm exec playwright test --config tests/e2e/playwright.config.ts tests/e2e/onboarding.spec.ts --reporter=line - pnpm --filter @paperclipai/ui typecheck - pnpm check:token-gates ## Risks Low risk. The initial ownership check still waits for a fresh company list. A later successful retry can restore a retained draft. Later background refetches preserve live wizard state. ## Model Used OpenAI Codex, GPT-5. Reasoning, tool use, code editing, terminal execution, and browser testing were used. The execution environment does not expose a context-window size. ## 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 described the issue in-PR following the bug issue template - [x] I have not referenced internal or instance-local Paperclip issues or links - [x] My branch name describes the change 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 - [x] All Paperclip CI gates are green - [x] 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: Paperclip --- .../canary-onboarding.spec.ts | 23 ++++ tests/e2e/onboarding.spec.ts | 21 +++ ui/src/components/OnboardingWizard.test.tsx | 126 ++++++++++++++++++ ui/src/components/OnboardingWizard.tsx | 37 ++++- 4 files changed, 202 insertions(+), 5 deletions(-) diff --git a/tests/canary-onboarding/canary-onboarding.spec.ts b/tests/canary-onboarding/canary-onboarding.spec.ts index 31fd25be21..0ff4a784c6 100644 --- a/tests/canary-onboarding/canary-onboarding.spec.ts +++ b/tests/canary-onboarding/canary-onboarding.spec.ts @@ -16,6 +16,28 @@ test("the exact published canary installs and reaches Connect a model", async ({ const pageErrors: string[] = []; page.on("pageerror", (error) => pageErrors.push(error.message)); + // Browser storage is scoped to the Paperclip origin, not to a data directory. + // A customer can therefore start a freshly installed server with an existing + // onboarding draft. Creating the organization invalidates the company list; + // this release check must prove that a refetch does not remount the wizard + // from that old draft and leave the customer on the name screen. + await page.addInitScript(() => { + localStorage.setItem("paperclip-onboarding-state", JSON.stringify({ + step: 1, + companyName: "", + createdCompanyId: null, + })); + }); + let delayCompanyListRefetch = false; + await page.route("**/api/companies", async (route) => { + if (delayCompanyListRefetch && route.request().method() === "GET") { + // Make the invalidation window observable. The previous implementation + // unmounted the live wizard for this entire request. + await new Promise((resolve) => setTimeout(resolve, 250)); + } + await route.continue(); + }); + const health = await getJson<{ status: string; version: string; @@ -30,6 +52,7 @@ test("the exact published canary installs and reaches Connect a model", async ({ page.getByRole("heading", { name: "What is the name of your organization?" }), ).toBeVisible(); await page.getByRole("textbox").fill(companyName); + delayCompanyListRefetch = true; await page.getByRole("button", { name: "Continue", exact: true }).click(); const agentNameField = page.locator("#onboarding-agent-name"); diff --git a/tests/e2e/onboarding.spec.ts b/tests/e2e/onboarding.spec.ts index f8094a4464..f6db6b766d 100644 --- a/tests/e2e/onboarding.spec.ts +++ b/tests/e2e/onboarding.spec.ts @@ -29,6 +29,26 @@ test.describe("Onboarding wizard", () => { const pageErrors: string[] = []; page.on("pageerror", (err) => pageErrors.push(err.message)); + // `--data-dir` starts a new server, not a new browser profile. Keep a + // resumable draft here so this ordinary browser condition is covered when + // the company-list invalidation runs after the create request. + await page.addInitScript(() => { + localStorage.setItem("paperclip-onboarding-state", JSON.stringify({ + step: 1, + companyName: "", + createdCompanyId: null, + })); + }); + let delayCompanyListRefetch = false; + await page.route("**/api/companies", async (route) => { + if (delayCompanyListRefetch && route.request().method() === "GET") { + // Keep the invalidation observable: a background fetch must not reset + // the in-progress wizard. + await new Promise((resolve) => setTimeout(resolve, 250)); + } + await route.continue(); + }); + // New-NUX surfaces are flag-gated default-OFF (PAP-136/137/138): turn the // experimental flag on for this throwaway instance before driving them. const flagRes = await page.request.patch("/api/instance/settings/experimental", { @@ -56,6 +76,7 @@ test.describe("Onboarding wizard", () => { page.getByRole("heading", { name: "What is the name of your organization?" }), ).toBeVisible({ timeout: 15_000 }); await page.getByPlaceholder("e.g. Northwind Labs").fill(COMPANY_NAME); + delayCompanyListRefetch = true; await page.getByRole("button", { name: /^Continue/ }).click(); // Step 1's "Next" now creates the company and goes straight to the agent. diff --git a/ui/src/components/OnboardingWizard.test.tsx b/ui/src/components/OnboardingWizard.test.tsx index add0e145d8..6497046447 100644 --- a/ui/src/components/OnboardingWizard.test.tsx +++ b/ui/src/components/OnboardingWizard.test.tsx @@ -1196,6 +1196,132 @@ describe("OnboardingWizard restore-gate (stale localStorage across accounts)", ( }); }); + it("keeps the live wizard mounted while a post-create company-list refetch runs", async () => { + // The authorization fetch needs to delay the first mount when a draft + // exists. Once the customer has typed, though, invalidating that query is + // normal background work. Unmounting for the refetch remounted the wizard + // from this page-load draft and made a successful create look like a reset. + window.localStorage.setItem( + ONBOARDING_STORAGE_KEY, + JSON.stringify({ step: 1, companyName: "", createdCompanyId: null }), + ); + let resolveRefetch: (companies: Array<{ id: string; name: string; issuePrefix: string }>) => void = + () => {}; + mockCompaniesApi.list + .mockResolvedValueOnce([]) + .mockImplementationOnce( + () => + new Promise>((resolve) => { + resolveRefetch = resolve; + }), + ); + mockCompaniesApi.create.mockResolvedValue({ + id: "created", + name: "Created Co", + issuePrefix: "CRE", + }); + + const { root, queryClient } = render(); + await act(async () => { + root.render( + + + , + ); + }); + await flushReact(); + + const companyNameInput = document.querySelector("input") as HTMLInputElement; + setControlledValue(companyNameInput, "Created Co"); + await act(async () => { + [...document.body.querySelectorAll("button")] + .find((button) => button.textContent?.trim() === "Continue")! + .dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + + expect(mockCompaniesApi.create).toHaveBeenCalledWith({ name: "Created Co" }); + expect(mockCompaniesApi.list).toHaveBeenCalledTimes(2); + expect(document.querySelector("#onboarding-agent-name")).not.toBeNull(); + + await act(async () => { + resolveRefetch([{ id: "created", name: "Created Co", issuePrefix: "CRE" }]); + }); + await act(async () => { + root.unmount(); + }); + }); + + it("restores a draft after its initial ownership check fails and a retry succeeds", async () => { + // A failed first request cannot authorize the draft, so the wizard opens + // with safe defaults. The original gate remounted on a later successful + // retry so the now-verified draft could be restored; keep that recovery + // while preserving the post-create refetch fix above. + window.localStorage.setItem( + ONBOARDING_STORAGE_KEY, + JSON.stringify({ + step: 3, + companyName: "Saved Co", + agentName: "Ops Lead", + createdCompanyId: "c1", + }), + ); + const company = { id: "c1", name: "Saved Co", issuePrefix: "SC" }; + let resolveRetry: (companies: Array<{ id: string; name: string; issuePrefix: string }>) => void = + () => {}; + mockCompaniesApi.list + .mockRejectedValueOnce(new Error("company list unavailable")) + .mockImplementationOnce( + () => + new Promise>((resolve) => { + resolveRetry = resolve; + }), + ); + + const { root, queryClient } = render(); + await act(async () => { + root.render( + + + , + ); + }); + await flushReact(); + + expect(document.querySelector("#onboarding-agent-name")).toBeNull(); + + mockCompany.companies = [company]; + await act(async () => { + void queryClient.invalidateQueries({ + queryKey: queryKeys.companies.list(SESSION_USER_ID), + }); + }); + await flushReact(); + + // The retry is intentionally still in flight. The wrapper removes the + // safe-default wizard so a completed validation can mount the saved draft. + expect(document.body.textContent).toBe(""); + + await act(async () => { + resolveRetry([company]); + }); + await flushReact(); + + expect(mockCompaniesApi.list).toHaveBeenCalledTimes(2); + expect(queryClient.getQueryData(queryKeys.companies.list(SESSION_USER_ID))).toEqual({ + companies: [company], + unauthorized: false, + }); + const agentNameInput = document.querySelector( + "#onboarding-agent-name", + ) as HTMLInputElement | null; + expect(agentNameInput?.value).toBe("Ops Lead"); + + await act(async () => { + root.unmount(); + }); + }); + it("discards a saved draft for a company the signed-in account does not own, and wipes the stale blob", async () => { // The actual vulnerability this fix closes: localStorage is per-origin, // not per-account, so a browser that already onboarded "company-old" for diff --git a/ui/src/components/OnboardingWizard.tsx b/ui/src/components/OnboardingWizard.tsx index 26f6df8e68..09dc2cf6e8 100644 --- a/ui/src/components/OnboardingWizard.tsx +++ b/ui/src/components/OnboardingWizard.tsx @@ -332,6 +332,19 @@ export function OnboardingWizard() { return null; // malformed: treated as stale below } }, []); + // The ownership gate is closed after the initial validation succeeds, or + // when no validation is needed. A later company-list invalidation is + // ordinary background work. If it unmounted the inner wizard then, all of + // its live useState values would be reconstructed from `rawBlob` above — + // the value from page load, not the draft the customer just typed — and a + // successful organization submission would appear to do nothing. + // + // A failed validation must remain retryable. A later successful fetch needs + // to remount the wizard with the now-authorized draft, rather than keeping + // the defaults it showed while ownership was unknown. + const [initialDraftValidationComplete, setInitialDraftValidationComplete] = useState( + rawBlob === undefined || rawBlob === null, + ); // Whether this account owns the company the draft names is an authorization // question, and the answer has to be about the account asking now. @@ -404,10 +417,11 @@ export function OnboardingWizard() { onboardingDraftStorage.clear(); }, [staleStateDetected]); - // A saved blob exists and the verification fetch is still in flight: wait, - // rather than mount the inner wizard with a premature and unrecoverable - // guess at the draft. Its ~20 `useState(saved?.x ?? default)` initializers - // only read `saved` once. + // A saved blob exists and its *initial* verification fetch is still in + // flight: wait, rather than mount the inner wizard with a premature and + // unrecoverable guess at the draft. Its ~20 `useState(saved?.x ?? default)` + // initializers only read `saved` once. After that first mount this is a + // background refetch, which must not tear down the customer's live state. // // `isFetching`, not `isLoading`. `isLoading` is false whenever the cache // holds retained data, so a refetch over a warm cache would mount the wizard @@ -424,7 +438,20 @@ export function OnboardingWizard() { // it is itself gated on `effectiveOnboardingOpen`, so a mounted-but-closed // wizard writes nothing. If the wizard is open the customer is onboarding // right now, which supersedes the draft anyway. - if (rawBlob !== undefined && companiesQuery.isFetching) { + const waitForInitialDraftValidation = + !initialDraftValidationComplete && rawBlob !== undefined && companiesQuery.isFetching; + + useEffect(() => { + if ( + !initialDraftValidationComplete && + ownershipDecidable && + !companiesQuery.isFetching + ) { + setInitialDraftValidationComplete(true); + } + }, [initialDraftValidationComplete, ownershipDecidable, companiesQuery.isFetching]); + + if (waitForInitialDraftValidation) { return null; }