mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-09 07:10:34 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The Runner Product E2E suite proves that agent work survives a control-plane restart > - A restart chaos campaign found three restart cells that fail in the test harness, not in the product > - A harness failure hides the real restart result, so the campaign cannot classify these cells > - This pull request repairs the harness drift in those cells and names the one failure the harness cannot prevent > - The benefit is that these cells now test restart behavior, or fail with a clear cause ## Linked Issues or Issue Description Refs #14811 (connector setup became one screen). Refs #19475 (completion-update restart fixture). **What happened?** 1. `lifecycle-baseline.runner-codex.local.tool-review-restart` clicked a "Save and continue" button. #14811 removed that button. The cell failed during setup. After that fix, the cell also failed after the restart: the expanded review card does not survive the post-restart reload, so "Approve & run" was not visible. 2. `local-session-integrity.runner-acpx-claude.local.structured-question-restart-resume` failed with `page.goto: net::ERR_ABORTED` after the controller restart. The old document's Vite client reloads when the replacement server starts, and this aborts the test navigation. The code tolerated only `TimeoutError`. 3. `completion-updates.runner-acpx-claude.local.handoff-completion-restart` waited for the source agent to run an instructed wait command. Claude replied without the command (2/2 attempts), so the restart never occurred. The cell then failed with a generic timeout. **Expected behavior** Each cell reaches its restart boundary and tests restart behavior. If the boundary cannot be reached, the cell names the cause. **Steps to reproduce** Run `pnpm test:e2e:runner -- --max-parallel 1 --max-automatic-retries 0 --id <cell>` for each cell above on `master`. ## What Changed - `tests/runner-e2e/connection-reviews.ts`: accept the one-screen "Check link" step and the older "Save and continue" step. This uses the same regex approach as `connection-flow.ts`. - `tests/runner-e2e/runner.spec.ts` (tool review): click "Review request" after the restart reload, on the document that acts on the card. - `tests/runner-e2e/chat-restart.ts`: add `gotoAfterServerRestart`. It retries only `net::ERR_ABORTED`, within the caller's deadline and at most 5 attempts. It still accepts `TimeoutError`. It rethrows all other errors. The structured-question restart uses it. The existing sentinel and durable-state assertions still prove the new document. - `tests/runner-e2e/completion-updates.ts` and `completion-update-flow.ts`: add `completionRestartGateSkipped` and `completionRestartGateTimeoutDetail`. The gate poll stops at once with `restart boundary not reached (agent skipped gate)` when no completion turn is active and the turn that carries this task's Done facts (`chatCompletionUpdates`, as in `completionDelivery`) succeeded or published its reply before the gate reports `waiting`. A failed or cancelled turn is not blamed on the agent, because delivery can retry it; the timeout detail names it instead. The post-restart chat navigation now uses `gotoAfterServerRestart`. - A fixture-level gate is not possible without a product hook. The native run finalizer publishes the completion reply inside its own transaction, through the issue service closure. The isolated-server entrypoint can patch only class prototypes and HTTP responses, and the reply uses no tool call or HTTP request. - `tests/runner-e2e/README.md`: document the skipped-gate failure. - Docs: use `pnpm --filter @paperclipai/paperclip-runner run build:runner-binaries` in `tests/runner-e2e/README.md`, `tests/restart-chaos/README.md`, `tests/restart-chaos/main.ts` and `doc/DEVELOPING.md`. - No product code changes. ## Verification - Unit: `pnpm exec vitest run --config tests/runner-e2e/vitest.config.ts tests/runner-e2e/chat-restart.test.ts tests/runner-e2e/completion-updates.test.ts tests/runner-e2e/connection-reviews.test.ts` (36 passed). - Typecheck: `node scripts/typecheck-test-projects.mjs tests/runner-e2e` passed. - Live cells, local, `--max-parallel 1 --max-automatic-retries 0`, macOS, load average 35–65: - `lifecycle-baseline.runner-codex.local.tool-review-restart`: PASS (first run found the review-card reload drift, fixed in the second commit). - `local-session-integrity.runner-acpx-claude.local.structured-question-restart-resume`: PASS. A first attempt failed before the restart with `PRP command session.open timed out` at load ~36 (environment). - `completion-updates.runner-acpx-claude.local.handoff-completion-restart`: FAIL as designed, twice (before and after the review fix), with `restart boundary not reached (agent skipped gate): completion turn … ended succeeded without the reference gate reporting waiting`. Claude skipped the wait both times, so this cell still does not exercise a restart. ## Risks - Low risk. Test harness and docs only. - The navigation retry could hide a real abort. It is limited to `net::ERR_ABORTED`, 5 attempts and the deadline, and the test still asserts that a new document loaded. - The ACPX Claude completion restart cell still does not exercise a restart when the agent skips the gate. It now reports this cause by name. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edit, browser test runs). ## 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 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
55 lines
2.4 KiB
TypeScript
55 lines
2.4 KiB
TypeScript
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<Error | undefined>) {
|
|
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);
|
|
});
|
|
});
|