diff --git a/doc/DEVELOPING.md b/doc/DEVELOPING.md index 814cb9750d0..da29540ff9c 100644 --- a/doc/DEVELOPING.md +++ b/doc/DEVELOPING.md @@ -3425,7 +3425,7 @@ successful warm-session retention or admit another provider turn. Run the credential-free real-process restart suite with: ```sh -pnpm --filter @paperclipai/paperclip-runner build:runner-binaries +pnpm --filter @paperclipai/paperclip-runner run build:runner-binaries pnpm exec vitest run server/src/services/native-runtime/native-runner-restart-recovery.integration.test.ts pnpm --filter @paperclipai/paperclip-runner exec vitest run src/live/runnerd-codex-transport-adoption.test.ts -t 'adopts a live runner' ``` diff --git a/tests/restart-chaos/README.md b/tests/restart-chaos/README.md index 14207e23782..348747af498 100644 --- a/tests/restart-chaos/README.md +++ b/tests/restart-chaos/README.md @@ -12,7 +12,7 @@ T-oracle. ```sh pnpm install --frozen-lockfile -pnpm --filter @paperclipai/paperclip-runner build:runner-binaries # native runner (debug) +pnpm --filter @paperclipai/paperclip-runner run build:runner-binaries # native runner (debug) set -a; source ~/.secrets; set +a # provider keys, process env only node cli/node_modules/tsx/dist/cli.mjs tests/restart-chaos/main.ts --env local \ --adapters codex,claude,opencode --concurrency 4 --tasks 8 --faults K1,K2,K4 --seed 42 diff --git a/tests/restart-chaos/main.ts b/tests/restart-chaos/main.ts index 7fd6981e028..c17c08d6c03 100644 --- a/tests/restart-chaos/main.ts +++ b/tests/restart-chaos/main.ts @@ -53,7 +53,7 @@ function preflight(plan: WavePlan) { if (missing.length) throw new Error(`Missing credentials ${missing.join(", ")}. Run: set -a; source ~/.secrets; set +a`); const runner = process.env.PAPERCLIP_RUNNER_BINARY ?? path.join(repositoryRoot, "packages/paperclip-runner/runner/target/debug/paperclip-runnerd"); - if (!existsSync(runner)) throw new Error(`Runner binary missing at ${runner}. Run: pnpm --filter @paperclipai/paperclip-runner build:runner-binaries`); + if (!existsSync(runner)) throw new Error(`Runner binary missing at ${runner}. Run: pnpm --filter @paperclipai/paperclip-runner run build:runner-binaries`); // Same provider bundle preparation as tests/runner-e2e/verified-provider-bundles.setup.mjs. const build = spawnSync(process.execPath, [path.join(repositoryRoot, "packages/paperclip-runner/scripts/build-verified-provider-entrypoints.mjs")], { cwd: repositoryRoot, stdio: "inherit" }); diff --git a/tests/runner-e2e/README.md b/tests/runner-e2e/README.md index f6be16f3c1d..dcd913f3133 100644 --- a/tests/runner-e2e/README.md +++ b/tests/runner-e2e/README.md @@ -686,7 +686,7 @@ so a content mismatch cannot suppress the communication evidence. The source thread is observed for 120 seconds. The probe retains a later correction even if an earlier reply already passes delivery and access. A later clarification does not erase an earlier accessible delivery. -The busy cell holds a separate source reply open until the worker finishes; the multiple cell delegates two notes and requires one completion per task. The restart cell holds the source provider at a fixture reference gate, then restarts the server after durable Done but before publication and releases the gate. The gate makes the interruption boundary observable and prevents a fast successful reply from racing the restart assertion. The onboarding cell records its naturally occurring timing. +The busy cell holds a separate source reply open until the worker finishes; the multiple cell delegates two notes and requires one completion per task. The restart cell holds the source provider at a fixture reference gate, then restarts the server after durable Done but before publication and releases the gate. The gate makes the interruption boundary observable and prevents a fast successful reply from racing the restart assertion. Reaching the gate depends on the source agent running the instructed wait: the completion reply is published inside the run finalizer's own transaction, so the isolated-server entrypoint cannot hold it without a product hook. If no completion turn is still active and the turn that received this task's Done facts has succeeded or published its reply before the gate reports `waiting`, the cell fails at once with `restart boundary not reached (agent skipped gate)` rather than a generic timeout. That failure is agent compliance (the restart was never exercised), not restart robustness. A completion turn that fails or is cancelled before the gate is not blamed on the agent, because delivery can retry it; if the wait then times out, the timeout names that turn and its status as `restart boundary not reached`. After the restart, the chat navigation retries an abort caused by the old document's reload. The onboarding cell records its naturally occurring timing. The mechanical oracle requires a run-attributed source reply after durable completion, plus the actual saved output or a navigable task/output link. @@ -839,7 +839,7 @@ runner binaries: ```bash pnpm install pnpm exec playwright install chromium -pnpm --filter @paperclipai/paperclip-runner build:runner-binaries +pnpm --filter @paperclipai/paperclip-runner run build:runner-binaries ``` List cells without loading credentials or starting Paperclip: diff --git a/tests/runner-e2e/chat-restart.test.ts b/tests/runner-e2e/chat-restart.test.ts new file mode 100644 index 00000000000..c2e2e5544d2 --- /dev/null +++ b/tests/runner-e2e/chat-restart.test.ts @@ -0,0 +1,54 @@ +import { describe, expect, it } from "vitest"; +import { gotoAfterServerRestart, isAbortedNavigation } from "./chat-restart.js"; + +const aborted = () => new Error("page.goto: net::ERR_ABORTED at http://127.0.0.1:1/RUN/issues/RUN-1"); +const timeout = () => Object.assign(new Error("page.goto: Timeout 30000ms exceeded."), { name: "TimeoutError" }); +function page(outcomes: Array) { + const calls: string[] = []; + return { + calls, + goto: async (url: string) => { + calls.push(url); + const outcome = outcomes.shift(); + if (outcome) throw outcome; + return null; + }, + }; +} + +describe("navigation after a server restart", () => { + it("retries a navigation the old document's reload aborted", async () => { + const p = page([aborted(), aborted(), undefined]); + await gotoAfterServerRestart(p as never, "/issue", { deadlineAt: Date.now() + 10_000, retryDelayMs: 0 }); + expect(p.calls).toEqual(["/issue", "/issue", "/issue"]); + }); + + it("keeps tolerating an unsettled lifecycle timeout without retrying", async () => { + const p = page([timeout()]); + await gotoAfterServerRestart(p as never, "/issue", { deadlineAt: Date.now() + 10_000, retryDelayMs: 0 }); + expect(p.calls).toHaveLength(1); + }); + + it("rethrows other navigation errors immediately", async () => { + const p = page([new Error("page.goto: net::ERR_CONNECTION_REFUSED"), undefined]); + await expect(gotoAfterServerRestart(p as never, "/issue", { deadlineAt: Date.now() + 10_000, retryDelayMs: 0 })) + .rejects.toThrow("ERR_CONNECTION_REFUSED"); + expect(p.calls).toHaveLength(1); + }); + + it("stops retrying aborts at the attempt bound and at the deadline", async () => { + const bounded = page([aborted(), aborted(), aborted(), undefined]); + await expect(gotoAfterServerRestart(bounded as never, "/issue", { deadlineAt: Date.now() + 10_000, retryDelayMs: 0, maxAttempts: 3 })) + .rejects.toThrow("ERR_ABORTED"); + expect(bounded.calls).toHaveLength(3); + const late = page([aborted(), undefined]); + await expect(gotoAfterServerRestart(late as never, "/issue", { deadlineAt: 0, retryDelayMs: 0 })).rejects.toThrow("ERR_ABORTED"); + expect(late.calls).toHaveLength(1); + }); + + it("recognizes only Chromium's aborted-navigation error", () => { + expect(isAbortedNavigation(aborted())).toBe(true); + expect(isAbortedNavigation(new Error("net::ERR_ABORTED_SOMETHING"))).toBe(false); + expect(isAbortedNavigation("net::ERR_ABORTED")).toBe(false); + }); +}); diff --git a/tests/runner-e2e/chat-restart.ts b/tests/runner-e2e/chat-restart.ts index ef488ab3bd6..ed6bcfc0368 100644 --- a/tests/runner-e2e/chat-restart.ts +++ b/tests/runner-e2e/chat-restart.ts @@ -5,3 +5,37 @@ export async function restartChatServer(page: Page, restart: () => Promise await page.goto("about:blank", { waitUntil: "commit", timeout: 30_000 }); await restart(); } + +/** Chromium reports a navigation another navigation replaced as net::ERR_ABORTED. */ +export function isAbortedNavigation(error: unknown): boolean { + return error instanceof Error && /\bnet::ERR_ABORTED\b/.test(error.message); +} + +/** + * Open `url` on a page whose document predates a server restart. The old + * document's Vite client reloads itself when the replacement server comes up; + * if that reload lands while this navigation is in flight, Chromium aborts one + * of them with net::ERR_ABORTED. Retry only that abort, within the caller's + * deadline and a bounded attempt count, and rethrow every other error. A + * timeout still resolves: the replacement server can commit and render a fresh + * document while its lifecycle stays unsettled, so callers must assert the new + * document and durable state explicitly afterwards. + */ +export async function gotoAfterServerRestart( + page: Pick, + url: string, + input: { deadlineAt: number; maxAttempts?: number; retryDelayMs?: number; now?: () => number }, +): Promise { + const now = input.now ?? Date.now; + const maxAttempts = input.maxAttempts ?? 5; + for (let attempt = 1; ; attempt += 1) { + try { + await page.goto(url, { waitUntil: "commit" }); + return; + } catch (error) { + if (error instanceof Error && error.name === "TimeoutError") return; + if (!isAbortedNavigation(error) || attempt >= maxAttempts || now() >= input.deadlineAt) throw error; + } + await new Promise(resolve => setTimeout(resolve, input.retryDelayMs ?? 500)); + } +} diff --git a/tests/runner-e2e/completion-update-flow.ts b/tests/runner-e2e/completion-update-flow.ts index 9e6978ad53a..2b4bfbb423b 100644 --- a/tests/runner-e2e/completion-update-flow.ts +++ b/tests/runner-e2e/completion-update-flow.ts @@ -6,7 +6,8 @@ import { pollUntil, type RunnerApi } from "./api.js"; import { sendChatMessage, readChatOutputDocument, collectChatRunEvidence, type ChatFlowInput, type ChatRun } from "./chat-flow.js"; import { arm as armDocumentGate, clear as clearDocumentGate, release as releaseDocumentGate, waitUntilHeld } from "./context-comment-gate.js"; import { publishChatBrief, workspaceChatBrief } from "./chat-stories.js"; -import { completionDelivery, completionOutputUsesReleasedBrief, type CompletionObservation } from "./completion-updates.js"; +import { gotoAfterServerRestart } from "./chat-restart.js"; +import { completionDelivery, completionOutputUsesReleasedBrief, completionRestartGateSkipped, completionRestartGateTimeoutDetail, type CompletionObservation } from "./completion-updates.js"; type Row = Record; export async function observeCompletionUpdate(input: { @@ -232,10 +233,15 @@ export async function runChatCompletionUpdate(context: { load: async () => { const runs = await context.allRuns(); return { runs, ready: await readFile(completionWait!.at(taskDirectory(context.issue().id)).ready, "utf8").catch(() => ""), + comments: await api.get(`/api/issues/${context.issue().id}/comments?order=asc`), sourceCwds: runCwd(runs, context.issue().id) }; }, accept: state => state.ready === "waiting" && state.runs.some(r => r.contextSnapshot?.issueId === context.issue().id && - r.contextSnapshot?.wakeReason === "chat_task_completed" && r.status === "running") }); + r.contextSnapshot?.wakeReason === "chat_task_completed" && r.status === "running"), + // The gate depends on the agent running the instructed wait; name a + // skipped gate rather than timing out (see completionRestartGateSkipped). + reject: state => completionRestartGateSkipped({ sourceId: context.issue().id, workerId: task!.id, ...state }), + timeoutDetail: state => completionRestartGateTimeoutDetail(state && { sourceId: context.issue().id, workerId: task!.id, ...state }) }); const beforeRestart = await api.get(`/api/issues/${context.issue().id}/comments?order=asc`); const workerState = await api.get(`/api/issues/${task!.id}`); expect(beforeRestart.filter(c => c.authorAgentId && c.createdAt >= workerState.completedAt), "restart must precede completion publication").toEqual([]); @@ -245,7 +251,8 @@ export async function runChatCompletionUpdate(context: { await input.evidence("completion-restart-boundary.json", { worker: await api.get(`/api/issues/${task!.id}`), runs: reportingRun.runs, sourceReferenceGateReady: true }); await input.restart(); await publishBrief(completionWait!, context.issue().id, "The organizer is ready to receive the saved result. Report the completed work now."); - await page.goto(`/${f.company.issuePrefix}/chats/${f.agent.id}`, { waitUntil: "commit" }); + // The old document's Vite client can reload and abort this navigation. + await gotoAfterServerRestart(page, `/${f.company.issuePrefix}/chats/${f.agent.id}`, { deadlineAt: input.deadlineAt }); } for (const [index, item] of delegated.entries()) { await pollUntil({ label: "delegated note completed", deadlineAt: Date.now() + 180_000, intervalMs: 1000, diff --git a/tests/runner-e2e/completion-updates.test.ts b/tests/runner-e2e/completion-updates.test.ts index d06bbee5736..0900c9aa987 100644 --- a/tests/runner-e2e/completion-updates.test.ts +++ b/tests/runner-e2e/completion-updates.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { completionDelivery, completionOutputUsesReleasedBrief, type CompletionObservation } from "./completion-updates.js"; +import { COMPLETION_RESTART_GATE_SKIPPED, completionDelivery, completionOutputUsesReleasedBrief, completionRestartGateSkipped, completionRestartGateTimeoutDetail, type CompletionObservation } from "./completion-updates.js"; import { runnerMatrix } from "./catalog.js"; const output = "Welcome to our free Friday garden meetup for beginners. Join us at 10:30; reference GARDEN123."; @@ -82,3 +82,45 @@ describe("completion-update delivery oracle", () => { } }); }); + +describe("completion restart gate", () => { + const turn = (status: string, id = "turn", workerId: string | null = "worker") => ({ id, status, + contextSnapshot: { issueId: "source", wakeReason: "chat_task_completed", ...(workerId ? { chatCompletionUpdates: [{ id: workerId, status: "done" }] } : {}) } }); + const base = { sourceId: "source", workerId: "worker", runs: [] as Record[], comments: [] as Record[] }; + it("keeps waiting while the completion turn is pending or running at the gate", () => { + expect(completionRestartGateSkipped({ ...base, ready: "" })).toBeUndefined(); + expect(completionRestartGateSkipped({ ...base, ready: "", runs: [turn("queued", "turn", null)] })).toBeUndefined(); + expect(completionRestartGateSkipped({ ...base, ready: "", runs: [turn("running")] })).toBeUndefined(); + expect(completionRestartGateSkipped({ ...base, ready: "waiting", runs: [turn("succeeded")] })).toBeUndefined(); + }); + it("names a correlated completion turn that succeeded without reaching the gate", () => { + expect(completionRestartGateSkipped({ ...base, ready: "", runs: [turn("succeeded")] })) + .toBe(`${COMPLETION_RESTART_GATE_SKIPPED}: completion turn turn ended succeeded without the reference gate reporting waiting`); + }); + it("keeps waiting while a later completion turn is still active", () => { + expect(completionRestartGateSkipped({ ...base, ready: "", runs: [turn("succeeded", "first"), turn("queued", "retry", null)] })).toBeUndefined(); + }); + it("does not blame the agent for a turn that failed before the gate, and names it on timeout", () => { + const state = { ...base, ready: "", runs: [turn("failed")] }; + expect(completionRestartGateSkipped(state)).toBeUndefined(); + expect(completionRestartGateTimeoutDetail(state)) + .toBe("restart boundary not reached: completion turn(s) turn ended failed before the reference gate reported waiting"); + expect(completionRestartGateTimeoutDetail({ ...state, ready: "waiting" })).toBeUndefined(); + expect(completionRestartGateTimeoutDetail(undefined)).toBeUndefined(); + }); + it("names a correlated agent reply and ignores unrelated replies", () => { + const comments = [ + { id: "handoff", authorAgentId: "lead", createdByRunId: "initial" }, + { id: "user", authorUserId: "board" }, + ]; + expect(completionRestartGateSkipped({ ...base, ready: "", runs: [turn("failed")], comments })).toBeUndefined(); + expect(completionRestartGateSkipped({ ...base, ready: "", runs: [turn("failed")], + comments: [...comments, { id: "reply", authorAgentId: "lead", createdByRunId: "turn" }] })) + .toBe(`${COMPLETION_RESTART_GATE_SKIPPED}: agent reply reply was published without the reference gate reporting waiting`); + }); + it("ignores other conversations, other tasks and ordinary chat turns", () => { + const other = { id: "o", status: "succeeded", contextSnapshot: { issueId: "other", wakeReason: "chat_task_completed", chatCompletionUpdates: [{ id: "worker" }] } }; + const ordinary = { id: "u", status: "succeeded", contextSnapshot: { issueId: "source", wakeReason: "chat_message" } }; + expect(completionRestartGateSkipped({ ...base, ready: "", runs: [other, ordinary, turn("succeeded", "t2", "another-worker")] })).toBeUndefined(); + }); +}); diff --git a/tests/runner-e2e/completion-updates.ts b/tests/runner-e2e/completion-updates.ts index 2cb751ccf6a..1af507116b7 100644 --- a/tests/runner-e2e/completion-updates.ts +++ b/tests/runner-e2e/completion-updates.ts @@ -23,6 +23,47 @@ export const completionReviewRubric = [ "Does it avoid repeated completion announcements or redundant acknowledgements, while allowing corrections and updates about different tasks?", ] as const; +export const COMPLETION_RESTART_GATE_SKIPPED = "restart boundary not reached (agent skipped gate)"; + +type GateState = { sourceId: string; workerId: string; ready: string; runs: Row[]; comments: Row[] }; +const completionTurns = (state: GateState) => state.runs.filter(r => r.contextSnapshot?.issueId === state.sourceId && + r.contextSnapshot?.wakeReason === "chat_task_completed"); +/** Correlated as completionDelivery does: the turn received this task's Done facts. */ +const carriesWorker = (run: Row, workerId: string) => Array.isArray(run.contextSnapshot?.chatCompletionUpdates) && + run.contextSnapshot.chatCompletionUpdates.some((u: Row) => u.id === workerId); + +/** + * The restart cell's boundary is the source provider waiting at its reference + * gate. The completion reply is published inside the run finalizer's own + * transaction, which an isolated-server fixture cannot hold without a product + * hook, so reaching the boundary depends on the agent running the wait. Once + * no completion turn is active and this task's correlated turn has succeeded + * or published its reply without the gate ever reporting "waiting", the + * boundary can no longer be reached: report it by name instead of waiting for + * a generic timeout. A failed or cancelled turn is not evidence that the agent + * skipped the gate, and delivery may retry it; completionRestartGateTimeoutDetail + * names it if the wait then times out. + */ +export function completionRestartGateSkipped(state: GateState): string | undefined { + if (state.ready === "waiting") return undefined; + const turns = completionTurns(state); + if (turns.some(r => ["queued", "running"].includes(String(r.status)))) return undefined; + const correlated = turns.filter(r => carriesWorker(r, state.workerId)); + const reply = state.comments.find(c => c.authorAgentId && correlated.some(r => r.id === c.createdByRunId)); + if (reply) return `${COMPLETION_RESTART_GATE_SKIPPED}: agent reply ${reply.id} was published without the reference gate reporting waiting`; + const succeeded = correlated.find(r => r.status === "succeeded"); + if (succeeded) return `${COMPLETION_RESTART_GATE_SKIPPED}: completion turn ${succeeded.id} ended succeeded without the reference gate reporting waiting`; + return undefined; +} + +/** Names completion turns that ended unsuccessfully before the gate when the wait times out. */ +export function completionRestartGateTimeoutDetail(state: GateState | undefined): string | undefined { + if (!state || state.ready === "waiting") return undefined; + const ended = completionTurns(state).filter(r => !["queued", "running", "succeeded"].includes(String(r.status))); + if (!ended.length) return undefined; + return `restart boundary not reached: completion turn(s) ${ended.map(r => `${r.id} ended ${r.status}`).join(", ")} before the reference gate reported waiting`; +} + /** The start time exists only in the brief released after the source is idle. */ export function completionOutputUsesReleasedBrief(body: string): boolean { return /\b(?:10\s*[:.h]\s*30|ten[-\s]+thirty|half[\s-]+past[\s-]+(?:ten|10))\b/i.test(body); diff --git a/tests/runner-e2e/connection-reviews.test.ts b/tests/runner-e2e/connection-reviews.test.ts new file mode 100644 index 00000000000..81bb5917ac4 --- /dev/null +++ b/tests/runner-e2e/connection-reviews.test.ts @@ -0,0 +1,19 @@ +import { readFile } from "node:fs/promises"; +import { describe, expect, it } from "vitest"; +import { CONNECTOR_SETUP_NEXT_STEP } from "./connection-reviews.js"; + +describe("connection review setup", () => { + it("accepts the one-screen Check link step and the older Save and continue step", () => { + expect(CONNECTOR_SETUP_NEXT_STEP.test("Check link")).toBe(true); + expect(CONNECTOR_SETUP_NEXT_STEP.test("Save and continue")).toBe(true); + expect(CONNECTOR_SETUP_NEXT_STEP.test("Continue")).toBe(false); + expect(CONNECTOR_SETUP_NEXT_STEP.test("Checking…")).toBe(false); + }); + + it("matches a label the current connector setup actually renders", async () => { + const source = await readFile(new URL("../../ui/src/features/connections/ConnectionSetupFlow.tsx", import.meta.url), "utf8"); + const labels = [...source.matchAll(/"(Check link|Save and continue)"/g)].map(match => match[1]!); + expect(labels.length).toBeGreaterThan(0); + expect(labels.every(label => CONNECTOR_SETUP_NEXT_STEP.test(label))).toBe(true); + }); +}); diff --git a/tests/runner-e2e/connection-reviews.ts b/tests/runner-e2e/connection-reviews.ts index a92cfff2035..2962bdd0a8a 100644 --- a/tests/runner-e2e/connection-reviews.ts +++ b/tests/runner-e2e/connection-reviews.ts @@ -3,6 +3,10 @@ import { expect, type Page } from "@playwright/test"; import type { RunnerApi } from "./api.js"; import { startReviewProvider } from "../fixtures/connection-review-provider.js"; +/** The button after the URL step: Check link today, Save and continue before #14811. */ +export const CONNECTOR_SETUP_NEXT_STEP = /^(Save and continue|Check link)$/i; +const CHECK_LINK = /^Check link$/i; + /** Uses the same production connection and permission screens as the fixture browser suite. */ export async function setupConnectionReview(input: { page: Page; @@ -62,8 +66,15 @@ export async function setupConnectionReview(input: { .getByPlaceholder("https://example.com/actions") .fill(provider.url); await page.getByRole("button", { name: "Continue", exact: true }).click(); - await page.getByRole("button", { name: "Save and continue" }).click(); - await page.getByRole("button", { name: /Check link/i }).click(); + // #14811 made connector setup one screen: Continue now leads straight to + // Check link. Accept the older intermediate "Save and continue" step too, + // as connection-flow.ts does, so either layout reaches the link check. + const next = page + .getByRole("button", { name: CONNECTOR_SETUP_NEXT_STEP }) + .first(); + await expect(next).toBeVisible(); + if (!CHECK_LINK.test((await next.textContent()) ?? "")) await next.click(); + await page.getByRole("button", { name: CHECK_LINK }).click(); await expect( page.getByRole("heading", { name: /is ready/i }), ).toBeVisible(); diff --git a/tests/runner-e2e/runner.spec.ts b/tests/runner-e2e/runner.spec.ts index c0c4524b1a2..3113cf0ad9a 100644 --- a/tests/runner-e2e/runner.spec.ts +++ b/tests/runner-e2e/runner.spec.ts @@ -64,7 +64,7 @@ import { armResponseTiming } from "./response-timing.js"; import { projectFirstTaskResult, runFirstTaskFlow, setupFirstTaskFixtures } from "./first-task-flow.js"; import { runChatFlow } from "./chat-flow.js"; -import { restartChatServer } from "./chat-restart.js"; +import { gotoAfterServerRestart, restartChatServer } from "./chat-restart.js"; import { matchesRunCount, minimumRunCount } from "./run-count.js"; import { createHash, randomBytes } from "node:crypto"; import { mkdir, readFile, rename, writeFile } from "node:fs/promises"; @@ -1489,11 +1489,13 @@ for (const execution of executions) { await pollUntil({ label: "governed waiting turn", deadlineAt, load: loadTaskState, accept: state => state.taskRuns.length === 1 && state.taskRuns.every(run => TERMINAL_RUN_STATUSES.has(run.status)) }); await captureScreenshot("tool-review-pending", "Connection review awaiting a human", "tool-review-pending.png"); await page.getByRole("button", { name: "Dismiss Approve tool action" }).click(); - await page.getByRole("button", { name: "Review request", exact: true }).click(); if (execution.task.toolReviewDecision === "restart") { await restartIsolatedPaperclipServer({ api, requestId: `tool-review-${nonce}`, deadlineAt }); await page.reload(); } + // Open the review card on the document that will act on it: an + // expanded card does not survive the post-restart reload. + await page.getByRole("button", { name: "Review request", exact: true }).click(); if (execution.task.toolReviewDecision === "always") { await page.getByRole("button", { name: "Approval options", exact: true }).click(); await page.getByRole("menuitem", { name: "Always allow", exact: true }).click(); @@ -1602,20 +1604,16 @@ for (const execution of executions) { (key) => Reflect.set(window, key, true), documentSentinel, ); - try { - await page.goto( - `/${encodeURIComponent(issuePrefix)}/issues/${encodeURIComponent(issue.identifier ?? issue.id)}`, - // The replacement Vite server can commit and render a fresh - // document while its navigation lifecycle remains unsettled. - // The sentinel and explicit assertions below prove the new - // document and durable state even when Playwright times out. - { waitUntil: "commit" }, - ); - } catch (error) { - if (!(error instanceof Error) || error.name !== "TimeoutError") { - throw error; - } - } + // The replacement Vite server can commit and render a fresh document + // while its navigation lifecycle remains unsettled, and the old + // document's Vite client can reload and abort this navigation. The + // sentinel and explicit assertions below prove the new document and + // durable state either way. + await gotoAfterServerRestart( + page, + `/${encodeURIComponent(issuePrefix)}/issues/${encodeURIComponent(issue.identifier ?? issue.id)}`, + { deadlineAt }, + ); await expect( page .getByRole("radio", {