From 8e956da1cf5091e33681614a058ee6c64a48a8f3 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Thu, 12 Nov 2026 02:59:46 -0600 Subject: [PATCH] fix(native): keep a live reattached Runner when its restart recovery read times out (#21053) 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 > - Native runs on remote sandboxes (Daytona) use a detached Runner. A server replacement detaches the controller and leaves the Runner alive. The new server reattaches to it and continues the same turn > - On staging, about half of the in-flight runs at each upgrade lost their turn. The new controller reattached to the live Runner (same process verified), its recovery read timed out (`PRP command session.snapshot timed out`), and the failure cleanup killed the Runner > - The next same-run reattachment then found `process_not_running`. #21039 continues these runs in a fresh session, but the in-flight turn is lost and its outcome is unknown > - The cause of the kill is in the Codex driver's `recoverSession`: its cleanup closed the transport without retention. For a reattached Runner that means SIGKILL after the close deadline. #21023 already keeps the Runner for a failed open session, but not for a failed recovery > - This pull request keeps the live Runner when a recovery fails or is aborted on its way to it. Identity refusals and protocol integrity faults still stop it > - The benefit is that a slow reattachment no longer destroys a turn that was still running ## Linked Issues or Issue Description Refs #21023, #20923, #21039, #21052 **What happened?** Staging replaced its server at about 07:10Z with 17 remote native runs in flight. For each, the old server logged "Server shutdown suspended native controller ownership without cancelling provider work". The new server reattached. Example run: the Runner process (same pid and start time) was verified at 07:12:37 and the transport authenticated at 07:12:32. At 07:13:28 the recovery failed with `native_session_recovery_failed: ... PRP command session.snapshot timed out`, and a same-run resume was saved. At 07:14:12 the same-run reattachment found `process_verification` / `process_not_running`, with the Runner journal still `ready` and no exit record from the Runner itself. The run failed with `runner_remote_process_identity_unavailable` / `native_continuation_requires_reconciliation`. 90 of 93 such failures in two hours of upgrades had this sequence. **What did you expect to happen?** A recovery that times out against a live Runner is a transient failure. The same-run reattachment should find the Runner alive and adopt it again. Killing the Runner is only for a deliberate stop, an identity refusal, or an integrity fault. **Root cause** `packages/paperclip-runner/src/drivers/codex/codex-app-server-driver-impl.ts` `recoverSession`: the `catch` block and the abort handler call `bootstrapCancellation(...).close()`, which calls `transport.close()` without `retainUnsettledRemoteRunner`. In `runnerd-codex-transport.ts` `#closeOnce`, an unresponsive adopted Runner that is not retained gets `SIGKILL` after the close deadline. The Codex driver serves remote Codex, OpenCode and ACPX runs, which matches the failures seen on staging for all three driver kinds. ## What Changed - `codex-app-server-driver-impl.ts`: `recoverSession` classifies a thrown recovery failure. A transient one (`provider_transport_failed`, a `PRP command ... timed out`, or the transport closing while waiting) now detaches this controller with the transport's restart detach (`detachControllerForRestart`). That path queues no `turn.stop` and no `runner.suspend` and sends no signal, the same as a server restart. The Runner and its in-flight turn stay for the same run's next reattachment. An abort (a Stop or the recovery deadline), any message that names a mismatch, a different session, identity, authority, integrity or incompatibility, any unclassified failure, any returned identity refusal, and a protocol integrity fault still close and stop the Runner. `openSession` is unchanged. - `codex-app-server-driver.recovery.test.ts`: the staging message (`codex_history_read_failed: ... provider_transport_failed: PRP command session.snapshot timed out`), and a plain snapshot timeout now detach and never close. An abort of a hung read still closes and stops the Runner. A thrown different-session refusal, `native_adopted_provider_identity_mismatch`, an identity refusal that also mentions a timeout, and an unclassified failure still close without retention. The existing integrity-fault cases check that the close does not retain the Runner. - `doc/execution-semantics.md`: describes the rule. ## Verification - Red on `origin/master` with only the new tests: `cd packages/paperclip-runner && npx vitest run src/drivers/codex/codex-app-server-driver.recovery.test.ts` fails 2 of 69 (the two detach cases; master calls `close()` and stops the Runner). - Green with the fix: 69 of 69. - `npx vitest run src/drivers/codex src/native-session-runtime.test.ts src/backends src/live/runnerd-codex-transport-remote-retention.test.ts --maxWorkers=4` passed except two timing tests (Codex command resolution, background cleanup containment) that pass alone and do not touch this code. - `npx tsc --noEmit -p packages/paperclip-runner/tsconfig.json` passes. - `detachControllerForRestart` is the existing restart path. It stops the pump, disconnects the Runner, and drains the controller journal. It queues no command and never signals the process. - Not run: a live Daytona restart. The next post-upgrade audit should show fewer `runner_remote_process_identity_unavailable` failures after a `session.snapshot timed out` resume. Instead, the run log should show "controller authority detached for restart; durable runner left alive" and then a successful same-run reattachment. ## Risks - Medium. A Runner that the recovery could not reach now stays alive, with its turn, for the same-run reattachment. This is the state a server restart already leaves. The same-run reattachment adopts it, or a Stop cleanup or saved-work recovery stops it. - If the Runner stays unresponsive across every same-run attempt, the run no longer gets the `process_not_running` proof that #21039 uses for its automatic fresh-session continuation. The run then keeps its visible Retry. The staging data shows that reattachments reaching the Runner sooner succeed, so a later attempt is expected to succeed. This is not proven against a live sandbox. - An abort of the recovery (a Stop or the recovery deadline) is unchanged: it closes and stops the Runner. `plugin-runner-release-resume.integration.test.ts` (14 of 14) covers a Stop during an adopted resume. - No turn is replayed and no second Runner is launched. Adopting an existing Runner never launches a new one. - Previous-release story: no persisted state changes. The decision is made in the process that runs the recovery, so a deploy during a run changes nothing. - No wait is added before a normal turn start. Only the recovery failure path changes. > 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 (Anthropic) via Claude Code, with tool use (shell, file edits, test runs, read-only staging API reads). Model ID as attributed in the commit trailer: Claude Fable 5.2. ## 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 Fable 5.2 --- doc/execution-semantics.md | 8 ++- .../codex/codex-app-server-driver-impl.ts | 30 ++++++-- .../codex-app-server-driver.recovery.test.ts | 69 +++++++++++++++++++ 3 files changed, 102 insertions(+), 5 deletions(-) diff --git a/doc/execution-semantics.md b/doc/execution-semantics.md index 54c2e54e616..596b3a21c51 100644 --- a/doc/execution-semantics.md +++ b/doc/execution-semantics.md @@ -1753,7 +1753,13 @@ The runtime's own cleanup after a failed turn start or a failed event stream (for example `runnerd did not report the provider turn identity` or the execution timeout) revokes the turn's output but is not a Stop, so it keeps the Runner for the same-run reattachment too. A protocol integrity fault, which -needs an operator, still stops the Runner. +needs an operator, still stops the Runner. A reattachment after a restart whose recovery read +times out or loses its transport (for example +`PRP command session.snapshot timed out`) detaches from the Runner the same way a server restart does: it sends no turn +stop and no suspend and does not signal the process. The Runner and its +in-flight turn stay for the next same-run reattachment. An abort (a Stop or the +recovery deadline), any identity or authority refusal, any other recovery +failure, and a protocol integrity fault still stop it. A remote native run can lose its original runner or sandbox before reattachment. This happens after a server restart, and for a same-run retry after a turn ends diff --git a/packages/paperclip-runner/src/drivers/codex/codex-app-server-driver-impl.ts b/packages/paperclip-runner/src/drivers/codex/codex-app-server-driver-impl.ts index c8b15fffb34..66b3d7a96ba 100644 --- a/packages/paperclip-runner/src/drivers/codex/codex-app-server-driver-impl.ts +++ b/packages/paperclip-runner/src/drivers/codex/codex-app-server-driver-impl.ts @@ -75,18 +75,34 @@ const DRIVER_KIND = "codex_app_server"; const DRIVER_VERSION = "codex-v2"; interface BootstrapCancellation { - close(): Promise; + /** `keepRunner` detaches this controller from a durable Runner exactly as a + * server restart does: no turn stop, no suspend, no signal. The Runner and + * its in-flight turn stay for the same run's next reattachment. Only the + * first close decides. */ + close(keepRunner?: boolean): Promise; detach(): void; wait(operation: Promise): Promise; } +/** A recovery failure that says nothing about the Runner's identity or state: + * a command or transport timed out or was lost. Any identity or authority + * refusal, even one that also mentions a timeout, is not transient. */ +export function isTransientRunnerRecoveryFailure(error: unknown): boolean { + if (error instanceof NativeSessionProtocolIntegrityError) return false; + const message = error instanceof Error ? error.message : String(error); + if (/mismatch|different|identity|authority|integrity|incompatible/i.test(message)) return false; + return /provider_transport_failed|PRP command \S+ timed out|runnerd transport closed while waiting/.test(message); +} + function bootstrapCancellation( transport: CodexAppServerTransport, signal: AbortSignal | undefined, ): BootstrapCancellation { let closePromise: Promise | null = null; - const close = (): Promise => { - closePromise ??= transport.close(); + const close = (keepRunner = false): Promise => { + closePromise ??= keepRunner && transport.detachControllerForRestart + ? transport.detachControllerForRestart() + : transport.close(); return closePromise; }; let rejectAborted!: (reason: unknown) => void; @@ -453,6 +469,12 @@ export class CodexAppServerDriver implements HarnessDriver { activeTurnId: snapshot.activeTurnId, }, }); + // A recovery whose Runner command timed out or lost its transport detaches + // from the Runner, as a restart does, for the same run's next + // reattachment: the Runner may still own an in-flight turn, and stopping + // or killing it loses the turn. An abort (a Stop or the recovery + // deadline), identity refusals, other recovery failures and protocol + // integrity faults still close and stop it. const cancellation = bootstrapCancellation(transport, options.signal); try { await cancellation.wait(this.#persistProcessOwnership(transport)); @@ -772,7 +794,7 @@ export class CodexAppServerDriver implements HarnessDriver { session, }; } catch (error) { - await cancellation.close().catch(() => {}); + await cancellation.close(isTransientRunnerRecoveryFailure(error)).catch(() => {}); if (error instanceof NativeSessionProtocolIntegrityError) throw error; if (options.signal.aborted) options.signal.throwIfAborted(); // A Runner with no saved control-plane state for this session can never diff --git a/packages/paperclip-runner/src/drivers/codex/codex-app-server-driver.recovery.test.ts b/packages/paperclip-runner/src/drivers/codex/codex-app-server-driver.recovery.test.ts index 845eec9d141..133d8005941 100644 --- a/packages/paperclip-runner/src/drivers/codex/codex-app-server-driver.recovery.test.ts +++ b/packages/paperclip-runner/src/drivers/codex/codex-app-server-driver.recovery.test.ts @@ -250,6 +250,73 @@ describe("Codex app-server Codex driver", () => { }); expect(second.calls.some((call) => call.method === "turn/start" || call.method === "thread/goal/set")).toBe(false); }); + // Staging: a server replacement left the remote Runner alive mid-turn. The + // new controller reattached, verified the same process, and its recovery + // read failed with "PRP command session.snapshot timed out". The failure + // cleanup then killed the live Runner, so the same-run reattachment found + // the process gone and the in-flight turn was lost. + describe("a recovery that fails on its way to a live remote Runner", () => { + async function recoverWith(fail: (method: string) => Promise | null, signal?: AbortSignal) { + const originalTransport = new FakeCodexTransport(); + const recoveryTransport = new FakeCodexTransport(); + const request = recoveryTransport.request.bind(recoveryTransport); + vi.spyOn(recoveryTransport, "request").mockImplementation(async (method, params) => + fail(method) ?? request(method, params)); + const close = vi.spyOn(recoveryTransport, "close"); + // The runnerd transport's restart detach: no turn stop, no suspend, no signal. + const detach = vi.fn(async () => {}); + Object.assign(recoveryTransport, { detachControllerForRestart: detach }); + const driver = makeDriver([originalTransport, recoveryTransport]); + const original = await driver.openSession({ + runId: "run-runner-kept", normalizedSessionId: "session-runner-kept", workingDirectory: WORKSPACE, + }); + await original.startTurn({ message: { role: "user", text: "Work." } }); + const checkpoint = await original.snapshot(); + await original.close({ reason: "controller detached for restart" }); + const recovery = driver.recoverSession!(checkpoint, { signal: signal ?? new AbortController().signal }); + return { recovery, close, detach, recoveryTransport }; + } + const rejectRead = (error: Error) => (method: string) => method === "thread/read" ? Promise.reject(error) : null; + + it.each([ + ["the runner snapshot timed out", new Error("provider_transport_failed: PRP command session.snapshot timed out")], + ["the history read failed behind a snapshot timeout (the staging message)", new HarnessReconciliationError( + "codex_history_read_failed: thread/turns/list requires supported paginated history; Error: provider_transport_failed: PRP command session.snapshot timed out")], + ])("detaches and keeps the Runner and its turn when %s", async (_title, error) => { + const { recovery, close, detach, recoveryTransport } = await recoverWith(rejectRead(error)); + expect(await recovery).toMatchObject({ recovered: false, reason: expect.stringContaining("session.snapshot timed out") }); + expect(detach).toHaveBeenCalledTimes(1); + expect(close).not.toHaveBeenCalled(); + expect(recoveryTransport.calls.some(call => ["turn/start", "thread/start", "turn/interrupt"].includes(call.method))).toBe(false); + }); + + it("still stops the Runner when an abort (a Stop or the recovery deadline) ends a hung read", async () => { + const controller = new AbortController(); + const { recovery, close, detach } = await recoverWith(method => { + if (method !== "thread/read") return null; + setTimeout(() => controller.abort(new Error("Stop requested")), 0); + return new Promise(() => {}); + }, controller.signal); + await expect(recovery).rejects.toThrow("Stop requested"); + expect(detach).not.toHaveBeenCalled(); + expect(close).toHaveBeenCalledTimes(1); + expect(close.mock.calls[0]).toEqual([]); + }); + + it.each([ + ["a thrown different-session refusal", new HarnessReconciliationError("thread/read returned a different driver session")], + ["an adopted provider identity mismatch", new Error("native_adopted_provider_identity_mismatch")], + ["an identity refusal that also timed out", new Error("provider_transport_failed: PRP command session.snapshot timed out; native_adopted_provider_identity_mismatch")], + ["an unclassified failure", new Error("codex_history_incomplete: thread/turns/list omitted data")], + ])("still stops the Runner after %s", async (_title, error) => { + const { recovery, close, detach } = await recoverWith(rejectRead(error)); + expect(await recovery).toMatchObject({ recovered: false }); + expect(detach).not.toHaveBeenCalled(); + expect(close).toHaveBeenCalledTimes(1); + expect(close.mock.calls[0]).toEqual([]); + }); + }); + it.each([ "initial-read", "reconcile-read", @@ -302,6 +369,8 @@ describe("Codex app-server Codex driver", () => { }), ).rejects.toBe(fault); expect(close).toHaveBeenCalledTimes(1); + // An integrity fault still stops the Runner. + expect(close.mock.calls[0]).toEqual([]); expect( recoveryTransport.calls.some((call) => call.method === "thread/start"), ).toBe(false);