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