mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-06 10:48:12 +02:00
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 <noreply@paperclip.ing>
This commit is contained in:
1 parent
9064cfd09e
commit
8c89340444
4 files changed
+202
-5
No files matched your search
@@ -16,6 +16,28 @@ test("the exact published canary installs and reaches Connect a model", async ({
|
|||||||
const pageErrors: string[] = [];
|
const pageErrors: string[] = [];
|
||||||
page.on("pageerror", (error) => pageErrors.push(error.message));
|
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<{
|
const health = await getJson<{
|
||||||
status: string;
|
status: string;
|
||||||
version: 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?" }),
|
page.getByRole("heading", { name: "What is the name of your organization?" }),
|
||||||
).toBeVisible();
|
).toBeVisible();
|
||||||
await page.getByRole("textbox").fill(companyName);
|
await page.getByRole("textbox").fill(companyName);
|
||||||
|
delayCompanyListRefetch = true;
|
||||||
await page.getByRole("button", { name: "Continue", exact: true }).click();
|
await page.getByRole("button", { name: "Continue", exact: true }).click();
|
||||||
|
|
||||||
const agentNameField = page.locator("#onboarding-agent-name");
|
const agentNameField = page.locator("#onboarding-agent-name");
|
||||||
|
|||||||
@@ -29,6 +29,26 @@ test.describe("Onboarding wizard", () => {
|
|||||||
const pageErrors: string[] = [];
|
const pageErrors: string[] = [];
|
||||||
page.on("pageerror", (err) => pageErrors.push(err.message));
|
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
|
// New-NUX surfaces are flag-gated default-OFF (PAP-136/137/138): turn the
|
||||||
// experimental flag on for this throwaway instance before driving them.
|
// experimental flag on for this throwaway instance before driving them.
|
||||||
const flagRes = await page.request.patch("/api/instance/settings/experimental", {
|
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?" }),
|
page.getByRole("heading", { name: "What is the name of your organization?" }),
|
||||||
).toBeVisible({ timeout: 15_000 });
|
).toBeVisible({ timeout: 15_000 });
|
||||||
await page.getByPlaceholder("e.g. Northwind Labs").fill(COMPANY_NAME);
|
await page.getByPlaceholder("e.g. Northwind Labs").fill(COMPANY_NAME);
|
||||||
|
delayCompanyListRefetch = true;
|
||||||
await page.getByRole("button", { name: /^Continue/ }).click();
|
await page.getByRole("button", { name: /^Continue/ }).click();
|
||||||
|
|
||||||
// Step 1's "Next" now creates the company and goes straight to the agent.
|
// Step 1's "Next" now creates the company and goes straight to the agent.
|
||||||
|
|||||||
@@ -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<Array<{ id: string; name: string; issuePrefix: string }>>((resolve) => {
|
||||||
|
resolveRefetch = resolve;
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
mockCompaniesApi.create.mockResolvedValue({
|
||||||
|
id: "created",
|
||||||
|
name: "Created Co",
|
||||||
|
issuePrefix: "CRE",
|
||||||
|
});
|
||||||
|
|
||||||
|
const { root, queryClient } = render();
|
||||||
|
await act(async () => {
|
||||||
|
root.render(
|
||||||
|
<QueryClientProvider client={queryClient}>
|
||||||
|
<OnboardingWizard />
|
||||||
|
</QueryClientProvider>,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
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<Array<{ id: string; name: string; issuePrefix: string }>>((resolve) => {
|
||||||
|
resolveRetry = resolve;
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
|
||||||
|
const { root, queryClient } = render();
|
||||||
|
await act(async () => {
|
||||||
|
root.render(
|
||||||
|
<QueryClientProvider client={queryClient}>
|
||||||
|
<OnboardingWizard />
|
||||||
|
</QueryClientProvider>,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
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 () => {
|
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,
|
// The actual vulnerability this fix closes: localStorage is per-origin,
|
||||||
// not per-account, so a browser that already onboarded "company-old" for
|
// not per-account, so a browser that already onboarded "company-old" for
|
||||||
|
|||||||
@@ -332,6 +332,19 @@ export function OnboardingWizard() {
|
|||||||
return null; // malformed: treated as stale below
|
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
|
// Whether this account owns the company the draft names is an authorization
|
||||||
// question, and the answer has to be about the account asking now.
|
// question, and the answer has to be about the account asking now.
|
||||||
@@ -404,10 +417,11 @@ export function OnboardingWizard() {
|
|||||||
onboardingDraftStorage.clear();
|
onboardingDraftStorage.clear();
|
||||||
}, [staleStateDetected]);
|
}, [staleStateDetected]);
|
||||||
|
|
||||||
// A saved blob exists and the verification fetch is still in flight: wait,
|
// A saved blob exists and its *initial* verification fetch is still in
|
||||||
// rather than mount the inner wizard with a premature and unrecoverable
|
// flight: wait, rather than mount the inner wizard with a premature and
|
||||||
// guess at the draft. Its ~20 `useState(saved?.x ?? default)` initializers
|
// unrecoverable guess at the draft. Its ~20 `useState(saved?.x ?? default)`
|
||||||
// only read `saved` once.
|
// 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
|
// `isFetching`, not `isLoading`. `isLoading` is false whenever the cache
|
||||||
// holds retained data, so a refetch over a warm cache would mount the wizard
|
// 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
|
// it is itself gated on `effectiveOnboardingOpen`, so a mounted-but-closed
|
||||||
// wizard writes nothing. If the wizard is open the customer is onboarding
|
// wizard writes nothing. If the wizard is open the customer is onboarding
|
||||||
// right now, which supersedes the draft anyway.
|
// 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;
|
return null;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in new issue
Block a user