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; };