From 8c0dbffa704b4d0cb4fd7acee7be906ea11692ea Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Tue, 10 Nov 2026 20:54:53 -0600 Subject: [PATCH] fix(heartbeat): retry a legacy run whose lease this process lost before dispatch (#20694) 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 heartbeat orphan reaper settles legacy (non-native) runs that lost their controller lease > - #16355 taught the reaper to retry a legacy run that a restart stopped while setup was still `preparing`, because revocation fences the old controller and `preparing` proves no adapter dispatch > - That proof was applied only when the lapsed lease belonged to a different boot > - A stall can make the current process lose its own lease while setup is still `preparing`, and its executor can end without settling the row > - The reaper then wrote "retrying once" but recorded no bootstrap evidence, so the retry was refused and the task was blocked for reconciliation > - This pull request applies the same revocation proof to a lease from the current boot > - The benefit is that these tasks recover by themselves instead of waiting for a person to press Retry ## Linked Issues or Issue Description Refs #16355 **What happened?** On a hosted instance, one server process stalled for about 100 seconds (lease renewals were 42–46 s late). Several legacy `paperclip_runner` runs on a remote sandbox were still in setup (`executionStage: "preparing"`, no process id). Their controller leases expired. Some executors settled their own runs as `controller_lease_lost`. Two runs had no executor left, so the orphan reaper of the same process settled them 4 minutes later. Each run got `error: "Process lost -- server may have restarted; retrying once"` and `processLossDiagnostic.retryEligible: true`, but no `executionRecovery` evidence. `enqueueProcessLossRetry` then saw a run that needs reconciliation and scheduled nothing. The next sweep recorded `legacy_execution_requires_reconciliation` with `preserve_without_replay_v1`, and both tasks stayed `blocked` until a person pressed Retry. **Expected behavior** Revocation commits the stage. Renewal requires the old boot id, so no controller can move a revoked run to `dispatching`. A revoked run that is still `preparing` with no process identity did no provider work, whichever boot owned the lease. The reaper should record the bootstrap evidence and use the bounded retry. **Steps to reproduce** 1. Claim a legacy run in this process: `controllerBootId` is the current boot, `executionStage: "preparing"`, no process id. 2. Let its controller lease expire with no local executor (for example after an event-loop or database stall). 3. Run the orphan reaper. 4. Before this change: `process_lost; retrying once`, no `executionRecovery`, no retry, and a reconciliation hold. After this change: bootstrap evidence and a `scheduled_retry` successor. ## What Changed - `server/src/services/heartbeat/recovery.ts`: `restartInterruptedBeforeDispatch` no longer requires the lease to come from another boot. The other clauses stay: legacy runtime, a stamped controller boot id, and `preparing` with no process id or group in the state committed by the revocation. Locally tracked runs and runs with a live lease are still skipped before this point. - `server/src/__tests__/heartbeat-process-recovery.test.ts`: two cases in the hosted shape (assignment wake and due-monitor wake, `paperclip_runner`, prepared remote workspace, same-boot expired lease, `preparing`). They expect bootstrap evidence, a scheduled retry, no recovery hold after a second sweep, and no adapter call. A control case keeps a same-boot `dispatching` run held for reconciliation. ## Verification - Red on `origin/master` (source change reverted): both new cases fail. The monitor case shows the exact hosted shape: `retryEligible: true` with no `executionRecovery`. - Green with the change: `pnpm --filter @paperclipai/server exec vitest run src/__tests__/heartbeat-process-recovery.test.ts -t "lease this process lost|this process dispatched held|restart stopped before adapter dispatch|reached adapter dispatch held|lost monitor|older build without a controller stamp|failure budget"` (10 passed). - Related files, run together: `heartbeat-process-recovery`, `live-recovery-work`, `interrupted-preparation-hold-settle`, `resource-wait-stop`, `legacy-controller-lease`, `execution-recovery-dead-controller`, `legacy-process-settlement`, `run-failure-report` (876 passed; 2 timing failures under load in unrelated setup-failure tests, both pass when run alone). - `pnpm --filter @paperclipai/server exec tsc --noEmit` passes. ## Risks - Low risk. The proof is the one #16355 already relies on across boots: the revocation compare-and-set replaces the boot id, so the in-memory controller of this process also fails its next renewal and cannot dispatch. - A same-boot run reaches this code only when it is not in `activeRunExecutions` or `runningProcesses` and its lease expired. If an executor is stalled before it registers, it aborts at its first renewal. Setup work can run again in the retry; provider work cannot. - Upgrade safety: no persisted shape changes. The reaper reads the same fields (`executionStage`, `controllerBootId`, process ids) that older releases write. Runs that older code already parked keep their hold and their working Retry. - Runs that reached `dispatching`, recorded a process, or have no controller stamp keep the current behavior. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), used through Claude Code with tool use, shell, 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 (the code comment carries the contract) - [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 --- .../heartbeat-process-recovery.test.ts | 64 +++++++++++++++++++ server/src/services/heartbeat/recovery.ts | 5 +- 2 files changed, 66 insertions(+), 3 deletions(-) diff --git a/server/src/__tests__/heartbeat-process-recovery.test.ts b/server/src/__tests__/heartbeat-process-recovery.test.ts index 74a0803bf67..559d9890075 100644 --- a/server/src/__tests__/heartbeat-process-recovery.test.ts +++ b/server/src/__tests__/heartbeat-process-recovery.test.ts @@ -3557,6 +3557,70 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { expect(mockAdapterExecute).not.toHaveBeenCalled(); }); + it.each([ + ["an assignment", {}], + ["a due monitor", { wakeReason: "issue_monitor_due" }], + ] as const)("retries %s run whose lease this process lost before adapter dispatch", async (_label, contextSnapshot) => { + const { companyId, runId, issueId } = await seedRunFixture({ + adapterType: "paperclip_runner", agentStatus: "idle", runtimeMode: "legacy", contextSnapshot, + }); + await db.update(heartbeatRuns).set({ + controllerBootId: controllerLeases.legacyControllerBootId, + controllerProcess: currentControllerProcessStamp(), + controllerLeaseExpiresAt: sql`clock_timestamp() - interval '1 second'`, + executionStage: "preparing", + runnerProfileJson: { + adapterDispatch: { adapterType: "paperclip_runner" }, + remoteWorkspace: { schema: "paperclip.remote-workspace-run.v1", epoch: 6, state: "prepared", + leaseId: randomUUID(), providerLeaseId: randomUUID(), workspaceId: randomUUID(), + remoteCwd: "/home/daytona/paperclip-workspace", snapshotId: randomUUID() }, + }, + }).where(eq(heartbeatRuns.id, runId)); + + const heartbeat = heartbeatService(db); + expect(await heartbeat.reapOrphanedRuns()).toEqual({ reaped: 1, runIds: [runId] }); + + const [reaped] = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, runId)); + expect(reaped).toMatchObject({ status: "failed", errorCode: "process_lost", + error: "Process lost -- server may have restarted; retrying once" }); + expect(reaped?.resultJson).toMatchObject({ + executionRecovery: { kind: "bootstrap", providerWorkStarted: false }, + processLossDiagnostic: { localCheck: "no_identifiers", retryEligible: true }, + }); + expect(await db.select().from(heartbeatRuns).where(and( + eq(heartbeatRuns.companyId, companyId), eq(heartbeatRuns.retryOfRunId, runId), + ))).toEqual([expect.objectContaining({ status: "scheduled_retry" })]); + await heartbeat.reapOrphanedRuns(); + expect(await db.select().from(issueRecoveryActions) + .where(eq(issueRecoveryActions.sourceIssueId, issueId))).toEqual([]); + expect(mockAdapterExecute).not.toHaveBeenCalled(); + }); + + it("keeps a run this process dispatched held for reconciliation after its lease lapses", async () => { + const { companyId, runId, issueId } = await seedRunFixture({ + adapterType: "paperclip_runner", agentStatus: "idle", runtimeMode: "legacy", + }); + await db.update(heartbeatRuns).set({ + controllerBootId: controllerLeases.legacyControllerBootId, + controllerProcess: currentControllerProcessStamp(), + controllerLeaseExpiresAt: sql`clock_timestamp() - interval '1 second'`, + executionStage: "dispatching", + }).where(eq(heartbeatRuns.id, runId)); + + await heartbeatService(db).reapOrphanedRuns(); + + const [reaped] = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, runId)); + expect(reaped).toMatchObject({ status: "failed", errorCode: "process_lost" }); + expect(reaped?.resultJson).not.toHaveProperty("executionRecovery"); + expect(await db.select().from(heartbeatRuns).where(and( + eq(heartbeatRuns.companyId, companyId), eq(heartbeatRuns.retryOfRunId, runId), + ))).toEqual([]); + expect(await db.select().from(issueRecoveryActions) + .where(eq(issueRecoveryActions.sourceIssueId, issueId))).toEqual([ + expect.objectContaining({ cause: "legacy_execution_requires_reconciliation" }), + ]); + }); + it("keeps a lost preparation from an older build without a controller stamp in the charged lane", async () => { const { companyId, runId } = await seedRestartOrphanedLegacySetup("preparing"); await db.update(heartbeatRuns).set({ runnerProfileJson: { adapterDispatch: { adapterType: "paperclip_runner" } } }) diff --git a/server/src/services/heartbeat/recovery.ts b/server/src/services/heartbeat/recovery.ts index b6482a6bc92..f6ea8aa8dae 100644 --- a/server/src/services/heartbeat/recovery.ts +++ b/server/src/services/heartbeat/recovery.ts @@ -2660,8 +2660,8 @@ export function createHeartbeatRecovery(db: Db, dependencies: HeartbeatRecoveryD continue; } // A legacy controller persists "dispatching" in the same lease renewal - // that admits the adapter. An expired lease from another boot that is - // still "preparing" proves a restart stopped setup before provider work. + // that admits the adapter. An expired lease that is still "preparing" + // proves setup stopped before provider work. // Use the state committed with the revocation, not the earlier snapshot: // the old controller may have dispatched before its lease expired. // The revocation matched the snapshot's boot ID before replacing it. @@ -2669,7 +2669,6 @@ export function createHeartbeatRecovery(db: Db, dependencies: HeartbeatRecoveryD run.runtimeMode === "legacy" && revokedState.executionStage === "preparing" && !!run.controllerBootId && - run.controllerBootId !== legacyControllerBootId && !revokedState.processPid && !revokedState.processGroupId; const shouldRetry =