mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-10 12:37:55 +01:00
fix(native): bound an idle warm local runner whose controller died (#19879)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - A warm runner (`lifecycleMode: warm`) keeps runnerd alive between runs so a follow-up turn starts fast. The follow-up default flip (next PR) makes warm the default for local Codex and OpenCode agents. > - The controller owns a warm runner's idle window. Runnerd ignores the idle timeout itself. > - A graceful controller shutdown suspends idle warm sessions. A controller that dies (SIGKILL, crash) does not. Local runners launch with no process reconnect grace, so the idle runner kept running until its 1-hour connection lease ran out, owned by nobody. > - This pull request gives warm local runners the 7-minute reconnect grace that remote runners already use. > - The benefit: idle runners are reaped by proof within a bounded time, never left orphaned, before warm becomes the default. ## Linked Issues or Issue Description Refs #19874 (warm completion contract), #19846 (TTFT baseline), #19852. **What existing behavior does this improve?** Ownership of an idle warm local runner after its controller dies. **Current behavior** `runnerProcessReconnectGraceMs` is `undefined` for local targets, so runnerd launches without `--reconnect-grace-ms`. An idle warm runner whose controller was killed stays alive until its connection lease (one hour) expires. The restart-chaos harness cannot show this: its K1 kills the controller's whole process tree, runner included. **Proposed behavior** `localRunnerProcessReconnectGraceMs(policy)` returns the remote grace (7 minutes) for a warm policy and `undefined` for per-turn. After the grace, runnerd records `transport_reconnect_grace_exceeded`, stops its provider, keeps durable state and exits. The next run resumes cold. **Reason and benefit** The restart plan requires idle runners to be reaped by proof, never orphaned. Seven minutes is far longer than a controller restart (13–20 s locally) and the long-outage fault holds (3–5 minutes), so a live turn still reattaches. ## What Changed - `native-runner-recovery-contract.ts`: `localRunnerProcessReconnectGraceMs`. - `native-session-executor.ts`: the local runnerd transport passes it as `runnerProcessReconnectGraceMs`. Remote targets keep `null` (lease-owned). - `runnerd-warm-idle-orphan.test.ts` (new): real runnerd and fake Codex. An idle warm runner, after its turn completed, whose controller detaches. - `doc/DEVELOPING.md`: the grace and its bound. ## Verification - `pnpm --filter @paperclipai/paperclip-runner exec vitest run src/live/runnerd-warm-idle-orphan.test.ts`: with a process grace, the idle runner exits with `transport_reconnect_grace_exceeded` and the next transport resumes the same provider session cold. With no grace (`null`, today's local launch), the runner is still alive after the controller left. - `cd server && npx vitest run src/services/native-runtime/native-runner-recovery-contract.test.ts`: warm gets the remote grace; per-turn keeps none; the grace outlasts a controller restart and a long outage. - `tsc --noEmit` passes for `server` and `packages/paperclip-runner`. - Local restart measurement with warm runners (K1/K2, codex, claude, opencode, n = 3 each): every interrupted run recovered, with recovery latency equal or lower than per-turn. The chaos waves with this change are in the default-flip PR. ## Risks - A warm local runner whose controller stays down for more than 7 minutes exits. A turn in progress then resumes through `resume_dead_runner`, the same path as a controller crash that kills the runner. Remote runners already behave this way. - Previous-release story (`rules/upgrade-safe-run-state.mdc`): no persisted state changes. A warm runner launched by the previous release keeps the launch arguments it started with (no grace) until it exits. > 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 - Anthropic Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code, with tool use (shell, file edit, test execution). ## 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 <noreply@anthropic.com>
This commit is contained in:
1 parent
d53fa727e1
commit
0189fedffc
6 files changed
+155
-5
No files matched your search
@@ -3563,6 +3563,13 @@ that classification finishes.
|
||||
|
||||
A resumed sandbox lease can contain a workspace whose provider never started. A new attempt may create its exact session directory only when durable control-plane evidence proves zero connections, zero events, and untouched bootstrap commands, and no backup or remote session directory exists. Directory creation is atomic; partial state or uncertain ownership remains blocked.
|
||||
|
||||
A warm local runner outlives its turn, so it carries a 7-minute reconnect grace
|
||||
(`localRunnerProcessReconnectGraceMs`, the same grace remote runners use). If its
|
||||
controller dies without suspending it, runnerd proves after the grace that no
|
||||
controller re-authenticated, stops its provider, keeps its durable state for a
|
||||
cold resume, and exits. A graceful controller shutdown suspends idle warm
|
||||
sessions first. A per-turn local runner keeps its connection-lease bound.
|
||||
|
||||
Only successful turns retain a live warm provider session. A structured failed
|
||||
or cancelled result must retire that session and collect its managed files before
|
||||
heartbeat stops the reusable sandbox. A later retry can resume the same sandbox
|
||||
|
||||
@@ -0,0 +1,90 @@
|
||||
import { createServer } from "node:http";
|
||||
import { mkdtemp, readFile, rm } from "node:fs/promises";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { expect, it, vi } from "vitest";
|
||||
import type { DurablePrpControlPlane } from "../control-plane/durable-prp-control-plane.js";
|
||||
import { createCapabilityRunnerdCodexTransport } from "./runnerd-codex-transport.js";
|
||||
import { defaultCapabilityRunnerdBinary, fakeCodex, fakeCodexArgs } from "../../test/support/runnerd-transport.js";
|
||||
|
||||
const alive = (pid: number) => { try { process.kill(pid, 0); return true; } catch { return false; } };
|
||||
|
||||
// A warm runner outlives its turn. When its controller dies without
|
||||
// suspending it (SIGKILL, crash), nothing else owns the idle process. The
|
||||
// process grace bounds it: runnerd proves no controller re-authenticated,
|
||||
// stops its provider, keeps durable state, and exits. Without a grace it lives
|
||||
// until its connection lease (one hour for local runners).
|
||||
it.each([
|
||||
{ policy: "a process grace", processGraceMs: 500, exits: true },
|
||||
// null: no process grace, as a per-turn local runner launches today.
|
||||
{ policy: "no process grace", processGraceMs: null, exits: false },
|
||||
])("bounds an idle warm runner whose controller vanished with $policy", async ({ processGraceMs, exits }) => {
|
||||
const root = await mkdtemp(join(tmpdir(), "runner-warm-idle-orphan-"));
|
||||
const server = createServer();
|
||||
let authority: DurablePrpControlPlane | null = null;
|
||||
server.on("upgrade", (request, socket, head) => {
|
||||
if (!authority) { socket.destroy(); return; }
|
||||
authority.handleUpgrade(request, socket, "/runner", head);
|
||||
});
|
||||
await new Promise<void>(resolveListen => server.listen(0, "127.0.0.1", resolveListen));
|
||||
const address = server.address();
|
||||
if (!address || typeof address === "string") throw new Error("Missing orphan test listener");
|
||||
const identity = {
|
||||
runnerInstanceId: "runner-warm-orphan", environmentLeaseId: "lease-warm-orphan", runId: "run-warm-orphan",
|
||||
normalizedSessionId: "session-warm-orphan", turnId: "turn-warm-orphan", itemId: "item-warm-orphan",
|
||||
};
|
||||
const shared = {
|
||||
runnerBinary: defaultCapabilityRunnerdBinary(),
|
||||
codexCommand: fakeCodex,
|
||||
codexArgs: fakeCodexArgs(root),
|
||||
stateDirectory: root,
|
||||
prpIdentity: identity,
|
||||
lifecyclePolicy: { mode: "warm" as const, idleTimeoutMs: 60_000 },
|
||||
runnerReconnectGraceMs: 500,
|
||||
runnerProcessReconnectGraceMs: processGraceMs,
|
||||
controlPlaneRegistration: async (next: DurablePrpControlPlane) => {
|
||||
authority = next;
|
||||
return {
|
||||
connectUrl: `ws://127.0.0.1:${address.port}/runner`,
|
||||
release: () => { if (authority === next) authority = null; },
|
||||
};
|
||||
},
|
||||
};
|
||||
const first = createCapabilityRunnerdCodexTransport(shared);
|
||||
first.transport.setServerRequestHandler(async () => ({ success: true, contentItems: [] }));
|
||||
let resumed: ReturnType<typeof createCapabilityRunnerdCodexTransport> | undefined;
|
||||
let pid: number | null = null;
|
||||
try {
|
||||
const opened = await first.transport.request("thread/start", { cwd: root, dynamicTools: [] });
|
||||
const notifications = first.transport.notifications()[Symbol.asyncIterator]();
|
||||
await first.transport.request("turn/start", { input: [{ type: "text", text: "Finish, then stay warm." }] });
|
||||
for (;;) if ((await notifications.next()).value?.method === "turn/completed") break;
|
||||
pid = first.evidence().runnerPid;
|
||||
expect(pid).toEqual(expect.any(Number));
|
||||
// The controller disappears without suspending its idle warm runner.
|
||||
await first.detachControllerForRestart();
|
||||
if (!exits) {
|
||||
await new Promise(resolveWait => setTimeout(resolveWait, 1_500));
|
||||
expect(alive(pid!)).toBe(true);
|
||||
return;
|
||||
}
|
||||
await vi.waitFor(() => expect(alive(pid!)).toBe(false), { timeout: 10_000 });
|
||||
const runnerState = JSON.parse(await readFile(join(root, "runner", "runner-state.json"), "utf8"));
|
||||
expect(runnerState.recoverableFailure).toBe("transport_reconnect_grace_exceeded");
|
||||
// Durable state survives: the next run resumes the same provider session cold.
|
||||
const thread = opened.thread as { id: string; sessionId: string };
|
||||
resumed = createCapabilityRunnerdCodexTransport({
|
||||
...shared,
|
||||
resumeProviderSession: { driverSessionId: thread.id, providerSessionId: thread.sessionId },
|
||||
});
|
||||
const restored = await resumed.transport.request("thread/read", {});
|
||||
expect(restored.thread).toMatchObject({ id: thread.id, sessionId: thread.sessionId });
|
||||
} finally {
|
||||
await first.transport.close().catch(() => undefined);
|
||||
await resumed?.transport.close().catch(() => undefined);
|
||||
if (pid && alive(pid)) { try { process.kill(-pid, "SIGKILL"); } catch { /* already stopped */ } }
|
||||
server.closeAllConnections();
|
||||
if (server.listening) await new Promise<void>(resolveClose => server.close(() => resolveClose()));
|
||||
await rm(root, { recursive: true, force: true });
|
||||
}
|
||||
}, 45_000);
|
||||
@@ -0,0 +1,18 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { REMOTE_RUNNER_RECONNECT_GRACE_MS, localRunnerProcessReconnectGraceMs } from "./native-runner-recovery-contract.js";
|
||||
|
||||
describe("local runner process reconnect grace", () => {
|
||||
it("bounds a warm local runner that loses its controller", () => {
|
||||
// A warm runner outlives its turn; without a grace a controller crash leaves it ownerless.
|
||||
expect(localRunnerProcessReconnectGraceMs({ mode: "warm" })).toBe(REMOTE_RUNNER_RECONNECT_GRACE_MS);
|
||||
});
|
||||
|
||||
it("leaves a per-turn local runner on its connection-lease bound", () => {
|
||||
expect(localRunnerProcessReconnectGraceMs({ mode: "per_turn" })).toBeUndefined();
|
||||
});
|
||||
|
||||
it("outlasts a controller restart so a live turn can reattach", () => {
|
||||
// Local control-plane restarts take 13-20 s; long outage faults hold 3-5 minutes.
|
||||
expect(localRunnerProcessReconnectGraceMs({ mode: "warm" })!).toBeGreaterThan(5 * 60_000);
|
||||
});
|
||||
});
|
||||
@@ -1,2 +1,19 @@
|
||||
export const REMOTE_RUNNER_RECONNECT_GRACE_MS = 7 * 60_000;
|
||||
export const REMOTE_CONTROLLER_LEASE_MS = 60_000;
|
||||
|
||||
/**
|
||||
* How long a local runnerd keeps running with no authenticated controller.
|
||||
*
|
||||
* A per-turn runner exits with its turn, so it keeps its connection-lease
|
||||
* bound (undefined: no separate process grace). A warm runner outlives its
|
||||
* turn: if its controller dies without suspending it (SIGKILL, crash), nothing
|
||||
* else owns it. It therefore gets the remote grace, which is far longer than a
|
||||
* controller restart, so a live turn still reattaches. After the grace, runnerd
|
||||
* proves the controller is gone, stops its provider, keeps its durable state for
|
||||
* a cold resume, and exits.
|
||||
*/
|
||||
export function localRunnerProcessReconnectGraceMs(
|
||||
lifecyclePolicy: { mode: "per_turn" | "warm" },
|
||||
): number | undefined {
|
||||
return lifecyclePolicy.mode === "warm" ? REMOTE_RUNNER_RECONNECT_GRACE_MS : undefined;
|
||||
}
|
||||
@@ -17573,6 +17573,8 @@ describe("runnerd provider runtime wiring", () => {
|
||||
"ambiguous turn start",
|
||||
"state symlink",
|
||||
"newer provider checkpoint",
|
||||
"warm reconnect grace expired",
|
||||
"other recoverable failure",
|
||||
])(
|
||||
"automatically recovers only a proven settled local session: %s",
|
||||
async (scenario) => {
|
||||
@@ -17728,6 +17730,13 @@ describe("runnerd provider runtime wiring", () => {
|
||||
join(root, "runner", "runner-state.json"),
|
||||
JSON.stringify({
|
||||
...durableRunnerState(identity, "ready"),
|
||||
// An idle warm runner whose controller died exits after its
|
||||
// reconnect grace with this settled, preserved state.
|
||||
...(scenario === "warm reconnect grace expired"
|
||||
? { lifecycle: "recoverable_failure", recoverableFailure: "transport_reconnect_grace_exceeded" }
|
||||
: scenario === "other recoverable failure"
|
||||
? { lifecycle: "recoverable_failure", recoverableFailure: "transport_reconnect_deadline_exceeded" }
|
||||
: {}),
|
||||
outbox:
|
||||
scenario === "unacknowledged events" ? [{ sourceSeq: 10 }] : [],
|
||||
pendingTerminalDelivery: null,
|
||||
@@ -17839,6 +17848,7 @@ describe("runnerd provider runtime wiring", () => {
|
||||
"empty retry shell",
|
||||
"unsuspended current",
|
||||
"active goal",
|
||||
"warm reconnect grace expired",
|
||||
].includes(scenario);
|
||||
if (shouldRecover) {
|
||||
await createRunnerdBackend({
|
||||
@@ -17891,7 +17901,7 @@ describe("runnerd provider runtime wiring", () => {
|
||||
"utf8",
|
||||
),
|
||||
).lifecycle,
|
||||
).toBe("ready");
|
||||
).toBe(scenario === "other recoverable failure" ? "recoverable_failure" : "ready");
|
||||
expect(onLog).not.toHaveBeenCalled();
|
||||
expect(state.createBackend).not.toHaveBeenCalled();
|
||||
}
|
||||
|
||||
@@ -142,7 +142,7 @@ import {
|
||||
import { hostname, tmpdir, userInfo } from "node:os";
|
||||
import { DatabaseSync } from "node:sqlite";
|
||||
import { NATIVE_CHECKPOINT_HEADER_INSPECTION_MAX_BYTES, loadNativeRunnerCheckpoint, loadNativeRunnerCheckpointHeader, loadNativeUnclaimedBootstrapCheckpoint, nativeRunnerCheckpointPersistence, nativeRunnerCheckpointRegistration, restoreNativeRunnerCheckpoint } from "./native-runner-checkpoint.js";
|
||||
import { REMOTE_CONTROLLER_LEASE_MS, REMOTE_RUNNER_RECONNECT_GRACE_MS } from "./native-runner-recovery-contract.js";
|
||||
import { REMOTE_CONTROLLER_LEASE_MS, REMOTE_RUNNER_RECONNECT_GRACE_MS, localRunnerProcessReconnectGraceMs } from "./native-runner-recovery-contract.js";
|
||||
import type { NativeTaskSessionPublicationContext } from "./native-task-session-publication.js";
|
||||
import { basename, dirname, join, posix, resolve } from "node:path";
|
||||
import type {
|
||||
@@ -5922,9 +5922,14 @@ async function recoverQuiescentRunnerdState(input: {
|
||||
const control = record(JSON.parse(controlBytes));
|
||||
const provider = record(JSON.parse(providerBytes));
|
||||
const events = control.committedEvents;
|
||||
// A warm runner whose controller died exits after its reconnect grace
|
||||
// with settled, preserved state; it is as recoverable as a dead "ready"
|
||||
// runner. Any other recoverable failure stays unproven.
|
||||
const graceExpired = runner.lifecycle === "recoverable_failure"
|
||||
&& runner.recoverableFailure === "transport_reconnect_grace_exceeded";
|
||||
if (
|
||||
runner.schema !== RUNNERD_STATE_SCHEMA ||
|
||||
!["ready", "suspended"].includes(String(runner.lifecycle)) ||
|
||||
(!["ready", "suspended"].includes(String(runner.lifecycle)) && !graceExpired) ||
|
||||
(!currentEmpty && runner.lifecycle === "suspended") ||
|
||||
[
|
||||
"runId",
|
||||
@@ -6116,7 +6121,9 @@ async function recoverQuiescentRunnerdState(input: {
|
||||
const temporary = `${statePath}.${randomUUID()}.tmp`;
|
||||
writeFileSync(
|
||||
temporary,
|
||||
JSON.stringify({ ...candidate.runner, lifecycle: "suspended" }),
|
||||
// A grace-expired runner's failure cause is resolved by this proof.
|
||||
JSON.stringify({ ...candidate.runner, lifecycle: "suspended",
|
||||
...(candidate.runner.recoverableFailure === "transport_reconnect_grace_exceeded" ? { recoverableFailure: null } : {}) }),
|
||||
{ encoding: "utf8", mode: 0o600, flag: "wx" },
|
||||
);
|
||||
renameSync(temporary, statePath);
|
||||
@@ -16156,7 +16163,8 @@ async function createRunnerdBackendWithinSessionClaim(
|
||||
// A controller's bounded reconnect attempt is not the remote owner's
|
||||
// lifetime. Retain it through restarts until its authenticated lease
|
||||
// expires or explicit Stop/suspension proves that execution ended.
|
||||
runnerProcessReconnectGraceMs: remoteTarget ? null : undefined,
|
||||
// A warm local runner outlives its turn; bound it if its controller dies.
|
||||
runnerProcessReconnectGraceMs: remoteTarget ? null : localRunnerProcessReconnectGraceMs(input.execution.session.lifecyclePolicy),
|
||||
environment: effectiveRunnerEnvironment,
|
||||
observability: input.observability,
|
||||
onDiagnostic: message => diagnostics.record(
|
||||
|
||||
Reference in new issue
Block a user