From 377e98f1a2ccbe041d5b893e435dc8dbd71df2ef Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Thu, 5 Nov 2026 13:20:20 -0600 Subject: [PATCH] test(runner): keep the production close budget for recovered prepared-descriptor closes (#19309) 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 > - The Paperclip Runner closes a native session only after it proves that the provider drained and the runner suspended > - The test "runs an environment-prepared descriptor without Core filesystem preparation (rotated-recovery)" closes recovered transports with a 5s close grace > - A 5s grace leaves only 2.5s for the provider drain, and a recovered successor needs several drain round trips > - On a loaded CI runner with the debug runnerd, the drain does not finish in 2.5s, so close correctly fails closed and the test fails > - This pull request makes those closes use the production close budget > - The benefit is that the Runner Vitest lane stops failing on unrelated PRs, and the close assertions stay the same ## Linked Issues or Issue Description No issue exists. The problem, in the bug report format: **What happened?** The Runner Vitest lane failed on PR #19265 (run 42075789869, job "Verify Paperclip Runner (vitest package)"). The change in that PR is not related to the Runner: ``` FAIL src/live/runnerd-codex-transport.test.ts > runs an environment-prepared descriptor without Core filesystem preparation (rotated-recovery) NativeSessionCloseUnrecoverableError: provider_transport_failed: runner did not durably suspend before checkpoint ❯ src/live/runnerd-codex-transport.test.ts:2768:21 Serialized Error: { settlement: { runnerSuspended: true, providerDrained: false, suspensionState: null, ... finalProviderState: null } } ``` **Expected behavior** The test fails only when recovered close behavior is wrong. It does not fail because the CI machine is slow. **Steps to reproduce** 1. In the rotated loop of the test, set the close grace to 700ms instead of 5_000. 2. Run `npx vitest run src/live/runnerd-codex-transport.test.ts -t rotated-recovery`. 3. The second (recovered) close fails with the same error and the same settlement (`runnerSuspended: true`, `providerDrained: false`, `suspensionState: null`). Root cause: `runnerCloseDeadlines` keeps half of the grace (max 2.5s) for suspension. The rest is the preparation budget for the provider drain. With `closeGraceMs: 5_000`, the drain gets 2.5s. In a local measurement, the recovered close needed 4 `runner.drain` commands and about 440ms on an idle machine. Each drain is a full runner round trip, because a drain receipt is `true` only after the controller ACKs the prefix that the previous drain moved. On a loaded CI runner with the debug runnerd binary, those round trips use more than 2.5s. The transport then fails closed, which is correct product behavior. Production uses a 10s default grace (7.5s for preparation, with the drain capped at 5s). **Paperclip version or commit** `master` at cb0b7525e0. ## What Changed - `packages/paperclip-runner/src/live/runnerd-codex-transport.test.ts`: the rotated-recovery, omitted-context, removed-context, and resumed closes in the environment-prepared descriptor test now use the production default close budget. Before, they used a 5s override. - The `timeout-attachment` case keeps its deliberate 500ms budget, because that case asserts the timeout. - The test timeout goes from 30s to 60s. Up to three closes now run under the 10s budget, so a close regression shows its settlement error and not a bare test timeout. - No assertion changed. No sleep or retry is added. ## Verification - Reproduced the CI failure signature locally by shrinking the rotated close grace to 700ms. - `npx vitest run src/live/runnerd-codex-transport.test.ts -t "environment-prepared descriptor"`: 10 of 10 runs passed (7 of 7 cases each) with 30 CPU-saturating processes running at the same time on an 18-core machine. - `npx vitest run src/live/runnerd-codex-transport.test.ts`: 253 passed. - Both runs used the debug runnerd from `pnpm run build:rust`, as CI does. ## Risks Low risk. The change is limited to a test budget. - If recovered close really hangs, this test now takes up to about 10s per close to report it, not 5s. The test timeout is now 60s. - The production close path does not change. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), run through Claude Code with tool use and code execution. ## 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 --- .../src/live/runnerd-codex-transport.test.ts | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/packages/paperclip-runner/src/live/runnerd-codex-transport.test.ts b/packages/paperclip-runner/src/live/runnerd-codex-transport.test.ts index 2faa2cb9c82..08d75ae2859 100644 --- a/packages/paperclip-runner/src/live/runnerd-codex-transport.test.ts +++ b/packages/paperclip-runner/src/live/runnerd-codex-transport.test.ts @@ -2741,7 +2741,7 @@ it.each(["reuse", "large-instructions", "rotated-recovery", "removed-context", " await expect(stat(join(root, "runtime-context.json"))).rejects.toHaveProperty("code", "ENOENT"); await transport.close(); if (mode === "removed-context") { - const rotated = create(5_000, "run-prepared-removed"); + const rotated = create(undefined, "run-prepared-removed"); try { await expect(rotated.transport.request("thread/read", { threadId: (opened.thread as { id: string }).id })) .rejects.toThrow("cannot change the durable Codex provider profile"); @@ -2758,7 +2758,11 @@ it.each(["reuse", "large-instructions", "rotated-recovery", "removed-context", " } if (mode === "rotated-recovery" || mode === "omitted-context") { for (let attempt = 0; attempt < 2; attempt++) { - const rotated = create(5_000, "run-prepared-rotated"); + // Close settlement is bounded by the close grace and must prove the + // provider drain. A recovered successor needs several drain receipts, + // each a runner round trip, so keep the production close budget here. + // A shortened budget fails closed on a loaded CI runner. + const rotated = create(undefined, "run-prepared-rotated"); try { await rotated.transport.request("thread/read", { threadId: (opened.thread as { id: string }).id }); expect(requestedRun).toBe("run-prepared-rotated"); @@ -2769,7 +2773,7 @@ it.each(["reuse", "large-instructions", "rotated-recovery", "removed-context", " } return; } - const resumed = create(mode === "timeout-attachment" ? 500 : 5_000); + const resumed = create(mode === "timeout-attachment" ? 500 : undefined); try { await resumed.transport.request("thread/read", { threadId: (opened.thread as { id: string }).id }); expect(recovered).toBe(true); @@ -2826,7 +2830,9 @@ it.each(["reuse", "large-instructions", "rotated-recovery", "removed-context", " materialize.mockRestore(); discover.mockRestore(); await rm(root, { recursive: true, force: true }); } -}, 30_000); + // Up to three closes run under the 10s production close budget. Keep a close + // regression visible as its settlement error, not as a bare test timeout. +}, 60_000); it("rejects an environment-prepared policy change before registration or process launch", async () => { const root = await mkdtemp(join(tmpdir(), "runner-environment-policy-"));