From 1ee1275f11a27ae2babf4b6c03457a20c3ba5bde Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Fri, 31 Jul 2026 14:43:46 -0700 Subject: [PATCH] fix(adapters): persist ACPX process identity for hot restart (#9838) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source control plane people use to manage AI agents for work. > - Local agent heartbeats need durable process identity so the server can supervise them. > - The ACPX runtime owns the child process used by `codex_local` sessions. > - ACPX did not expose the child PID and start time to the Paperclip adapter. > - Warm ACPX runtimes can also serve a later heartbeat without a new spawn event. > - A hot restart could therefore classify a live Codex run as lost because its heartbeat row had no process identity. > - This pull request forwards ACPX spawn identity, reuses it for compatible warm heartbeats, and fails closed when identity cannot be persisted. > - The benefit is reliable hot-restart adoption for eligible local Codex runs. ## Linked Issues or Issue Description No matching public GitHub issue was found. **What happened?** A `codex_local` heartbeat could run through ACPX without a persisted `processPid` or `processStartedAt`. A Paperclip hot restart then had no durable identity for the live ACP child. Recovery could classify the run as `process_lost` even while the child was still alive. **Expected behavior** ACPX reports the real child PID and start time before the first prompt. A compatible warm runtime reports the same known identity to each later heartbeat that reuses the child. ACPX stops the child if the identity is invalid or persistence fails. Hot-restart recovery can then adopt the live run. **Steps to reproduce** 1. Start a `codex_local` heartbeat through the ACPX execution lane. 2. Keep the run active during a Paperclip hot restart. 3. Inspect the heartbeat row before this change. 4. Observe that the process identity can be null and recovery cannot adopt the live child. **Reproduced on** - Paperclip `master` before this change. - Linux source deployment. - `codex_local` with ACPX `0.12.0`. ## What Changed - Add an awaited `onAgentSpawn` lifecycle hook to the patched ACPX runtime. - Forward the ACP child PID and start time through the adapter `onSpawn` callback. - Keep a mutable callback sink for cached runtimes so a later respawn updates the current heartbeat. - Reuse the last known process identity when a compatible warm heartbeat reuses the existing child. - Kill the ACP child and fail session startup when the PID is invalid or identity persistence rejects. - Add ACPX and heartbeat recovery tests for callback ordering, warm reuse, failure cleanup, durable row identity, and hot-restart adoption. - Document the one-time drain required when an installed pre-fix run already lacks process metadata. ## Verification - `PAPERCLIP_HOME="$PAPERCLIP_RUN_SCRATCH_DIR/test-home-execute-escalated" pnpm exec vitest run packages/adapter-utils/src/acpx-engine/execute.test.ts` — 89 passed. - `PAPERCLIP_HOME="$PAPERCLIP_RUN_SCRATCH_DIR/test-home-recovery-escalated" pnpm exec vitest run server/src/__tests__/heartbeat-process-recovery.test.ts` — 92 passed. - `PAPERCLIP_HOME="$PAPERCLIP_RUN_SCRATCH_DIR/test-home-remote-smoke-escalated" pnpm exec vitest run packages/adapter-utils/src/acpx-engine/remote-spawn-smoke.test.ts` — 3 passed. - `PAPERCLIP_HOME="$PAPERCLIP_RUN_SCRATCH_DIR/test-home-ci-repro-escalated" pnpm exec vitest run server/src/__tests__/heartbeat-dependency-scheduling.test.ts` — 6 passed. - `pnpm --filter @paperclipai/adapter-utils typecheck` — passed. - Reverse and forward dry-run application of `patches/acpx@0.12.0.patch` — passed. - `git diff --check` — passed. - `git diff --exit-code origin/master...HEAD -- pnpm-lock.yaml` — passed. - `git diff --exit-code origin/master...HEAD -- .github/workflows` — passed. ## Risks - Runtime risk is low to moderate. ACPX now awaits process-identity persistence during child startup. - ACPX kills the child when persistence fails. This prevents an unsupervised process, but it makes that heartbeat fail visibly. - A compatible warm heartbeat reuses the identity of the existing ACP child. Regression tests verify that identity is persisted before the next prompt. - The change updates the vendored ACPX patch. Package installation must apply that patch. - There are no schema, migration, public API, UI, workflow, or lockfile changes. > 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 - OpenAI Codex used GPT-5.3-Codex for the earlier implementation. - OpenAI Codex used GPT-5 for the lifecycle-hook revision and the current fail-closed review fix. The runtime did not expose a more specific snapshot ID or context-window size. Both runs used reasoning, repository tools, and code 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 - [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/DEVELOPING.md | 39 ++++++ .../src/acpx-engine/execute.test.ts | 88 ++++++++++++++ .../adapter-utils/src/acpx-engine/execute.ts | 54 +++++++-- .../acpx-engine/remote-spawn-smoke.test.ts | 49 +++++++- patches/acpx@0.12.0.patch | 27 ++++- .../heartbeat-process-recovery.test.ts | 114 +++++++++++++++++- 6 files changed, 357 insertions(+), 14 deletions(-) diff --git a/doc/DEVELOPING.md b/doc/DEVELOPING.md index 0e0fddf80f..cd7b9de2c5 100644 --- a/doc/DEVELOPING.md +++ b/doc/DEVELOPING.md @@ -119,6 +119,45 @@ Use `--drain-required` only when the deploy intentionally requires the old termi A healthy guarded deploy must compare the report against `/api/health` (`version` or `serverVersion`) and treat any `lostRunIds` entry as a continuity failure that needs recovery before marking deployment complete. +### Recovering a deploy blocked by missing process metadata + +If the currently installed version already has a running local-agent heartbeat +whose `processPid` and `processGroupId` are both null, that pre-fix run cannot be +made adoptable retroactively. Cross that version boundary once with the normal +drain-and-retry path: + +```sh +old_main_pid="$(systemctl show paperclip.service -p MainPID --value)" +pnpm --filter @paperclipai/server exec tsx ../scripts/request-hot-restart.ts \ + --server-pid "$old_main_pid" --drain-required +systemctl restart paperclip.service +``` + +After the fixed server starts, wait for the replacement `codex_local` heartbeat +to spawn, then confirm its run record has an identity (use an authenticated API +request in authenticated mode): + +```sh +PAPERCLIP_API_BASE="${PAPERCLIP_API_URL:-http://127.0.0.1:3100}" +PAPERCLIP_API_BASE="${PAPERCLIP_API_BASE%/api}" +curl -fsS "$PAPERCLIP_API_BASE/api/heartbeat-runs/$RUN_ID" \ + | jq -e '.status == "running" and (.processPid != null or .processGroupId != null)' +``` + +The next continuity check should use the ordinary marker without +`--drain-required`. After restart, require both an empty loss list and an +explicit outcome for the run that was live before restart: + +```sh +jq -e --arg run "$RUN_ID" \ + '(.lostRunIds | length) == 0 and ((.adoptedRunIds + .finalizedWhileDownRunIds) | index($run) != null)' \ + "$PAPERCLIP_HOME/hot-restart-report.json" +``` + +An alive child appears in `adoptedRunIds`; a child that completed during the +restart window appears in `finalizedWhileDownRunIds`. Either is continuous. A +`lostRunIds` entry remains a failed deploy and must not be waived. + Tailscale/private-auth dev mode: ```sh diff --git a/packages/adapter-utils/src/acpx-engine/execute.test.ts b/packages/adapter-utils/src/acpx-engine/execute.test.ts index 6006e05e22..b2b7e64fe2 100644 --- a/packages/adapter-utils/src/acpx-engine/execute.test.ts +++ b/packages/adapter-utils/src/acpx-engine/execute.test.ts @@ -290,6 +290,94 @@ const ALLOWED_STARTUP_SPAN_ATTRIBUTE_KEYS = new Set([ ]); describe("shared ACPX engine runtime behavior", () => { + it("persists ACP agent process identity before prompting and reuses it for the next warm heartbeat", async () => { + const root = await makeTempRoot(); + const startedAt = "2026-07-30T07:00:00.000Z"; + const processPid = 43_210; + let runtimeCreateCount = 0; + let processIdentityPersisted = false; + let turnStartedBeforeProcessIdentity = false; + const warmHandles = new Map(); + const execute = createAcpxEngineExecutor({ + warmHandles, + createRuntime: (options) => { + runtimeCreateCount += 1; + const patchedOptions = options as AcpRuntimeOptions & { + onAgentSpawn?: (meta: { pid: number; startedAt: string }) => Promise; + }; + return { + ensureSession: async () => { + await patchedOptions.onAgentSpawn?.({ pid: processPid, startedAt }); + return { + backendSessionId: "backend-session", + agentSessionId: "agent-session", + runtimeSessionName: "runtime-session", + }; + }, + startTurn: () => { + turnStartedBeforeProcessIdentity = !processIdentityPersisted; + return { + events: (async function* () {})(), + result: Promise.resolve({ status: "completed" as const, stopReason: "end_turn" }), + cancel: async () => {}, + }; + }, + close: async () => {}, + } as never; + }, + }); + const config = { + agent: "codex", + cwd: root, + stateDir: path.join(root, "state"), + warmHandleIdleMs: 60_000, + }; + const context = { + taskId: "issue-1", + paperclipWorkspace: { cwd: root }, + }; + const firstOnSpawn = vi.fn(async (meta: unknown) => { + expect(meta).toEqual({ pid: processPid, processGroupId: null, startedAt }); + processIdentityPersisted = true; + }); + const first = await execute({ + runId: "run-process-identity-1", + agent: { id: "agent-1", companyId: "company-1" }, + runtime: {}, + config, + context, + onLog: async () => {}, + onMeta: async () => {}, + onSpawn: firstOnSpawn, + } as never); + + expect(first.exitCode).toBe(0); + expect(firstOnSpawn).toHaveBeenCalledOnce(); + expect(turnStartedBeforeProcessIdentity).toBe(false); + + processIdentityPersisted = false; + turnStartedBeforeProcessIdentity = false; + const secondOnSpawn = vi.fn(async (meta: unknown) => { + expect(meta).toEqual({ pid: processPid, processGroupId: null, startedAt }); + processIdentityPersisted = true; + }); + const second = await execute({ + runId: "run-process-identity-2", + agent: { id: "agent-1", companyId: "company-1" }, + runtime: { sessionParams: first.sessionParams }, + config, + context, + onLog: async () => {}, + onMeta: async () => {}, + onSpawn: secondOnSpawn, + } as never); + + expect(second.exitCode).toBe(0); + expect(runtimeCreateCount).toBe(1); + expect(secondOnSpawn).toHaveBeenCalledOnce(); + expect(turnStartedBeforeProcessIdentity).toBe(false); + }); + it("sets Codex model, effort, and fast mode through CODEX_CONFIG without session config calls", async () => { const { configOptions, meta } = await runExecutor({ agent: "codex", diff --git a/packages/adapter-utils/src/acpx-engine/execute.ts b/packages/adapter-utils/src/acpx-engine/execute.ts index 645c105834..f62972089a 100644 --- a/packages/adapter-utils/src/acpx-engine/execute.ts +++ b/packages/adapter-utils/src/acpx-engine/execute.ts @@ -130,12 +130,24 @@ function flushChildStderr(state: ChildStderrState) { state.pendingLiveLine = ""; } -type AcpxRuntimeFactory = (options: AcpRuntimeOptions) => AcpRuntime; +type AcpxAgentProcessIdentity = { pid: number; startedAt: string }; + +type PaperclipAcpRuntimeOptions = AcpRuntimeOptions & { + onAgentSpawn?: (meta: AcpxAgentProcessIdentity) => Promise; +}; + +type AcpxProcessIdentitySink = { + current: AdapterExecutionContext["onSpawn"]; + latest: AcpxAgentProcessIdentity | null; +}; + +type AcpxRuntimeFactory = (options: PaperclipAcpRuntimeOptions) => AcpRuntime; export interface RuntimeCacheEntry { runtime: AcpRuntime; handle: AcpRuntimeHandle; childStderrState: ChildStderrState; + processIdentitySink: AcpxProcessIdentitySink; fingerprint: string; lastUsedAt: number; cleanupTimer?: NodeJS.Timeout; @@ -2972,9 +2984,17 @@ export function createAcpxEngineExecutor(deps: AcpxEngineExecutorOptions = {}) { const resumeSessionId = canResume ? asString(previousParams.acpSessionId, "") || undefined : undefined; const cached = canResume ? warmHandles.get(prepared.sessionKey) : undefined; const childStderrState = cached?.childStderrState ?? { logPath: null, pendingLiveLine: "" }; + const processIdentitySink = cached?.processIdentitySink ?? { + current: ctx.onSpawn, + latest: null, + }; + // ACPX runtimes can stay warm across heartbeat runs. Keep the callback + // target mutable so a later agent respawn records identity on the current + // heartbeat instead of the run that originally created the runtime. + processIdentitySink.current = ctx.onSpawn; flushChildStderr(childStderrState); childStderrState.logPath = prepared.childStderrLogPath; - const runtimeOptions: AcpRuntimeOptions = { + const runtimeOptions: PaperclipAcpRuntimeOptions = { cwd: prepared.cwd, // Host-only spawn cwd for the relay proxy on the remote process-session // lane; `undefined` elsewhere so acpx falls back to `cwd` (byte-identical). @@ -2995,14 +3015,23 @@ export function createAcpxEngineExecutor(deps: AcpxEngineExecutorOptions = {}) { onAgentStderr: prepared.childStderrLogPath ? (chunk) => routeChildStderr(childStderrState, chunk) : undefined, + onAgentSpawn: async (meta) => { + processIdentitySink.latest = meta; + await processIdentitySink.current?.({ + pid: meta.pid, + processGroupId: null, + startedAt: meta.startedAt, + }); + }, }; // Open Q2: split the ~7s `acp.handshake` into the two in-repo-observable // sub-phases — the ACP runtime construction (`createRuntime`) vs the session - // establishment envelope (`ensureSession`). The finer spawn/`initialize`/ - // `session/new` split lives inside external `acpx` and is gated on an - // upstream lifecycle hook (not bundled here). `createRuntime` runs once and - // only on a cold start; a warm-handle hit reuses `cached.runtime`, so - // `createRuntimeMs` stays undefined and the split reports nothing for it. + // establishment envelope (`ensureSession`). The patched spawn lifecycle + // hook records process identity, but the finer spawn/`initialize`/ + // `session/new` timing split still lives inside external `acpx`. + // `createRuntime` runs once and only on a cold start; a warm-handle hit + // reuses `cached.runtime`, so `createRuntimeMs` stays undefined and the + // split reports nothing for it. let createRuntimeMs: number | undefined; let runtime: AcpRuntime; if (cached?.runtime) { @@ -3083,6 +3112,16 @@ export function createAcpxEngineExecutor(deps: AcpxEngineExecutorOptions = {}) { }); } } + // A compatible warm handle reuses the already-running ACP agent and does + // not emit another spawn event. Persist its known identity on this run + // before the next prompt starts so every running heartbeat is adoptable. + if (handle && cached && processIdentitySink.latest && ctx.onSpawn) { + await ctx.onSpawn({ + pid: processIdentitySink.latest.pid, + processGroupId: null, + startedAt: processIdentitySink.latest.startedAt, + }); + } } catch (err) { // Bring-up failed at the handshake — close the root span with error status. rootSpan.end(true); @@ -3303,6 +3342,7 @@ export function createAcpxEngineExecutor(deps: AcpxEngineExecutorOptions = {}) { runtime, handle: sessionHandle, childStderrState, + processIdentitySink, fingerprint: prepared.fingerprint, lastUsedAt: now(), }; diff --git a/packages/adapter-utils/src/acpx-engine/remote-spawn-smoke.test.ts b/packages/adapter-utils/src/acpx-engine/remote-spawn-smoke.test.ts index c8ddb04eca..efaa315b9e 100644 --- a/packages/adapter-utils/src/acpx-engine/remote-spawn-smoke.test.ts +++ b/packages/adapter-utils/src/acpx-engine/remote-spawn-smoke.test.ts @@ -35,6 +35,7 @@ const fixturePath = path.join(repoRoot, "scripts", "mcp-fixtures", "servers", "a const tempRoots: string[] = []; type PatchedAcpRuntimeOptions = AcpRuntimeOptions & { + onAgentSpawn?: (meta: { pid: number; startedAt: string }) => Promise; spawnCwd?: string; }; @@ -56,7 +57,11 @@ async function makeTempDir(prefix: string): Promise { * `spawnCwd`, when set, is the host-only spawn cwd the acpx patch consumes as * `spawnCwd ?? cwd`. */ -async function ensureRealAcpSession(input: { cwd: string; spawnCwd?: string }) { +async function ensureRealAcpSession(input: { + cwd: string; + spawnCwd?: string; + onAgentSpawn?: (meta: { pid: number; startedAt: string }) => Promise; +}) { const stateRoot = await makeTempDir("paperclip-acpx-remote-spawn-state-"); const stderrChunks: string[] = []; const agentCommand = `${JSON.stringify(process.execPath.replaceAll("\\", "/"))} ${JSON.stringify(fixturePath.replaceAll("\\", "/"))}`; @@ -70,6 +75,7 @@ async function ensureRealAcpSession(input: { cwd: string; spawnCwd?: string }) { permissionMode: "approve-all", nonInteractivePermissions: "deny", onAgentStderr: (chunk: string) => stderrChunks.push(chunk), + onAgentSpawn: input.onAgentSpawn, }; const runtime = createAcpRuntime(runtimeOptions); @@ -89,6 +95,23 @@ async function ensureRealAcpSession(input: { cwd: string; spawnCwd?: string }) { } } +function isProcessAlive(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch { + return false; + } +} + +async function waitForProcessExit(pid: number, timeoutMs = 2_000): Promise { + const deadline = Date.now() + timeoutMs; + while (isProcessAlive(pid)) { + if (Date.now() >= deadline) throw new Error(`Timed out waiting for ACP agent ${pid} to exit`); + await new Promise((resolve) => setTimeout(resolve, 20)); + } +} + it("reproduces host-spawn ENOENT when the advertised session cwd is host-nonexistent", async () => { // The in-sandbox `remoteCwd` that does not exist on the host. Intentionally // NOT created: this is what trips the acpx host `spawn()` `chdir`. @@ -124,3 +147,27 @@ it("spawnCwd redirects the host spawn to a host-valid dir while the advertised s // unchanged — still `remoteCwd`. expect(outcome.stderr).toContain(`SESSION_NEW_CWD=${remoteCwd}`); }); + +it("kills the ACP agent when process identity persistence rejects", async () => { + const hostCwd = await makeTempDir("paperclip-acpx-spawn-persistence-"); + let spawnedPid: number | null = null; + + const outcome = await ensureRealAcpSession({ + cwd: hostCwd, + onAgentSpawn: async ({ pid }) => { + spawnedPid = pid; + throw new Error("process identity persistence failed"); + }, + }); + + expect(outcome.resolved).toBe(false); + if (outcome.resolved) return; + expect(outcome.error.message).toContain("process identity persistence failed"); + expect(spawnedPid).not.toBeNull(); + + try { + await waitForProcessExit(spawnedPid!); + } finally { + if (isProcessAlive(spawnedPid!)) process.kill(spawnedPid!, "SIGKILL"); + } +}); diff --git a/patches/acpx@0.12.0.patch b/patches/acpx@0.12.0.patch index a01c1dc19f..8d5500a705 100644 --- a/patches/acpx@0.12.0.patch +++ b/patches/acpx@0.12.0.patch @@ -20,7 +20,22 @@ index 243c9d13bcba520923b63adfddad75cf2d94362d..336a1699b80b9e99416f9da4346ca73e current: state.current, quote: state.quote, escaping: true, -@@ -3960,6 +3960,10 @@ var AcpClient = class { +@@ -3959,7 +3959,24 @@ var AcpClient = class { +- this.attachAgentLifecycleObservers(child); ++ this.attachAgentLifecycleObservers(child); ++ if (this.options.onAgentSpawn) { ++ try { ++ if (typeof child.pid !== "number" || child.pid <= 0) { ++ throw new Error("ACPX agent spawn did not expose a valid process id."); ++ } ++ await this.options.onAgentSpawn({ pid: child.pid, startedAt: this.agentStartedAt }); ++ } catch (error) { ++ try { ++ child.kill("SIGKILL"); ++ } catch {} ++ throw error; ++ } ++ } const startupStderr = []; child.stderr.on("data", (chunk) => { this.captureStartupStderr(startupStderr, chunk); @@ -31,7 +46,7 @@ index 243c9d13bcba520923b63adfddad75cf2d94362d..336a1699b80b9e99416f9da4346ca73e if (!this.options.verbose) return; process.stderr.write(chunk); }); -@@ -3994,7 +3998,7 @@ var AcpClient = class { +@@ -3994,7 +4011,7 @@ var AcpClient = class { geminiAcp: isGeminiAcpCommand(spawnCommand, args), copilotAcp: isCopilotAcpCommand(spawnCommand, args), claudeAcp: isClaudeAcpCommand(spawnCommand, args), @@ -44,11 +59,12 @@ diff --git a/dist/runtime.d.ts b/dist/runtime.d.ts index ccdbe5b032521518022223733049b8b38793473b..3d4e04231e78efeecd8540735b08e1e43547ff2d 100644 --- a/dist/runtime.d.ts +++ b/dist/runtime.d.ts -@@ -266,6 +266,8 @@ type AcpRuntimeOptions = { +@@ -266,6 +266,9 @@ type AcpRuntimeOptions = { timeoutMs?: number; probeAgent?: string; verbose?: boolean; + onAgentStderr?: (chunk: string) => void; ++ onAgentSpawn?: (meta: { pid: number; startedAt: string }) => Promise; + spawnCwd?: string; onPermissionRequest?: (req: AcpPermissionRequest, ctx: { signal: AbortSignal; @@ -62,7 +78,7 @@ index 6c9cc999e50a11c399c68b3a0f1b7af4bc2317c0..33b5054b2906502d1d4b512bfa259bd2 } createClient(options) { - return this.deps.clientFactory?.(options) ?? new AcpClient(options); -+ const clientOptions = { ...options, onAgentStderr: this.options.onAgentStderr, spawnCwd: this.options.spawnCwd }; ++ const clientOptions = { ...options, onAgentStderr: this.options.onAgentStderr, onAgentSpawn: this.options.onAgentSpawn, spawnCwd: this.options.spawnCwd }; + return this.deps.clientFactory?.(clientOptions) ?? new AcpClient(clientOptions); } async readPendingPersistentClient(record, options) { @@ -71,11 +87,12 @@ diff --git a/dist/session-options-jkYbBxGE.d.ts b/dist/session-options-jkYbBxGE. index 9d37f377fb6a0828e0d2bc5a48754f3aa71509a4..680bc080fc5d6ffd266ed1b27d3d5056add9d980 100644 --- a/dist/session-options-jkYbBxGE.d.ts +++ b/dist/session-options-jkYbBxGE.d.ts -@@ -84,6 +84,8 @@ type AcpClientOptions = { +@@ -84,6 +84,9 @@ type AcpClientOptions = { terminal?: boolean; suppressSdkConsoleErrors?: boolean; verbose?: boolean; + onAgentStderr?: (chunk: string) => void; ++ onAgentSpawn?: (meta: { pid: number; startedAt: string }) => Promise; + spawnCwd?: string; sessionOptions?: { model?: string; diff --git a/server/src/__tests__/heartbeat-process-recovery.test.ts b/server/src/__tests__/heartbeat-process-recovery.test.ts index fd23f53a49..9a5e85558c 100644 --- a/server/src/__tests__/heartbeat-process-recovery.test.ts +++ b/server/src/__tests__/heartbeat-process-recovery.test.ts @@ -50,7 +50,7 @@ const mockTelemetryClient = vi.hoisted(() => ({ track: vi.fn() })); const mockTrackAgentFirstHeartbeat = vi.hoisted(() => vi.fn()); const mockTerminateLocalService = vi.hoisted(() => vi.fn()); const mockAdapterExecute = vi.hoisted(() => - vi.fn(async () => ({ + vi.fn(async (_input?: unknown) => ({ exitCode: 0, signal: null, timedOut: false, @@ -1489,6 +1489,118 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { }); }); + it("persists codex_local spawn identity before hot restart and never loses the live run for missing metadata", async () => { + let releaseAdapter: (() => void) | null = null; + let spawnedPid: number | null = null; + const adapterStarted = new Promise((resolve) => { + mockAdapterExecute.mockImplementationOnce(async (rawInput?: unknown) => { + const input = rawInput as { + onSpawn?: (meta: { pid: number; processGroupId: number | null; startedAt: string }) => Promise; + }; + const child = spawnAliveProcess(); + childProcesses.add(child); + if (!child.pid) throw new Error("Test codex_local child did not expose a pid"); + spawnedPid = child.pid; + await input.onSpawn?.({ + pid: child.pid, + processGroupId: null, + startedAt: new Date("2026-07-30T07:00:00.000Z").toISOString(), + }); + resolve(); + await new Promise((release) => { + releaseAdapter = release; + }); + return { + exitCode: 0, + signal: null, + timedOut: false, + errorMessage: null, + summary: "Codex run completed after hot restart adoption.", + provider: "test", + model: "test-model", + }; + }); + }); + const { runId } = await seedRunFixture({ + adapterType: "codex_local", + agentStatus: "idle", + runStatus: "queued", + processPid: null, + processGroupId: null, + includeIssue: false, + }); + const heartbeat = heartbeatService(db); + await heartbeat.resumeQueuedRuns(); + await Promise.race([ + adapterStarted, + new Promise((_, reject) => { + setTimeout(() => reject(new Error("Timed out waiting for codex_local spawn identity")), 3_000); + }), + ]); + + const running = await waitForValue(async () => + db + .select() + .from(heartbeatRuns) + .where(eq(heartbeatRuns.id, runId)) + .then((rows) => { + const row = rows[0] ?? null; + return row?.status === "running" && row.processPid ? row : null; + }), + ); + expect(running).toMatchObject({ + id: runId, + status: "running", + processPid: spawnedPid, + processGroupId: null, + processStartedAt: new Date("2026-07-30T07:00:00.000Z"), + }); + + await withTempPaperclipHome(async (home) => { + await writeHotRestartIntent({ + previousServerPid: process.pid, + previousServerVersion: "old-version", + requestedAt: new Date("2026-07-30T07:01:00.000Z"), + }); + await heartbeat.prepareHotRestartShutdown( + "SIGTERM", + new Date("2026-07-30T07:02:00.000Z"), + ); + + const adoption = await heartbeat.reconcileHotRestartAdoption( + new Date("2026-07-30T07:03:00.000Z"), + ); + expect(adoption).toMatchObject({ + mode: "reported", + adoptedRunIds: [runId], + finalizedWhileDownRunIds: [], + lostRunIds: [], + }); + const report = JSON.parse( + await fs.readFile(resolveHotRestartReportPath(home), "utf8"), + ) as { runs?: Array> }; + expect(report.runs).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + runId, + classification: "adopted", + reason: "process_pid_alive", + }), + ]), + ); + expect(report.runs).not.toEqual( + expect.arrayContaining([ + expect.objectContaining({ runId, reason: "missing_process_metadata" }), + ]), + ); + }); + + if (!releaseAdapter) throw new Error("Adapter release handle was not captured"); + releaseAdapter(); + const settled = await waitForRunToSettle(heartbeat, runId, 5_000); + expect(settled?.status).toBe("succeeded"); + }); + it("reports adopted hot-restart runs before startup reap can mark them process_lost", async () => { const child = spawnAliveProcess(); childProcesses.add(child);