mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-12 06:10:44 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Native runs record the server and adapter processes that own them, and recovery must prove an owner gone before it takes over or releases its work > - Phase 2 of the durable ownership plan gave the repository one prover, `proveOwnerGone`. The native runtime still had four private proofs of the same fact > - The private proofs disagreed with the shared rules in edge cases: some judged a PID from another PID space, and one accepted an inconsistent start time as proof of reuse > - This pull request moves those four proofs onto `proveOwnerGone` and deletes the private liveness code > - The benefit is one rule for "the owner is gone" across native recovery, with the stricter shared behaviour where they differed ## Linked Issues or Issue Description Refs #19933, #19950, #19953 (the shared ownership path and earlier Phase 2 kinds). **What is the problem?** Native recovery proved dead owners with four private checks instead of the shared prover. The checks used different rules for PID spaces, reused PIDs and unreadable start times. **What should happen?** Every native owner proof uses `proveOwnerGone`. Only a `gone` verdict releases or takes over. The remote fence and the one-release legacy fallback stay as they are. **Why does it matter?** Recovery decisions about the same process must agree. A dead PID in another PID space must never be used as proof. ## What Changed - `native-restart-recovery.ts` `evaluateNativeControllerTakeover`: the local-process branch now calls `proveOwnerGone`. The remote fenced branch (#19830), the `operator_attested` sentinel and the null-host fallback (`processSpaceUnrecorded` with `legacyHostFallback`, recorded by callers as `legacy_host_unknown_dead_pid_proof`) are kept. Reasons are unchanged, plus a new `controller_pid_start_inconsistent`. Owners built from receipts with no PID space are judged in this PID space: their callers check the recorded host first. - `native-failed-run-owner-release.ts`: `recordedProcessMayBeLive` is deleted. `runProcessMayBeLive` and `controllerMayBeLive` release only on a `gone` verdict. Each use of the legacy fallback is recorded as `legacy_host_unknown_dead_pid_proof` when the release commits. - `native-session-executor.ts`: `verifiedRecoveryProcessIsAlive` is replaced by `recoveryProcessMayBeAlive`, which is false only when the prover proves the runner gone. Signalling a recovered runner requires a proven same process. - `native-local-process-stop.ts` `recordedLocalExecutionStopped`: the bound identity (launch event, host, PID, group, start time) is judged by the prover. It uses `process_host_hash` when the row has one. - New `native-runtime/native-owner-stamps.ts`: controller and adapter-process stamp builders, shared with the ownership registry. Not changed: `proveLocalWorkspaceOwnerStopped` (workspaces), `evaluateNativeProviderProcesses`, the stop observation in `recordNativeLocalProcessStop`, and `runProcessMayBeLive` in `native-workspace-sync-retention.ts`. ## Verification - `cd server && npx vitest run src/services/native-runtime/native-restart-recovery.test.ts src/services/controller-process-identity.test.ts`: 51 passed. - `cd server && npx vitest run src/services/native-runtime/native-failed-run-owner-release.test.ts`: 14 passed. - `cd server && npx vitest run src/services/native-runtime/native-controller-host-recovery.test.ts`: 2 passed. - `cd server && npx vitest run src/services/quarantined-task-retry.test.ts -t "keeps quarantine"`: 18 passed. `explicit-native-continuation.test.ts`: 414 passed. `ownership-reconciliation-report.test.ts`: 23 passed. - Red with the master versions of the two modules: 3 new tests fail ("accepts a reused PID only when the live process started definitely later", "never releases on a dead PID recorded in another PID space", "keeps the one-release fallback for a run process saved before its PID space, and records it"). The tests were written after the code. - Two fixtures changed to match the shared rules: - `controller-process-identity.test.ts`: the "recycled" case recorded a start after the live process, which is an inconsistent stamp. It now records an earlier start (a real reuse), and the inconsistent case is asserted as alive. - `quarantined-task-retry.test.ts` `worker_alive`: the live worker now records this process's real start time. The old fixture was a reused PID, which the prover correctly proves gone. - `cd server && npx vitest run src/services/native-local-process-stop.test.ts`: 5 passed (review round 1). With the master module, 2 fail: the other-PID-space case and the reused-PID case. - Not run locally: typecheck (CI runs it) and the wide suites. ## Risks Behaviour changes, all from the shared rules: - A live PID observed to start before the recorded controller is no longer accepted as recycled (`controller_pid_start_inconsistent`; the run waits for evidence). - A failed-run owner whose recorded adapter process or controller is in another PID space, or is a remote sandbox PID, is no longer released by a local probe. It keeps the lease path. - An unreadable start time is no longer exit evidence for a recovered local runner. - A local execution whose PID was reused by a later process now counts as stopped. Before, any live PID blocked the proof. - A local execution whose row records another PID space is no longer counted as stopped by a dead local PID. - A PID space that cannot be read here makes a hostless receipt owner unprovable. No persisted state changes. Rows saved by the previous release keep the one-release null-host fallback, as before. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits, local test runs). ## 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 <noreply@anthropic.com>
72 lines
4.1 KiB
TypeScript
72 lines
4.1 KiB
TypeScript
import { randomUUID } from "node:crypto";
|
|
import { hostname } from "node:os";
|
|
import { afterAll, beforeAll, describe, expect, it } from "vitest";
|
|
import { agents, companies, createDb, heartbeatRuns } from "@paperclipai/db";
|
|
import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "../__tests__/helpers/embedded-postgres.js";
|
|
import { currentProcessSpaceHash, currentProcessStartedAtMs } from "./controller-process-identity.js";
|
|
import { appendHeartbeatRunEvent } from "./heartbeat-run-events.js";
|
|
import { PROCESS_IDENTITY_RECORDED, recordedLocalExecutionStopped } from "./native-local-process-stop.js";
|
|
|
|
const support = await getEmbeddedPostgresTestSupport();
|
|
|
|
// Phase 2: a bound local execution is judged by the shared prover
|
|
// (proveOwnerGone), in the PID space the row recorded.
|
|
(support.supported ? describe : describe.skip)("recordedLocalExecutionStopped", () => {
|
|
let database: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>>;
|
|
let db: ReturnType<typeof createDb>;
|
|
let companyId: string;
|
|
let agentId: string;
|
|
beforeAll(async () => {
|
|
database = await startEmbeddedPostgresTestDatabase("native-local-stop-");
|
|
db = createDb(database.connectionString, { maxConnections: 2 });
|
|
companyId = randomUUID(); agentId = randomUUID();
|
|
await db.insert(companies).values({ id: companyId, name: "Local stop", issuePrefix: `L${companyId.slice(0, 7)}` });
|
|
await db.insert(agents).values({ id: agentId, companyId, name: "Agent", role: "general", adapterType: "codex_local",
|
|
status: "idle" });
|
|
}, 60_000);
|
|
afterAll(async () => { await db?.$client.end({ timeout: 1 }); await database?.cleanup(); });
|
|
|
|
/** A failed run with its process identity bound by the launch event. */
|
|
async function execution(input: { pid: number; startedAt: Date; processHostHash: string | null }) {
|
|
const [run] = await db.insert(heartbeatRuns).values({ companyId, agentId, status: "failed", runtimeMode: "native",
|
|
processPid: input.pid, processGroupId: input.pid, processStartedAt: input.startedAt,
|
|
processHostHash: input.processHostHash }).returning();
|
|
await appendHeartbeatRunEvent(db, { companyId, runId: run!.id, agentId, eventType: PROCESS_IDENTITY_RECORDED,
|
|
payload: { targetKind: "local", hostname: hostname(), processPid: input.pid, processGroupId: input.pid,
|
|
processStartedAt: input.startedAt.toISOString() } });
|
|
return run!;
|
|
}
|
|
const deadPid = 2_000_000_001;
|
|
const minuteAgo = () => new Date(Date.now() - 60_000);
|
|
const ownStart = async () => new Date((await currentProcessStartedAtMs())!);
|
|
|
|
it("is stopped when its dead PID was recorded in this PID space", async () => {
|
|
expect(await recordedLocalExecutionStopped(db, await execution({ pid: deadPid, startedAt: minuteAgo(),
|
|
processHostHash: currentProcessSpaceHash() }))).toBe(true);
|
|
});
|
|
|
|
it("is never stopped by a dead PID recorded in another PID space", async () => {
|
|
expect(await recordedLocalExecutionStopped(db, await execution({ pid: deadPid, startedAt: minuteAgo(),
|
|
processHostHash: "another-pid-space" }))).toBe(false);
|
|
});
|
|
|
|
it("keeps the host-bound proof for a row saved before its PID space", async () => {
|
|
expect(await recordedLocalExecutionStopped(db, await execution({ pid: deadPid, startedAt: minuteAgo(),
|
|
processHostHash: null }))).toBe(true);
|
|
});
|
|
|
|
it("is not stopped while the same process is alive", async () => {
|
|
expect(await recordedLocalExecutionStopped(db, await execution({ pid: process.pid, startedAt: await ownStart(),
|
|
processHostHash: currentProcessSpaceHash() }))).toBe(false);
|
|
});
|
|
|
|
it("is stopped when a later process reused its PID and leads the group", async () => {
|
|
// This test process started definitely after the recorded execution.
|
|
const recorded = new Date((await ownStart()).getTime() - 5_000);
|
|
// A group with this id is either empty or led by this later process, so
|
|
// it cannot hold the recorded execution's children.
|
|
expect(await recordedLocalExecutionStopped(db, await execution({ pid: process.pid, startedAt: recorded,
|
|
processHostHash: currentProcessSpaceHash() }))).toBe(true);
|
|
});
|
|
});
|