From dda4dff645da9eebe532cf9c59d6c5debdf05070 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Tue, 1 Sep 2026 10:10:30 -0500 Subject: [PATCH] fix(onboarding): restore browser launch and gate canaries (#12667) 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. > - The onboarding command starts the local server and opens the first-run wizard. > - Interactive onboarding stopped opening the browser by default. > - Organization creation could also succeed in the API while the wizard stayed on the name step. > - The npm canary workflow did not prove that the published package could complete this path. > - This pull request repairs the startup and organization transitions and adds an exact-version canary smoke gate. > - The benefit is a quickstart that works for users and is tested after each canary publish. ## Linked Issues or Issue Description Related: #12557 covers a separate final-route onboarding handoff. **What happened?** Interactive `paperclipai onboard` runs did not open the onboarding page. The organization API request could succeed while a same-company context update caused the wizard to stay on the organization step. The canary release lane did not test the exact published npm package through this path. **Expected behavior** Interactive onboarding must open the browser once. A successful organization request must advance to the first-agent step when the surrounding context adopts the same organization. Each published canary must install in a clean environment and reach the model connection step. **Steps to reproduce** 1. Run `npx paperclipai@canary onboard --data-dir "$(mktemp -d /tmp/paperclip-canary.XXXXXX)"` in an interactive terminal. 2. Enter an organization name while the company context refreshes from the create response. 3. Observe that the browser does not open or that the wizard can remain on the organization step after the API creates it. 4. Inspect the canary release lane and observe that no post-publish onboarding test runs against the exact npm version. **Paperclip version or commit** The issue reproduced with `2026.901.0-canary.8` and the source state before this pull request. **Deployment mode** Local trusted quickstart with embedded PostgreSQL. The install source can be npm or a source checkout. ## What Changed - Open the browser once for interactive foreground onboarding. - Preserve explicit browser opt-outs and restore the prior environment value after startup. - Accept a same-company context update after organization creation and reject a different-company takeover with an explicit error. - Export the exact canary version from the publish job. - Install and test that exact npm version in a clean Playwright smoke job through the "Connect a model" step. - Upload server logs, traces, screenshots, and the Playwright report when the canary smoke fails. - Document the interactive default and headless opt-outs. ## Verification - `pnpm exec vitest run cli/src/__tests__/onboard.test.ts ui/src/components/OnboardingWizard.step.test.tsx --reporter=dot` passes with 37 tests. - `node --test scripts/__tests__/release-verify-workflow.test.mjs` passes with 9 tests. - `pnpm exec playwright test --config tests/e2e/playwright.config.ts tests/e2e/onboarding.spec.ts` passes with 2 tests. - `PAPERCLIPAI_VERSION=2026.901.0-canary.8 pnpm run test:canary-onboarding-smoke` passes against the published npm package. - `pnpm check:token-gates` passes. - `pnpm -r typecheck` passes. - `pnpm build` passes. - A fresh interactive source run opens the browser and reaches "Connect a model" after organization and agent naming. ## Risks - Low risk. Automatic browser opening only applies to interactive foreground onboarding. - `PAPERCLIP_NO_BROWSER=1` and `PAPERCLIP_OPEN_ON_LISTEN=false` keep headless runs silent. - A different organization context still blocks the pending create transition. - The canary package is immutable before the smoke runs. A smoke failure leaves the package published but makes the release workflow red. - This change does not modify REST APIs, database schemas, or shared data types. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - OpenAI Codex based on GPT-5. The runtime does not expose the exact deployment snapshot or context-window size. The model used reasoning, browser automation, repository tools, shell commands, code editing, and test execution. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [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 --- .github/workflows/release.yml | 55 +++++++++++ cli/src/__tests__/onboard.test.ts | 53 ++++++++++- cli/src/commands/onboard.ts | 33 ++++++- docs/cli/setup-commands.md | 10 +- package.json | 1 + .../release-verify-workflow.test.mjs | 40 ++++++++ tests/canary-onboarding/.gitignore | 2 + .../canary-onboarding.spec.ts | 78 ++++++++++++++++ tests/canary-onboarding/playwright.config.ts | 93 +++++++++++++++++++ .../components/OnboardingWizard.step.test.tsx | 44 +++++++++ ui/src/components/OnboardingWizard.tsx | 26 +++++- 11 files changed, 422 insertions(+), 13 deletions(-) create mode 100644 tests/canary-onboarding/.gitignore create mode 100644 tests/canary-onboarding/canary-onboarding.spec.ts create mode 100644 tests/canary-onboarding/playwright.config.ts diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 1f0511e051..3621bc98c9 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -76,6 +76,8 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 90 environment: npm-canary + outputs: + canary_version: ${{ steps.canary_tag.outputs.version }} permissions: contents: write id-token: write @@ -127,6 +129,7 @@ jobs: done - name: Push canary tag + id: canary_tag run: | tag="$(git tag --points-at HEAD | grep '^canary/v' | head -1)" if [ -z "$tag" ]; then @@ -134,6 +137,58 @@ jobs: exit 1 fi git push origin "refs/tags/${tag}" + echo "version=${tag#canary/v}" >> "$GITHUB_OUTPUT" + + # The package is already public when this gate runs. A red result leaves the + # immutable canary in npm, but makes the release workflow visibly fail before + # anyone mistakes an installable package for an onboardable one. + smoke_canary_onboarding: + needs: publish_canary + if: needs.publish_canary.result == 'success' + runs-on: ubuntu-latest + timeout-minutes: 20 + permissions: + contents: read + env: + PAPERCLIPAI_VERSION: ${{ needs.publish_canary.outputs.canary_version }} + PAPERCLIP_PLAYWRIGHT_CHANNEL: chrome + + steps: + - name: Checkout repository + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + + - name: Setup pnpm + uses: pnpm/action-setup@0977fd99725f1db4007ccb2928dbb4e90d06cc86 # v6 + with: + version: 9.15.4 + + - name: Setup Node.js + uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7 + with: + node-version: 24 + + - name: Install test dependencies + run: pnpm install --frozen-lockfile + + - name: Show browser version + run: google-chrome --version + + - name: Smoke exact published canary through onboarding + env: + PAPERCLIP_CANARY_SMOKE_SERVER_LOG: ${{ runner.temp }}/canary-onboarding-server.log + run: pnpm run test:canary-onboarding-smoke + + - name: Upload failed canary onboarding diagnostics + if: failure() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + with: + name: canary-onboarding-smoke-${{ needs.publish_canary.outputs.canary_version }} + if-no-files-found: warn + retention-days: 14 + path: | + ${{ runner.temp }}/canary-onboarding-server.log + tests/canary-onboarding/playwright-report/ + tests/canary-onboarding/test-results/ # ----- Nightly lane ----------------------------------------------------- # Once a night (or on a forced nightly dispatch), promote the newest master diff --git a/cli/src/__tests__/onboard.test.ts b/cli/src/__tests__/onboard.test.ts index 6aa033e251..6a77d75d4a 100644 --- a/cli/src/__tests__/onboard.test.ts +++ b/cli/src/__tests__/onboard.test.ts @@ -109,6 +109,7 @@ describe("onboard", () => { delete process.env.PAPERCLIP_BIND_HOST; delete process.env.PAPERCLIP_TAILNET_BIND_HOST; delete process.env.PAPERCLIP_OPEN_ON_LISTEN; + delete process.env.PAPERCLIP_NO_BROWSER; delete process.env.HOST; runCommandMock.mockReset(); }); @@ -139,7 +140,7 @@ describe("onboard", () => { expect(fs.existsSync(path.join(path.dirname(fixture.configPath), ".env"))).toBe(true); }); - it("does not opt into opening a browser when --yes starts an existing setup", async () => { + it("does not opt into opening a browser for a non-interactive existing setup", async () => { const fixture = createExistingConfigFixture(); await onboard({ config: fixture.configPath, yes: true }); @@ -148,15 +149,59 @@ describe("onboard", () => { expect(process.env.PAPERCLIP_OPEN_ON_LISTEN).toBeUndefined(); }); - it("does not opt into opening a browser when --yes starts a fresh setup", async () => { - const configPath = createFreshConfigPath(); + it.each([ + ["existing", () => createExistingConfigFixture().configPath], + ["fresh", () => createFreshConfigPath()], + ])("opens the browser once while an interactive %s setup starts", async (_label, configPathForTest) => { + const configPath = configPathForTest(); + const stdinIsTTY = process.stdin.isTTY; + const stdoutIsTTY = process.stdout.isTTY; + let openOnListenDuringRun: string | undefined; + Object.defineProperty(process.stdin, "isTTY", { configurable: true, value: true }); + Object.defineProperty(process.stdout, "isTTY", { configurable: true, value: true }); + runCommandMock.mockImplementation(async () => { + openOnListenDuringRun = process.env.PAPERCLIP_OPEN_ON_LISTEN; + }); - await onboard({ config: configPath, yes: true }); + try { + await onboard({ config: configPath, yes: true }); + } finally { + Object.defineProperty(process.stdin, "isTTY", { configurable: true, value: stdinIsTTY }); + Object.defineProperty(process.stdout, "isTTY", { configurable: true, value: stdoutIsTTY }); + } expect(runCommandMock).toHaveBeenCalledWith({ config: configPath, repair: true, yes: true }); + expect(openOnListenDuringRun).toBe("true"); expect(process.env.PAPERCLIP_OPEN_ON_LISTEN).toBeUndefined(); }); + it.each([ + ["PAPERCLIP_NO_BROWSER", "1"], + ["PAPERCLIP_OPEN_ON_LISTEN", "false"], + ])("respects the interactive browser opt-out %s", async (key, value) => { + const configPath = createFreshConfigPath(); + const stdinIsTTY = process.stdin.isTTY; + const stdoutIsTTY = process.stdout.isTTY; + let openOnListenDuringRun: string | undefined; + Object.defineProperty(process.stdin, "isTTY", { configurable: true, value: true }); + Object.defineProperty(process.stdout, "isTTY", { configurable: true, value: true }); + process.env[key] = value; + runCommandMock.mockImplementation(async () => { + openOnListenDuringRun = process.env.PAPERCLIP_OPEN_ON_LISTEN; + }); + + try { + await onboard({ config: configPath, yes: true }); + } finally { + Object.defineProperty(process.stdin, "isTTY", { configurable: true, value: stdinIsTTY }); + Object.defineProperty(process.stdout, "isTTY", { configurable: true, value: stdoutIsTTY }); + } + + expect(runCommandMock).toHaveBeenCalledWith({ config: configPath, repair: true, yes: true }); + expect(openOnListenDuringRun).not.toBe("true"); + expect(process.env[key]).toBe(value); + }); + it("backs up invalid config bytes and refuses --yes replacement", async () => { const configPath = createFreshConfigPath(); const invalidBytes = Buffer.from('{"database": invalid}\n', "utf8"); diff --git a/cli/src/commands/onboard.ts b/cli/src/commands/onboard.ts index f6e4be7e2c..a950d48b4d 100644 --- a/cli/src/commands/onboard.ts +++ b/cli/src/commands/onboard.ts @@ -115,6 +115,33 @@ function parseBooleanFromEnv(rawValue: string | undefined): boolean | null { return null; } +async function runOnboardedForeground(configPath: string): Promise { + const previousOpenOnListen = process.env.PAPERCLIP_OPEN_ON_LISTEN; + const browserDisabled = parseBooleanFromEnv(process.env.PAPERCLIP_NO_BROWSER) === true; + const interactive = Boolean(process.stdin.isTTY && process.stdout.isTTY); + + // The server consumes this flag in its listen callback. Keep it scoped to + // this foreground start so a later in-process restart does not open another + // tab. Explicit configuration wins over the interactive default, while the + // broad no-browser switch wins over an earlier explicit opt-in. + if (browserDisabled) { + process.env.PAPERCLIP_OPEN_ON_LISTEN = "false"; + } else if (interactive && previousOpenOnListen === undefined) { + process.env.PAPERCLIP_OPEN_ON_LISTEN = "true"; + } + + try { + const { runCommand } = await import("./run.js"); + await runCommand({ config: configPath, repair: true, yes: true }); + } finally { + if (previousOpenOnListen === undefined) { + delete process.env.PAPERCLIP_OPEN_ON_LISTEN; + } else { + process.env.PAPERCLIP_OPEN_ON_LISTEN = previousOpenOnListen; + } + } +} + function parseNumberFromEnv(rawValue: string | undefined): number | null { if (!rawValue) return null; const parsed = Number(rawValue); @@ -477,8 +504,7 @@ export async function onboard(opts: OnboardOptions): Promise { } if (shouldRunNow && !opts.invokedByRun) { - const { runCommand } = await import("./run.js"); - await runCommand({ config: configPath, repair: true, yes: true }); + await runOnboardedForeground(configPath); return; } @@ -745,8 +771,7 @@ export async function onboard(opts: OnboardOptions): Promise { } if (shouldRunNow && !opts.invokedByRun) { - const { runCommand } = await import("./run.js"); - await runCommand({ config: configPath, repair: true, yes: true }); + await runOnboardedForeground(configPath); return; } diff --git a/docs/cli/setup-commands.md b/docs/cli/setup-commands.md index ff6a8d070f..cf93845ec6 100644 --- a/docs/cli/setup-commands.md +++ b/docs/cli/setup-commands.md @@ -46,16 +46,20 @@ Start immediately after onboarding: pnpm paperclipai onboard --run ``` -Non-interactive defaults + immediate start (prints the URL without opening a browser): +Quickstart defaults + immediate start: ```sh pnpm paperclipai onboard --yes ``` -Browser opening is opt-in. Set the environment variable explicitly when that is the desired behavior: +When onboarding starts Paperclip from an interactive terminal, it opens the +onboarding page in your browser once. Non-interactive terminals stay silent. +Suppress browser opening explicitly for headless or automated runs with either +environment variable: ```sh -PAPERCLIP_OPEN_ON_LISTEN=true pnpm paperclipai onboard --yes +PAPERCLIP_NO_BROWSER=1 pnpm paperclipai onboard --yes +PAPERCLIP_OPEN_ON_LISTEN=false pnpm paperclipai onboard --yes ``` On an existing install, `--yes` now preserves the current config and just starts Paperclip with that setup. diff --git a/package.json b/package.json index 1e73bb19d3..a265d5bedf 100644 --- a/package.json +++ b/package.json @@ -70,6 +70,7 @@ "evals:smoke": "cd evals/promptfoo && npx promptfoo@0.103.3 eval", "test:release-smoke": "npx playwright test --config tests/release-smoke/playwright.config.ts", "test:release-smoke:headed": "npx playwright test --config tests/release-smoke/playwright.config.ts --headed", + "test:canary-onboarding-smoke": "npx playwright test --config tests/canary-onboarding/playwright.config.ts", "metrics:paperclip-commits": "tsx scripts/paperclip-commit-metrics.ts", "perf:issue-chat-long-thread": "node scripts/measure-issue-chat-long-thread.mjs", "connections:ingest-app-definitions": "node scripts/ingest-app-definitions.mjs" diff --git a/scripts/__tests__/release-verify-workflow.test.mjs b/scripts/__tests__/release-verify-workflow.test.mjs index 7ddc23bf08..a5b33ad768 100644 --- a/scripts/__tests__/release-verify-workflow.test.mjs +++ b/scripts/__tests__/release-verify-workflow.test.mjs @@ -73,6 +73,46 @@ test("post-publish beta smoke survives the skipped candidate-verification ancest ); }); +test("published canaries are gated by the exact-version onboarding browser smoke", () => { + const releaseWorkflow = readWorkflow("release.yml"); + + assert.match( + releaseWorkflow, + /publish_canary:[\s\S]*?outputs:\n\s+canary_version: \$\{\{ steps\.canary_tag\.outputs\.version \}\}/, + ); + assert.match( + releaseWorkflow, + /smoke_canary_onboarding:\n\s+needs: publish_canary\n\s+if: needs\.publish_canary\.result == 'success'/, + ); + assert.match( + releaseWorkflow, + /PAPERCLIPAI_VERSION: \$\{\{ needs\.publish_canary\.outputs\.canary_version \}\}/, + ); + assert.match(releaseWorkflow, /test:canary-onboarding-smoke/); + assert.match( + releaseWorkflow, + /smoke_canary_onboarding:[\s\S]*?uses: actions\/checkout@[0-9a-f]{40} # v7[\s\S]*?uses: pnpm\/action-setup@[0-9a-f]{40} # v6[\s\S]*?uses: actions\/setup-node@[0-9a-f]{40} # v7/, + ); + assert.match( + releaseWorkflow, + /smoke_canary_onboarding:[\s\S]*?Install test dependencies\n\s+run: pnpm install --frozen-lockfile/, + ); + assert.doesNotMatch( + releaseWorkflow.match(/smoke_canary_onboarding:[\s\S]*?(?=\n # ----- Nightly lane)/)?.[0] ?? "", + /cache: pnpm/, + ); + assert.match( + releaseWorkflow, + /name: Smoke exact published canary through onboarding\n\s+env:\n\s+PAPERCLIP_CANARY_SMOKE_SERVER_LOG: \$\{\{ runner\.temp \}\}\/canary-onboarding-server\.log/, + ); + assert.match( + releaseWorkflow, + /smoke_canary_onboarding:[\s\S]*?uses: actions\/upload-artifact@[0-9a-f]{40} # v7/, + ); + assert.match(releaseWorkflow, /canary-onboarding-server\.log/); + assert.match(releaseWorkflow, /tests\/canary-onboarding\/playwright-report/); +}); + test("every lane's tag push degrades to recovery instructions when rejected", () => { const releaseWorkflow = readWorkflow("release.yml"); diff --git a/tests/canary-onboarding/.gitignore b/tests/canary-onboarding/.gitignore new file mode 100644 index 0000000000..5c4ffa21fa --- /dev/null +++ b/tests/canary-onboarding/.gitignore @@ -0,0 +1,2 @@ +playwright-report/ +test-results/ diff --git a/tests/canary-onboarding/canary-onboarding.spec.ts b/tests/canary-onboarding/canary-onboarding.spec.ts new file mode 100644 index 0000000000..31fd25be21 --- /dev/null +++ b/tests/canary-onboarding/canary-onboarding.spec.ts @@ -0,0 +1,78 @@ +import { expect, test, type APIRequestContext } from "@playwright/test"; + +const expectedVersion = process.env.PAPERCLIPAI_VERSION!; +const companyName = `Canary Smoke ${Date.now()}`; +const agentName = "Canary Smoke Lead"; + +async function getJson(request: APIRequestContext, url: string): Promise { + const response = await request.get(url); + expect(response.ok()).toBe(true); + return (await response.json()) as T; +} + +test("the exact published canary installs and reaches Connect a model", async ({ + page, +}) => { + const pageErrors: string[] = []; + page.on("pageerror", (error) => pageErrors.push(error.message)); + + const health = await getJson<{ + status: string; + version: string; + serverVersion: string; + }>(page.request, "/api/health"); + expect(health.status).toBe("ok"); + expect(health.version).toBe(expectedVersion); + expect(health.serverVersion).toBe(expectedVersion); + + await page.goto("/onboarding"); + await expect( + page.getByRole("heading", { name: "What is the name of your organization?" }), + ).toBeVisible(); + await page.getByRole("textbox").fill(companyName); + await page.getByRole("button", { name: "Continue", exact: true }).click(); + + const agentNameField = page.locator("#onboarding-agent-name"); + await expect(agentNameField).toBeVisible({ timeout: 30_000 }); + + const companies = await getJson>( + page.request, + "/api/companies", + ); + const company = companies.find((candidate) => candidate.name === companyName); + expect(company, `organization ${companyName} should exist through the API`).toBeTruthy(); + + expect( + await getJson>( + page.request, + `/api/companies/${company!.id}/agents`, + ), + ).toEqual([]); + expect( + await getJson>( + page.request, + `/api/companies/${company!.id}/issues`, + ), + ).toEqual([]); + + await agentNameField.fill(agentName); + await page.getByRole("button", { name: "Next", exact: true }).click(); + await expect(page.getByRole("heading", { name: "Connect a model" })).toBeVisible(); + + // Stop before Connect: a public-repository gate must not require a Claude or + // Codex credential, and reaching this screen has not hired an agent or made + // a first task. + expect( + await getJson>( + page.request, + `/api/companies/${company!.id}/agents`, + ), + ).toEqual([]); + expect( + await getJson>( + page.request, + `/api/companies/${company!.id}/issues`, + ), + ).toEqual([]); + expect(pageErrors, pageErrors.join("\n")).toEqual([]); +}); diff --git a/tests/canary-onboarding/playwright.config.ts b/tests/canary-onboarding/playwright.config.ts new file mode 100644 index 0000000000..20345b25f7 --- /dev/null +++ b/tests/canary-onboarding/playwright.config.ts @@ -0,0 +1,93 @@ +import { defineConfig } from "@playwright/test"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +const canaryVersion = process.env.PAPERCLIPAI_VERSION?.trim(); +if (!canaryVersion || !/^[0-9A-Za-z.+-]+$/.test(canaryVersion)) { + throw new Error( + "PAPERCLIPAI_VERSION must name the exact published canary version to test", + ); +} + +const baseUrl = + process.env.PAPERCLIP_CANARY_SMOKE_BASE_URL ?? "http://127.0.0.1:3233"; +const parsedBaseUrl = new URL(baseUrl); +if (parsedBaseUrl.hostname !== "127.0.0.1" || !parsedBaseUrl.port) { + throw new Error("PAPERCLIP_CANARY_SMOKE_BASE_URL must use 127.0.0.1 and an explicit port"); +} + +const workspace = fs.mkdtempSync( + path.join(os.tmpdir(), "paperclip-canary-onboarding-smoke-"), +); +const dataDir = path.join(workspace, "data"); +const npmCache = path.join(workspace, "npm-cache"); +fs.mkdirSync(dataDir); +fs.mkdirSync(npmCache); + +const serverLog = + process.env.PAPERCLIP_CANARY_SMOKE_SERVER_LOG ?? + path.join(workspace, "canary-onboarding-server.log"); + +function shellQuote(value: string): string { + return `'${value.replaceAll("'", `'"'"'`)}'`; +} + +const command = [ + "npx", + "--yes", + shellQuote(`paperclipai@${canaryVersion}`), + "onboard", + "--yes", + "--data-dir", + shellQuote(dataDir), + ">", + shellQuote(serverLog), + "2>&1", +].join(" "); + +export default defineConfig({ + testDir: ".", + testMatch: "**/*.spec.ts", + timeout: 90_000, + expect: { + timeout: 20_000, + }, + retries: 0, + workers: 1, + use: { + baseURL: baseUrl, + headless: true, + screenshot: "only-on-failure", + trace: "retain-on-failure", + }, + projects: [ + { + name: "chromium", + use: { + browserName: "chromium", + ...(process.env.PAPERCLIP_PLAYWRIGHT_CHANNEL + ? { channel: process.env.PAPERCLIP_PLAYWRIGHT_CHANNEL } + : {}), + }, + }, + ], + webServer: { + command, + url: `${baseUrl}/api/health`, + reuseExistingServer: false, + timeout: 300_000, + env: { + ...process.env, + PORT: parsedBaseUrl.port, + PAPERCLIP_NO_BROWSER: "1", + PAPERCLIP_OPEN_ON_LISTEN: "false", + npm_config_cache: npmCache, + }, + }, + outputDir: "./test-results", + reporter: [ + ["list"], + ["html", { open: "never", outputFolder: "./playwright-report" }], + ], +}); diff --git a/ui/src/components/OnboardingWizard.step.test.tsx b/ui/src/components/OnboardingWizard.step.test.tsx index fdccbc6fdd..2cd21b29f6 100644 --- a/ui/src/components/OnboardingWizard.step.test.tsx +++ b/ui/src/components/OnboardingWizard.step.test.tsx @@ -736,6 +736,50 @@ describe("OnboardingWizard — which step it lands on", () => { // here rather than read as a pass. expect(currentStep()).toBe("agent"); expect(companyState.setSelectedCompanyId).not.toHaveBeenCalled(); + expect(document.body.textContent).toContain( + "Organization created, but onboarding switched to another organization.", + ); + }); + + it("finishes advancing when the returned company was already adopted", async () => { + // The company-created live update can make the surrounding app adopt the + // returned company before the POST continuation runs. That is not a + // different-company takeover: both signals name the same company, so + // dropping the continuation leaves the customer on the name step even + // though the organization now exists. + let resolveCreate: (company: { id: string; issuePrefix: string }) => void = () => {}; + mockCompaniesApi.create.mockReturnValue( + new Promise<{ id: string; issuePrefix: string }>((resolve) => { + resolveCreate = resolve; + }), + ); + routerState.pathname = "/onboarding"; + await render(); + await settle(); + + const nameInput = document.body.querySelector("input")! as HTMLInputElement; + setControlledValue(nameInput, "Initech"); + await settle(); + const next = [...document.body.querySelectorAll("button")].find( + (button) => button.textContent?.trim() === "Continue", + )!; + await act(async () => { + next.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + + // Model the surrounding app adopting exactly the company that the pending + // request is about, without choosing a new step on the wizard's behalf. + dialogState.onboardingOpen = true; + dialogState.onboardingOptions = { companyId: "company-created" }; + await rerender(); + await settle(); + + await act(async () => resolveCreate({ id: "company-created", issuePrefix: "INI" })); + await settle(); + + expect(currentStep()).toBe("agent"); + expect(companyState.setSelectedCompanyId).toHaveBeenCalledWith("company-created"); + expect(mockCompaniesApi.create).toHaveBeenCalledTimes(1); }); it("applies the step again when the wizard is re-opened", async () => { diff --git a/ui/src/components/OnboardingWizard.tsx b/ui/src/components/OnboardingWizard.tsx index 35f2d90655..e960043176 100644 --- a/ui/src/components/OnboardingWizard.tsx +++ b/ui/src/components/OnboardingWizard.tsx @@ -1093,6 +1093,26 @@ function OnboardingWizardInner({ return createdCompanyIdRef.current === companyIdAtStart; } + /** + * Whether a just-created company can still be committed to this wizard. + * + * Company-list refreshes can make the surrounding app adopt the POST result + * before the continuation runs. That is the same successful transition, not + * a takeover. A different id still means navigation moved the wizard to a + * different organization while the request was in flight. + */ + function canCommitCreatedCompany( + companyIdAtStart: string | null, + returnedCompanyId: string, + ) { + const companyIdNow = createdCompanyIdRef.current; + if (companyIdNow === companyIdAtStart || companyIdNow === returnedCompanyId) { + return true; + } + setError("Organization created, but onboarding switched to another organization."); + return false; + } + async function handleLaunchToDashboard() { if (!createdCompanyId || !createdAgentId) { setError(INCOMPLETE_ONBOARDING_STATE_MESSAGE); @@ -1365,6 +1385,7 @@ function OnboardingWizardInner({ } setLoading(true); setError(null); + const companyIdAtStart = createdCompanyIdRef.current; try { const company = await companiesApi.create({ name: companyName.trim() }); queryClient.invalidateQueries({ queryKey: queryKeys.companies.all }); @@ -1373,7 +1394,7 @@ function OnboardingWizardInner({ // a company while the request was open has taken over the wizard, and // adopting the company just created would fight it — and would leave the // customer on a company they never navigated to. - if (!stillTheSameCompany(null)) return; + if (!canCommitCreatedCompany(companyIdAtStart, company.id)) return; setCreatedCompanyId(company.id); // Keep the mirror current here rather than waiting for the next render. // The goal write below asks `stillTheSameCompany(company.id)`, and a ref @@ -1429,6 +1450,7 @@ function OnboardingWizardInner({ creatingCompanyRef.current = true; setLoading(true); setError(null); + const companyIdAtStart = createdCompanyIdRef.current; try { const company = await companiesApi.create({ name: companyName.trim() }); queryClient.invalidateQueries({ queryKey: queryKeys.companies.all }); @@ -1437,7 +1459,7 @@ function OnboardingWizardInner({ // taken over the wizard, and adopting the company just created would // fight it — and would leave the customer on a company they never // navigated to. - if (!stillTheSameCompany(null)) return; + if (!canCommitCreatedCompany(companyIdAtStart, company.id)) return; setCreatedCompanyId(company.id); // Keep the mirror current rather than waiting for the next render, for // the same reason the mission path does: anything downstream that asks