mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-10 12:37:55 +01:00
fix(recovery): settle a workspace finalize attempt whose process died mid-copyback (#19985)
## 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 <noreply@anthropic.com>
This commit is contained in:
1 parent
68d5fb5a6e
commit
95f3cab0cd
4 files changed
+171
-6
No files matched your search
@@ -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<string, unknown>) => ({ 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 });
|
||||
|
||||
@@ -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`)),
|
||||
|
||||
@@ -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<SettleOutcome> {
|
||||
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<string, unknown> | 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;
|
||||
}
|
||||
@@ -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<string>();
|
||||
|
||||
/**
|
||||
* 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<string>();
|
||||
|
||||
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);
|
||||
}
|
||||
},
|
||||
};
|
||||
|
||||
Reference in new issue
Block a user