From 09412e652a83325af2645a062cb35f5f026d67c3 Mon Sep 17 00:00:00 2001 From: Nicky Leach Date: Mon, 9 Nov 2026 07:43:27 -0800 Subject: [PATCH] fix(runner): allow warm release to drain after provider restart (#20290) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Warm Runner sessions park after a turn so follow-up tasks can claim the same process. > - Claude restarts its ACPX provider after the controller receives the terminal result. > - That restart can delay the drain receipt beyond the release path's one-second limit. > - A failed drain prevents parking, so the idle timer later closes the retained Runner. > - This pull request uses the existing warm admission budget for release drain. > - The benefit is retained Runner reuse without moving provider restart onto task dispatch. ## Linked Issues or Issue Description Fixes: #20225. Refs #20190 and #20145. Related performance follow-up: #20224. ## What Changed - Align the release drain timeout with the existing 30-second warm admission budget for every provider. Close paths keep their explicit remaining deadlines. - Keep the existing immediate completion when the drain receipt proves settlement. Add no sleep, retry, provider branch or dispatch work. - Add a real Runner regression with a fake ACPX sidecar whose replacement start takes 1.5 seconds. One release call must park the Runner, then a follow-up must claim the same Runner process. - Document the release drain budget. ## Verification - Targeted command in `packages/paperclip-runner`: `vitest run src/live/runnerd-codex-transport-release-resume.test.ts src/live/runnerd-codex-transport-settlement.test.ts src/live/runnerd-codex-transport-deadlines.test.ts`: **46 passed** across 3 files, 157.78 seconds. - Independent parent verification reran the slow-restart and ordinary cached cases: **2 passed**. Source review confirmed that release is the only caller using the changed default; close callers supply explicit deadlines. - Negative control: with the original one-second timeout, the new test fails because the single release call returns `false`. The fixed code passes the new case. - Built only the current-source Runner and two fixture binaries required by these tests. The first sandboxed invocation could not bind its loopback listener (`EPERM`); the test ran with loopback access afterward. - Full GitHub PR workflow [42644210993](https://github.com/paperclipai/paperclip/actions/runs/42644210993) passed on head `9cfda0927b514d8d40443b612afa92525b35b700`. Standard Greptile scored this same head 5/5 with no review threads. Full repository checks ran in GitHub CI, per the requested local test scope. No live Claude session or hosted deployment was tested. ## Risks - An unresponsive Runner can now hold release drain for up to the existing 30-second warm admission limit. A healthy Runner returns as soon as its receipt arrives. The provider restart stays at turn end; task dispatch gets no fixed delay. - Saved state, protocol and provider identity formats do not change. Previous-release state carries forward under the existing validation. - The controlled fake-sidecar test proves the restart timing race and process reuse. It does not qualify live provider latency or a hosted rollout. ## Model Used - OpenAI GPT-6 through Codex, with repository tools, code editing and targeted test execution. The exact serving revision and context window are not exposed in this session. ## 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 Co-authored-by: Paperclip --- .../paperclip-runner/docs/durable-recovery.md | 7 +++++++ ...runnerd-codex-transport-release-resume.test.ts | 15 ++++++++++++--- .../src/live/runnerd-codex-transport.ts | 3 ++- 3 files changed, 21 insertions(+), 4 deletions(-) diff --git a/packages/paperclip-runner/docs/durable-recovery.md b/packages/paperclip-runner/docs/durable-recovery.md index 3f8938c272a..63e25852e53 100644 --- a/packages/paperclip-runner/docs/durable-recovery.md +++ b/packages/paperclip-runner/docs/durable-recovery.md @@ -83,6 +83,13 @@ a scripted harness, removes the controller's launch files, then recovers the same process and provider session and completes another turn. This exercises the real transport; it does not qualify a hosted OpenCode or ACPX deployment. +## Warm Runner release + +Before parking a settled Runner, the controller waits for its durable drain +receipt within the same 30-second budget used for warm admission. A Claude +turn-end provider restart can delay that receipt beyond one second. The wait +returns as soon as the drain is proved; it adds no fixed delay to a ready Runner. + ## ACPX checkpoints after a rejected turn start A matching `turn.start` response with `SESSION_RESUME_REQUIRED` and diff --git a/packages/paperclip-runner/src/live/runnerd-codex-transport-release-resume.test.ts b/packages/paperclip-runner/src/live/runnerd-codex-transport-release-resume.test.ts index 36b8843de43..d0f174eb3bc 100644 --- a/packages/paperclip-runner/src/live/runnerd-codex-transport-release-resume.test.ts +++ b/packages/paperclip-runner/src/live/runnerd-codex-transport-release-resume.test.ts @@ -78,16 +78,19 @@ async function nativeWire(port: number, path: string, closeGate?: Promise) }); } -it.skipIf(process.platform === "win32").each(["cached", "lost claim reply", "cache adoption", "acpx cache adoption", "unavailable", "claim cancellation", "expired ticket", "late claim reply", "held past deadline", "release admission", "no cleanup callback", "adoption no cleanup callback", "resume no cleanup callback", "deferred turn after close", "late wire close", "empty goal snapshot", "early claim", "early claim abandoned", "early claim cancellation", "early claim expired", "early claim stale ticket"] as const)( +it.skipIf(process.platform === "win32").each(["cached", "lost claim reply", "cache adoption", "acpx cache adoption", "acpx slow restart", "unavailable", "claim cancellation", "expired ticket", "late claim reply", "held past deadline", "release admission", "no cleanup callback", "adoption no cleanup callback", "resume no cleanup callback", "deferred turn after close", "late wire close", "empty goal snapshot", "early claim", "early claim abandoned", "early claim cancellation", "early claim expired", "early claim stale ticket"] as const)( "releases and resumes the native Codex source before attachment (%s)", async mode => { const root = await mkdtemp(join(tmpdir(), "runner-release-resume-")); - const acpx = mode === "acpx cache adoption"; + const acpx = mode === "acpx cache adoption" || mode === "acpx slow restart"; const sidecar = join(root, "sidecar"), sidecarScript = join(root, "sidecar-entry"); + const restartStarted = join(root, "restart-started"); if (acpx) { const quote = (value: string) => `'${value.replaceAll("'", "'\"'\"'")}'`; const fake = resolve(import.meta.dirname, "../../runner/target/debug/fake-acpx-sidecar"); await writeFile(sidecarScript, "qualified fixture entry"); - await writeFile(sidecar, `#!/bin/sh\nexec ${quote(fake)} --mode source-reconcile --profile-digest ${quote(QUALIFIED_ACPX_PROFILES.claude.commandDigest)} --journal ${quote(join(root, "calls"))} --source-fault ${quote(join(root, "fault"))}\n`, { mode: 0o700 }); + const restartDelay = mode === "acpx slow restart" + ? `if [ -e ${quote(join(root, "calls"))} ]; then touch ${quote(restartStarted)}; sleep 1.5; fi\n` : ""; + await writeFile(sidecar, `#!/bin/sh\n${restartDelay}exec ${quote(fake)} --mode source-reconcile --profile-digest ${quote(QUALIFIED_ACPX_PROFILES.claude.commandDigest)} --journal ${quote(join(root, "calls"))} --source-fault ${quote(join(root, "fault"))}\n`, { mode: 0o700 }); } const listener = createServer(); await new Promise(resolve => listener.listen(0, "127.0.0.1", resolve)); @@ -214,6 +217,12 @@ it.skipIf(process.platform === "win32").each(["cached", "lost claim reply", "cac const opened = await bundle.transport.request("thread/start", { cwd: root, dynamicTools: [], ...(acpx ? { model: "explicit-source-model" } : {}) }); const threadId = String((opened.thread as { id: string }).id); await complete(); + if (mode === "acpx slow restart") { + await vi.waitFor(async () => expect(await readFile(restartStarted, "utf8")).toBe("")); + expect(await bundle.transport.release!()).toEqual(identity); + expect(pair.release).toHaveBeenCalledOnce(); + expect(authority!.store.state.commands.filter(command => command.type === "runner.drain")).toHaveLength(1); + } if (mode === "empty goal snapshot") { await writeFile(goalGate, "release late goal"); await vi.waitFor(() => expect(authority!.store.state.committedEvents.some(event => event.eventType === "session.goal.updated" diff --git a/packages/paperclip-runner/src/live/runnerd-codex-transport.ts b/packages/paperclip-runner/src/live/runnerd-codex-transport.ts index 700127cbc2e..bddf9effa94 100644 --- a/packages/paperclip-runner/src/live/runnerd-codex-transport.ts +++ b/packages/paperclip-runner/src/live/runnerd-codex-transport.ts @@ -5185,8 +5185,9 @@ class DurablePrpCodexTransport implements CodexAppServerTransport { return false; } + // Release shares the warm admission budget: a turn-end restart can delay the drain receipt. async #drainSettledProviderEventsBeforeSuspend( - timeoutMs = 1_000, + timeoutMs = coldAdmissionTimeoutMs(this.options.provider, this.options.acpxAgent, false), providerStopConfirmed = false, ): Promise { const core = this.#core;