From 586caa5f4935c0d79eed5c1a4456ce7fe171b1fc Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Fri, 6 Nov 2026 15:10:44 -0600 Subject: [PATCH] fix(native): hand a shutdown-interrupted startup to the next controller (#19596) 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 > - The native Runner path runs an agent turn through a runnerd process that the control plane owns > - A restart of the control plane must keep the task going without a click, if that is safe > - A graceful shutdown closes the HTTP listener first and then drains the heartbeat runs > - A native run that was between provider dispatch and runnerd backend setup in that gap failed with `local_runtime_api_listener_unavailable`, so the drain did not see it as running and did not suspend it for the next boot > - This pull request makes that failure a restart detach: the run stays running with its claim, the drain suspends it, and the next controller adopts it > - The benefit is that a SIGTERM in this window no longer spends a failure and a same-run retry, and the task continues by itself ## Linked Issues or Issue Description Refs #19530 (the same restart window, before provider dispatch). **What happened?** In a restart chaos replay (ACPX Claude, local runner), a restart-recovered run got SIGTERM while `createRunnerdBackendWithinSessionClaim` built the runnerd backend. `resolveLocalRuntimeApiUrl()` threw `local_runtime_api_listener_unavailable` because shutdown had already closed the listener. The run settled `failed`. The drain then logged `restartSuspendedRunIds: []`. The next boot used the one bootstrap retry. A second SIGTERM hit the same window, and the task stopped at a manual Retry. **Expected behavior** A run that this process cannot continue because it is shutting down is handed to the next controller. It is not settled as failed. **Steps to reproduce** 1. Start a native Runner task and restart the server (K1) so that startup recovery resumes the run. 2. Send SIGTERM while the resumed run sets up its runnerd backend. 3. See the run fail with `local_runtime_api_listener_unavailable` and the drain report no suspended runs. ## What Changed - `server/src/runtime-api.ts`: `resolveLocalRuntimeApiUrl()` throws the new `LocalRuntimeApiListenerClosedError` when this process had a listener and closed it (the `listening` flag is false or the `close` event fired). The error message stays `local_runtime_api_listener_unavailable`, so the existing classifiers do not change. A listener that never started still throws the plain error. - `server/src/services/native-runtime/native-session-executor.ts`: for a local Runner, the startup failure handler treats `LocalRuntimeApiListenerClosedError` like a restart detach. It stops lease renewal, keeps the claim and throws `NativeControllerDetachedForRestartError`. Heartbeat already keeps such a run `running`. A remote Runner keeps the same-run bootstrap retry from #19462, because startup recovery has no process evidence to adopt a claimed remote startup (`native-runner-restart-preparation.integration.test.ts` still covers that path). - `doc/execution-semantics.md`: describe the new rule. - Tests: a heartbeat regression that closes the real runtime API listener before backend setup, then drains and restarts; a unit test for the closed versus never-started listener; two `vi.mock` factories keep the real module exports. ## Verification - Red before the fix: `pnpm exec vitest run src/__tests__/heartbeat-native-restart-suspension.test.ts -t listener` failed with `restartSuspendedRunIds: []` and the run `failed` (`local_runtime_api_listener_unavailable`, `bootstrap_retry`). - Green after the fix: the same test passes. The drain suspends the run, the run stays `running` with `awaiting_runner_reattach`, and the next controller claims it and reaches the backend once. - `pnpm exec vitest run src/__tests__/heartbeat-native-restart-suspension.test.ts src/http/server.test.ts src/__tests__/server-startup-feedback-export.test.ts src/services/native-runtime/native-session-executor.test.ts src/services/native-runtime/native-recovered-owner-followup.test.ts`: 1333 tests pass. One timing test in `native-session-executor.test.ts` failed once under host load average ~30 and passed alone. - `pnpm exec tsc --noEmit -p server/tsconfig.json`: pass. - Live, restart chaos harness, local ACPX Claude (`claude-sonnet-5`), `--rc06-masked`, this branch at a35ec8b845 (before the remote-scope commit 5dc63f24fb, which does not change the local path). The original replay schedule no longer reached the window, because the task now ends before the second fault. So I moved the faults earlier: K1 at 10 s, then K2 as soon as the recovered server is ready. - K1 + K2, 2 runs: both runs PASS with **R1 pass**. Each drain logged `restartSuspendedRunIds: []` (before: `[]`). The next boot classified the run `resume_dead_runner` and the same run completed. No `local_runtime_api_listener_unavailable` failure. - K1 + K2 + K2, 2 runs: each drain suspended the run. The fourth boot blocked the run with `native_restart_recovery_blocked`, because three restart claims used the three-attempt recovery budget. Retry then completed the task (R3 pass). That budget count is a separate open item. This pull request does not change it. ## Risks - Low. The new path applies only after this process closed its own listener, which happens only during shutdown. The run then waits for the drain or the next boot, as for the other restart detaches. - If shutdown does not reach the drain (for example SIGKILL after the listener closes), the run keeps its live claim until the claim expires. Then startup recovery handles it as a hard restart. This is the same as any run that is running at a hard kill. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, through Claude Code with tool use (shell, file edits, GitHub CLI). Extended reasoning on. ## 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 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 --- doc/execution-semantics.md | 7 +++ ...eartbeat-native-restart-suspension.test.ts | 51 +++++++++++++++++++ .../src/__tests__/helpers/native-restart.ts | 2 +- server/src/http/server.test.ts | 26 +++++++++- server/src/runtime-api.ts | 38 ++++++++++++-- .../native-recovered-owner-followup.test.ts | 2 +- .../native-session-executor.test.ts | 2 +- .../native-runtime/native-session-executor.ts | 11 +++- tests/runner-e2e/workflow-target.test.ts | 3 +- 9 files changed, 131 insertions(+), 11 deletions(-) diff --git a/doc/execution-semantics.md b/doc/execution-semantics.md index 986c65cbbf5..5c71fd55f82 100644 --- a/doc/execution-semantics.md +++ b/doc/execution-semantics.md @@ -3317,6 +3317,13 @@ A graceful shutdown can also suspend a restart-recovery claim whose execution has not yet reached provider dispatch. That execution has lost its authority: it leaves the run running, its budget hold held and its workspace unmarked, and the next controller adopts the same run. It does not settle the run as failed. +Shutdown closes the runtime API listener before it drains runs. A local Runner +execution that then needs the listener (for example, to give a new runner its +callback origin) detaches the same way instead of failing. It keeps its claim, +the drain suspends it, and the next controller adopts the run. A remote Runner +execution has no process evidence on the controller host, so it keeps the +same-run bootstrap retry. A listener that never started is still a startup +failure. Remote Runner processes use their authenticated connection lease as the bound on disconnected lifetime. The controller's shorter connection-recovery budget limits its own attempt; it must not terminate the remote owner while another diff --git a/server/src/__tests__/heartbeat-native-restart-suspension.test.ts b/server/src/__tests__/heartbeat-native-restart-suspension.test.ts index 892cb063735..654f152bcf3 100644 --- a/server/src/__tests__/heartbeat-native-restart-suspension.test.ts +++ b/server/src/__tests__/heartbeat-native-restart-suspension.test.ts @@ -2,6 +2,7 @@ import { randomUUID } from "node:crypto"; import { and, eq } from "drizzle-orm"; import { authUsers, budgetReservations, companyMemberships, completionContracts, heartbeatRunEvents, heartbeatRuns, issues, nativeRunFinalizations, nativeRunResults, workspaceOperations } from "@paperclipai/db"; import { expect, it, vi } from "vitest"; +import { resolveLocalRuntimeApiUrl } from "../runtime-api.js"; import { heartbeatService } from "../services/heartbeat.js"; import { instanceSettingsService } from "../services/instance-settings.js"; import { currentNativeControllerIdentity } from "../services/native-runtime/native-restart-recovery.js"; @@ -77,6 +78,56 @@ describeEmbeddedPostgres("heartbeat native recovery interrupted by graceful shut 90_000, ); + it("hands a recovery to the next controller when shutdown closed the listener before backend setup", async () => { + const f = await seedRecoverableRun("LISTENER-CLOSED"); + const listener = fixture.current.server!; + const address = listener.address(); + if (!address || typeof address === "string") throw new Error("Expected a TCP runtime API listener"); + let listenerClosed: Promise | undefined; + // Graceful shutdown closes HTTP admission before it drains heartbeat runs. + // Backend setup then asks for the callback origin, as runnerd setup does. + const dying = vi.fn(() => ({ + descriptor: async () => { resolveLocalRuntimeApiUrl(); throw new Error("unreachable"); }, + openSession: async () => { resolveLocalRuntimeApiUrl(); throw new Error("unreachable"); }, + })); + const heartbeat = heartbeatService(f.db, { + nativeSessionBackendFactory: dying as never, + beforeNativeRuntimeSelection: async () => { + listener.closeIdleConnections(); + listenerClosed = new Promise(resolve => listener.close(() => resolve())); + }, + }); + let drained: unknown; + try { + try { + const { claims } = await heartbeat.recoverNativeRunsAfterRestart(new Date(), [f.runId]); + expect(claims).toEqual([expect.objectContaining({ runId: f.runId })]); + } finally { await heartbeat.drainActiveRunExecutions(); } + expect(dying).toHaveBeenCalled(); + drained = await heartbeat.drainRunningRunsForShutdown("SIGTERM", new Date(), [f.runId]); + await heartbeat.drainActiveRunExecutions(); + } finally { + await listenerClosed; + await new Promise(resolve => listener.listen(address.port, "127.0.0.1", resolve)); + } + expect(drained).toMatchObject({ restartSuspendedRunIds: [f.runId] }); + const [run] = await f.db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, f.runId)); + const [owner] = await f.db.select().from(nativeRunFinalizations).where(eq(nativeRunFinalizations.runId, f.runId)); + expect({ status: run.status, errorCode: run.errorCode, finishedAt: run.finishedAt }).toEqual({ + status: "running", errorCode: null, finishedAt: null }); + expect(owner).toMatchObject({ phase: "observed", resultId: null, recoveryState: "awaiting_runner_reattach" }); + + await f.db.update(nativeRunFinalizations).set({ controllerPid: 2_000_000_001 }) + .where(eq(nativeRunFinalizations.runId, f.runId)); + const resumed = vi.fn(() => { throw new Error("next boot reached provider dispatch"); }); + const next = heartbeatService(f.db, { nativeSessionBackendFactory: resumed }); + try { + const { claims } = await next.recoverNativeRunsAfterRestart(new Date(), [f.runId]); + expect(claims).toEqual([expect.objectContaining({ runId: f.runId })]); + } finally { await next.drainActiveRunExecutions(); } + expect(resumed).toHaveBeenCalledOnce(); + }, 90_000); + it("still settles a recovery whose claim merely expired without a shutdown suspension", async () => { const f = await seedRecoverableRun("EXPIRED-NOT-SUSPENDED"); const dying = vi.fn(() => { throw new Error("An expired claim must not open a provider session"); }); diff --git a/server/src/__tests__/helpers/native-restart.ts b/server/src/__tests__/helpers/native-restart.ts index deccd592af1..8d642f4ca12 100644 --- a/server/src/__tests__/helpers/native-restart.ts +++ b/server/src/__tests__/helpers/native-restart.ts @@ -244,7 +244,7 @@ export function nativeRestartFixture() { }; } - return { seedRun, transportOptions, get current() { return { temporary, runtimeRoot, paperclipHome, apiUrl, companyId, agentId, successor }; } }; + return { seedRun, transportOptions, get current() { return { temporary, runtimeRoot, paperclipHome, apiUrl, companyId, agentId, successor, server }; } }; } export function remotePreparedWorkspace() { diff --git a/server/src/http/server.test.ts b/server/src/http/server.test.ts index b548858d23f..c4616ec6751 100644 --- a/server/src/http/server.test.ts +++ b/server/src/http/server.test.ts @@ -8,7 +8,7 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import express from "express"; import { DurablePrpControlPlane } from "../vendor/paperclip-runner/index.js"; import { registerRunnerPrpAuthority, runnerPrpWebSocketInternals } from "../realtime/runner-prp-ws.js"; -import { resolveLocalRuntimeApiUrl } from "../runtime-api.js"; +import { LocalRuntimeApiListenerClosedError, resolveLocalRuntimeApiUrl } from "../runtime-api.js"; import { createPaperclipHttpServer } from "./server.js"; import { privateHostnameGuard } from "../middleware/private-hostname-guard.js"; @@ -206,6 +206,30 @@ describe("Paperclip HTTP server runner transport", () => { } }); + it("distinguishes a listener closed for shutdown from one that never served", async () => { + expect(() => resolveLocalRuntimeApiUrl()).toThrow("local_runtime_api_listener_unavailable"); + expect(() => resolveLocalRuntimeApiUrl()).not.toThrow(LocalRuntimeApiListenerClosedError); + const server = createPaperclipHttpServer((_req, res) => res.end("closing"), { apiUrl: "http://ignored.example.test" }); + server.listen(0, "127.0.0.1"); + await once(server, "listening"); + const client = await fetch(resolveLocalRuntimeApiUrl(), { keepalive: true }); + // `close()` stops admission at once; its close event waits for open requests. + const closed = new Promise(resolve => server.close(() => resolve())); + expect(() => resolveLocalRuntimeApiUrl()).toThrow(LocalRuntimeApiListenerClosedError); + await client.text(); + server.closeAllConnections(); + await closed; + expect(() => resolveLocalRuntimeApiUrl()).toThrow(LocalRuntimeApiListenerClosedError); + expect(() => resolveLocalRuntimeApiUrl()).toThrow("local_runtime_api_listener_unavailable"); + server.listen(0, "127.0.0.1"); + await once(server, "listening"); + try { + expect(resolveLocalRuntimeApiUrl()).toMatch(/^http:\/\/127\.0\.0\.1:\d+$/); + } finally { + await new Promise(resolve => server.close(() => resolve())); + } + }); + it.each(["127.0.0.1", "::1", "::ffff:127.0.0.1"])("routes a guarded native read through the real %s listener", async host => { await guardedNativeRead(host); }); diff --git a/server/src/runtime-api.ts b/server/src/runtime-api.ts index 4c4f87868a6..b7d1a3d64f4 100644 --- a/server/src/runtime-api.ts +++ b/server/src/runtime-api.ts @@ -4,15 +4,37 @@ import { isIP } from "node:net"; const runtimeApiListeners = new Set(); const registeredRuntimeApiListeners = new WeakSet(); +let runtimeApiListenerClosed = false; + +/** + * The process had a runtime API listener and has since closed it, which only + * happens while it shuts down. Work that needs a callback origin must hand off + * to the next controller instead of failing as a provider or startup error. + */ +export class LocalRuntimeApiListenerClosedError extends Error { + constructor() { + // Keep the established failure code for classifiers and persisted rows. + super("local_runtime_api_listener_unavailable"); + this.name = "LocalRuntimeApiListenerClosedError"; + } +} /** Track listener ownership without treating configured or advertised URLs as authority. */ export function registerLocalRuntimeApiServer(server: Server): void { if (!registeredRuntimeApiListeners.has(server)) { registeredRuntimeApiListeners.add(server); - server.on("listening", () => runtimeApiListeners.add(server)); - server.on("close", () => runtimeApiListeners.delete(server)); + server.on("listening", () => { + runtimeApiListeners.add(server); + runtimeApiListenerClosed = false; + }); + server.on("close", () => { + if (runtimeApiListeners.delete(server)) runtimeApiListenerClosed = true; + }); + } + if (server.listening) { + runtimeApiListeners.add(server); + runtimeApiListenerClosed = false; } - if (server.listening) runtimeApiListeners.add(server); } function normalizeHost(value: string | null | undefined): string { @@ -94,6 +116,11 @@ export function chooseLocalRuntimeApiUrl(input: { export function resolveLocalRuntimeApiUrl(): string { const listeners = [...runtimeApiListeners].filter(server => server.listening); + // `server.close()` clears `listening` at once; its close event waits for + // admitted requests. Either state means this process stopped serving. + if (listeners.length === 0 && (runtimeApiListenerClosed || runtimeApiListeners.size > 0)) { + throw new LocalRuntimeApiListenerClosedError(); + } if (listeners.length !== 1) { throw new Error(listeners.length ? "local_runtime_api_listener_ambiguous" : "local_runtime_api_listener_unavailable"); } @@ -103,7 +130,10 @@ export function resolveLocalRuntimeApiUrl(): string { } export const runtimeApiInternals = { - resetForTests(): void { runtimeApiListeners.clear(); }, + resetForTests(): void { + runtimeApiListeners.clear(); + runtimeApiListenerClosed = false; + }, }; export function choosePrimaryRuntimeApiUrl(input: { diff --git a/server/src/services/native-runtime/native-recovered-owner-followup.test.ts b/server/src/services/native-runtime/native-recovered-owner-followup.test.ts index f12aad1871f..8185661619d 100644 --- a/server/src/services/native-runtime/native-recovered-owner-followup.test.ts +++ b/server/src/services/native-runtime/native-recovered-owner-followup.test.ts @@ -25,7 +25,7 @@ import { import { nativeRuntimeContextFixture } from "./runtime-context.test-fixture.js"; const toolDefinitions = vi.hoisted(() => vi.fn()); -vi.mock("../../runtime-api.js", () => ({ resolveLocalRuntimeApiUrl: () => "http://127.0.0.1:43219" })); +vi.mock("../../runtime-api.js", async importOriginal => ({ ...await importOriginal(), resolveLocalRuntimeApiUrl: () => "http://127.0.0.1:43219" })); vi.mock("./paperclip-runner-tool-authority.js", () => ({ PaperclipRunnerToolAuthority: class { readonly binding = {}; diff --git a/server/src/services/native-runtime/native-session-executor.test.ts b/server/src/services/native-runtime/native-session-executor.test.ts index c568d5350f2..dd9be079322 100644 --- a/server/src/services/native-runtime/native-session-executor.test.ts +++ b/server/src/services/native-runtime/native-session-executor.test.ts @@ -275,7 +275,7 @@ const state = vi.hoisted(() => ({ release: null as null | (() => void), })); -vi.mock("../../runtime-api.js", () => ({ resolveLocalRuntimeApiUrl: state.localRuntimeApiUrl })); +vi.mock("../../runtime-api.js", async importOriginal => ({ ...await importOriginal(), resolveLocalRuntimeApiUrl: state.localRuntimeApiUrl })); const accountingEvents = vi.hoisted(() => ({ committed: undefined as undefined | ((event: PrpEvent) => Promise), duplicate: undefined as undefined | ((event: PrpEvent) => Promise) })); vi.mock("./paperclip-control-plane-port.js", async importOriginal => { diff --git a/server/src/services/native-runtime/native-session-executor.ts b/server/src/services/native-runtime/native-session-executor.ts index c7fddc8388b..fd1216fe57c 100644 --- a/server/src/services/native-runtime/native-session-executor.ts +++ b/server/src/services/native-runtime/native-session-executor.ts @@ -5,7 +5,7 @@ import { logger } from "../../middleware/logger.js"; import { canRestoreMigratedNativeSession } from "./native-workspace-migration.js"; import { NATIVE_PROVIDER_DURABLE_EVENT_TYPES, nativeHarnessBackupManifestSha256, retiredSourceAuthorizesNativeBackupRestore } from "./native-retired-backup-source.js"; import { prepareCursorCloudRuntime } from "./cursor-cloud-runtime.js"; -import { resolveLocalRuntimeApiUrl } from "../../runtime-api.js"; +import { LocalRuntimeApiListenerClosedError, resolveLocalRuntimeApiUrl } from "../../runtime-api.js"; import { providerFailureFromError } from "../../vendor/paperclip-runner/index.js"; import { readProviderFailure } from "@paperclipai/shared"; import { nativeCancellationHasUnsettledOwner, notifyNativeCancellation, waitForNativeCancellation } from "./native-cancellation-delivery.js"; @@ -10676,7 +10676,14 @@ async function executePaperclipNativeSessionWithinScope( } else { error = identityRedactor.redact(error); } - if (nativeRunsDetachingForRestart.has(input.execution.binding.runId)) { + // A closed runtime API listener means this process is shutting down; it + // closes HTTP admission before it drains runs. For a local Runner, keep the + // claim for the drain to suspend: the next controller proves the Runner + // process identity and adopts the run without spending a failure or a + // same-run retry. A remote Runner has no process evidence on this host, so + // it keeps the bootstrap retry below. + if (nativeRunsDetachingForRestart.has(input.execution.binding.runId) + || (error instanceof LocalRuntimeApiListenerClosedError && input.runnerExecutionTarget?.kind !== "remote")) { await leaseRenewal.stop().catch(() => undefined); liveQuestions.close(); activeNativeSessions.delete(input.execution.binding.runId); diff --git a/tests/runner-e2e/workflow-target.test.ts b/tests/runner-e2e/workflow-target.test.ts index d02c01d647b..7f7aab50686 100644 --- a/tests/runner-e2e/workflow-target.test.ts +++ b/tests/runner-e2e/workflow-target.test.ts @@ -18,7 +18,8 @@ async function resolveTarget(event: string, cadence: string, branch = "master") encoding: "utf8", env: { ...process.env, EVENT_NAME: event, EVAL_CADENCE: cadence, IMAGE_ONLY: "false", TESTOPS_REQUESTED: "false", TARGET_BRANCH: branch, DEFAULT_BRANCH: "master", GITHUB_SHA: queuedSha, - MOCK_BRANCH_SHA: advancedSha, GITHUB_OUTPUT: output, REPOSITORY: "paperclipai/paperclip" }, + MOCK_BRANCH_SHA: advancedSha, GITHUB_OUTPUT: output, REPOSITORY: "paperclipai/paperclip", + IMAGE_ONLY: "false", TESTOPS_REQUESTED: "false" }, }); return { status: result.status, stderr: result.stderr, output: await readFile(output, "utf8").catch(() => "") };