From 51f200243f68555775b7b0cfcef76557de6bed77 Mon Sep 17 00:00:00 2001 From: Dotta Date: Sun, 27 Sep 2026 18:05:37 -0500 Subject: [PATCH] fix: require copyback stop proof after abrupt controller death Co-Authored-By: Paperclip --- doc/native-workspace-finalization-recovery.md | 12 ++++--- ...native-workspace-finalization-ownership.ts | 20 +++++++++--- .../native-workspace-finalizer.test.ts | 32 ++++++++++++++++++- 3 files changed, 55 insertions(+), 9 deletions(-) diff --git a/doc/native-workspace-finalization-recovery.md b/doc/native-workspace-finalization-recovery.md index 45621debae8..c17babb0d32 100644 --- a/doc/native-workspace-finalization-recovery.md +++ b/doc/native-workspace-finalization-recovery.md @@ -15,12 +15,16 @@ receipt. Losing the lock connection does not prove physical copyback stopped: a contender still refuses a receipt whose controller is alive. The original callback joins before its token is released, and publication checks the lock connection and token. Graceful completion clears the receipt. A controller that -has exited on the same host can be recovered; PID reuse is checked against its -recorded start time rather than trusted by PID alone. +has exited on the same host can be recovered automatically only when a durable +successful workspace barrier proves its copyback finished. A dead parent can +leave tar/Git children alive, so incomplete copyback requires operator stop +verification even on the same host. PID reuse is checked against its recorded +start time rather than trusted by PID alone. -## Controller replacement on a different host +## Unverified copyback after controller replacement -The controller cannot verify a process on a foreign or unknown host. It surfaces +The controller cannot verify a process on a foreign or unknown host, or orphaned +copyback children after an abrupt parent death before the success barrier. It surfaces `native_workspace_finalization_owner_unverified` as board-owned recovery, with no automatic provider wake. This is an intentional limit: elapsed time or a missing database connection never proves the old copyback process stopped. diff --git a/server/src/services/native-runtime/native-workspace-finalization-ownership.ts b/server/src/services/native-runtime/native-workspace-finalization-ownership.ts index f0e0d78de87..fc30be3c786 100644 --- a/server/src/services/native-runtime/native-workspace-finalization-ownership.ts +++ b/server/src/services/native-runtime/native-workspace-finalization-ownership.ts @@ -3,7 +3,7 @@ import { nativeSha256 } from "./canonical.js"; import { randomUUID } from "node:crypto"; import os from "node:os"; import { and, eq, sql } from "drizzle-orm"; -import { heartbeatRuns, withDedicatedDbConnection, type Db } from "@paperclipai/db"; +import { heartbeatRuns, workspaceOperations, withDedicatedDbConnection, type Db } from "@paperclipai/db"; import { currentNativeControllerIdentity, evaluateNativeControllerTakeover, @@ -68,8 +68,7 @@ export async function withNativeWorkspaceFinalizationOwnership( const rawPrior = run.profile?.[OWNER_KEY]; if (rawPrior != null) { const prior = readOwner(rawPrior); - // A foreign host or malformed receipt cannot prove the old physical owner stopped. - if (!prior || prior.hostname !== owner.hostname) { + const requireStopVerification = async () => { if (run.issueId) await issueRecoveryActionService(input.db).upsertSourceScoped({ companyId: input.companyId, sourceIssueId: run.issueId, kind: "active_run_watchdog", ownerType: "board", returnOwnerAgentId: run.agentId, @@ -80,12 +79,25 @@ export async function withNativeWorkspaceFinalizationOwnership( wakePolicy: null, maxAttempts: 1, supersedeOnIdentityChange: true, }); return { acquired: false } as const; - } + }; + // A foreign host or malformed receipt cannot prove the old physical owner stopped. + if (!prior || prior.hostname !== owner.hostname) return requireStopVerification(); const takeover = await evaluateNativeControllerTakeover({ owner: { leaseOwner: prior.token, leaseExpiresAt: new Date(0), controllerPid: prior.pid, controllerProcessStartedAt: new Date(prior.processStartedAt) }, now: new Date(), }); if (!takeover.allowed) return { acquired: false } as const; + // A dead parent may have orphaned tar/Git children. Only a durable + // completed-copyback barrier proves they joined; otherwise board proof + // is required even on the same host. Both callers reuse this barrier. + const [completedCopyback] = await input.db.select({ id: workspaceOperations.id }).from(workspaceOperations).where(and( + eq(workspaceOperations.companyId, input.companyId), + eq(workspaceOperations.heartbeatRunId, input.runId), + eq(workspaceOperations.issueId, run.issueId!), + eq(workspaceOperations.phase, "workspace_finalize"), + eq(workspaceOperations.status, "succeeded"), + )).limit(1); + if (!completedCopyback) return requireStopVerification(); } await input.db.update(heartbeatRuns).set({ runnerProfileJson: sql`jsonb_set(coalesce(${heartbeatRuns.runnerProfileJson}, '{}'::jsonb), array[${OWNER_KEY}], ${JSON.stringify(owner)}::jsonb)`, diff --git a/server/src/services/native-runtime/native-workspace-finalizer.test.ts b/server/src/services/native-runtime/native-workspace-finalizer.test.ts index c24624100f0..a3ba8aab9f6 100644 --- a/server/src/services/native-runtime/native-workspace-finalizer.test.ts +++ b/server/src/services/native-runtime/native-workspace-finalizer.test.ts @@ -220,8 +220,38 @@ describe("native workspace finalization recovery", () => { .toEqual({ acquired: true, value: "recovered" }); }); - it("recovers a receipt after the exact controller process on this host exited", async () => { + it("never steals pending copyback from an orphan child after its controller is killed", async () => { + const seeded = await seedRun({ executionWorkspaceId: randomUUID(), title: "Orphan copyback child" }); + const output = path.join(workspaceRoot, "orphan-copyback.txt"); + const childScript = `const fs = require('node:fs'); setInterval(() => fs.appendFileSync(${JSON.stringify(output)}, 'copying\\n'), 25);`; + const parentScript = `const {spawn} = require('node:child_process'); const child = spawn(process.execPath, ['-e', ${JSON.stringify(childScript)}], {detached:true, stdio:'ignore'}); console.log(child.pid); child.unref(); setInterval(() => {}, 1000);`; + const parent = spawn(process.execPath, ["-e", parentScript], { stdio: ["ignore", "pipe", "ignore"] }); + const [chunk] = await once(parent.stdout!, "data"); + const childPid = Number(String(chunk).trim()); + expect(childPid).toBeGreaterThan(0); + try { + const startedAt = await readProcessStartedAt(parent.pid!); + const exited = once(parent, "close"); + parent.kill("SIGKILL"); await exited; + process.kill(childPid, 0); // The copyback child outlived the exact controller. + await db.update(heartbeatRuns).set({ runnerProfileJson: { nativeWorkspaceFinalizationOwner: { + token: randomUUID(), hostname: os.hostname(), pid: parent.pid, processStartedAt: startedAt, + } } }).where(eq(heartbeatRuns.id, seeded.runId)); + const work = vi.fn(async () => "second physical writer"); + expect(await withNativeWorkspaceFinalizationOwnership({ db, companyId, runId: seeded.runId }, work)).toEqual({ acquired: false }); + expect(work).not.toHaveBeenCalled(); + const [action] = await db.select().from(issueRecoveryActions).where(eq(issueRecoveryActions.sourceIssueId, seeded.issueId)); + expect(action).toMatchObject({ ownerType: "board", cause: "native_workspace_finalization_owner_unverified", wakePolicy: null }); + } finally { + parent.kill("SIGKILL"); + try { process.kill(-childPid, "SIGKILL"); } catch { /* fixture already exited */ } + } + }); + + it("recovers completed copyback after the exact controller process on this host exited", async () => { const seeded = await seedRun({ executionWorkspaceId: randomUUID(), title: "Dead copyback controller" }); + await db.insert(workspaceOperations).values({ companyId, heartbeatRunId: seeded.runId, + issueId: seeded.issueId, phase: "workspace_finalize", status: "succeeded" }); const child = spawn(process.execPath, ["-e", "setInterval(() => {}, 1000)"], { stdio: "ignore" }); await once(child, "spawn"); const startedAt = await readProcessStartedAt(child.pid!);