From e5abcbf7a9e58f2380b9808a84131369d3cd9c1d Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Tue, 10 Nov 2026 09:55:21 -0600 Subject: [PATCH] fix(native): release a restart claim that lapsed during a failed preparation (#20529) 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 > - After a restart, native restart recovery claims an unfinished run and prepares it again on the same heartbeat run. > - The restart claim holds a lease on the run's native coordinator row. For a remote claim with durable authority, the lease is 60 s. > - When that preparation fails, the failure path first preserves the remote workspace. Then it settles the run. > - When preparation outlived the claim lease, preservation took the run's own lapsed claim for another owner. It threw `NativeWorkspaceFinalizationBusyError`, and the execution returned without settling. The run stayed `running` with no event until a later scan reclaimed it (4.5 min in one wave, never in another). > - This pull request releases exactly that lapsed claim and asks restart recovery to decide on the run right away. It also logs the cause when the failure path defers. > - The benefit is that a failed restart-claim preparation is reclaimed or blocked visibly within seconds, instead of hanging. ## Linked Issues or Issue Description Refs #20514 (the same restart-chaos investigation). **What happened?** In restart-chaos wave `w-20261110T101517-s11936` (Daytona R1, K2, ST1), run `9d39550a` got a `bootstrap_incomplete` restart claim (generation 3). Its `heartbeat.prepare.run_output` step waited 141 s through an ST1 storage outage. The 60 s claim lease lapsed meanwhile, and the begin failed with `run_log_writer_authority_lost` when storage came back. The inner failure path then called `preserveNativeFailedWorkspace`. Its `ownsClaim` check required an unexpired lease, so the run's own claim counted as another owner and the call threw `NativeWorkspaceFinalizationBusyError`. `preserveNativeWorkspaceOnFailure` set `nativeOwnershipHeld` and the execution returned before `runCompletion.failRun`. No failure was recorded, and nothing was logged. The run stayed `running` until a later scan reclaimed it 4.5 minutes later. In a second wave no such scan came, and the run stayed `running` for 30 minutes. **Expected behavior** A failed preparation must settle or retry, or be claimed promptly by its next owner. It must not stay `running` with no record. **Steps to reproduce** 1. Claim a remote native run for restart recovery. 2. Keep its preparation busy past the 60 s claim lease, for example with a storage outage during `runOutput.begin`. 3. Let preparation fail. The run stays `running` with no failure event. **Paperclip version or commit** `master` at 23fa37f5bb. ## What Changed - `native-execution-lease.ts`: new `releaseLapsedNativeRestartClaim`. It is one conditional UPDATE that releases the claim only when the run is `running`, the coordinator is `observed` with no result, and it still names this exact `leaseOwner`, `controllerGeneration` and `attempt` with an expired lease. A successor claim, a result, or a live lease leaves the row unchanged. - `heartbeat.ts`: when failed-workspace preservation defers on a restart-claim execution, the failure path tries that release. If it succeeds, the execution schedules a restart-recovery pass for the run (`scheduleNativeSessionResumeDispatch(..., "restart")`) after it leaves `activeRunExecutions`. Recovery then reclaims the run, or blocks it with a named reason, right away. The path also logs the redacted cause, the claim kind and whether the claim lapsed. Before, this return was silent. - `preserveNativeFailedWorkspace` is unchanged. It still refuses an expired claim (`refuses a stale expired recovery claim`). - New `heartbeat-restart-claim-lapse.test.ts`. A restart-claim execution whose preparation fails after its claim lease lapsed, on a run with a prepared remote workspace, gets an immediate new restart-recovery decision. It failed before the change: the run stayed `running` with the lapsed claim and recovery answered `controller_still_alive`. Not in this PR: renewing the claim lease during preparation for `bootstrap_incomplete` claims (recovery.ts). Another restart-chaos fix owns that change. This PR makes sure a lapsed claim's failure is handed back at once either way. ## Verification - `cd server && npx vitest run src/__tests__/heartbeat-restart-claim-lapse.test.ts`: passed. Without the heartbeat change it times out. - `cd server && npx vitest run src/services/native-runtime/native-failed-workspace-recovery.test.ts src/__tests__/heartbeat-native-restart-suspension.test.ts src/services/native-runtime/native-execution-lease.test.ts`: passed (195 tests across the four files). - `cd server && pnpm typecheck`: no errors. - Live Daytona proof uses `--native-trigger` from #20538. Results follow in a comment. ## Risks - Low risk. The release matches only the exact lapsed claim of a running, result-less, `observed` run in one statement. A takeover between the check and the release changes the owner or generation, so the release does nothing. - The run is not settled by this execution. Restart recovery makes the decision under its own rules, which is the same path a later sweep would take. - Previous-release story: no persisted shape or identity changes. Rows saved by the previous release are read and released the same way. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use and code execution in Claude Code. ## 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 --- .../heartbeat-restart-claim-lapse.test.ts | 98 +++++++++++++++++++ server/src/services/heartbeat.ts | 22 ++++- .../native-runtime/native-execution-lease.ts | 17 ++++ 3 files changed, 135 insertions(+), 2 deletions(-) create mode 100644 server/src/__tests__/heartbeat-restart-claim-lapse.test.ts diff --git a/server/src/__tests__/heartbeat-restart-claim-lapse.test.ts b/server/src/__tests__/heartbeat-restart-claim-lapse.test.ts new file mode 100644 index 00000000000..0e8058421c5 --- /dev/null +++ b/server/src/__tests__/heartbeat-restart-claim-lapse.test.ts @@ -0,0 +1,98 @@ +import { randomUUID } from "node:crypto"; +import { and, eq, sql } from "drizzle-orm"; +import { expect, it, vi } from "vitest"; +import { authUsers, companyMemberships, heartbeatRunEvents, heartbeatRuns, nativeRunFinalizations } from "@paperclipai/db"; +import { heartbeatService } from "../services/heartbeat.js"; +import { releaseLapsedNativeRestartClaim } from "../services/native-runtime/native-execution-lease.js"; +import { instanceSettingsService } from "../services/instance-settings.js"; +import { describeEmbeddedPostgres, nativeRestartFixture } from "./helpers/native-restart.js"; +import { waitForCondition } from "./helpers/native-restart-fixture.js"; + +const preparationFault = vi.hoisted(() => ({ + current: null as null | { runId: string; phase: string; act: () => Promise }, +})); +vi.mock("../services/heartbeat/preparation-timing.js", async (importOriginal) => { + const actual = await importOriginal(); + return { + createHeartbeatPreparationTiming: (runId: string) => { + const timing = actual.createHeartbeatPreparationTiming(runId); + return (async (phase, work) => { + const fault = preparationFault.current; + if (fault?.runId === runId && fault.phase === phase) { + preparationFault.current = null; + await fault.act(); + } + return timing(phase, work); + }) as typeof timing; + }, + }; +}); + +describeEmbeddedPostgres("restart-claim preparation that outlives its claim lease", () => { + const fixture = nativeRestartFixture(); + + it.each(["live lease", "another owner", "newer generation", "other attempt", "run not running", "lapsed own claim"] as const)( + "releases a restart claim only when it is this exact lapsed claim (%s)", async (variant) => { + const f = await fixture.seedRun(`RELEASE-${variant.replace(/\W+/g, "-")}`); + const leaseOwner = `native-recovery:${randomUUID()}`; + await f.db.update(nativeRunFinalizations).set({ phase: "observed", leaseOwner, controllerGeneration: 3, attempt: 1, + leaseExpiresAt: new Date(Date.now() + (variant === "live lease" ? 60_000 : -1_000)) }) + .where(eq(nativeRunFinalizations.runId, f.runId)); + if (variant === "run not running") await f.db.update(heartbeatRuns).set({ status: "failed" }).where(eq(heartbeatRuns.id, f.runId)); + const released = await releaseLapsedNativeRestartClaim({ db: f.db, companyId: fixture.current.companyId, runId: f.runId, + leaseOwner: variant === "another owner" ? `native-recovery:${randomUUID()}` : leaseOwner, + controllerGeneration: variant === "newer generation" ? 2 : 3, providerAttempt: variant === "other attempt" ? 2 : 1 }); + const [owner] = await f.db.select().from(nativeRunFinalizations).where(eq(nativeRunFinalizations.runId, f.runId)); + if (variant === "lapsed own claim") { + expect(released).toBe(true); + expect(owner).toMatchObject({ leaseOwner: null, leaseExpiresAt: null, phase: "observed", controllerGeneration: 3 }); + return; + } + expect(released).toBe(false); + expect(owner).toMatchObject({ leaseOwner, phase: "observed", controllerGeneration: 3, attempt: 1 }); + }, 60_000); + + it("hands the run straight back to restart recovery instead of leaving it running unowned", async () => { + const f = await fixture.seedRun("LAPSED-RESTART-CLAIM"); + await instanceSettingsService(f.db).updateExperimental({ enableNativeRunner: true }); + const userId = randomUUID(); + await f.db.insert(authUsers).values({ id: userId, name: "Operator", email: `${userId}@example.invalid`, + createdAt: new Date(), updatedAt: new Date() }); + await f.db.insert(companyMemberships).values({ companyId: fixture.current.companyId, principalType: "user", + principalId: userId, membershipRole: "owner" }); + await f.db.update(heartbeatRuns).set({ responsibleUserId: userId, completionContractId: null, + completionContractSha256: null }).where(eq(heartbeatRuns.id, f.runId)); + const transitions = async () => (await f.db.select({ id: heartbeatRunEvents.id }).from(heartbeatRunEvents) + .where(and(eq(heartbeatRunEvents.runId, f.runId), eq(heartbeatRunEvents.eventType, "native.recovery.transition")))).length; + + preparationFault.current = { runId: f.runId, phase: "run_output", act: async () => { + await f.db.update(heartbeatRuns).set({ runnerProfileJson: sql`coalesce(${heartbeatRuns.runnerProfileJson}, '{}'::jsonb) + || jsonb_build_object('nativeWorkspaceSync', jsonb_build_object('schema', 'paperclip.native-workspace-sync/v3', + 'state', 'prepared', 'descriptorSha256', ${"d".repeat(64)}::text, 'baselineSha256', ${"b".repeat(64)}::text, + 'finalHostSha256', null, 'workspaceId', ${randomUUID()}::text, 'leaseId', ${randomUUID()}::text, + 'providerLeaseId', ${randomUUID()}::text, 'remoteCwd', '/work', 'resourceDisposition', 'destroy'))` }) + .where(eq(heartbeatRuns.id, f.runId)); + await f.db.update(nativeRunFinalizations).set({ leaseExpiresAt: new Date(Date.now() - 1_000) }) + .where(eq(nativeRunFinalizations.runId, f.runId)); + throw new Error("Object storage is unavailable"); + } }; + const heartbeat = heartbeatService(f.db, { + nativeSessionBackendFactory: vi.fn(() => { throw new Error("A lapsed recovery must not open a provider session"); }), + }); + let before = 0; + try { + const { claims } = await heartbeat.recoverNativeRunsAfterRestart(new Date(), [f.runId]); + expect(claims).toEqual([expect.objectContaining({ runId: f.runId })]); + before = await transitions(); + await waitForCondition("restart recovery reclaims or blocks the lapsed run", async () => { + const [owner] = await f.db.select().from(nativeRunFinalizations).where(eq(nativeRunFinalizations.runId, f.runId)); + return await transitions() > before && (owner.controllerGeneration !== claims[0]!.controllerGeneration + || owner.phase !== "observed" || owner.recoveryState === "blocked"); + }, 15_000); + } finally { + preparationFault.current = null; + await heartbeat.drainActiveRunExecutions(); + } + expect(await transitions()).toBeGreaterThan(before); + }, 90_000); +}); diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 9c4365abde7..16b9897657d 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -53,7 +53,7 @@ import { prepareHeartbeatTaskContext } from "./heartbeat/task-context.js"; import { selectHeartbeatEnvironment } from "./heartbeat/environment-selection.js"; import { reconcileMuseNativeTerminalRun } from "./native-runtime/muse-terminal-reconciliation.js"; import { prepareHeartbeatWorkspace } from "./heartbeat/workspace-preparation.js"; -import { nativeRecoveryClaimRelinquished } from "./native-runtime/native-execution-lease.js"; +import { nativeRecoveryClaimRelinquished, releaseLapsedNativeRestartClaim } from "./native-runtime/native-execution-lease.js"; import { executeHeartbeatRuntime, NativeSessionResumeScheduledError, NativeWorkspaceFinalizeScheduledError } from "./heartbeat/runtime-execution.js"; import { preserveNativeFailedWorkspace } from "./native-runtime/native-failed-workspace-recovery.js"; import { nativeRunHasNoWorkspaceAccess } from "./workspace-restore-recovery-state.js"; @@ -2026,6 +2026,7 @@ export function heartbeatService( let githubLauncherLocation: Parameters[0] | null = null; let nativeSessionResumeScheduled = false; + let restartClaimLapsed = false; const shutdownHandedOff = watchShutdownHandoff(run.id); let nativeOwnershipHeld = false; let nativeStartupCleanupScheduled = false; @@ -3813,7 +3814,23 @@ export function heartbeatService( return; } if (await leaveToNextController(err)) return; - if (!(await preserveNativeWorkspaceOnFailure())) return; + if (!(await preserveNativeWorkspaceOnFailure())) { + const claim = runOptions.nativeRestartRecovery; + let releaseFailed = false; + restartClaimLapsed = claim !== undefined && await releaseLapsedNativeRestartClaim({ db, companyId: run.companyId, runId: run.id, + leaseOwner: claim.leaseOwner, controllerGeneration: claim.controllerGeneration, providerAttempt: claim.providerAttempt }) + .catch(releaseError => { + logger.warn({ err: releaseError, runId: run.id }, "lapsed native restart claim could not be released; restart recovery will reclaim it after expiry"); + releaseFailed = true; + return false; + }); + logger.warn({ runId: run.id, err: identityRedactor.redact({ message: err instanceof Error ? err.message : String(err), + stack: err instanceof Error ? err.stack : undefined }), restartClaim: claim?.kind ?? null, restartClaimLapsed }, + restartClaimLapsed ? "native restart claim lapsed during a failed preparation; released it for restart recovery" + : releaseFailed ? "native run failure left to restart recovery after its claim release failed" + : "native run failure left to the workspace owner that holds the run"); + return; + } await runCompletion.failRun({ run, agent, issueId, issueRef, signal: executionControl.controller.signal, output: runOutput, @@ -4129,6 +4146,7 @@ export function heartbeatService( } finally { controllerLease.stop(); activeRunExecutions.delete(run.id); + if (restartClaimLapsed) scheduleNativeSessionResumeDispatch(run.id, new Date(), "restart"); // A failed owned Stop remains visible until this exact executor settles, // including a graceful exit result arriving after the cancellation error. // It is never retained beyond the active execution's cleanup. diff --git a/server/src/services/native-runtime/native-execution-lease.ts b/server/src/services/native-runtime/native-execution-lease.ts index 851c608574a..015d6b822c8 100644 --- a/server/src/services/native-runtime/native-execution-lease.ts +++ b/server/src/services/native-runtime/native-execution-lease.ts @@ -145,6 +145,23 @@ export async function nativeRecoveryClaimRelinquished(input: { && row.lastDisposition === "restart_suspended"; } +export async function releaseLapsedNativeRestartClaim(input: { + db: Db; companyId: string; runId: string; leaseOwner: string; controllerGeneration: number; providerAttempt: number; +}): Promise { + const released = await input.db.update(nativeRunFinalizations) + .set({ leaseOwner: null, leaseExpiresAt: null, updatedAt: new Date() }) + .where(and(eq(nativeRunFinalizations.companyId, input.companyId), eq(nativeRunFinalizations.runId, input.runId), + eq(nativeRunFinalizations.phase, "observed"), isNull(nativeRunFinalizations.resultId), + eq(nativeRunFinalizations.leaseOwner, input.leaseOwner), + eq(nativeRunFinalizations.controllerGeneration, input.controllerGeneration), + eq(nativeRunFinalizations.attempt, input.providerAttempt), + sql`${nativeRunFinalizations.leaseExpiresAt} <= clock_timestamp()`, + sql`exists (select 1 from ${heartbeatRuns} where ${heartbeatRuns.id} = ${nativeRunFinalizations.runId} + and ${heartbeatRuns.companyId} = ${input.companyId} and ${heartbeatRuns.status} = 'running')`)) + .returning({ runId: nativeRunFinalizations.runId }); + return released.length > 0; +} + /** * Preparation failed after a fresh claim and before any provider turn. Release * only this exact claim, so ordinary failure settlement can retain the source