From 648e0563326be2e014fc27875e70212e1370c845 Mon Sep 17 00:00:00 2001 From: Dotta Date: Wed, 23 Sep 2026 06:02:53 -0700 Subject: [PATCH] fix(runner): wait for remote process containment before reuse Await the ownership-fenced remote termination monitor before session close resolves. Bound EC2 verification checks to retain failure evidence before fleet shutdown. Co-authored-by: Paperclip --- .github/workflows/docker-runner-check.yml | 9 +++++-- .../src/live/runnerd-codex-transport.test.ts | 24 +++++++++++++++++++ .../src/live/runnerd-codex-transport.ts | 18 ++++++++++---- 3 files changed, 45 insertions(+), 6 deletions(-) diff --git a/.github/workflows/docker-runner-check.yml b/.github/workflows/docker-runner-check.yml index b251eab7306..45f3e297408 100644 --- a/.github/workflows/docker-runner-check.yml +++ b/.github/workflows/docker-runner-check.yml @@ -167,6 +167,8 @@ jobs: echo "Image: $immutable" >> "$GITHUB_STEP_SUMMARY" - name: Verify repository on EC2 if: inputs.verify_source + # Leave time for artifact retention before the fleet's one-hour lifetime. + timeout-minutes: 38 run: | set -uo pipefail pnpm install --frozen-lockfile --ignore-scripts @@ -174,8 +176,11 @@ jobs: for check in 'pnpm -r typecheck' 'pnpm test:run' 'pnpm check:token-gates' 'pnpm test:e2e:runner:typecheck' 'pnpm test:e2e:runner:unit' 'pnpm build'; do label="$(echo "$check" | tr -cs 'a-zA-Z0-9' '-')" echo "Starting $check" - bash -c "$check" > "remote-verification/$label.log" 2>&1 || status=1 - echo "Finished $check" + check_status=0 + timeout --signal=TERM --kill-after=15s 15m bash -c "$check" > "remote-verification/$label.log" 2>&1 || check_status=$? + printf '%s\t%s\n' "$check_status" "$check" >> remote-verification/check-status.tsv + if [ "$check_status" -ne 0 ]; then status=1; fi + echo "Finished $check (exit $check_status)" done exit "$status" - name: Build canonical eval report viewer 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 3a5842abada..b1113094ecb 100644 --- a/packages/paperclip-runner/src/live/runnerd-codex-transport.test.ts +++ b/packages/paperclip-runner/src/live/runnerd-codex-transport.test.ts @@ -1089,6 +1089,30 @@ it("quiesces the control route before checkpoint and containment regardless of p expect(failedCheckpointSteps).toEqual(["release", "checkpoint", "kill"]); }); +it("does not release a session before asynchronous process containment settles", async () => { + let finishKill!: () => void; + const killed = new Promise((resolve) => { finishKill = resolve; }); + const steps: string[] = []; + const released = runnerdRecoveryInternals.releaseRunnerProcessOwnership({ + runnerSettled: false, + release: () => { steps.push("route-closed"); }, + checkpoint: () => { steps.push("checkpoint"); }, + forceKill: async () => { steps.push("kill-dispatched"); await killed; steps.push("process-exited"); }, + }).then(() => { steps.push("session-reusable"); }); + await new Promise(resolve => setImmediate(resolve)); + expect(steps).toEqual(["route-closed", "checkpoint", "kill-dispatched"]); + finishKill(); + await released; + expect(steps).toEqual(["route-closed", "checkpoint", "kill-dispatched", "process-exited", "session-reusable"]); +}); + +it("refuses session reuse when asynchronous process containment fails", async () => { + await expect(runnerdRecoveryInternals.releaseRunnerProcessOwnership({ + runnerSettled: true, release: null, checkpoint: null, + forceKill: async () => { throw new Error("remote process still alive"); }, + })).rejects.toThrow("remote process still alive"); +}); + it("waits for the exact durable suspension command behind prior close work", async () => { const commands = [ { diff --git a/packages/paperclip-runner/src/live/runnerd-codex-transport.ts b/packages/paperclip-runner/src/live/runnerd-codex-transport.ts index 50914b7e7d1..ea7281dff0a 100644 --- a/packages/paperclip-runner/src/live/runnerd-codex-transport.ts +++ b/packages/paperclip-runner/src/live/runnerd-codex-transport.ts @@ -678,7 +678,7 @@ async function releaseRunnerProcessOwnership(input: { runnerSettled: boolean; checkpoint: ((settlement: "settled" | "unsettled") => Promise | void) | null; - forceKill: () => void; + forceKill: () => Promise | void; release: (() => Promise | void) | null; }): Promise { let releaseFailure: unknown; @@ -701,7 +701,7 @@ async function releaseRunnerProcessOwnership(input: { } catch (error) { checkpointFailure = error; } finally { - input.forceKill(); + await input.forceKill(); } if (checkpointFailure !== undefined) throw checkpointFailure; if (releaseFailure !== undefined) throw releaseFailure; @@ -4269,8 +4269,18 @@ class DurablePrpCodexTransport implements CodexAppServerTransport { adoptedRunner && !this.#adoptedRunnerAuthenticated ? null : this.#controlPlaneCheckpoint, - forceKill: () => { - this.#handle?.child.kill("SIGKILL"); + forceKill: async () => { + const handle = this.#handle; + if (!handle || this.#evidence.runnerExited) return; + handle.child.kill("SIGKILL"); + // A remote kill dispatches an asynchronous, ownership-fenced RPC. + // Do not release the session for reuse until its process monitor has + // settled: the successor would otherwise overwrite that ownership + // marker while the previous runner still holds the fixed listener. + const result = await waitForProcess(handle, 15_000); + this.#evidence.runnerExited = true; + this.#evidence.runnerExitCode = result.code; + this.#evidence.runnerSignal = result.signal as NodeJS.Signals | null; }, release: this.#controlPlaneRelease, });