From beec70ad9f93fb02b56b108c4d3aa5626fe6adae Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Sun, 8 Nov 2026 05:32:46 -0600 Subject: [PATCH] fix(native): judge sync-retention run processes with the shared run-process prover (#20025) 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 > - A native run syncs its workspace into a local folder. A retention job deletes old folders, but never one whose run's adapter process may still write it > - That job had its own liveness check. It probed the recorded PID in this PID space, whatever host recorded it, and compared process start times to the millisecond > - So a dead PID here could delete the synced files of a process still running on another host. And a live process whose recorded start differed by under a second (`ps lstart` has 1 s precision) counted as a recycled PID, so its folder could be deleted too > - The durable ownership plan proves owners by identity with one shared prover. The lost-process reapers already use it for exactly this run identity (#20004) > - This pull request makes retention use that prover, so anything but a proof the process is gone keeps the folder > - The benefit is that preserved files are no longer deleted on a wrong-host or sub-second-skew judgment ## Linked Issues or Issue Description Refs #19831 (plan, Phase 2), #19838 (inventory: "One fact, many proofs"). Phase 2's native agent flagged `runProcessMayBeLive` in `native-workspace-sync-retention.ts` as still bespoke after #19997. **What happened?** `runProcessMayBeLive` ignored `process_host_hash` and required an exact start-time match. **Expected behavior** The same verdict as the lost-process reapers (`proveRunProcessGone`): - the PID is judged only in its recorded PID space; - rows saved before `process_host_hash` keep the one-release local fallback on single-host servers; - a reused PID needs a definitely later start; - EPERM is alive. ## What Changed - `native-workspace-sync-retention.ts`: `runProcessMayBeLive` calls `proveRunProcessGone` (reading `process_host_hash`). The injectable retention probe is adapted to the shared probe for tests. Remote-only runs keep their existing shortcut. ## Verification - `native-workspace-sync-retention.test.ts`: 59 pass. Two new tests fail on `master` and pass here: - a run recorded in another PID space is kept, even if that PID is dead here; - a live PID whose start time is within the 1 s precision is kept. The new "deletes a terminal run whose process, recorded in this PID space, has exited" and all existing retention cases (recycled PID, dead process, destroyed sandbox, live group) pass. - The server package typecheck passes. ## Risks - Folders of runs recorded on another host are now kept until their other owners settle, rather than deleted on a local PID probe. That is the safe direction; the folders are bounded by the existing retention limits. ## 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 --- .../native-workspace-sync-retention.test.ts | 20 +++++++++++++ .../native-workspace-sync-retention.ts | 29 +++++++++++++------ 2 files changed, 40 insertions(+), 9 deletions(-) diff --git a/server/src/__tests__/native-workspace-sync-retention.test.ts b/server/src/__tests__/native-workspace-sync-retention.test.ts index a35da0f7372..15647bec73c 100644 --- a/server/src/__tests__/native-workspace-sync-retention.test.ts +++ b/server/src/__tests__/native-workspace-sync-retention.test.ts @@ -1,3 +1,4 @@ +import { currentProcessSpaceHash } from "../services/controller-process-identity.js"; import { randomUUID } from "node:crypto"; import os from "node:os"; import path from "node:path"; @@ -55,6 +56,7 @@ type SeedInput = { processPid?: number | null; processGroupId?: number | null; processStartedAt?: Date | null; + processHostHash?: string | null; leaseProvider?: string | null; controllerLeaseDaysFromNow?: number | null; finalization?: { @@ -148,6 +150,7 @@ describeEmbeddedPostgres("native workspace sync retention", () => { processPid: input.processPid ?? null, processGroupId: input.processGroupId ?? null, processStartedAt: input.processStartedAt ?? null, + processHostHash: input.processHostHash ?? null, controllerLeaseExpiresAt: input.controllerLeaseDaysFromNow == null ? null : new Date(now.getTime() + input.controllerLeaseDaysFromNow * DAY_MS), @@ -297,6 +300,23 @@ describeEmbeddedPostgres("native workspace sync retention", () => { expect(await exists(folder)).toBe(false); }); + it("keeps a run whose process was recorded in another PID space, even if that pid is dead here", async () => { + await expectKept({ processPid: 1234, processStartedAt: processStart, processHostHash: "another-host-pid-space", + leaseProvider: "local" }, "process_live"); + }); + + it("deletes a terminal run whose process, recorded in this PID space, has exited", async () => { + const { folder } = await seed({ processPid: 1234, processStartedAt: processStart, + processHostHash: currentProcessSpaceHash(), leaseProvider: "local" }); + expect((await service().tick()).expired).toBe(1); + expect(await exists(folder)).toBe(false); + }); + + it("keeps a run whose live pid's start time matches within the 1 s start-time precision", async () => { + livePids.set(1234, new Date(processStart.getTime() + 400).toISOString()); + await expectKept({ processPid: 1234, processStartedAt: processStart, leaseProvider: "local" }, "process_live"); + }); + it("deletes a terminal run whose recorded pid was recycled by another process", async () => { livePids.set(1234, new Date(now.getTime() - DAY_MS).toISOString()); const { folder } = await seed({ processPid: 1234, processStartedAt: processStart }); diff --git a/server/src/services/native-runtime/native-workspace-sync-retention.ts b/server/src/services/native-runtime/native-workspace-sync-retention.ts index f62079a4e13..51c6e175443 100644 --- a/server/src/services/native-runtime/native-workspace-sync-retention.ts +++ b/server/src/services/native-runtime/native-workspace-sync-retention.ts @@ -7,6 +7,8 @@ import { logger } from "../../middleware/logger.js"; import type { createDeliveryWorkCoordinator } from "../delivery-work-coordinator.js"; import { DELIVERY_QUEUES } from "../delivery-work-notifications.js"; import { readProcessStartedAt } from "../hot-restart.js"; +import { currentProcessProbe, type ProcessProbe as OwnerProcessProbe } from "../owner-proof.js"; +import { proveRunProcessGone } from "../run-process-owner.js"; import { workspaceExportCauses } from "../workspace-recovery-source.js"; import { nativeWorkspaceFinalizationLockKey } from "./native-workspace-finalization-ownership.js"; import { NATIVE_WORKSPACE_OWNER_UNVERIFIED } from "./native-workspace-finalization-recovery-state.js"; @@ -65,6 +67,7 @@ type RunRow = { process_pid: number | null; process_group_id: number | null; process_started_at: string | Date | null; + process_host_hash: string | null; controller_live: boolean; owner_present: boolean; reference: unknown; @@ -229,25 +232,33 @@ const defaultProcessProbe: ProcessProbe = { }; async function runProcessMayBeLive( - tx: Tx, + _tx: Tx, input: { runId: string; run: RunRow; processProbe: ProcessProbe }, ): Promise { - const { run, processProbe } = input; + const { run } = input; if (run.process_pid === null && run.process_group_id === null) return false; - const leases = (await tx.execute(sql` + const leases = (await _tx.execute(sql` select provider from environment_leases where company_id = ${run.company_id} and heartbeat_run_id = ${input.runId} `)) as unknown as Array<{ provider: string }>; if (leases.length > 0 && leases.every((lease) => lease.provider !== "local")) return false; - if (run.process_group_id !== null && processProbe.mayBeAlive(-run.process_group_id)) return true; - if (run.process_pid === null || !processProbe.mayBeAlive(run.process_pid)) return false; - if (!run.process_started_at) return true; - const observed = await processProbe.startedAt(run.process_pid); - return !observed || new Date(observed).getTime() === new Date(run.process_started_at).getTime(); + const proof = await proveRunProcessGone({ + processPid: run.process_pid, processGroupId: run.process_group_id, + processStartedAt: run.process_started_at ? new Date(run.process_started_at) : null, + processHostHash: run.process_host_hash, + }, "native_workspace_sync_retention", sharedProbe(input.processProbe)); + return proof?.verdict !== "gone"; +} + +function sharedProbe(probe: ProcessProbe): OwnerProcessProbe | undefined { + if (probe === defaultProcessProbe) return undefined; + const base = currentProcessProbe(); + return { ...base, pidExists: probe.mayBeAlive, processGroupExists: (group) => probe.mayBeAlive(-group), + readStartedAt: async (pid) => { const value = await probe.startedAt(pid); return value ? new Date(value) : null; } }; } async function readRun(tx: Tx, runId: string): Promise { const [run] = (await tx.execute(sql` - select company_id, status, finished_at, process_pid, process_group_id, process_started_at, + select company_id, status, finished_at, process_pid, process_group_id, process_started_at, process_host_hash, (controller_lease_expires_at is not null and controller_lease_expires_at >= now()) as controller_live, (runner_profile_json->>'nativeWorkspaceFinalizationOwner' is not null) as owner_present, runner_profile_json->'nativeWorkspaceSync' as reference