From d1e4109df1173c3ab9cf46b262fc1760691805a9 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Wed, 11 Nov 2026 17:49:13 -0600 Subject: [PATCH] fix(runner): keep a connected adopted Runner through a failed liveness probe (#20965) 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 > - After a controller restart, the controller adopts the remote Runner that kept running in the sandbox. A monitor then checks the Runner's liveness every 250 ms through the environment plugin. > - #20940 and #20945 made a stalled plugin probe "unknown" for the launched Runner's monitor, reattachment and launch. The adopted Runner's monitor still failed the run on the first probe that failed without an answer. > - A short plugin stall after adoption therefore failed a healthy run with `native_adopted_runner_identity_unverifiable`. > - This pull request uses the evidence that is available: while the adopted Runner holds its authenticated connection to the controller, a liveness check that fails without an answer does not fail the run, for at most the reconnect grace. > - The benefit is that a healthy adopted Runner survives a short plugin stall. A check that reports the process gone still fails the run at once. ## Linked Issues or Issue Description Refs #20940, #20945 **What happened?** `#watchAdoptedRunner` in `packages/paperclip-runner/src/live/runnerd-codex-transport.ts` called `failTransport("native_adopted_runner_identity_unverifiable: runner liveness could not be revalidated")` on the first `isAlive()` rejection. For a remote adopted Runner, `isAlive()` is a plugin `environmentExecute` RPC. A plugin stall makes it reject, even while the Runner holds its authenticated connection and works normally. **Expected behavior** A rejection is not an answer. While the Runner holds its authenticated connection, the monitor keeps checking. The bound is the reconnect grace. A check that returns `false` (the process is gone) still fails the run at once. **Steps to reproduce** 1. Adopt a remote Runner after a controller restart. 2. Stall the plugin worker, so that the liveness probe rejects. 3. Within 250 ms the run fails with `native_adopted_runner_identity_unverifiable`. **Paperclip version or commit** master at ef755cecc6. ## What Changed - `#watchAdoptedRunner`: on an `isAlive()` rejection, the monitor records when verification was first lost. It does not fail the transport while the control plane has an authenticated Runner connection and less than `runnerReconnectGraceMs` has passed. A successful check resets the clock. - Unchanged: an `isAlive()` result of `false` fails the run at once. A rejection with no Runner connection fails it as before. A transport with no reconnect grace (local adoption) fails it as before. The close path still uses `isAlive()` directly. - `doc/execution-semantics.md` describes the rule. ## Verification - Red on master: `npx vitest run src/live/runnerd-codex-transport-adoption.test.ts -t "liveness probe rejects while"` (in `packages/paperclip-runner/`). It failed with `native_adopted_runner_identity_unverifiable: runner liveness could not be revalidated`. - Green: the test passes. The real adopted runnerd keeps its connection, `isAlive()` rejects for 1.5 seconds (6 monitor ticks), the run still answers `thread/read`, no signal is sent, and the runner is alive. - `runnerd-codex-transport-adoption.test.ts` passes (24 tests), including "retains authenticated adopted-runner ownership when a close probe rejects". `tsc --noEmit` passes for `packages/paperclip-runner`. ## Risks - A remote adopted Runner whose liveness cannot be verified, but which stays connected, keeps running for up to 7 minutes before the run fails closed. Its own authenticated connection is the evidence during that time, and no exit is reported. - The change touches `packages/paperclip-runner/src`, so it changes the Daytona runner image content ID. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, through Claude Code with tool use (shell, file edit, code search). Extended reasoning was on. ## 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 - [ ] All Paperclip CI gates are green - [ ] 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 --- doc/execution-semantics.md | 7 ++++- .../runnerd-codex-transport-adoption.test.ts | 31 +++++++++++++++++++ .../src/live/runnerd-codex-transport.ts | 6 ++++ 3 files changed, 43 insertions(+), 1 deletion(-) diff --git a/doc/execution-semantics.md b/doc/execution-semantics.md index b059505c464..636028ec192 100644 --- a/doc/execution-semantics.md +++ b/doc/execution-semantics.md @@ -4881,7 +4881,12 @@ transport failures keep the 30-second budget. At the launch of a new remote runner, a launch call that stalls this way is not retried. The controller reads the identity marker for the launch nonce instead, so the one launched runner is adopted or cleaned up by its nonce. The identity read uses the same 3-minute -stall budget. Each answer that the sandbox gives keeps its current meaning. Monitoring checks the saved process generation when available. +stall budget. Each answer that the sandbox gives keeps its current meaning. + +After the controller adopts a remote runner, it checks the runner's liveness every 250 ms. A check that +fails without an answer does not fail the run while the runner holds its authenticated connection. The +bound is the remote runner reconnect grace (7 minutes). A check that reports the process gone still fails +the run at once. A local adopted runner, which has no reconnect grace, keeps the old rule. Monitoring checks the saved process generation when available. Only a matching ownership marker and confirmed process exit permit automatic replacement. Missing or conflicting identity evidence, failed reads, and an exhausted probe budget retain the existing ownership and recovery safeguards. diff --git a/packages/paperclip-runner/src/live/runnerd-codex-transport-adoption.test.ts b/packages/paperclip-runner/src/live/runnerd-codex-transport-adoption.test.ts index 664f7db4a44..859329993c1 100644 --- a/packages/paperclip-runner/src/live/runnerd-codex-transport-adoption.test.ts +++ b/packages/paperclip-runner/src/live/runnerd-codex-transport-adoption.test.ts @@ -28,6 +28,7 @@ async function verifyLiveRunnerAdoption( uncertainAtClose = false, semanticRecoveryMode?: "denied-before-cancel" | "cleanup-only", stopExitProofFault?: "missing" | "false", + livenessUnverifiedWhileConnected?: "within-grace" | "past-grace" | "reset-by-success", ) { const stateDirectory = await mkdtemp(join(tmpdir(), "runnerd-live-adopt-")); const pendingCompletion = toolHandlerOrder?.startsWith("completion-") === true; @@ -214,6 +215,7 @@ async function verifyLiveRunnerAdoption( ...options, ...(preparedTask && !adoptionAuthorization.startsWith("legacy") ? { authorizeRunnerAdoption } : {}), ...(uncertainAtClose ? { closeGraceMs: 2_000 } : {}), + ...(livenessUnverifiedWhileConnected ? { runnerReconnectGraceMs: livenessUnverifiedWhileConnected === "within-grace" ? 60_000 : 2_000 } : {}), ...(semanticRecoveryMode === "cleanup-only" ? { recoveryPurpose: "stop_cleanup" as const } : {}), // Hash different stable bytes without replacing the real runner artifact // used by concurrent tests. Adoption must never execute this path. @@ -311,6 +313,31 @@ async function verifyLiveRunnerAdoption( }), ); if (toolHandlerOrder?.endsWith("after")) adopted.transport.setServerRequestHandler(redispatchedTool); + if (livenessUnverifiedWhileConnected) { + const pause = (ms: number) => new Promise(resolveWait => setTimeout(resolveWait, ms)); + expect(authority?.activeRunnerConnectionCount()).toBe(1); + rejectLiveness = true; + if (livenessUnverifiedWhileConnected === "reset-by-success") { + await pause(1_400); + rejectLiveness = false; + await pause(500); + rejectLiveness = true; + await pause(1_400); + } else { + await pause(livenessUnverifiedWhileConnected === "within-grace" ? 1_500 : 3_000); + } + rejectLiveness = false; + if (livenessUnverifiedWhileConnected === "past-grace") { + await expect(adopted.transport.request("thread/read", {})).rejects.toThrow("native_adopted_runner_identity_unverifiable"); + } else { + await expect(adopted.transport.request("thread/read", {})).resolves.toEqual( + expect.objectContaining({ thread: expect.objectContaining({ id: "codex-thread-1" }) })); + expect(adopted.evidence().diagnostics.join("\n")).not.toContain("native_adopted_runner_identity_unverifiable"); + } + expect(() => process.kill(runnerPid!, 0)).not.toThrow(); + expect(signal).not.toHaveBeenCalled(); + if (livenessUnverifiedWhileConnected === "past-grace") return; + } if (semanticRecoveryMode) { const retained = authority as DurablePrpControlPlane | null; expect(retained).not.toBeNull(); @@ -520,6 +547,10 @@ it("adopts a prepared runner after losing its controller launch cache without pr it.each(["deny", "legacy", "legacy-missing", "missing-binding"] as const)("preserves adoption authorization with %s", mode => verifyLiveRunnerAdoption(false, false, false, false, false, true, mode), 30_000); +it.each(["within-grace", "past-grace", "reset-by-success"] as const)( + "bounds a rejected liveness probe of a connected adopted runner by the reconnect grace (%s)", + mode => verifyLiveRunnerAdoption(false, false, false, false, false, false, "allow", undefined, false, undefined, undefined, mode), 30_000); + it("retains authenticated adopted-runner ownership when a close probe rejects", () => verifyLiveRunnerAdoption(false, false, false, false, false, false, "allow", undefined, true), 30_000); it( diff --git a/packages/paperclip-runner/src/live/runnerd-codex-transport.ts b/packages/paperclip-runner/src/live/runnerd-codex-transport.ts index 2d33e6a8182..64b9d87f167 100644 --- a/packages/paperclip-runner/src/live/runnerd-codex-transport.ts +++ b/packages/paperclip-runner/src/live/runnerd-codex-transport.ts @@ -8605,11 +8605,13 @@ class DurablePrpCodexTransport implements CodexAppServerTransport { >, ): void { let checking = false; + let unverifiedSince: number | null = null; this.#adoptedRunnerMonitor = setInterval(() => { if (checking || this.#closed || (this.#releaseEpisode && !this.#releaseEpisode.claimed)) return; checking = true; void Promise.resolve(adoptedRunner.isAlive()) .then((alive) => { + unverifiedSince = null; if (alive || this.#closed || (this.#releaseEpisode && !this.#releaseEpisode.claimed)) return; this.#evidence.runnerExited = true; this.#publish(); @@ -8620,6 +8622,10 @@ class DurablePrpCodexTransport implements CodexAppServerTransport { ); }) .catch(() => { + const now = Date.now(); + unverifiedSince ??= now; + if ((this.#core?.activeRunnerConnectionCount() ?? 0) > 0 + && now - unverifiedSince < (this.options.runnerReconnectGraceMs ?? 0)) return; if (!this.#closed && !(this.#releaseEpisode && !this.#releaseEpisode.claimed)) { this.#failTransport( new Error(