From 499712c68b3ca95801b6114d5f2f0f0ea6759318 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Thu, 5 Nov 2026 09:14:10 -0600 Subject: [PATCH] fix(runner): restore cold state before inspecting completed handoffs (#19254) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native Runner saves provider sessions and controller state across turns. > - A completed warm handoff leaves a receipt in the controller journal. > - A later cold continuation must restore external Runner state before it can inspect that receipt. > - The transport read the missing physical file before it called the restore hook. > - This pull request prepares admitted external state before that inspection for completed handoffs. > - Live Runner adoption and pending transitions retain their existing ownership checks. The provider thread can continue after a cold restore. ## Linked Issues or Issue Description Related: #19225 and #19157. This fix changes the shared transport read order. It does not depend on the open source-reconciliation extension in #19157. **What happened?** A cold continuation after a completed warm handoff could fail with `runner_remote_state_unavailable`. The saved controller journal was valid. The physical Runner state was absent because its restore hook had not run yet. A later automatic reattachment could then fail an allocation identity check. **Expected behavior** Restore admitted external state before inspecting a completed handoff. Retain the original provider thread. Refuse a denied restore or foreign session without starting provider work. **Steps to reproduce** 1. Open a Codex session and complete a warm handoff to a second run. 2. Suspend the session and retain the controller journal and provider backup. 3. Start a third run with no physical Runner state until the external preparation hook restores it. 4. Read the retained thread. Before this fix, the read fails before preparation. ## What Changed - Prepare external Runner state before inspecting a completed warm-handoff receipt. - Keep live Runner adoption, pending attachments, and active warm transitions on the existing exact-state path. - Add a real Runner and controlled Codex regression for successful restore, denied restoration, and a foreign restored session. - Document the cold-continuation order in the execution contract. ## Verification - Red: the new real-process regression fails on unpatched source because resume reads the absent `runner-state.json` before preparation. - Green: all three new cases pass. The restored thread keeps its identity. The controlled provider starts one thread and resumes it once. No turn is replayed. - Adjacent safety suites: 60 cases passed across warm attachment, interrupted attachment recovery, suspension, and cold restore. The new foreign-session case also passes separately. - Runner TypeScript and generated protocol checks passed. - Full workspace typecheck and build passed. Runner TypeScript checks passed again after the live-adoption guard. The full local test command remains in progress; all required CI test jobs passed on the current head. - The first CI run found two live-adoption regressions: an already completed staged handoff must not restore a backup while its Runner is live. The guard now excludes live adoption. All six cases across cold restore and staged handoff pass locally. The original CI failure remains recorded in run 42047027346. - Greptile reviewed the corrected head `f33a041c8902c39ca52b995d53128fc23b0b5499` at 5/5. There are no review threads or findings. All current-head CI gates passed. - Live staging verification remains pending normal deployment. No pinned serving image or bulk migration is part of this PR. ## Risks The change calls an existing external preparation hook earlier for completed handoffs. That hook must remain idempotent and must enforce restore authority. Live adoption, pending attachments, and active transitions do not gain this earlier call. The negative regressions assert that a refused restore and a foreign session start no provider work and do not change the authority journal. ## Model Used OpenAI Codex, based on GPT-6, with reasoning, tool use, and code execution. The host does not expose the exact serving model ID or context-window size. ## 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 --- doc/execution-semantics.md | 7 ++ ...nnerd-codex-transport-cold-restore.test.ts | 107 ++++++++++++++++++ .../src/live/runnerd-codex-transport.ts | 35 +++--- 3 files changed, 134 insertions(+), 15 deletions(-) create mode 100644 packages/paperclip-runner/src/live/runnerd-codex-transport-cold-restore.test.ts diff --git a/doc/execution-semantics.md b/doc/execution-semantics.md index 37092285f2b..44ff7b33b11 100644 --- a/doc/execution-semantics.md +++ b/doc/execution-semantics.md @@ -3173,6 +3173,13 @@ configuration, and controller-owned credential or instruction resources use verified suspension and saved-history resume. This recovery can restart a provider; it does not suspend every healthy warm turn. +A cold continuation can retain the completed receipt from a prior warm handoff. +Before inspecting that receipt against physical Runner state, the transport +materializes the admitted external backup. A refused restore or mismatched +session identity prevents provider startup. Adoption of a live Runner, pending +handoffs and active warm transitions still require their original physical state +and ownership checks without restoring a backup. + A different task can reuse the same physical sandbox after its prior task finishes. Provider ingress has one listener per allocation. Before a new logical session starts, Runner retires a locally retained idle session that still owns diff --git a/packages/paperclip-runner/src/live/runnerd-codex-transport-cold-restore.test.ts b/packages/paperclip-runner/src/live/runnerd-codex-transport-cold-restore.test.ts new file mode 100644 index 00000000000..d7598b004fa --- /dev/null +++ b/packages/paperclip-runner/src/live/runnerd-codex-transport-cold-restore.test.ts @@ -0,0 +1,107 @@ +import { cp, mkdir, mkdtemp, readFile, rename, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { expect, it, vi } from "vitest"; +import { createCapabilityRunnerdCodexTransport } from "./runnerd-codex-transport.js"; +import { defaultCapabilityRunnerdBinary, fakeCodex, fakeCodexArgs } from "../../test/support/runnerd-transport.js"; + +it.each(["restore", "refuse", "foreign-state"] as const)( + "continues a completed warm handoff through external cold-state preparation: %s", + async (mode) => { + const root = await mkdtemp(join(tmpdir(), "runnerd-warm-cold-restore-")); + const saved = await mkdtemp(join(tmpdir(), "runnerd-warm-saved-")); + const callsPath = join(root, "calls.log"); + const identity = { + runnerInstanceId: "cold-restore-runner", + environmentLeaseId: "cold-restore-lease", + runId: "first-run", + normalizedSessionId: "cold-restore-session", + turnId: "first-turn", + itemId: "first-item", + }; + const options = { + runnerBinary: defaultCapabilityRunnerdBinary(), + codexCommand: fakeCodex, + codexArgs: fakeCodexArgs(root, "--call-log", callsPath, "--record-process-start"), + stateDirectory: root, + lifecyclePolicy: { mode: "warm" as const, idleTimeoutMs: 60_000 }, + runnerReconnectGraceMs: 2_000, + }; + const first = createCapabilityRunnerdCodexTransport({ ...options, prpIdentity: identity }); + let resumed: ReturnType | undefined; + try { + const opened = await first.transport.request("thread/start", { cwd: tmpdir(), dynamicTools: [] }); + const openedThread = opened.thread as { id: string; sessionId: string }; + const thread = { id: openedThread.id, sessionId: openedThread.sessionId }; + await first.transport.attachRun!({ runId: "second-run", turnId: "second-turn", itemId: "second-item" }); + expect((await first.transport.request("thread/read", {})).thread).toMatchObject(thread); + await first.transport.close(); + const controlPath = join(root, "control-plane", "control-plane-state.json"); + const controlBytes = await readFile(controlPath); + const control = JSON.parse(controlBytes.toString()); + expect(control.completedWarmTransition.receipt.newIdentity.runId).toBe("second-run"); + const savedRunner = join(saved, "runner"); + await rename(join(root, "runner"), savedRunner); + const savedState = JSON.parse(await readFile(join(savedRunner, "runner-state.json"), "utf8")); + expect(savedState).toMatchObject({ lifecycle: "suspended", runId: "second-run" }); + const callsBefore = await readFile(callsPath, "utf8"); + const sentinel = "workspace content must survive cold provider recovery"; + await writeFile(join(root, "workspace-sentinel.txt"), sentinel); + let prepared = false; + const prepare = vi.fn(async () => { + if (prepared) return; + if (mode === "refuse") throw new Error("fixture cold restore authority refused"); + await cp(savedRunner, join(root, "runner"), { recursive: true, errorOnExist: true }); + if (mode === "foreign-state") { + await writeFile(join(root, "runner", "runner-state.json"), JSON.stringify({ + ...savedState, normalizedSessionId: "another-session", + })); + } + prepared = true; + }); + const read = vi.fn(async () => JSON.parse(await readFile(join(root, "runner", "runner-state.json"), "utf8"))); + const archive = vi.fn(async (request: { archiveKey: string }) => { + const state = await read(); + const directory = join(root, "authority-epochs", request.archiveKey); + await mkdir(directory, { recursive: true }); + await rename(join(root, "runner", "runner-state.json"), join(directory, "runner-state.json")); + return state; + }); + resumed = createCapabilityRunnerdCodexTransport({ + ...options, + prpIdentity: { ...identity, runId: "third-run", turnId: "third-turn", itemId: "third-item" }, + resumeProviderSession: { driverSessionId: thread.id, providerSessionId: thread.sessionId }, + readRunnerState: read, + prepareExternalRunnerState: prepare, + archiveExternalRunnerState: archive, + }); + if (mode === "refuse") { + await expect(resumed.transport.request("thread/read", {})).rejects.toThrow("fixture cold restore authority refused"); + expect(read).not.toHaveBeenCalled(); + expect(archive).not.toHaveBeenCalled(); + expect(await readFile(callsPath, "utf8")).toBe(callsBefore); + expect(await readFile(controlPath)).toEqual(controlBytes); + } else if (mode === "foreign-state") { + await expect(resumed.transport.request("thread/read", {})).rejects.toThrow("native_runner_authority_rotation_requires_settled_state"); + expect(read).toHaveBeenCalled(); + expect(archive).not.toHaveBeenCalled(); + expect(await readFile(callsPath, "utf8")).toBe(callsBefore); + expect(await readFile(controlPath)).toEqual(controlBytes); + } else { + expect((await resumed.transport.request("thread/read", {})).thread).toMatchObject(thread); + expect(prepare.mock.invocationCallOrder[0]).toBeLessThan(read.mock.invocationCallOrder[0]); + const calls = (await readFile(callsPath, "utf8")).trim().split(/\r?\n/); + expect(calls.filter(call => call === "thread/start")).toHaveLength(1); + expect(calls.filter(call => call === "thread/resume")).toHaveLength(1); + expect(calls.filter(call => call === "turn/start")).toHaveLength(0); + expect(JSON.parse(await readFile(controlPath, "utf8")).identity.runId).toBe("third-run"); + expect(await readFile(join(root, "workspace-sentinel.txt"), "utf8")).toBe(sentinel); + } + } finally { + await Promise.allSettled([first.transport.close(), resumed?.transport.close()]); + await rm(root, { recursive: true, force: true }); + await rm(saved, { recursive: true, force: true }); + } + }, + 30_000, +); diff --git a/packages/paperclip-runner/src/live/runnerd-codex-transport.ts b/packages/paperclip-runner/src/live/runnerd-codex-transport.ts index 82e23dfa1ae..f699208da3d 100644 --- a/packages/paperclip-runner/src/live/runnerd-codex-transport.ts +++ b/packages/paperclip-runner/src/live/runnerd-codex-transport.ts @@ -5526,23 +5526,30 @@ class DurablePrpCodexTransport implements CodexAppServerTransport { let identity = controlPlaneState ? controlPlaneIdentity(controlPlaneState) : desiredIdentity; + const hasControlPlaneWarmBoundary = + controlPlaneState?.warmTransition !== undefined || + controlPlaneState?.schema === + "paperclip.runner.durable.control-plane-state.warm-transition.v1"; + const hasPendingWarmAttachment = + Array.isArray(controlPlaneState?.commands) && + controlPlaneState.commands.some((entry) => { + const command = record(entry); + return ( + command.type === "run.attach" && + command.status === "pending" && + record(command.payload).paperclipNextAuthority !== undefined + ); + }); if ( this.options.readRunnerState && controlPlaneState && - (controlPlaneState.warmTransition !== undefined || + (hasControlPlaneWarmBoundary || controlPlaneState.completedWarmTransition !== undefined || - controlPlaneState.schema === - "paperclip.runner.durable.control-plane-state.warm-transition.v1" || - (Array.isArray(controlPlaneState.commands) && - controlPlaneState.commands.some((entry) => { - const command = record(entry); - return ( - command.type === "run.attach" && - command.status === "pending" && - record(command.payload).paperclipNextAuthority !== undefined - ); - }))) + hasPendingWarmAttachment) ) { + if (!this.options.adoptExistingRunner && !hasControlPlaneWarmBoundary && !hasPendingWarmAttachment) { + await this.options.prepareExternalRunnerState?.(); + } candidateRunnerState = await this.options.readRunnerState(); } const staged = inspectStagedRunAttachment(controlPlaneState, desiredIdentity); @@ -5558,9 +5565,7 @@ class DurablePrpCodexTransport implements CodexAppServerTransport { port?: number; } | null = null; const hasWarmBoundary = - controlPlaneState?.warmTransition !== undefined || - controlPlaneState?.schema === - "paperclip.runner.durable.control-plane-state.warm-transition.v1" || + hasControlPlaneWarmBoundary || hasRunnerWarmBoundary || candidateRunnerState?.warmTransition !== undefined || candidateRunnerState?.schema ===