From 95f3cab0cdad8ca78ecc11a81a5fc7f2268d9e25 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Sun, 8 Nov 2026 00:17:02 -0600 Subject: [PATCH] fix(recovery): settle a workspace finalize attempt whose process died mid-copyback (#19985) 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 durable-ownership plan settles in-progress records whose owner is proven gone, on one shared path. > - The crash-point suite (#19946) found two windows where a workspace finalize attempt stays `running` forever after a restart, even though its run succeeded through a later attempt. > - #19807's supersession covers only native-finalizer attempts. The local live-finalize path writes rows without that marker. > - This pull request stamps each finalize attempt's owner and settles an abandoned attempt on the shared path. > - The benefit is that no finalize row is left `running` with a dead owner, and the run's outcome never changes. ## Linked Issues or Issue Description Refs #19946 (crash-point suite), #19807 (RC-64 supersession), #19933 (shared settle path). **What happened?** At crash points `workspace.finalize.after_start_before_copyback` and `workspace.finalize.after_copyback_before_done` (run `cp-20261108T040253`, tasks t07 and t08), the run succeeded after the restart. The original `workspace_finalize` operation stayed `running`, and the ownership pass reported I2 (`owner_run_terminal`). **What did you expect?** The interrupted attempt is settled once its owner is proven gone, and the run outcome is left alone. **Steps to reproduce** 1. `node scripts/crash-points.mjs --point workspace.finalize.after_start_before_copyback`. Or run the new report test case below. ## What Changed - `workspace-operations.ts`: every `workspace_finalize` attempt records `finalizeOwner` (the process stamp, with no I/O) and is tracked in a process-local set while it runs (`isWorkspaceFinalizeOperationLive`). - `ownership-registry.ts`: `native_workspace_finalize_attempt` takes its owner from the stamp, or from the run for rows written before the stamp. It now registers a settle. - New `recovery/workspace-finalize-attempt-settlement.ts`: a compare-and-swap that marks the attempt `failed` with `abandonedFinalization` provenance and logs a `workspace_operation.reconciled` activity. It skips an attempt live in this process, an uncertain export and a save-work record. It does not change the run. ## Verification - `cd server && npx vitest run src/__tests__/ownership-reconciliation-report.test.ts`: 19 pass. The new case covers a dead stamped owner (settled), a current-process owner (kept), another PID space (kept), an uncertain export (kept), a pre-stamp row of a terminal run (settled) and of a running run (kept), a read-only pass (no change), and the run status unchanged. - The finalize-related suites pass (100 tests in 5 files): `heartbeat-workspace-finalize-branch`, `native-workspace-finalization-work`, `workspace-operations-reconciliation`, `offline-workspace-operation-reconciliation`, `finalized-workspace-recovery-migration`. - Red/green: the new case fails on master, where the attempt stays `running`. I wrote the test after the code. - Crash-point re-proof with `--point` for both windows: to be run once the harness slot is free, and reported on this PR. ## Risks - Fail-open risk: settle needs a `gone` proof from the shared prover, and another PID space is never proof. It re-checks the stamp in the transaction and skips attempts this process is running. - Previous-release story: rows without `finalizeOwner` are owned by their run, and they settle only when that run is terminal. - Nothing runs before `turn.start`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, in Claude Code with tool use (shell, file edits). ## 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 - [ ] 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 --- .../ownership-reconciliation-report.test.ts | 51 ++++++++++- .../services/recovery/ownership-registry.ts | 16 +++- .../workspace-finalize-attempt-settlement.ts | 87 +++++++++++++++++++ server/src/services/workspace-operations.ts | 23 ++++- 4 files changed, 171 insertions(+), 6 deletions(-) create mode 100644 server/src/services/recovery/workspace-finalize-attempt-settlement.ts diff --git a/server/src/__tests__/ownership-reconciliation-report.test.ts b/server/src/__tests__/ownership-reconciliation-report.test.ts index b86a6d2848e..b1986fa9880 100644 --- a/server/src/__tests__/ownership-reconciliation-report.test.ts +++ b/server/src/__tests__/ownership-reconciliation-report.test.ts @@ -16,6 +16,7 @@ import { issues, nativeRunFinalizations, sandboxCreationOperations, + workspaceOperations, } from "@paperclipai/db"; import { startEmbeddedPostgresTestDatabase } from "./helpers/embedded-postgres.js"; import { resolveExternalTestDatabaseUrl } from "./helpers/external-test-database.js"; @@ -65,7 +66,7 @@ describe("report-only ownership reconciliation", () => { beforeEach(async () => { await rm(orphanSpool, { recursive: true, force: true }); - await db.execute(sql`truncate budget_policies, activity_log, agent_wakeup_requests, sandbox_creation_operations, environment_leases, + await db.execute(sql`truncate budget_policies, activity_log, workspace_operations, agent_wakeup_requests, sandbox_creation_operations, environment_leases, native_run_finalizations, heartbeat_runs, issues, agents, companies cascade`); companyId = randomUUID(); agentId = randomUUID(); await db.insert(companies).values({ id: companyId, name: "Ownership fixture", issuePrefix: "OWN" }); @@ -142,7 +143,7 @@ describe("report-only ownership reconciliation", () => { expect(names.length).toBeGreaterThanOrEqual(40); expect(OWNABLE_KINDS.filter(kind => kind.settle !== null).map(kind => kind.id)) .toEqual(["native_accepted_result_lease", "native_finalizer_lease", "native_cleanup_lease", "environment_run_lease_active", - "environment_teardown_claim", "wake_request_claimed", "legacy_controller_lease"]); + "environment_teardown_claim", "native_workspace_finalize_attempt", "wake_request_claimed", "legacy_controller_lease"]); expect(OWNABLE_KINDS.filter(kind => !kind.listInProgress).every(kind => kind.coverageGap)).toBe(true); }); @@ -288,6 +289,52 @@ describe("report-only ownership reconciliation", () => { .toEqual([wakes.pending.id, wakes.unprojected.id].sort()); }); + it("settles a finalize attempt whose process died mid-copyback, and keeps live or independent ones", async () => { + // Crash points workspace.finalize.after_start_before_copyback and + // .after_copyback_before_done: the run finished through a later attempt, + // but the interrupted attempt stayed `running`. + const run = (id: string, status: string) => ({ id, companyId, agentId, status, invocationSource: "manual", + ...(status === "running" ? {} : { finishedAt: minutesAgo(2) }) }); + const runs = { done: randomUUID(), live: randomUUID(), legacyDone: randomUUID(), legacyLive: randomUUID() }; + await db.insert(heartbeatRuns).values([run(runs.done, "succeeded"), run(runs.live, "running"), + run(runs.legacyDone, "succeeded"), run(runs.legacyLive, "running")]); + const dead = { bootId: "previous-boot", pid: 4_194_303, hostHash: currentProcessSpaceHash(), + processStartedAt: "2026-11-07T09:00:00.000Z" }; + const ops = { dead: randomUUID(), live: randomUUID(), foreign: randomUUID(), uncertain: randomUUID(), + legacyDone: randomUUID(), legacyLive: randomUUID() }; + const op = (id: string, runId: string, metadata: Record) => ({ id, companyId, heartbeatRunId: runId, + phase: "workspace_finalize", status: "running", metadata }); + await db.insert(workspaceOperations).values([ + op(ops.dead, runs.done, { finalizeOwner: dead }), + // Stamped by this process and still running here (current boot): alive. + op(ops.live, runs.live, { finalizeOwner: { bootId: currentControllerBootId(), pid: process.pid, + hostHash: currentProcessSpaceHash(), processStartedAt: null } }), + // Another PID space is never proof. + op(ops.foreign, runs.done, { finalizeOwner: { ...dead, hostHash: "another-pid-space" } }), + op(ops.uncertain, runs.done, { finalizeOwner: dead, uncertain: true }), + // Written before the stamp: owned by its run. + op(ops.legacyDone, runs.legacyDone, {}), + op(ops.legacyLive, runs.legacyLive, {}), + ]); + const statuses = async () => Object.fromEntries((await db.select({ id: workspaceOperations.id, status: workspaceOperations.status }) + .from(workspaceOperations)).map(row => [Object.keys(ops).find(name => ops[name as keyof typeof ops] === row.id), row.status])); + + await runOwnershipReconciliationPass(db, { state: createOwnershipReportState(), record: false }); + expect(Object.values(await statuses()).every(status => status === "running")).toBe(true); + + const state = createOwnershipReportState(); + const first = await runOwnershipReconciliationPass(db, { state }); + expect(kindReport(first, "native_workspace_finalize_attempt")).toMatchObject({ inProgress: 6, settled: 2 }); + expect(await statuses()).toEqual({ dead: "failed", live: "running", foreign: "running", uncertain: "running", + legacyDone: "failed", legacyLive: "running" }); + const [settled] = await db.select().from(workspaceOperations).where(eq(workspaceOperations.id, ops.dead)); + expect(settled.metadata).toMatchObject({ abandonedFinalization: { reason: "abandoned_workspace_finalization", proof: "pid_dead" } }); + expect(settled.finishedAt).not.toBeNull(); + // The run's outcome never changes. + const [doneRun] = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, runs.done)); + expect(doneRun.status).toBe("succeeded"); + }); + it("writes nothing when recording is off", async () => { await seed(); const report = await runOwnershipReconciliationPass(db, { state: createOwnershipReportState(), record: false }); diff --git a/server/src/services/recovery/ownership-registry.ts b/server/src/services/recovery/ownership-registry.ts index b2ef3047448..787dd8160a2 100644 --- a/server/src/services/recovery/ownership-registry.ts +++ b/server/src/services/recovery/ownership-registry.ts @@ -32,6 +32,7 @@ import { settleNativeAcceptedResultLease, } from "../native-runtime/native-accepted-result-lease-settlement.js"; import { settleClaimedWakeRequest } from "./wake-request-claimed-settlement.js"; +import { settleAbandonedWorkspaceFinalizeAttempt } from "./workspace-finalize-attempt-settlement.js"; import { legacyControllerOwnerStamp, settleLegacyControllerLease } from "../legacy-controller-lease.js"; import { nativeLeaseHolderStamp, @@ -634,9 +635,18 @@ export const OWNABLE_KINDS: readonly OwnableKind[] = [ generation: null, host: null, processSpace: str(owner.hostHash), bootId: str(owner.ownerId), pid, processStartedAt: null, processGroupId: null } } : { kind: "none" }; }), - operationKind("native_workspace_finalize_attempt", "Native workspace finalize attempt (copyback export)", "workspace_preservation", - and(eq(workspaceOperations.phase, "workspace_finalize"), inArray(workspaceOperations.status, ["running", "pending"])), - row => runOwner(row.heartbeatRunId)), + { + // Shared path since Phase 2: a finalize attempt whose process died (a + // restart between start and copyback, or between copyback and done) is + // settled so it cannot stay `running` forever (crash points + // workspace.finalize.after_start_before_copyback / after_copyback_before_done). + ...operationKind("native_workspace_finalize_attempt", "Native workspace finalize attempt (copyback export)", "workspace_preservation", + and(eq(workspaceOperations.phase, "workspace_finalize"), inArray(workspaceOperations.status, ["running", "pending"])), + // Rows written before `finalizeOwner` existed are owned by their run. + row => json(row.metadata).finalizeOwner ? controllerStamp(json(row.metadata).finalizeOwner, "native_workspace_finalize_attempt") + : runOwner(row.heartbeatRunId)), + settle: settleAbandonedWorkspaceFinalizeAttempt, + }, operationKind("remote_provision_command", "Remote provision command intent", "workspace_preservation", and(eq(workspaceOperations.phase, "remote_workspace_provision"), or(eq(workspaceOperations.status, "running"), sql`(${workspaceOperations.metadata}->>'uncertain')::boolean is true`)), diff --git a/server/src/services/recovery/workspace-finalize-attempt-settlement.ts b/server/src/services/recovery/workspace-finalize-attempt-settlement.ts new file mode 100644 index 00000000000..4b73e69ab6d --- /dev/null +++ b/server/src/services/recovery/workspace-finalize-attempt-settlement.ts @@ -0,0 +1,87 @@ +import { and, eq, inArray } from "drizzle-orm"; +import { heartbeatRuns, workspaceOperations, type Db } from "@paperclipai/db"; +import { persistActivity, publishActivity, type ActivityPublication } from "../activity-log.js"; +import type { OwnerProof } from "../owner-proof.js"; +import { notifyTerminalWorkspaceWork } from "../terminal-workspace-work.js"; +import { isWorkspaceFinalizeOperationLive } from "../workspace-operations.js"; +import type { InProgressRecord, SettleOutcome } from "./ownership-registry.js"; + +export const ABANDONED_WORKSPACE_FINALIZE_REASON = "abandoned_workspace_finalization"; +const TERMINAL_RUN_STATUSES = ["succeeded", "failed", "cancelled", "timed_out", "interrupted"]; + +/** + * Settle of the `native_workspace_finalize_attempt` ownership kind (plan Phase 2). + * + * A restart between a finalize attempt's start and its copyback, or between + * copyback and the row's terminal write, leaves the attempt `running` for good: + * the process that drove it is gone, and the run finishes through a later + * attempt or recovery. The crash-point suite found both windows. + * + * The shared reconciler calls this only with a `gone` proof: the stamped + * `finalizeOwner` process is proven gone, or, for a row written before the + * stamp, its run is terminal. Settle marks only that attempt `failed` with its + * provenance. It never changes the run or its outcome, never touches an attempt + * this process is still driving, and leaves independent obligations alone + * (an uncertain export or a save-work record), like the RC-64 supersession. + */ +export async function settleAbandonedWorkspaceFinalizeAttempt(db: Db, record: InProgressRecord, + context: { now: Date; proof: OwnerProof }): Promise { + if (isWorkspaceFinalizeOperationLive(record.recordId)) return { settled: false, reason: "attempt_live_in_this_process" }; + let publication: ActivityPublication | undefined; + const outcome = await db.transaction(async transaction => { + const tx = transaction as unknown as Db; + const [operation] = await tx.select().from(workspaceOperations) + .where(and(eq(workspaceOperations.id, record.recordId), eq(workspaceOperations.phase, "workspace_finalize"), + inArray(workspaceOperations.status, ["running", "pending"]))) + .for("update"); + if (!operation) return { settled: false, reason: "attempt_no_longer_in_progress" }; + const metadata = operation.metadata ?? {}; + if (metadata.uncertain === true || metadata.uncertain === "true" || metadata.saveWork != null) { + return { settled: false, reason: "independent_obligation" }; + } + // A run-owned (pre-stamp) attempt needs its run to be terminal now. + if (record.owner.kind === "run") { + if (!operation.heartbeatRunId) return { settled: false, reason: "owner_run_missing" }; + const [run] = await tx.select({ status: heartbeatRuns.status }).from(heartbeatRuns) + .where(and(eq(heartbeatRuns.id, operation.heartbeatRunId), inArray(heartbeatRuns.status, TERMINAL_RUN_STATUSES))); + if (!run) return { settled: false, reason: "owner_run_active" }; + } else if (record.owner.kind === "process") { + // The proof judged this exact owner; a re-stamped attempt is another owner. + const owner = metadata.finalizeOwner as Record | undefined; + if (owner?.bootId !== record.owner.stamp.bootId || owner?.pid !== record.owner.stamp.pid) { + return { settled: false, reason: "owner_changed_since_proof" }; + } + } else { + return { settled: false, reason: "owner_not_provable" }; + } + const now = context.now; + const provenance = { + schema: "paperclip.workspace-finalize-abandonment.v1", + reason: ABANDONED_WORKSPACE_FINALIZE_REASON, + proof: context.proof.reason, + previousStatus: operation.status, + runId: operation.heartbeatRunId, + reconciledAt: now.toISOString(), + }; + // Register the wake intent before the write so drain and sleep checks see it. + const workKey = operation.executionWorkspaceId ? `workspace:${operation.executionWorkspaceId}` + : operation.issueId ? `issue:${operation.issueId}` : null; + if (workKey) await notifyTerminalWorkspaceWork(tx, workKey); + const [row] = await tx.update(workspaceOperations).set({ + status: "failed", finishedAt: now, updatedAt: now, + stderrExcerpt: `${operation.stderrExcerpt ?? ""}Workspace finalization was interrupted before it finished; the process that ran it is gone.\n`.slice(-4096), + metadata: { ...metadata, abandonedFinalization: provenance }, + }).where(and(eq(workspaceOperations.id, operation.id), eq(workspaceOperations.status, operation.status))) + .returning({ id: workspaceOperations.id }); + if (!row) return { settled: false, reason: "attempt_no_longer_in_progress" }; + const activity = await persistActivity(tx, { + companyId: operation.companyId, actorType: "system", actorId: "ownership-reconciler", + action: "workspace_operation.reconciled", entityType: "workspace_operation", entityId: operation.id, + runId: operation.heartbeatRunId, issueId: operation.issueId, details: provenance, + }); + publication = activity.publication; + return { settled: true, reason: context.proof.reason }; + }); + if (outcome.settled && publication) publishActivity(publication); + return outcome; +} diff --git a/server/src/services/workspace-operations.ts b/server/src/services/workspace-operations.ts index 06c6358d6b9..b05ba90a915 100644 --- a/server/src/services/workspace-operations.ts +++ b/server/src/services/workspace-operations.ts @@ -11,7 +11,7 @@ import { asc, desc, eq, gte, inArray, isNull, lt, or, and } from "drizzle-orm"; import { conflict, notFound } from "../errors.js"; import { redactCurrentUserText, redactCurrentUserValue } from "../log-redaction.js"; import { instanceSettingsService } from "./instance-settings.js"; -import { currentProcessSpaceHash, processExists, sameProcessSpace } from "./controller-process-identity.js"; +import { currentControllerProcessStamp, currentProcessSpaceHash, processExists, sameProcessSpace } from "./controller-process-identity.js"; import { getWorkspaceOperationLogStore } from "./workspace-operation-log-store.js"; import { crashPoint } from "@paperclipai/shared/crash-points"; @@ -106,6 +106,17 @@ function readRuntimeControlOwner(metadata: unknown): RuntimeControlOwnerStamp | */ const liveRuntimeControlOperationIds = new Set(); +/** + * Workspace-finalize operations this process is driving. A finalize row stamped + * by this process (`finalizeOwner`) but absent here was left by a request that + * died without unwinding. The ownership settle never touches an id in this set. + */ +const liveWorkspaceFinalizeOperationIds = new Set(); + +export function isWorkspaceFinalizeOperationLive(operationId: string): boolean { + return liveWorkspaceFinalizeOperationIds.has(operationId); +} + type NativeFinalizationActivityScope = { operationId: string; companyId: string; @@ -155,6 +166,7 @@ function updatedAtUnchanged(updatedAt: Date) { export function resetWorkspaceRuntimeControlStateForTests() { liveRuntimeControlOperationIds.clear(); + liveWorkspaceFinalizeOperationIds.clear(); } export function workspaceRuntimeControlOwnerIdForTests() { @@ -551,6 +563,13 @@ export function workspaceOperationService(db: Db) { } satisfies RuntimeControlOwnerStamp, } : recordInput.metadata ?? null; + // Finalize attempts record the process that runs them (no I/O), so + // the shared ownership path can settle one whose owner died mid-copyback. + const finalizeAttempt = recordInput.phase === "workspace_finalize"; + if (finalizeAttempt) { + currentMetadata = { ...(currentMetadata ?? {}), finalizeOwner: currentControllerProcessStamp() }; + liveWorkspaceFinalizeOperationIds.add(id); + } // Claim in-process before the row exists, so there is no ordering in which a recovery // sweep can observe an operation stamped by this process but not yet tracked by it — @@ -583,6 +602,7 @@ export function workspaceOperationService(db: Db) { if (!published) throw new Error("Workspace operation admission did not publish its running row"); } catch (insertError) { liveRuntimeControlOperationIds.delete(id); + liveWorkspaceFinalizeOperationIds.delete(id); throw insertError; } createdIds.push(id); @@ -742,6 +762,7 @@ export function workspaceOperationService(db: Db) { if (heartbeatTimer) clearInterval(heartbeatTimer); if (timeoutTimer) clearTimeout(timeoutTimer); liveRuntimeControlOperationIds.delete(id); + liveWorkspaceFinalizeOperationIds.delete(id); } }, };