mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-11 05:11:49 +01:00
fix(runner): stop relaunching a runner that keeps exiting after restore (#20608)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Agent runs execute through Paperclip Runner. The server-side
transport supervises the runnerd process and restarts it when it exits
unexpectedly
> - That recovery keeps launching replacements until one restores the
durable PRP session, within a grace window
> - Each successful restore starts a fresh grace window. So a runner
that restores and then exits again a few seconds later loops with no
limit
> - In a real run, a deterministic runnerd crash looped this way 62
times in 15 minutes, with no error recorded, until a server restart
ended it
> - This pull request fails the transport after 3 restored runners each
exit within a minute of their restore. The error carries the runner's
last diagnostic
> - The benefit is that a crashing runner fails the run in under a
minute, with the reason recorded, instead of looping silently
## Linked Issues or Issue Description
**What happened?**
After runnerd exited unexpectedly, `#recoverRunnerProcess` launched a
replacement. The replacement authenticated ("runner process restored its
durable PRP session") and then exited about 5 seconds later with the
same diagnostic. `#watchRunner` then started a new recovery with a new
420-second grace window. This repeated every ~14 seconds for 15 minutes,
until the server shut down. The run produced no output and recorded no
failure during that time.
**Expected behavior**
A runner that keeps exiting right after each restore fails the transport
quickly. The failure names the runner's own diagnostic.
**Steps to reproduce**
1. Start a transport with `runnerReconnectGraceMs` set.
2. Kill runnerd. Recovery launches a replacement, which restores the
session.
3. Kill each restored replacement within a few seconds. On `master`,
recovery relaunches every time; the new test shows no failure after 3
restored runners exit.
**Paperclip version or commit**
`master` at 491dfa1c6d.
## What Changed
- `packages/paperclip-runner/src/live/runnerd-codex-transport.ts`:
- Records the time of each successful restore.
- Both runner-exit paths in `#watchRunner` (exit and completion
rejection) now go through `#recoverOrFailRunnerProcess`. It counts an
exit that comes within `RUNNER_RESTORE_STABLE_MS` (60 s) of the last
restore. A runner that stays up longer than that resets the count.
- On the third such exit (`MAX_UNSTABLE_RUNNER_RESTORES`), it fails the
transport with `native_runner_process_exited: runnerd exited 3 times
within 60000ms of being restored: <last diagnostic>`, and does not
launch again.
- The counting rule is a pure function, `unstableRunnerRestoreCount`,
exposed through `runnerdRecoveryInternals` for tests.
- `runnerd-codex-transport.test.ts`:
- An integration test with the real runnerd: it kills each restored
runner and expects the failure after 3 restores, with exactly 4 launches
and no further relaunch.
- A unit test for the counting rule, including the reset after a stable
runner.
## Verification
- `npx vitest run src/live/runnerd-codex-transport.test.ts -t "keeps
exiting right after|stable window|replacement whose staging"`: 3 pass (2
new, and the existing replacement-restore test).
- Red check: with `master`'s transport and the same tests, the loop test
fails. The transport never reports the failure and keeps relaunching.
- Full `npx vitest run src/live/runnerd-codex-transport.test.ts`: 296
pass.
- `npx tsc -p tsconfig.json --noEmit` in `packages/paperclip-runner`
(after `pnpm run ensure:eval-build-deps`): clean.
## Risks
- A runner that crashes repeatedly right after restoring now fails the
run after 3 restores instead of retrying indefinitely. The server's own
recovery then handles the failed run as it does any other
`native_runner_process_exited` failure.
- A healthy runner that restores and stays up longer than 60 seconds is
unaffected, and a later exit starts a new count.
- A companion pull request, #20607, removes the runnerd crash that
caused the observed loop.
## Model Used
- Claude Opus 5.5 (`claude-opus-5-5`) via Claude Code, with tool use
(shell, file editing, vitest and cargo runs, read-only log analysis).
## 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 <noreply@paperclip.ing>
This commit is contained in:
1 parent
90dd864bff
commit
4f8dd0e25a
2 files changed
+90
-2
No files matched your search
@@ -538,6 +538,67 @@ it("retries a replacement whose staging failed before any process launched", asy
|
||||
}
|
||||
}, 15_000);
|
||||
|
||||
it("fails instead of relaunching a runner that keeps exiting right after it is restored", async () => {
|
||||
const directory = await mkdtemp(join(tmpdir(), "runnerd-restore-crash-loop-"));
|
||||
const crash = "paperclip-runnerd: redacted display activity exceeds its schema bounds";
|
||||
const children: ReturnType<typeof spawn>[] = [];
|
||||
const exits: Promise<void>[] = [];
|
||||
const launch = vi.fn((spec: durableControlPlane.RunnerProcessLaunchSpec) => {
|
||||
const child = spawn(spec.command, [...spec.args], { cwd: spec.cwd, env: spec.environment, stdio: "ignore" });
|
||||
children.push(child);
|
||||
exits.push(new Promise(resolveExit => child.once("exit", () => resolveExit())));
|
||||
return {
|
||||
child,
|
||||
completion: new Promise<durableControlPlane.RunnerProcessResult>((resolveExit, rejectExit) => {
|
||||
child.once("error", rejectExit);
|
||||
child.once("exit", (code, signal) => resolveExit({ code, signal, stdout: "", stderr: crash }));
|
||||
}),
|
||||
isExitConfirmed: () => child.exitCode !== null || child.signalCode !== null,
|
||||
};
|
||||
});
|
||||
const bundle = createCapabilityRunnerdCodexTransport({
|
||||
runnerBinary: defaultCapabilityRunnerdBinary(), codexCommand: fakeCodex,
|
||||
codexArgs: fakeCodexArgs(directory), stateDirectory: directory,
|
||||
lifecyclePolicy: { mode: "warm", idleTimeoutMs: 60_000 },
|
||||
runnerReconnectGraceMs: 5_000, runnerProcessLauncher: launch,
|
||||
});
|
||||
const restored = () => bundle.evidence().diagnostics
|
||||
.filter(line => line === "runner process restored its durable PRP session").length;
|
||||
try {
|
||||
await bundle.transport.request("thread/start", { cwd: directory });
|
||||
children[0]!.kill("SIGKILL");
|
||||
for (let restore = 1; restore <= 3; restore += 1) {
|
||||
await vi.waitFor(() => expect(restored()).toBe(restore), { timeout: 4_000 });
|
||||
children[restore]!.kill("SIGKILL");
|
||||
}
|
||||
await vi.waitFor(
|
||||
() => expect(bundle.evidence().diagnostics).toContain(
|
||||
`native_runner_process_exited: runnerd exited 3 times within 60000ms of being restored: ${crash}`,
|
||||
),
|
||||
{ timeout: 4_000 },
|
||||
);
|
||||
await new Promise(resolveWait => setTimeout(resolveWait, 500));
|
||||
expect(launch).toHaveBeenCalledTimes(4);
|
||||
expect(restored()).toBe(3);
|
||||
} finally {
|
||||
const providerPid = bundle.evidence().codexPid;
|
||||
if (providerPid) { try { process.kill(providerPid, "SIGKILL"); } catch {} }
|
||||
await bundle.transport.detachControllerForRestart!();
|
||||
for (const child of children) child.kill("SIGKILL");
|
||||
await Promise.all(exits);
|
||||
await rm(directory, { recursive: true, force: true });
|
||||
}
|
||||
}, 20_000);
|
||||
|
||||
it("counts only runner exits that follow a restore within the stable window", () => {
|
||||
const count = runnerdRecoveryInternals.unstableRunnerRestoreCount;
|
||||
expect(count(0, null, 1_000)).toBe(0);
|
||||
expect(count(0, 1_000, 1_000)).toBe(1);
|
||||
expect(count(2, 1_000, 60_999)).toBe(3);
|
||||
expect(count(2, 1_000, 61_000)).toBe(0);
|
||||
expect(count(5, null, 1_000)).toBe(0);
|
||||
});
|
||||
|
||||
it.each(["alive", "pending_liveness", "pending_registration"] as const)(
|
||||
"bounds adopted runner authentication at its exact deadline with %s evidence",
|
||||
async (mode) => {
|
||||
|
||||
@@ -117,6 +117,8 @@ const MAX_NOTIFICATION_COUNT = 2_048;
|
||||
const MAX_NOTIFICATION_BYTES = 4 * 1024 * 1024;
|
||||
const RUNNER_CLIENT_VERSION = "0.3.0";
|
||||
const RUNNER_BOOTSTRAP_TICKET_TTL_MS = 60_000;
|
||||
const RUNNER_RESTORE_STABLE_MS = 60_000;
|
||||
const MAX_UNSTABLE_RUNNER_RESTORES = 3;
|
||||
const RUNNERD_MAX_OUTBOX_BYTES = 16 * 1024 * 1024;
|
||||
const RUNNERD_P0_RESERVE_BYTES = 1024 * 1024;
|
||||
|
||||
@@ -4050,6 +4052,8 @@ class DurablePrpCodexTransport implements CodexAppServerTransport {
|
||||
readonly #failureSignal: Promise<never>;
|
||||
#rejectFailureSignal!: (error: Error) => void;
|
||||
#runnerRecoveryInProgress = false;
|
||||
#lastRunnerRestoreAt: number | null = null;
|
||||
#unstableRunnerRestores = 0;
|
||||
#startupComplete = false;
|
||||
readonly #authorityRunId: () => string | undefined;
|
||||
#startupFailureCode = "native_runner_process_exited";
|
||||
@@ -8489,7 +8493,7 @@ class DurablePrpCodexTransport implements CodexAppServerTransport {
|
||||
this.options.runnerReconnectGraceMs !== undefined &&
|
||||
handle.restart
|
||||
) {
|
||||
void this.#recoverRunnerProcess(handle, detail);
|
||||
this.#recoverOrFailRunnerProcess(handle, detail);
|
||||
return;
|
||||
}
|
||||
this.#failTransport(
|
||||
@@ -8507,7 +8511,7 @@ class DurablePrpCodexTransport implements CodexAppServerTransport {
|
||||
this.options.runnerReconnectGraceMs !== undefined &&
|
||||
handle.restart
|
||||
) {
|
||||
void this.#recoverRunnerProcess(
|
||||
this.#recoverOrFailRunnerProcess(
|
||||
handle,
|
||||
error instanceof Error ? error.message : String(error),
|
||||
);
|
||||
@@ -8528,6 +8532,23 @@ class DurablePrpCodexTransport implements CodexAppServerTransport {
|
||||
);
|
||||
}
|
||||
|
||||
#recoverOrFailRunnerProcess(handle: RunnerProcessHandle, detail: string): void {
|
||||
this.#unstableRunnerRestores = unstableRunnerRestoreCount(
|
||||
this.#unstableRunnerRestores,
|
||||
this.#lastRunnerRestoreAt,
|
||||
Date.now(),
|
||||
);
|
||||
if (this.#unstableRunnerRestores >= MAX_UNSTABLE_RUNNER_RESTORES) {
|
||||
this.#failTransport(
|
||||
new Error(
|
||||
`native_runner_process_exited: runnerd exited ${this.#unstableRunnerRestores} times within ${RUNNER_RESTORE_STABLE_MS}ms of being restored${detail ? `: ${detail.slice(-1_000)}` : ""}`,
|
||||
),
|
||||
);
|
||||
return;
|
||||
}
|
||||
void this.#recoverRunnerProcess(handle, detail);
|
||||
}
|
||||
|
||||
async #recoverRunnerProcess(
|
||||
failedHandle: RunnerProcessHandle,
|
||||
initialDetail: string,
|
||||
@@ -8622,6 +8643,7 @@ class DurablePrpCodexTransport implements CodexAppServerTransport {
|
||||
})();
|
||||
if (await Promise.race([completion, authenticated])) {
|
||||
this.#diagnostic("runner process restored its durable PRP session");
|
||||
this.#lastRunnerRestoreAt = Date.now();
|
||||
this.#watchRunner(recoveredHandle);
|
||||
return;
|
||||
}
|
||||
@@ -9057,7 +9079,12 @@ export const runnerdLaunchProfileInternals = Object.freeze({
|
||||
p0ReserveBytes: RUNNERD_P0_RESERVE_BYTES,
|
||||
});
|
||||
|
||||
function unstableRunnerRestoreCount(previous: number, restoredAt: number | null, now: number): number {
|
||||
return restoredAt !== null && now - restoredAt < RUNNER_RESTORE_STABLE_MS ? previous + 1 : 0;
|
||||
}
|
||||
|
||||
export const runnerdRecoveryInternals = Object.freeze({
|
||||
unstableRunnerRestoreCount,
|
||||
projectCopilotRunGrant,
|
||||
completedMaintenanceTerminalReceipt,
|
||||
completedMaintenanceTerminalReplayMatches,
|
||||
|
||||
Reference in new issue
Block a user