From 9a76390cfa4e58f466fa0c2e0dd5b744058337a5 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Tue, 22 Sep 2026 15:04:13 -0500 Subject: [PATCH] test(runner): improve blank-page diagnostics and infrastructure coverage (#13824) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Runner E2E tests verify real tasks and retain evidence for failures. > - Some exposure tests assumed that port 42000 was free. > - A blank task page could also fail without enough browser startup evidence. > - This pull request tests occupied ports and task reloads, and records private startup diagnostics. > - These changes make test failures easier to reproduce and explain. ## Linked Issues or Issue Description Refs #13815. Report publication already received its production fix in #13750; this PR adds a regression check for that workflow. **What happened?** Three exposure tests assumed that the allocator would select port 42000. The synthetic failure disappeared when another process occupied that port. A prior Daytona run also retained an empty task page after navigation, but its evidence did not record pending modules or service-worker control. **Expected behavior** Exposure tests must exercise the intended failure on the actual assigned port. Browser failure evidence must distinguish an empty root from loaded content. A saved task must remain usable after navigation and reload. **Steps to reproduce** Run the exposure regression with the base port pair marked unavailable. Run the browser-support tests with an unresolved entry module. Open a saved task under the service worker, navigate to the same URL, and reload it. **Paperclip version or commit** Based on master at a68f3d8e3. ## What Changed - Make synthetic exposure failures use the assigned port. Test both free and occupied base pairs. - Record document readiness, root children, service-worker control, pending module paths, and recent module error or 304 statuses in private runner evidence. - Add browser tests for pending startup, failed module responses, and diagnostic reset after navigation. - Add a provider-free browser test for persisted task content and the composer across three navigation/reload cycles. - Guard trusted report-job lockfile resolution before frozen install and AWS credential setup. - Document the new evidence and test commands. ## Verification - `pnpm test:e2e:runner:unit`: 443 tests passed. - `pnpm test:e2e:runner:browser-support`: 10 tests passed. - Exposure tests: 28 passed; 3 platform-specific skips. - Harness typecheck, workspace typecheck, and full build passed. - Task-reload browser test passed against a throwaway local instance. It checks three navigation/reload cycles with a controlling service worker. - All PR verification suites passed, including server, chat, workspace, serialized server, Rust, runner, build, typecheck, and browser shards. The existing sidebar navigation case showed an empty page on the first CI attempt and passed on one retry. - The monolithic local `pnpm test:run` was started, then stopped after CI completed the equivalent suites. Additional local browser reproduction attempts also hit embedded-Postgres startup failures; the focused checks listed above completed successfully. - Greptile: 5/5 on the current head, with no findings. ## Risks This change affects tests and private test evidence. It does not change production prompts or runtime behavior. Module paths omit queries; the collector reads no response bodies or headers. The existing sanitizer and publication allowlist still apply. A 304 response does not cause a test failure. The blank-page cause remains unconfirmed. The first CI attempt reproduced it in an existing sidebar navigation case: the trace shows an empty page after a service-worker-mediated 304 response for the large editor module. The single retry passed. This PR adds coverage and diagnostics; it does not claim to fix that intermittent symptom. ## Model Used OpenAI Codex, GPT-6, with repository tools, code execution, and browser tests. The exact deployed model variant and context-window size are not exposed in this session. ## 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 --- .../workspace-runtime-exposure.test.ts | 57 ++++++++++++------- tests/e2e/task-reload.spec.ts | 44 ++++++++++++++ tests/runner-e2e/README.md | 16 ++++++ .../browser-bootstrap-diagnostics.spec.ts | 44 ++++++++++++++ .../browser-bootstrap-diagnostics.ts | 51 +++++++++++++++++ tests/runner-e2e/playwright-support.config.ts | 2 +- tests/runner-e2e/runner.spec.ts | 4 ++ tests/runner-e2e/workflow-security.test.ts | 17 ++++++ 8 files changed, 212 insertions(+), 23 deletions(-) create mode 100644 tests/e2e/task-reload.spec.ts create mode 100644 tests/runner-e2e/browser-bootstrap-diagnostics.spec.ts create mode 100644 tests/runner-e2e/browser-bootstrap-diagnostics.ts diff --git a/server/src/services/workspace-runtime-exposure.test.ts b/server/src/services/workspace-runtime-exposure.test.ts index 9f8dfeac42..951b6f5e4a 100644 --- a/server/src/services/workspace-runtime-exposure.test.ts +++ b/server/src/services/workspace-runtime-exposure.test.ts @@ -131,18 +131,10 @@ setInterval(() => {}, 1000); * exposure-port pool. The start must surface the failure terminally after a single * allocation. */ -const SYNTHETIC_EADDRINUSE_ON_BASE_PORT_GUEST = ` -import http from "node:http"; +const SYNTHETIC_EADDRINUSE_ON_ASSIGNED_PORT_GUEST = ` const p = Number(process.env.PORT); -if (p === 42000) { - process.stderr.write("node:events:497\\nError: listen EADDRINUSE: address already in use 127.0.0.1:" + p + "\\n"); - process.exit(1); -} -const health = (rq, r) => { if (rq.url === "/api/health") { r.setHeader("content-type", "application/json"); r.end(JSON.stringify({ status: "ok" })); return true; } return false; }; -for (const q of [p, p + 10000]) { - http.createServer((rq, r) => { if (health(rq, r)) return; r.statusCode = 200; r.end("ok"); }).listen(q, "127.0.0.1"); -} -setInterval(() => {}, 1000); +process.stderr.write("node:events:497\\nError: listen EADDRINUSE: address already in use 127.0.0.1:" + p + "\\n"); +process.exit(1); `; /** @@ -189,7 +181,7 @@ beforeAll(async () => { // collision the host cannot confirm. The start must not quarantine the pair. await fs.writeFile( path.join(guestDir, "dev-runner-eaddrinuse-synthetic.mjs"), - SYNTHETIC_EADDRINUSE_ON_BASE_PORT_GUEST, + SYNTHETIC_EADDRINUSE_ON_ASSIGNED_PORT_GUEST, ); // A guest that fails on a fixed auxiliary port, not on its assigned app or HMR // port. It models an unrelated helper listener that an external process holds. @@ -757,7 +749,7 @@ describe("the deployed failure shape: loopback app port, wildcard HMR (PAP-17256 }, 20_000); }); -describe("recovers when a guest loses its assigned exposure port during startup (PAP-17256)", () => { +describe.each([false, true])("guest exposure-port collision reporting (base pair occupied: %s)", (basePairOccupied) => { // The quarantine decision itself is unit-tested through // `classifyExposureHostCollisions` below. A deterministic end-to-end quarantine // test is not reachable here: a real host listener that holds the assigned port @@ -773,7 +765,12 @@ describe("recovers when a guest loses its assigned exposure port during startup return broker.reserve(runtimeId, requested); }, }; - installDeps({ broker: recordingBroker }); + installDeps({ + broker: recordingBroker, + isPortAvailable: (port) => basePairOccupied && + [RUNTIME_EXPOSURE_APP_PORT_MIN, deriveViteHmrPort(RUNTIME_EXPOSURE_APP_PORT_MIN)].includes(port) + ? Promise.resolve(false) : isLoopbackPortFree(port), + }); const logs: string[] = []; const error = await startRuntimeServicesForWorkspaceControl({ @@ -788,12 +785,14 @@ describe("recovers when a guest loses its assigned exposure port during startup }, }).then(() => null, (err: unknown) => err as Error); - // No host listener owns 42000, so the printed EADDRINUSE line is unverified. + // The assigned pair is free, so the printed EADDRINUSE line is unverified. // The start fails terminally after ONE allocation and never burns the pool. expect(error).not.toBeNull(); - expect(error!.message).toContain("42000"); + expect(error!.message).toContain(String(reservedAppPorts[0])); expect(error!.message).toContain("not verified"); - expect(reservedAppPorts).toEqual([42_000]); + expect(reservedAppPorts).toHaveLength(1); + expect(reservedAppPorts[0]).toBeGreaterThanOrEqual(RUNTIME_EXPOSURE_APP_PORT_MIN + (basePairOccupied ? 1 : 0)); + expect(reservedAppPorts[0]).toBeLessThanOrEqual(RUNTIME_EXPOSURE_APP_PORT_MAX); // No quarantine and no re-allocation happened for the unverified claim. const diagnosis = logs.join(""); @@ -811,7 +810,12 @@ describe("recovers when a guest loses its assigned exposure port during startup return broker.reserve(runtimeId, requested); }, }; - installDeps({ broker: recordingBroker }); + installDeps({ + broker: recordingBroker, + isPortAvailable: (port) => basePairOccupied && + [RUNTIME_EXPOSURE_APP_PORT_MIN, deriveViteHmrPort(RUNTIME_EXPOSURE_APP_PORT_MIN)].includes(port) + ? Promise.resolve(false) : isLoopbackPortFree(port), + }); const logs: string[] = []; const error = await startRuntimeServicesForWorkspaceControl({ @@ -831,7 +835,9 @@ describe("recovers when a guest loses its assigned exposure port during startup // burns the bounded retries on a valid pair. expect(error).not.toBeNull(); expect(error!.message).toContain("39999"); - expect(reservedAppPorts).toEqual([42_000]); + expect(reservedAppPorts).toHaveLength(1); + expect(reservedAppPorts[0]).toBeGreaterThanOrEqual(RUNTIME_EXPOSURE_APP_PORT_MIN + (basePairOccupied ? 1 : 0)); + expect(reservedAppPorts[0]).toBeLessThanOrEqual(RUNTIME_EXPOSURE_APP_PORT_MAX); // No quarantine and no re-allocation happened for the auxiliary conflict. const diagnosis = logs.join(""); @@ -849,7 +855,12 @@ describe("recovers when a guest loses its assigned exposure port during startup return broker.reserve(runtimeId, requested); }, }; - installDeps({ broker: recordingBroker }); + installDeps({ + broker: recordingBroker, + isPortAvailable: (port) => basePairOccupied && + [RUNTIME_EXPOSURE_APP_PORT_MIN, deriveViteHmrPort(RUNTIME_EXPOSURE_APP_PORT_MIN)].includes(port) + ? Promise.resolve(false) : isLoopbackPortFree(port), + }); const logs: string[] = []; const error = await startRuntimeServicesForWorkspaceControl({ @@ -864,13 +875,15 @@ describe("recovers when a guest loses its assigned exposure port during startup }, }).then(() => null, (err: unknown) => err as Error); - // The assigned port 42000 appears on a benign line, but EADDRINUSE names only + // The assigned port appears on a benign line, but EADDRINUSE names only // the auxiliary port 39999. The parser matches the error and the port on the // same line, so the assigned pair is not a collision. The start fails // terminally after ONE allocation and never quarantines the valid pair. expect(error).not.toBeNull(); expect(error!.message).toContain("39999"); - expect(reservedAppPorts).toEqual([42_000]); + expect(reservedAppPorts).toHaveLength(1); + expect(reservedAppPorts[0]).toBeGreaterThanOrEqual(RUNTIME_EXPOSURE_APP_PORT_MIN + (basePairOccupied ? 1 : 0)); + expect(reservedAppPorts[0]).toBeLessThanOrEqual(RUNTIME_EXPOSURE_APP_PORT_MAX); const diagnosis = logs.join(""); expect(diagnosis).not.toContain("Quarantined pair"); diff --git a/tests/e2e/task-reload.spec.ts b/tests/e2e/task-reload.spec.ts new file mode 100644 index 0000000000..68f02b332f --- /dev/null +++ b/tests/e2e/task-reload.spec.ts @@ -0,0 +1,44 @@ +import { expect, test } from "@playwright/test"; +import { observeBrowserBootstrap } from "../runner-e2e/browser-bootstrap-diagnostics"; +import { json, setup } from "./agent-chat.shared"; + +test.use({ serviceWorkers: "allow", trace: "retain-on-failure" }); + +test("saved task content survives same-URL navigation and reload with a controlling service worker", async ({ page, request }) => { + test.setTimeout(120_000); + const fixture = await setup(request); + const diagnostics = observeBrowserBootstrap(page); + const title = "Task reload continuity"; + const comment = "Persisted context remains available after a full page load."; + try { + // Unassigned backlog work exercises the actual UI without a provider call. + const issue = await json(await request.post(`/api/companies/${fixture.company.id}/issues`, { + data: { title, status: "backlog" }, + })); + await json(await request.post(`/api/issues/${issue.id}/comments`, { data: { body: comment } })); + const route = `/${fixture.company.issuePrefix}/issues/${issue.identifier}`; + const assertLoaded = async () => { + await expect(page.getByRole("heading", { name: title, exact: true })).toBeVisible({ timeout: 30_000 }); + await expect(page.getByText(comment, { exact: true })).toBeVisible(); + await expect(page.getByTestId("task-chat-composer-input")).toBeVisible(); + }; + await page.goto(route, { waitUntil: "domcontentloaded" }); + await assertLoaded(); + await page.waitForFunction(() => Boolean(navigator.serviceWorker?.controller)); + for (let index = 0; index < 3; index += 1) { + // The warm-continuity eval used same-URL goto, not just location.reload. + await page.goto(route, { waitUntil: "domcontentloaded" }); + await assertLoaded(); + await page.reload({ waitUntil: "domcontentloaded" }); + await assertLoaded(); + } + expect(await json(await request.get(`/api/issues/${issue.id}/runs`))).toEqual([]); + } finally { + await test.info().attach("browser-bootstrap", { + contentType: "application/json", + body: Buffer.from(JSON.stringify(await diagnostics.snapshot(), null, 2)), + }); + diagnostics.dispose(); + await fixture.restore(); + } +}); diff --git a/tests/runner-e2e/README.md b/tests/runner-e2e/README.md index cbe7695cc7..d10684439e 100644 --- a/tests/runner-e2e/README.md +++ b/tests/runner-e2e/README.md @@ -985,3 +985,19 @@ Current proof and remaining decisions are recorded in the original worker-loss attempts quarantined both providers. The version 9 crash eval requires a usable fresh conversation after verified cleanup. A passing quarantine guard alone is not a recovered workflow. + +### Blank-page investigation + +Private `browser-diagnostics.json` includes the final document readiness, whether +`#root` mounted content, whether a service worker controls the page, outstanding +script/style paths, and recent module 304/error statuses. These fields contain no +response bodies, headers, or query strings. A 304 is ordinary cache validation; +recording it does not change the grade or retry the page. The public report still +uses the existing evidence allowlist. + +The provider-free `tests/e2e/task-reload.spec.ts` regression opens a persisted task +with the production service worker, navigates to the same URL, and reloads it. It +requires the saved content and usable composer to remain visible. Run it with the +standard `tests/e2e/playwright.config.ts`; no provider or Daytona credentials are +needed. Browser-support tests separately exercise blank-root/pending-module +failure evidence, so a future blank page is distinguishable from a loaded task. diff --git a/tests/runner-e2e/browser-bootstrap-diagnostics.spec.ts b/tests/runner-e2e/browser-bootstrap-diagnostics.spec.ts new file mode 100644 index 0000000000..ef52d54b1d --- /dev/null +++ b/tests/runner-e2e/browser-bootstrap-diagnostics.spec.ts @@ -0,0 +1,44 @@ +import { expect, test } from "@playwright/test"; +import { observeBrowserBootstrap } from "./browser-bootstrap-diagnostics.js"; + +const html = '
'; + +test("records an empty root and pending module, then recognizes completed startup", async ({ page }) => { + let release!: () => void; + const gate = new Promise(resolve => { release = resolve; }); + await page.route("http://bootstrap.test/**", async route => { + if (new URL(route.request().url()).pathname === "/entry.js") { + await gate; + await route.fulfill({ contentType: "text/javascript", body: 'document.getElementById("root").innerHTML = "
Ready
";' }); + } else await route.fulfill({ contentType: "text/html", body: html }); + }); + const diagnostics = observeBrowserBootstrap(page); + try { + await page.goto("http://bootstrap.test/", { waitUntil: "commit" }); + await expect.poll(async () => (await diagnostics.snapshot()).pendingModules).toEqual(["/entry.js"]); + const before = await diagnostics.snapshot(); + expect(before.state).toMatchObject({ rootPresent: true, rootChildCount: 0, serviceWorkerControlled: false }); + expect(JSON.stringify(before)).not.toContain("do-not-record"); + release(); + await expect(page.getByRole("main")).toHaveText("Ready"); + await expect.poll(async () => (await diagnostics.snapshot()).pendingModuleCount).toBe(0); + expect((await diagnostics.snapshot()).state?.rootChildCount).toBe(1); + } finally { release(); diagnostics.dispose(); } +}); + +for (const status of [304, 503]) { + test(`retains module status ${status} without credentials and clears it on navigation`, async ({ page }) => { + await page.route("http://bootstrap.test/**", async route => { + const pathname = new URL(route.request().url()).pathname; + await route.fulfill(pathname === "/entry.js" ? { status, body: "" } + : { contentType: "text/html", body: pathname === "/ready" ? '
Ready
' : html }); + }); + const diagnostics = observeBrowserBootstrap(page); + try { + await page.goto("http://bootstrap.test/", { waitUntil: "load" }); + expect((await diagnostics.snapshot()).moduleResponses).toEqual([{ path: "/entry.js", status }]); + await page.goto("http://bootstrap.test/ready"); + expect((await diagnostics.snapshot()).moduleResponses).toEqual([]); + } finally { diagnostics.dispose(); } + }); +} diff --git a/tests/runner-e2e/browser-bootstrap-diagnostics.ts b/tests/runner-e2e/browser-bootstrap-diagnostics.ts new file mode 100644 index 0000000000..838beecf70 --- /dev/null +++ b/tests/runner-e2e/browser-bootstrap-diagnostics.ts @@ -0,0 +1,51 @@ +import type { Page, Request, Response } from "@playwright/test"; + +/** Private failure evidence only: no response bodies, headers, or query strings. */ +export function observeBrowserBootstrap(page: Page) { + const pending = new Map(); + const moduleResponses: { path: string; status: number }[] = []; + const pathname = (url: string) => { + try { return new URL(url).pathname; } catch { return "[invalid URL]"; } + }; + const isModule = (request: Request) => ["script", "stylesheet"].includes(request.resourceType()); + const onRequest = (request: Request) => { + if (isModule(request)) pending.set(request, pathname(request.url())); + }; + const onFinished = (request: Request) => { pending.delete(request); }; + const onResponse = (response: Response) => { + // A 304 is normally valid, but is useful evidence when module evaluation + // never mounts the app. It must not itself turn a successful run into a fail. + if (isModule(response.request()) && (response.status() === 304 || response.status() >= 400)) { + moduleResponses.push({ path: pathname(response.url()), status: response.status() }); + if (moduleResponses.length > 50) moduleResponses.shift(); + } + }; + const onNavigation = (frame: ReturnType) => { + if (frame !== page.mainFrame()) return; + pending.clear(); + moduleResponses.length = 0; + }; + page.on("request", onRequest); + page.on("requestfinished", onFinished); + page.on("requestfailed", onFinished); + page.on("response", onResponse); + page.on("framenavigated", onNavigation); + return { + async snapshot() { + const state = await page.evaluate(() => ({ + readyState: document.readyState, + rootPresent: document.getElementById("root") !== null, + rootChildCount: document.getElementById("root")?.childElementCount ?? 0, + serviceWorkerControlled: Boolean(navigator.serviceWorker?.controller), + })).catch(() => null); + return { state, pendingModuleCount: pending.size, pendingModules: [...pending.values()].slice(0, 50), moduleResponses: [...moduleResponses] }; + }, + dispose() { + page.off("request", onRequest); + page.off("requestfinished", onFinished); + page.off("requestfailed", onFinished); + page.off("response", onResponse); + page.off("framenavigated", onNavigation); + }, + }; +} diff --git a/tests/runner-e2e/playwright-support.config.ts b/tests/runner-e2e/playwright-support.config.ts index 53e6b3a47c..32dd9e6b29 100644 --- a/tests/runner-e2e/playwright-support.config.ts +++ b/tests/runner-e2e/playwright-support.config.ts @@ -3,7 +3,7 @@ import { defineConfig } from "@playwright/test"; /** Browser-only harness regressions: no Paperclip instance or provider credentials. */ export default defineConfig({ testDir: ".", - testMatch: ["screenshot-readiness.spec.ts", "lost-send.spec.ts", "chat-restart.spec.ts", "settings-toggle.spec.ts"], + testMatch: ["screenshot-readiness.spec.ts", "lost-send.spec.ts", "chat-restart.spec.ts", "settings-toggle.spec.ts", "browser-bootstrap-diagnostics.spec.ts"], workers: 1, retries: 0, timeout: 10_000, diff --git a/tests/runner-e2e/runner.spec.ts b/tests/runner-e2e/runner.spec.ts index e5008fd746..9649bf1da5 100644 --- a/tests/runner-e2e/runner.spec.ts +++ b/tests/runner-e2e/runner.spec.ts @@ -1,3 +1,4 @@ +import { observeBrowserBootstrap } from "./browser-bootstrap-diagnostics.js"; import { runContinuationFlow } from "./continuation-flow.js"; import { runEverydayFlow } from "./everyday-flow.js"; import { createTaskThroughUi, submitTaskReply } from "./user-actions.js"; @@ -533,6 +534,7 @@ for (const execution of executions) { const consoleDiagnostics: Array> = []; const networkDiagnostics: Array> = []; const pageLifecycleDiagnostics: Array> = []; + const browserBootstrap = observeBrowserBootstrap(page); let fixtures: LiveFixtureValues | undefined; let reviewProvider: Awaited> | undefined; let issue: IssueRecord | undefined; @@ -2449,6 +2451,7 @@ for (const execution of executions) { console: consoleDiagnostics, network: networkDiagnostics, lifecycle: pageLifecycleDiagnostics, + bootstrap: await browserBootstrap.snapshot(), }, secrets, ); @@ -2459,6 +2462,7 @@ for (const execution of executions) { ); failureClassOverride = "secret_leak"; } + browserBootstrap.dispose(); if (fixtures) { await captureRuntimeLeases().catch((error) => { networkDiagnostics.push({ diff --git a/tests/runner-e2e/workflow-security.test.ts b/tests/runner-e2e/workflow-security.test.ts index 6e80147d17..1a9c021dd0 100644 --- a/tests/runner-e2e/workflow-security.test.ts +++ b/tests/runner-e2e/workflow-security.test.ts @@ -682,6 +682,23 @@ describe("public repository paid workflow security", () => { ); }); + it("resolves each trusted reporting lockfile before frozen install and AWS credentials", async () => { + const workflow = await readFile( + path.join(repositoryRoot, ".github/workflows/runner-full-stack-e2e.yml"), "utf8", + ); + for (const jobName of ["report", "publish_history"]) { + const job = workflow.split(`\n ${jobName}:`)[1]!.split(/\n [a-z_]+:/u)[0]!; + const resolve = job.indexOf("pnpm install --lockfile-only --ignore-scripts --no-frozen-lockfile"); + const install = job.indexOf("pnpm install --frozen-lockfile"); + expect(job).toContain("ref: ${{ github.sha }}"); + expect(job).not.toContain("resolved-target-lockfile"); + expect(resolve, jobName).toBeGreaterThan(-1); + expect(install, jobName).toBeGreaterThan(resolve); + const credentials = job.indexOf("aws-actions/configure-aws-credentials@"); + if (credentials >= 0) expect(install).toBeLessThan(credentials); + } + }); + it("binds rerun evidence and Pages artifacts to the exact workflow attempt", async () => { const workflow = await readFile( path.join(repositoryRoot, ".github/workflows/runner-full-stack-e2e.yml"),