From 6681c71b40ec2c80fb33abb74b4b45ede1150565 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Wed, 23 Sep 2026 15:22:21 -0700 Subject: [PATCH] fix(ci): stabilize chat startup and close the initial live-update gap (#13895) 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. > - Browser tests check chat state across navigation, reload, and agent runs. > - Runtime tests check that a service is ready before Paperclip publishes its address. > - CI for #13891 failed in these paths, then passed attempt 3 with the same code. > - The browser traces stopped during development asset startup. A separate sidebar assertion used an unstable focus path through the rich editor. > - This PR gives browser tests fresh built assets and a direct keyboard path to the star button. It adds service-worker reload coverage under CPU throttling. > - A later browser failure exposed a real reload race: comments can change between the first query and the first live subscription. The UI now refreshes active queries when that subscription opens. > - Runtime fixtures now have separate registry state and better failure evidence. The readiness deadline remains unchanged. > - A later CI run exposed a wall-clock backoff assertion and three authorization cases sharing one test lifecycle. The PR anchors the assertion to transport time and separates the cases. ## Linked Issues or Issue Description Refs #13891. Related runtime ownership and cleanup work: #11791, #11389, #11278. Evidence: [original CI run, attempt 3](https://github.com/paperclipai/paperclip/actions/runs/35910335089/attempts/3). Attempt 3 passed both affected shards without code changes. This PR is separate from the wake-payload change, which has since merged. | Failure | Diagnosis and classification | | --- | --- | | Sidebar blank page and retry text missing after reload in CI | Both traces show a blank document before React startup. Vite connects, but the failing page makes no application API requests. The service worker forwards the unbundled development module graph. The retry response is already stored and its process-adapter run succeeded before reload. This places the failure in browser bootstrap, not wake payload or reply persistence. The exact reason the development module graph stopped is not established by the retained trace. The harness now serves built assets, and the new test covers startup and controlled reload at 4x CPU throttling. | | Star opacity remains zero on macOS | Reproduced on unchanged master `f55759942b`. The old test clicked the rich editor, focused the star, then used Tab and Shift+Tab. An instrumented baseline run captured the sequence: Tab moved from the star to the next sidebar link, then the editor bundle called `focus()` on its contenteditable before Shift+Tab. That key reached the composer, where it is a work-mode shortcut. This confirms a pending editor selection update stole focus; it was not the browser skipping sidebar buttons. The assertion did not prove the star retained focus. The test now moves the pointer away, focuses the preceding sidebar link, presses Tab once, and asserts both actual focus and opacity. This is test synchronization and keyboard traversal, not a demonstrated CSS defect. | | Runtime readiness exceeds 10 seconds | The old error only says `fetch failed`. There is no child startup output in that failure, so it cannot distinguish slow process startup from a refused or stalled probe. It passed unchanged locally and took 3.416 seconds in attempt 3. Resource contention is plausible but unproved. This PR does not claim a proven historical runtime root cause: it isolates fixture registry/log files, checks ports before spawn and after stop, checks live backends at publication, and preserves transport errors, probe count, elapsed time, and fixture startup timestamps for the next occurrence. | | Later CI: recovery reply disappears after reload | Product synchronization bug, distinct from the blank-page bootstrap failure. Reproduced locally with a trace: the comments request started at `21:56:07.443`, the server saved the reply at `.520`, and the first live subscription started at `.541`. The reload fetched a successful run and its complete log, but missed the comment event. The provider refreshed after reconnects only. It now refreshes active queries on the first connection too. A deterministic regression test fails before the fix. No browser assertion was changed. | | Later CI: Slack backoff and authorization tests | The 30-second backoff assertion required more than 25 seconds to remain when it read the saved action. CI spent 10.311 seconds in the test, exceeding that five-second allowance. A local six-second read delay reproduces the failure; the new transport-anchored lower and upper bounds pass the same fault injection. The neighbouring authorization test ran three independent fixtures in one test and timed out at 15 seconds. Each case now has its own fixture cleanup and the normal per-test deadline, so earlier cases do not remain active during later global worker sweeps. No specific production slow call was established. | | Self-hosted runner loses communication or shuts down | The original lost-communication failure has no assertion. On final-head [attempt 1](https://github.com/paperclipai/paperclip/actions/runs/35925901613/attempts/1), Build, Runner Vitest 1/2, and chat 2/3 ran on three separate fleet instances. All received a runner shutdown signal at `22:04:55 UTC`, within 35 milliseconds, then cancellation. Server shard 6/12 received the same shutdown signal one minute later. All four were Spot `m7i-flex.xlarge` instances in `us-east-1a`. No test assertion or build error preceded those stops. This is infrastructure interruption; the reason the fleet stopped the runners is not available in job logs. | ## What Changed - Build the browser fixture UI into the static server's preferred directory, `server/ui-dist`, and disable Vite middleware. Ignore these generated assets. Start the source CLI directly from the repository root, as required by the CLI invocation safety contract. - Keep all existing chat assertions. Add first takeover and three service-worker-controlled reloads under CPU throttling. Assert that the page uses built module assets and that opening it creates no chat task. - Use forward keyboard traversal from the Zeta link to its star. Check focus before checking the reveal style. - Give each runtime exposure test a temporary Paperclip home and restore environment state after process cleanup. - Check that reserved ports are free before spawn, serve the fixture response before exposure, and become free after stop. - Include the nested fetch error, probe count, and elapsed time in readiness failures. Add a unit test for this diagnostic contract. - Refresh active queries when the first live connection opens, covering events missed during initial page loading. Keep reconnect toast suppression unchanged. - Measure Slack retry timing from the transport attempt and recovery completion. This checks the full provider-requested delay without spending a small wall-clock allowance on unrelated processing. - Run each Slack authorization-revocation scenario as a separate test, with cleanup between cases. All assertions remain. - Document the browser fixture's build and serving mode. No configured assertion deadline, readiness deadline, retry count, or skip was added. ## Verification - Final-head [Linux CI, attempt 2](https://github.com/paperclipai/paperclip/actions/runs/35925901613/attempts/2): **green**. All 52 check runs passed; two conditional checks were skipped. The legacy Snyk status also passed. No pending or failed checks remain. - `pnpm -r typecheck` passed. - `pnpm build` passed again after the final UI fix. - Focused runtime suites: 38 passed, 3 existing platform skips. - CLI invocation safety suite: 39 passed. - Slack timing negative control: inserting a six-second delay before reading the saved action fails the old assertion. All four revised authorization/backoff cases pass with that same delay. The diagnostic delay is not committed. - Live update suites: 92 passed, including the new regression. UI typecheck and token gates passed. - Recovery browser negative control: the unchanged tests reproduced the missing reply (1 failed, 9 passed). After the first-connection fix, all six recovery paths passed twice (12 passed). Browser assertions and deadlines are unchanged. - Server typecheck passed after the Slack test adjustment. - `GITHUB_WORKFLOW=PR pnpm test:run:general -- --group general-chat --shard-index 0 --shard-count 3`: 342 passed. The other 682 tests belong to the remaining shards; collection verified exact coverage. - Final direct-source CLI launch: all five chat session tests passed. - `PAPERCLIP_E2E_PORT=32993 PAPERCLIP_PLAYWRIGHT_CHANNEL=chrome pnpm exec playwright test --config tests/e2e/playwright.config.ts tests/e2e/agent-chat-sessions.spec.ts --repeat-each=3`: 15 passed, including nine controlled reloads at 4x CPU throttling. - `GITHUB_WORKFLOW=PR pnpm test:run:general -- --group general-server-without-chat --shard-index 10 --shard-count 12`: 55 files passed, 867 tests passed, 21 existing skips. An earlier run could not initialize PostgreSQL because this Mac exhausted its System V shared-memory slots. After reclaiming the orphaned segment from this task's stopped browser server, the full shard passed. - On final commit `1f1fafc08d`, [browser 4/8](https://github.com/paperclipai/paperclip/actions/runs/35925901613/job/107400837947) passed all 16 tests; [browser 8/8](https://github.com/paperclipai/paperclip/actions/runs/35925901613/job/107400838155) passed all 23 tests; [server 11/12](https://github.com/paperclipai/paperclip/actions/runs/35925901613/job/107400838328) passed 887 tests with one existing skip. The readiness lifecycle case took 1.842 seconds. Chat 1/3 also passed. All four jobs interrupted by runner shutdowns passed unchanged on their single rerun. - Greptile reviewed `1f1fafc08d`: **5/5**, with no unresolved comments. - Full local `pnpm test:run` was attempted and stopped after confirming failures outside this patch: a sibling `skills` directory shadows bundled Slack/AgentMail skills; macOS rejects rename of read-only skill-cache directories (`EACCES`, also reproduced in an isolated filesystem probe); and the host exhausts PostgreSQL System V shared-memory slots. Focused reruns confirmed these limits. The complete Linux CI run is the repository-wide verification; the full local run is not green. - Baseline evidence: the original sidebar test failed on unchanged master; a development-mode run at 4x CPU throttling passed six selected cases, so CPU pressure alone did not reproduce the CI bootstrap stall. ## Risks - Default browser tests now exercise the shipped static UI. They no longer implicitly cover Vite middleware or HMR; use the development server for those checks. - The new browser startup test uses Chromium CDP, matching the only configured browser project. - Opening a live subscription now causes one active-query refresh to close the initial event gap. This adds startup API reads but no recurring poll. - Runtime behavior and deadlines are unchanged except for error details. The historical readiness stall remains unconfirmed; a green rerun alone cannot establish its cause. - Local verification runs on macOS. The runtime lifecycle checks also passed on Linux CI. ## Model Used - OpenAI GPT-6 through Codex. The session identifies the model family as GPT-6; an exact served model ID and context window size are not exposed. Used reasoning, repository inspection, shell tools, code editing, and test execution. No subagents were used. ## 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 #123` / `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 — targeted suites passed; full local host limits are listed above - [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 --- .gitignore | 1 + doc/DEVELOPING.md | 8 ++++ .../chat-channels.integration.test.ts | 31 ++++++++----- ...orkspace-runtime-start-terminality.test.ts | 21 +++++++++ .../workspace-runtime-exposure.test.ts | 41 ++++++++++++----- server/src/services/workspace-runtime.ts | 18 +++++++- tests/e2e/agent-chat-sessions.spec.ts | 44 +++++++++++++++++-- tests/e2e/playwright.config.ts | 9 +++- .../LiveUpdatesProvider.recovery.test.tsx | 20 ++++++++- ui/src/context/LiveUpdatesProvider.tsx | 5 ++- 10 files changed, 167 insertions(+), 31 deletions(-) diff --git a/.gitignore b/.gitignore index 3b81cf1a12..232855e0b9 100644 --- a/.gitignore +++ b/.gitignore @@ -5,6 +5,7 @@ node_modules/ dist/ dist-preview/ dist-flow-preview/ +server/ui-dist/ packages/paperclip-runner/runner/target/ ui/storybook-static/ .env diff --git a/doc/DEVELOPING.md b/doc/DEVELOPING.md index c5fec8a253..d3e04011e1 100644 --- a/doc/DEVELOPING.md +++ b/doc/DEVELOPING.md @@ -368,6 +368,14 @@ pnpm test:release-smoke These browser suites are intended for targeted local verification and CI, not the default agent/human test command. +The default E2E configuration builds the UI into `server/ui-dist` before starting +its throwaway instance and serves that build with +`PAPERCLIP_UI_DEV_MIDDLEWARE=false`. This exercises the +shipped assets, including service-worker takeover and reload, without traversing +the development server's unbundled module graph on each navigation. Browser +assertion deadlines and retries remain unchanged. Use `pnpm dev` separately when +verifying Vite/HMR behavior. + For normal issue work, start with the smallest targeted check that proves the change. Reserve repo-wide typecheck/build/test runs for PR-ready handoff or changes broad enough that narrow checks do not cover the risk. ### Task search evaluation diff --git a/server/src/__tests__/chat-channels.integration.test.ts b/server/src/__tests__/chat-channels.integration.test.ts index 889431f76a..cb174f99ec 100644 --- a/server/src/__tests__/chat-channels.integration.test.ts +++ b/server/src/__tests__/chat-channels.integration.test.ts @@ -48797,12 +48797,15 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => { ).resolves.toEqual([{ state: "processed" }]); }); - it("rechecks linked authority and channel reach before admitting a provider-confirmed Slack task", async () => { - for (const authorizationChange of [ - "link_revoked", - "viewer", - "resource_disabled", - ] as const) { + // Each authorization case owns its service lifecycle. The global recovery + // worker must not see active fixtures from an earlier loop iteration. + it.each([ + "link_revoked", + "viewer", + "resource_disabled", + ] as const)( + "rechecks linked authority and channel reach before admitting a provider-confirmed Slack task (%s)", + async (authorizationChange) => { const fixture = await seedCompany(); const { endpoint, runtime, service, wakeup } = await configuredSlackEndpoint(fixture, { @@ -48950,8 +48953,8 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => { `must-not-create-${authorizationChange}`, ); await service.shutdown(); - } - }); + }, + ); it("recovers a provider-confirmed Slack starter after restart but rechecks revoked reach", async () => { const fixture = await seedCompany(); @@ -49296,7 +49299,7 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => { }); const providerRuntime = runtime.endpoints.get(endpoint.id); if (!providerRuntime) throw new Error("Expected Slack provider runtime"); - const transportAttempts = vi.fn(); + const transportAttempts = vi.fn(() => Date.now()); providerRuntime.postHook = async () => { transportAttempts(); }; @@ -49307,6 +49310,7 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => { await service.processPendingDeliveries(); expect(transportAttempts).toHaveBeenCalledTimes(1); + const recoveryFinishedAt = Date.now(); const [deferred] = await db .select() .from(chatActions) @@ -49325,9 +49329,12 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => { retryAt: expect.any(String), }, }); - expect( - new Date(String(deferred!.result?.retryAt)).getTime(), - ).toBeGreaterThan(Date.now() + 25_000); + // Anchor the 30-second provider delay to the transport attempt. Other + // recovery work and the database read must not consume assertion slack. + const retryAt = new Date(String(deferred!.result?.retryAt)).getTime(); + const attemptedAt = transportAttempts.mock.results[0]!.value; + expect(retryAt).toBeGreaterThanOrEqual(attemptedAt + 30_000); + expect(retryAt).toBeLessThanOrEqual(recoveryFinishedAt + 30_000); await service.processPendingDeliveries(); expect(transportAttempts).toHaveBeenCalledTimes(1); diff --git a/server/src/__tests__/workspace-runtime-start-terminality.test.ts b/server/src/__tests__/workspace-runtime-start-terminality.test.ts index 01bbcbe88f..35e96a6db8 100644 --- a/server/src/__tests__/workspace-runtime-start-terminality.test.ts +++ b/server/src/__tests__/workspace-runtime-start-terminality.test.ts @@ -27,6 +27,27 @@ describe("managed runtime start terminality", () => { readiness: { type: "http", timeoutSec: 1, intervalMs: 50 }, } as Record; + it("reports the fetch transport cause and probe count without extending the deadline", async () => { + const transportError = new Error("connect ECONNREFUSED 127.0.0.1:42000"); + const fetchError = new TypeError("fetch failed", { cause: transportError }); + let now = 0; + const refusingFetch = (async () => { + now = 1_000; + throw fetchError; + }) as typeof fetch; + + await expect(waitForRuntimeServiceReadiness({ + service: hangingService, + url: "http://127.0.0.1:42000/", + readinessUrl: null, + fetchImpl: refusingFetch, + now: () => now, + })).rejects.toMatchObject({ + message: "Readiness check failed for http://127.0.0.1:42000/: fetch failed: connect ECONNREFUSED 127.0.0.1:42000 (1 probes over 1000ms)", + cause: fetchError, + }); + }); + it("fails a readiness check whose probes never answer instead of hanging forever", async () => { let probes = 0; const abortedProbes: string[] = []; diff --git a/server/src/services/workspace-runtime-exposure.test.ts b/server/src/services/workspace-runtime-exposure.test.ts index 951b6f5e4a..c1e2899ec9 100644 --- a/server/src/services/workspace-runtime-exposure.test.ts +++ b/server/src/services/workspace-runtime-exposure.test.ts @@ -3,7 +3,7 @@ import net from "node:net"; import os from "node:os"; import path from "node:path"; -import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it } from "vitest"; +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; import { deriveViteHmrPort, @@ -31,24 +31,27 @@ const HANDLE = "handle-abcdef1234567890"; // The shared test setup pins the automatic default off so unrelated suites do // not probe for a real host broker. This suite is about the default, so it opts // back in and restores the harness value afterwards. -let previousHttpsMode: string | undefined; -beforeEach(() => { - previousHttpsMode = process.env.PAPERCLIP_MANAGED_RUNTIME_HTTPS; - process.env.PAPERCLIP_MANAGED_RUNTIME_HTTPS = "auto"; +beforeEach(async () => { + vi.stubEnv("PAPERCLIP_MANAGED_RUNTIME_HTTPS", "auto"); + // Registry records and append-only logs must not share a developer's instance + // or a previous test's service identity. + vi.stubEnv("PAPERCLIP_HOME", await fs.mkdtemp(path.join(guestDir, "home-"))); }); afterEach(async () => { - if (previousHttpsMode === undefined) delete process.env.PAPERCLIP_MANAGED_RUNTIME_HTTPS; - else process.env.PAPERCLIP_MANAGED_RUNTIME_HTTPS = previousHttpsMode; // These tests spawn real loopback backends on dedicated-range ports; reap them // rather than leaving one squatting 42xxx/52xxx for every test in the file. - await resetRuntimeServicesForTests({ terminateProcesses: true }); + try { + await resetRuntimeServicesForTests({ terminateProcesses: true }); + } finally { + vi.unstubAllEnvs(); + } }); function serviceCommand() { // Answers `/api/health` the way a real Paperclip dev runtime does: managed // publication requires semantic health, not just a 200 (PAP-17572). - return `node -e 'const http=require("http");const p=Number(process.env.PORT);for(const q of [p,p+10000].filter(q=>q<65536))http.createServer((rq,r)=>{if(rq.url==="/api/health"){r.setHeader("content-type","application/json");r.end(JSON.stringify({status:"ok"}));return}r.statusCode=200;r.end("ok")}).listen(q,"127.0.0.1");setInterval(()=>{},1000)'`; + return `node -e 'console.log("fixture started",process.pid,Date.now());const http=require("http");const p=Number(process.env.PORT);for(const q of [p,p+10000].filter(q=>q<65536))http.createServer((rq,r)=>{if(rq.url==="/api/health"){r.setHeader("content-type","application/json");r.end(JSON.stringify({status:"ok"}));return}r.statusCode=200;r.end("ok")}).listen(q,"127.0.0.1",()=>console.log("fixture listening",q,Date.now()));setInterval(()=>{},1000)'`; } /** @@ -362,7 +365,24 @@ function startInput(options?: { describe("workspace runtime tailscale_https lifecycle", () => { it("reserves before spawn, exposes after backend readiness, and removes on stop", async () => { const { broker, calls } = createBroker(); - installDeps({ broker }); + let reservedPorts: number[] = []; + installDeps({ + broker: { + ...broker, + async reserve(runtimeId, listeners) { + reservedPorts = listeners.map((listener) => listener.port); + for (const port of reservedPorts) expect(await isLoopbackPortFree(port)).toBe(true); + return broker.reserve(runtimeId, listeners); + }, + async expose(...args) { + for (const port of reservedPorts) { + const response = await fetch(`http://127.0.0.1:${port}/`, { signal: AbortSignal.timeout(5_000) }); + expect(await response.text()).toBe("ok"); + } + return broker.expose(...args); + }, + }, + }); const [runtime] = await startRuntimeServicesForWorkspaceControl(startInput()); expect(calls.slice(0, 2)).toEqual(["reserve", "expose"]); @@ -375,6 +395,7 @@ describe("workspace runtime tailscale_https lifecycle", () => { runtimeServiceId: runtime.id, }); expect(calls).toEqual(["reserve", "expose", "remove"]); + for (const port of reservedPorts) expect(await isLoopbackPortFree(port)).toBe(true); }, 15_000); it("fails closed and removes the mapping when external HTTPS validation fails", async () => { diff --git a/server/src/services/workspace-runtime.ts b/server/src/services/workspace-runtime.ts index e96f793bac..224ed7395e 100644 --- a/server/src/services/workspace-runtime.ts +++ b/server/src/services/workspace-runtime.ts @@ -5114,21 +5114,35 @@ export async function waitForRuntimeServiceReadiness(input: { const now = input.now ?? Date.now; const timeoutSec = resolveWorkspaceRuntimeReadinessTimeoutSec(input.service); const intervalMs = Math.max(100, asNumber(readiness.intervalMs, 500)); - const deadline = now() + timeoutSec * 1000; + const startedAt = now(); + const deadline = startedAt + timeoutSec * 1000; let lastError = "service did not become ready"; + let lastCause: unknown; + let probes = 0; while (now() < deadline) { const probeBudgetMs = Math.max(1, Math.min(RUNTIME_SERVICE_READINESS_PROBE_TIMEOUT_MS, deadline - now())); + probes += 1; try { const response = await fetchImpl(readinessUrl, { signal: AbortSignal.timeout(probeBudgetMs) }); if (response.ok) return; lastError = `received HTTP ${response.status}`; + lastCause = undefined; } catch (err) { + lastCause = err; lastError = err instanceof Error ? err.message : String(err); + // Node fetch hides connection errors behind "fetch failed". Retain the + // transport cause so a refused port is distinguishable from a timeout. + if (err instanceof Error && err.cause instanceof Error) { + lastError += `: ${err.cause.message}`; + } } if (now() >= deadline) break; await delay(Math.min(intervalMs, Math.max(0, deadline - now()))); } - throw new Error(`Readiness check failed for ${readinessUrl}: ${lastError}`); + throw new Error( + `Readiness check failed for ${readinessUrl}: ${lastError} (${probes} probes over ${now() - startedAt}ms)`, + { cause: lastCause }, + ); } async function waitForAllocatedPortBind(input: { diff --git a/tests/e2e/agent-chat-sessions.spec.ts b/tests/e2e/agent-chat-sessions.spec.ts index 477bf7a10f..8f860e304f 100644 --- a/tests/e2e/agent-chat-sessions.spec.ts +++ b/tests/e2e/agent-chat-sessions.spec.ts @@ -13,6 +13,41 @@ test.setTimeout(120_000); * live in ./agent-chat.shared.ts; project, attachment, and history flows run * in agent-chat-projects.spec.ts. */ +test("built chat initializes after service worker takeover and reload with a slow CPU", async ({ + page, + context, + request, +}) => { + const f = await setup(request); + try { + const cdp = await context.newCDPSession(page); + try { + await cdp.send("Emulation.setCPUThrottlingRate", { rate: 4 }); + await page.goto(f.route); + await expect(page.getByTestId("task-chat-composer-input")).toBeVisible(); + // The failed CI traces stopped before React evaluated, while a service + // worker forwarded the Vite module graph. Keep this test on shipped assets + // and cover both first takeover and subsequent controlled navigations. + const scripts = await page.locator('script[type="module"][src]').evaluateAll( + (elements) => elements.map((element) => element.getAttribute("src")), + ); + expect(scripts.length).toBeGreaterThan(0); + expect(scripts.every((src) => src?.startsWith("/assets/"))).toBe(true); + await page.evaluate(async () => { await navigator.serviceWorker.ready; }); + for (let reload = 0; reload < 3; reload += 1) { + await page.reload(); + await expect(page.getByTestId("task-chat-composer-input")).toBeVisible(); + expect(await page.evaluate(() => !!navigator.serviceWorker.controller)).toBe(true); + } + expect(await json(await request.get(f.chatPath))).toBeNull(); + } finally { + await cdp.detach(); + } + } finally { + await f.restore(); + } +}); + test("chat first open is read-only; concurrent first sends and retries share one task", async ({ page, context, @@ -240,11 +275,14 @@ test("sidebar discovery, stars, recent agents, configuration links, and drafts s } await expect(chatLinks).toHaveText(["Alpha", "Zeta", "Epsilon", "Delta", "Gamma"]); const star = page.getByRole("button", { name: "Star Zeta", exact: true }); - await page.getByTestId("task-chat-composer-input").click(); + await page.getByTestId("task-chat-composer-input").hover(); await expect(star).toHaveCSS("opacity", "0"); - await star.focus(); + // Enter from the preceding link. Clicking the rich editor first can leave + // a pending selection update that restores editor focus; Shift+Tab there + // also cycles work modes instead of moving backwards through the sidebar. + await nav.getByRole("link", { name: "Zeta", exact: true }).focus(); await page.keyboard.press("Tab"); - await page.keyboard.press("Shift+Tab"); + await expect(star).toBeFocused(); await expect(star).toHaveCSS("opacity", "1"); await star.click(); await page.goto(f.route); diff --git a/tests/e2e/playwright.config.ts b/tests/e2e/playwright.config.ts index 5f17a3f1b1..0d68ae5724 100644 --- a/tests/e2e/playwright.config.ts +++ b/tests/e2e/playwright.config.ts @@ -61,7 +61,13 @@ export default defineConfig({ // The webServer directive bootstraps a throwaway instance and then starts it. // `onboard --yes --run` works in a non-interactive temp PAPERCLIP_HOME. webServer: { - command: `pnpm paperclipai onboard --yes --run`, + cwd: path.resolve(import.meta.dirname, "../.."), + // Exercise the shipped UI. Source-checkout onboarding otherwise enables + // Vite middleware: every reload traverses thousands of modules, including + // service-worker-intercepted requests, before React can even start. + // Build the server's first-choice static directory so a prior package build + // cannot shadow the UI under test with stale server/ui-dist assets. + command: "pnpm --filter @paperclipai/ui build --outDir ../server/ui-dist --emptyOutDir && node cli/node_modules/tsx/dist/cli.mjs cli/src/index.ts onboard --yes --run", url: `${BASE_URL}/api/health`, // Always boot a dedicated throwaway instance for e2e so browser tests // never attach to the developer's active Paperclip home/server. @@ -72,6 +78,7 @@ export default defineConfig({ env: { ...process.env, NODE_ENV: "test", + PAPERCLIP_UI_DEV_MIDDLEWARE: "false", NODE_OPTIONS: `${process.env.NODE_OPTIONS ?? ""} --import=${path.resolve(import.meta.dirname, "fixtures/agent-chat-github.mjs")}`, PORT: String(PORT), PAPERCLIP_OPEN_ON_LISTEN: "false", diff --git a/ui/src/context/LiveUpdatesProvider.recovery.test.tsx b/ui/src/context/LiveUpdatesProvider.recovery.test.tsx index 6dcf566c00..edd3b41769 100644 --- a/ui/src/context/LiveUpdatesProvider.recovery.test.tsx +++ b/ui/src/context/LiveUpdatesProvider.recovery.test.tsx @@ -39,7 +39,8 @@ describe("LiveUpdatesProvider connection recovery", () => { beforeEach(() => { vi.useFakeTimers(); vi.spyOn(document, "visibilityState", "get").mockReturnValue("visible"); - read.mockClear(); + read.mockReset(); + read.mockResolvedValue("current task data"); Socket.instances = []; client = new QueryClient({ defaultOptions: { queries: { retry: false, gcTime: Infinity, staleTime: Infinity } } }); client.setQueryData(queryKeys.auth.session, { user: { id: "viewer" }, session: { id: "session", userId: "viewer" } }); @@ -67,6 +68,23 @@ describe("LiveUpdatesProvider connection recovery", () => { await act(async () => vi.advanceTimersByTimeAsync(1)); } + it("reconciles updates between the initial query and the first socket connection", async () => { + vi.stubGlobal("WebSocket", Socket); + await render(); + expect(container.textContent).toBe("current task data"); + expect(Socket.instances).toHaveLength(1); + + // The server saves a reply after the page read but before it subscribes. + // There is no event replay and this is not a reconnect. + read.mockResolvedValue("reply saved during connection setup"); + await act(async () => Socket.instances[0].onopen?.()); + await act(async () => vi.advanceTimersByTimeAsync(1)); + expect(container.textContent).toBe("reply saved during connection setup"); + const readsAfterConnect = read.mock.calls.length; + await act(async () => vi.advanceTimersByTimeAsync(30_000)); + expect(read).toHaveBeenCalledTimes(readsAfterConnect); + }); + it.each(["missing", "throws"])("polls visible data and resumes realtime when the constructor %s", async (failure) => { vi.stubGlobal("WebSocket", failure === "missing" ? undefined : class { constructor() { throw new DOMException("Blocked", "SecurityError"); } diff --git a/ui/src/context/LiveUpdatesProvider.tsx b/ui/src/context/LiveUpdatesProvider.tsx index f86c71b47b..925dd6dc45 100644 --- a/ui/src/context/LiveUpdatesProvider.tsx +++ b/ui/src/context/LiveUpdatesProvider.tsx @@ -1991,9 +1991,10 @@ export function LiveUpdatesProvider({ children }: { children: ReactNode }) { stopPolling(); if (reconnectAttempt > 0) { gateRef.current.suppressUntil = Date.now() + RECONNECT_SUPPRESS_MS; - // Reconcile all visible data after a gap: missed events cannot be replayed. - void queryClient.invalidateQueries({ type: "active" }, { cancelRefetch: false }); } + // The initial page queries can finish before the first subscription, + // too. Reconcile that gap as well as reconnects: events are not replayed. + void queryClient.invalidateQueries({ type: "active" }, { cancelRefetch: false }); reconnectAttempt = 0; };