From 0cb9770d1c02f91a327a6ae7a94a12cf77c96d2b Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Sat, 7 Nov 2026 23:17:40 -0600 Subject: [PATCH] feat(recovery): name the owner of teardown claims and login/probe leases (#19965) 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 > - Environment leases carry several in-progress records: login/probe leases and in-flight teardown claims among them > - The ownership reconciler (durable ownership plan, Phase 2) must know who owns each record, or invariant I1 reports a gap > - The ownership data showed one teardown claim and one login/probe lease with no owner stamp > - A teardown claim could also show the wrong owner: a controller stamp left by an earlier attempt named a dead process while a live claimant held the claim > - This pull request names an honest owner for both kinds, without a new write before `turn.start` > - The benefit is that I1 measures real gaps, and the later settle for these kinds can trust the owner it is given ## Linked Issues or Issue Description Refs #19964 Refs #19933 **What happened?** The ownership reconciler reported `environment_teardown_claim` and `environment_login_probe_lease` records as I1 `no_owner_stamp`. Workspace recovery and native export retry claim teardown without a controller stamp. A login/probe lease never had an owner stamp. A teardown claim that kept an older attempt's controller stamp was attributed to that older process. **Expected behavior** Each in-progress record names the owner that can still move it: a process proved by identity, or a fence whose expiry is never proof by itself. A stamp for a different attempt never names the owner. **Steps to reproduce** 1. Let workspace recovery claim a pending-cleanup lease (no controller stamp), or open a login session lease. 2. Run the ownership reconciliation pass. 3. See `no_owner_stamp` for the record, or a dead earlier controller for a claim that a live claimant holds. ## What Changed - `recovery/ownership-registry.ts`, `environment_login_probe_lease`: the owner is the holder process stamped by the write that makes the lease active (`metadata.leaseController`, added in #19964). A lease saved by the previous release still names no owner; it ends with its login session. - `recovery/ownership-registry.ts`, `environment_teardown_claim`: a controller stamp counts only when its `attemptId` equals the claim's `pendingCleanupAttemptId`. Any other claim with an attempt id is a fence: the attempt id is the token and `pendingCleanupLeaseExpiresAtMs` (renewed by the claimant) is the expiry. An expired fence stays unprovable. A claim with no attempt id names no owner. - Report-only. No settle is registered for these kinds in this change. ## Verification - `cd server && npx vitest run src/__tests__/environment-lease-owner-stamps.test.ts src/__tests__/ownership-reconciliation-report.test.ts` (embedded Postgres). - Red on the previous registry, green here (tests written after the code): - login/probe lease is owned by the process whose write made it active - login/probe lease is proven gone when that process has exited - teardown claim never reads a controller stamped for an earlier attempt (fence held by its token) - an unstamped teardown claim whose fence expired stays unprovable, never gone - Green on both: a stamped claim for this attempt with a dead PID is gone; an unstamped login lease and an attempt-less claim report I1. - Full typecheck is left to CI (local memory limit). ## Risks - Low risk. The change only alters the owner the report-only reconciler reads. No record is changed. - Stacked on #19964 (the lease holder stamp). Merge #19964 first. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edit, embedded Postgres tests). ## 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 --- .../environment-lease-owner-stamps.test.ts | 117 ++++++++++++++++++ .../services/recovery/ownership-registry.ts | 35 +++++- 2 files changed, 150 insertions(+), 2 deletions(-) create mode 100644 server/src/__tests__/environment-lease-owner-stamps.test.ts diff --git a/server/src/__tests__/environment-lease-owner-stamps.test.ts b/server/src/__tests__/environment-lease-owner-stamps.test.ts new file mode 100644 index 00000000000..7e494ec0569 --- /dev/null +++ b/server/src/__tests__/environment-lease-owner-stamps.test.ts @@ -0,0 +1,117 @@ +import { spawn } from "node:child_process"; +import { randomUUID } from "node:crypto"; +import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest"; +import { agents, companies, createDb, environmentLeases, environments } from "@paperclipai/db"; +import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "./helpers/embedded-postgres.js"; +import { currentControllerProcessStamp, currentProcessSpaceHash } from "../services/controller-process-identity.js"; +import { ENVIRONMENT_LEASE_CONTROLLER_KEY } from "../services/environment-lease-controller.js"; +import { environmentService } from "../services/environments.js"; +import { createOwnershipReportState, runOwnershipReconciliationPass } from "../services/recovery/ownership-invariant-report.js"; + +const support = await getEmbeddedPostgresTestSupport(); +const suite = support.supported ? describe : describe.skip; + +async function exitedPid(): Promise { + const child = spawn(process.execPath, ["-e", "setInterval(() => {}, 1000)"], { stdio: "ignore" }); + const pid = child.pid!; + await new Promise(resolve => { child.once("exit", () => resolve()); child.kill("SIGKILL"); }); + return pid; +} + +suite("environment lease owner stamps (login/probe lease, teardown claim)", () => { + let db!: ReturnType; + let temporary: Awaited> | null = null; + let companyId = ""; + let environmentId = ""; + + beforeAll(async () => { + temporary = await startEmbeddedPostgresTestDatabase("paperclip-environment-lease-owner-stamps-"); + db = createDb(temporary.connectionString); + companyId = randomUUID(); + environmentId = randomUUID(); + await db.insert(companies).values({ id: companyId, name: "Paperclip", issuePrefix: "ELS", + requireBoardApprovalForNewAgents: false }); + await db.insert(agents).values({ id: randomUUID(), companyId, name: "Coder", role: "engineer", status: "idle", + adapterType: "codex_local", adapterConfig: {}, runtimeConfig: {}, permissions: {} }); + await db.insert(environments).values({ id: environmentId, name: "Fake Sandbox", driver: "sandbox", + status: "active", config: { provider: "fake", image: "ubuntu:24.04" } }); + }, 30_000); + + afterEach(async () => { + await db.delete(environmentLeases); + }); + + afterAll(async () => { + await temporary?.cleanup(); + }); + + async function report(kind: string) { + const result = await runOwnershipReconciliationPass(db, { state: createOwnershipReportState(), record: false }); + return { kind: result.kinds.find(entry => entry.kind === kind)!, violations: result.violations.filter(entry => entry.kind === kind) }; + } + + async function claim(metadata: Record) { + const id = randomUUID(); + await db.insert(environmentLeases).values({ id, companyId, environmentId, status: "pending_cleanup", + provider: "fake", providerLeaseId: `sandbox://fake/${id}`, metadata: { pendingCleanupInFlight: true, ...metadata } }); + return id; + } + + describe("login/probe lease", () => { + it("is owned by the process whose write made it active", async () => { + await environmentService(db).acquireLease({ companyId, environmentId, provider: "fake", + providerLeaseId: "sandbox://fake/login" }); + const { kind, violations } = await report("environment_login_probe_lease"); + expect(kind).toMatchObject({ inProgress: 1, verdicts: { alive: 1 }, reasons: { current_process: 1 } }); + expect(violations).toEqual([]); + }); + + it("is proven gone when that process has exited", async () => { + await db.insert(environmentLeases).values({ companyId, environmentId, status: "active", metadata: { + [ENVIRONMENT_LEASE_CONTROLLER_KEY]: { bootId: "earlier-boot", pid: await exitedPid(), + hostHash: currentProcessSpaceHash(), processStartedAt: null } } }); + const { kind } = await report("environment_login_probe_lease"); + expect(kind).toMatchObject({ verdicts: { gone: 1 }, reasons: { pid_dead: 1 } }); + }); + + it("names no owner when the previous release saved it without a stamp", async () => { + await db.insert(environmentLeases).values({ companyId, environmentId, status: "active", metadata: {} }); + const { kind, violations } = await report("environment_login_probe_lease"); + expect(kind).toMatchObject({ verdicts: { unprovable: 1 }, reasons: { no_owner_stamp: 1 } }); + expect(violations.map(violation => violation.rule)).toEqual(["I1_owned"]); + }); + }); + + describe("teardown claim", () => { + it("is owned by the claimant process stamped for this exact attempt", async () => { + const attemptId = randomUUID(); + await claim({ pendingCleanupAttemptId: attemptId, pendingCleanupLeaseExpiresAtMs: Date.now() + 60_000, + pendingCleanupController: { bootId: "earlier-boot", pid: await exitedPid(), hostHash: currentProcessSpaceHash(), + processStartedAt: null, attemptId } }); + const { kind } = await report("environment_teardown_claim"); + expect(kind).toMatchObject({ verdicts: { gone: 1 }, reasons: { pid_dead: 1 } }); + }); + + it("never reads a controller stamped for an earlier attempt: the claim is a fence held by its token", async () => { + const attemptId = randomUUID(); + await claim({ pendingCleanupAttemptId: attemptId, pendingCleanupLeaseExpiresAtMs: Date.now() + 60_000, + pendingCleanupController: { ...currentControllerProcessStamp(), bootId: "earlier-boot", attemptId: randomUUID() } }); + const { kind, violations } = await report("environment_teardown_claim"); + expect(kind).toMatchObject({ verdicts: { alive: 1 }, reasons: { fence_held: 1 } }); + expect(violations).toEqual([]); + }); + + it("keeps an unstamped claim whose fence expired unprovable, never gone", async () => { + await claim({ pendingCleanupAttemptId: randomUUID(), pendingCleanupLeaseExpiresAtMs: Date.now() - 1 }); + const { kind } = await report("environment_teardown_claim"); + expect(kind).toMatchObject({ verdicts: { unprovable: 1 }, reasons: { fence_expired_without_provider_proof: 1 } }); + }); + + it("names no owner for an in-flight flag without an attempt", async () => { + await claim({}); + const { kind, violations } = await report("environment_teardown_claim"); + expect(kind).toMatchObject({ verdicts: { unprovable: 1 }, reasons: { no_owner_stamp: 1 } }); + expect(violations.map(violation => violation.rule)).toEqual(["I1_owned"]); + }); + }); +}); diff --git a/server/src/services/recovery/ownership-registry.ts b/server/src/services/recovery/ownership-registry.ts index e0fbe89ce7d..c4fc7da85b0 100644 --- a/server/src/services/recovery/ownership-registry.ts +++ b/server/src/services/recovery/ownership-registry.ts @@ -319,6 +319,34 @@ function runLeaseKind(): OwnableKind { }; } +/** + * Owner of an in-flight teardown claim. A claimant that stamps + * `pendingCleanupController` for this exact attempt is a process, proved by + * identity. Other claimants (workspace recovery, native export retry, and + * claims saved by an earlier release) hold the claim as a fence: the attempt + * id is the token and `pendingCleanupLeaseExpiresAtMs` is the deadline that + * the claimant renews. An expired fence is never proof by itself; another + * host takes over only through the fence. A controller stamp left by an + * earlier attempt names a different claimant and is ignored. + */ +function teardownClaimOwner(row: LeaseRow): RecordOwner { + const metadata = json(row.metadata); + const attemptId = str(metadata.pendingCleanupAttemptId); + const controller = json(metadata.pendingCleanupController); + if (attemptId && controller.attemptId === attemptId) { + const stamped = controllerStamp(controller, "environment_teardown_claim"); + if (stamped.kind !== "none") return stamped; + } + if (!attemptId) return { kind: "none" }; + const expiresAtMs = typeof metadata.pendingCleanupLeaseExpiresAtMs === "number" ? metadata.pendingCleanupLeaseExpiresAtMs : null; + return { + kind: "fence", + stamp: { kind: "remote", scope: "environment_teardown_claim", generation: null, fenceToken: attemptId }, + fence: { currentFenceToken: attemptId, currentGeneration: null, + leaseExpiresAt: expiresAtMs !== null && Number.isFinite(expiresAtMs) ? new Date(expiresAtMs) : null }, + }; +} + /** `ControllerProcess` metadata ({bootId, pid, hostHash, processStartedAt}). */ function controllerStamp(value: unknown, scope: string): RecordOwner { const controller = readControllerProcess(value); @@ -554,7 +582,7 @@ export const OWNABLE_KINDS: readonly OwnableKind[] = [ runLeaseKind(), leaseKind("environment_teardown_claim", "Teardown claim (in-flight cleanup)", "ownership", and(eq(environmentLeases.status, "pending_cleanup"), sql`(${environmentLeases.metadata}->>'pendingCleanupInFlight')::boolean is true`), - row => controllerStamp(json(row.metadata).pendingCleanupController, "environment_teardown_claim")), + teardownClaimOwner), leaseKind("environment_pending_cleanup", "Pending-cleanup obligation (not in flight)", "ownership", and(eq(environmentLeases.status, "pending_cleanup"), sql`coalesce((${environmentLeases.metadata}->>'pendingCleanupInFlight')::boolean, false) is false`), @@ -569,7 +597,10 @@ export const OWNABLE_KINDS: readonly OwnableKind[] = [ eq(environmentLeases.status, "retained"), () => ({ kind: "none" }), [1, 7]), spoolKind("sandbox_orphan_cleanup_spool", "Orphan cleanup buffer/spool", "ownership", idleOrphanSpoolPath), leaseKind("environment_login_probe_lease", "Login/probe lease", "ownership", - and(eq(environmentLeases.status, "active"), isNull(environmentLeases.heartbeatRunId)), () => ({ kind: "none" })), + and(eq(environmentLeases.status, "active"), isNull(environmentLeases.heartbeatRunId)), + // Stamped by the write that makes the lease active. A lease saved before + // the stamp existed names no owner; it ends with its login session. + row => controllerStamp(json(row.metadata)[ENVIRONMENT_LEASE_CONTROLLER_KEY], "environment_login_probe_lease")), // Workspaces operationKind("workspace_runtime_control_operation", "Managed runtime-control operation", "ownership",