mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-12 14:19:00 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Native runs keep their ownership in `native_run_finalizations`, and the shared ownership reconciler now settles proven-gone owners (Phase 2 of the durable ownership plan) > - The finalizer lease (`native-finalizer:*`, 5 min) and the retained-session cleanup lease (`native-cleanup:*`, 60 s) recorded only a random token. They named no holder process > - A server killed while it finalized or cleaned up kept the run waiting for the whole lease. The lease was released by age alone. A graceful shutdown also keeps an in-progress finalizer lease for the next boot to wait out > - This pull request records the holder process with each claim and moves both kinds onto the shared prover and a compare-and-swap settle > - The benefit is that a dead holder's run continues in the first pass after boot, and a live or unprovable holder is never released ## Linked Issues or Issue Description Refs #19933 (the settle hook and the first moved kind, merged). **What happened?** A finalizer or cleanup lease whose server process died stayed held until the lease expired (5 min or 60 s). The run stayed `running` and its follow-up work waited. **What did you expect?** The next boot proves the dead holder gone and frees the lease at once. A live holder, or one that cannot be proven gone, keeps its lease. **Steps to reproduce** 1. Accept a native result and let the finalizer claim it. 2. Kill the server (SIGKILL) before the finalizer commits. 3. Start the server. The run stays `running` for up to 5 minutes. ## What Changed - `native_run_finalizations.lease_holder` (jsonb, migration 0446): `{token, bootId, pid, hostHash, processStartedAt}` of the process that took a token-only lease. It names the holder only while `token` equals `lease_owner`, so renewals keep it and a later lease never inherits it. The executor's `controller_*` columns are not changed. - The finalizer claim (`native-run-finalizer.ts`) and the cleanup claim (`native-session-executor.ts`) write it in the same UPDATE as the lease, from `currentControllerProcessStamp()` (no I/O). - New `native-token-lease-settlement.ts`: compare-and-swap settle that expires exactly the proven-gone holder's outstanding lease, after it notifies the finalization or cleanup work that the expiry frees. - Both claims also wake the ownership worker queue in their transaction, so a quiet worker watches the new holder (Greptile round 1). - `ownership-registry.ts`: `native_finalizer_lease` and `native_cleanup_lease` register `settle`. Their owner is the holder process; rows without a holder keep the token fence. Listings cover outstanding leases only. - `heartbeat/recovery.ts`: restart recovery settles both kinds before it finalizes and schedules cleanup. A failure is logged and the reconciler pass retries it. - Two sparse partial indexes on `lease_expires_at` for the two listings, so the settle watch re-check stays indexed. Bespoke code deleted: none. These kinds had no liveness check; their only release was lease age (the takeover in the claim), which stays as the fence for unprovable holders. ## Verification - `cd server && npx vitest run src/__tests__/heartbeat-native-restart-suspension.test.ts -t "finalizer|cleanup|holder|exact lease"`: 12 passed (rerun after the rebase on master). The crash-point test also asserts the ownership wake for its run; it fails without the signal. - `cd server && npx vitest run src/__tests__/ownership-reconciliation-report.test.ts`: 17 passed. - Red on the base branch (server code at `bea3bfb20b`, tests and schema from this branch): 7 new tests fail, 5 pass (the 5 are existing tests). Red: the crash-point restart test, the two reconciler settle tests, the three live/unprovable counter-cases (owner was a token fence), and the exact-lease CAS test. The tests were written after the code. - New tests: - crash point: a finalizer claim was written and its holder killed before commit; the next restart finalizes the run, and this boot's claim stamps itself as holder; - a dead finalizer or cleanup holder is settled by a recording pass, never by a read-only pass (`record: false`); the accepted result and phase are untouched; - live counter-cases: a live earlier-boot PID (`pid_alive`), the current boot (`current_process`), a dead PID in another PID space (`different_process_space`, unprovable) keep the lease through a pass and a restart; - a holder written for another token, and a holderless row of the previous release, stay token fences, also with `PAPERCLIP_EPHEMERAL_SERVER=true`; - settle refuses a lease taken over after the proof (`owner_changed_since_proof`). - Not run locally: typecheck (CI runs it) and the wide suites. ## Risks - Previous-release story (rules/upgrade-safe-run-state.mdc): rows saved by the previous release have no `lease_holder`. They stay token fences and the existing claim takes them over at expiry. Neither lease outlives 5 min, so this covers at most 5 min after the upgrade. A rollback ignores the new column. - Unprovable holders (other host or PID space, unknown space): no operator action. The lease is short and self-expiring; at expiry the next claim takes it over and the old holder's next token compare-and-swap fails. That fence is the handoff. A different host is never proof. - The settle only expires a lease. Finalization and cleanup still run under their own checks (cleanup keeps its maintenance-history proofs). Settling ownership does not settle the accepted result, workspace preservation, or accounting. - Migration adds a nullable column and two partial indexes on a sparse predicate. Low risk. ## 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>
113 lines
6.6 KiB
TypeScript
113 lines
6.6 KiB
TypeScript
import { and, eq, gt, like, sql } from "drizzle-orm";
|
|
import { heartbeatRuns, nativeRunFinalizations, type Db } from "@paperclipai/db";
|
|
import { currentControllerProcessStamp, readControllerProcess, type ControllerProcess } from "../controller-process-identity.js";
|
|
import { DELIVERY_QUEUES, notifyDeliveryWork } from "../delivery-work-notifications.js";
|
|
import { notifyNativeFinalizationWork } from "../native-finalization-notifications.js";
|
|
import { notifyNativeResultCommit } from "../native-result-commit-notifications.js";
|
|
import { notifyRetainedNativeSessionCleanup } from "../retained-native-session-cleanup-notifications.js";
|
|
import type { LocalOwnerStamp, OwnerProof } from "../owner-proof.js";
|
|
import type { InProgressRecord, SettleOutcome } from "../recovery/ownership-registry.js";
|
|
|
|
/**
|
|
* Holder stamps and settle for the token-only native leases (plan Phase 2):
|
|
* the finalizer lease (`native-finalizer:*`, 5 min) and the retained-session
|
|
* cleanup lease (`native-cleanup:*`, 60 s).
|
|
*
|
|
* Both run inside one server process, but their claims used to record only a
|
|
* random token: the controller_* columns belong to the executor and must not
|
|
* be overwritten (the finalizer's attempt rule and restart recovery read
|
|
* them). A holder killed mid-lease therefore kept the run waiting for the
|
|
* whole TTL. The claim now writes `lease_holder` = this process's identity,
|
|
* bound to the token, in the same UPDATE (no I/O: `currentControllerProcessStamp`).
|
|
* The holder names the lease owner only while its token equals `lease_owner`,
|
|
* so renewals keep it and any later owner never inherits it.
|
|
*
|
|
* Previous release: rows it saved have no holder. They stay token fences, and
|
|
* the existing claim takes them over at expiry (at most 5 min / 60 s after the
|
|
* upgrade, since neither lease outlives that). Settle never touches them.
|
|
*/
|
|
|
|
export type NativeTokenLeaseKind = "native_finalizer_lease" | "native_cleanup_lease";
|
|
|
|
const PREFIX: Record<NativeTokenLeaseKind, string> = {
|
|
native_finalizer_lease: "native-finalizer:",
|
|
native_cleanup_lease: "native-cleanup:",
|
|
};
|
|
|
|
/** Wakes the ownership worker for a newly claimed token lease, so a quiet
|
|
* worker (on any server) starts watching the holder. Call it in the claim's
|
|
* transaction, before the write it announces. */
|
|
export async function notifyNativeTokenLeaseClaimed(tx: object, scope: { companyId: string; runId: string }) {
|
|
await notifyDeliveryWork(tx, DELIVERY_QUEUES.ownershipInvariantReport,
|
|
JSON.stringify({ companyId: scope.companyId, runId: scope.runId }));
|
|
}
|
|
|
|
/** The `lease_holder` value for a token-only lease claimed by this process. */
|
|
export function nativeLeaseHolder(token: string): Record<string, unknown> {
|
|
return { token, ...currentControllerProcessStamp() };
|
|
}
|
|
|
|
/** The process holding `token`, or null when the holder was written for
|
|
* another token (or not at all, on rows saved by the previous release). */
|
|
export function readNativeLeaseHolder(value: unknown, token: string | null | undefined): ControllerProcess | null {
|
|
if (!token || !value || typeof value !== "object" || (value as { token?: unknown }).token !== token) return null;
|
|
return readControllerProcess(value);
|
|
}
|
|
|
|
export function nativeLeaseHolderStamp(holder: ControllerProcess, scope: NativeTokenLeaseKind): LocalOwnerStamp {
|
|
return {
|
|
kind: "local", scope, generation: null, host: null, processSpace: holder.hostHash, bootId: holder.bootId,
|
|
pid: holder.pid, processStartedAt: holder.processStartedAt, processGroupId: null,
|
|
};
|
|
}
|
|
|
|
/**
|
|
* Called by the shared reconciler only after `proveOwnerGone` proved the
|
|
* recorded holder gone. Expires exactly that lease (compare-and-swap on the
|
|
* token while it is still outstanding and still held by the proven process),
|
|
* after announcing the work the expiry releases. It settles ownership only:
|
|
* the finalizer or cleanup that the notification wakes does the rest, under
|
|
* its own checks.
|
|
*/
|
|
export async function settleNativeTokenLease(kind: NativeTokenLeaseKind, db: Db, record: InProgressRecord,
|
|
context: { now: Date; proof: OwnerProof }): Promise<SettleOutcome> {
|
|
if (record.owner.kind !== "process") return { settled: false, reason: "owner_not_a_process" };
|
|
const stamp = record.owner.stamp;
|
|
const { now } = context;
|
|
return db.transaction(async transaction => {
|
|
const tx = transaction as unknown as Db;
|
|
const [row] = await tx.select({
|
|
runId: nativeRunFinalizations.runId, companyId: nativeRunFinalizations.companyId,
|
|
issueId: nativeRunFinalizations.issueId, decisionId: nativeRunFinalizations.decisionId,
|
|
agentId: heartbeatRuns.agentId, leaseOwner: nativeRunFinalizations.leaseOwner,
|
|
leaseHolder: nativeRunFinalizations.leaseHolder,
|
|
}).from(nativeRunFinalizations)
|
|
.innerJoin(heartbeatRuns, and(eq(heartbeatRuns.id, nativeRunFinalizations.runId),
|
|
eq(heartbeatRuns.companyId, nativeRunFinalizations.companyId)))
|
|
.where(and(eq(nativeRunFinalizations.runId, record.recordId), like(nativeRunFinalizations.leaseOwner, `${PREFIX[kind]}%`),
|
|
gt(nativeRunFinalizations.leaseExpiresAt, now)))
|
|
.for("update", { of: nativeRunFinalizations });
|
|
if (!row?.leaseOwner) return { settled: false, reason: "lease_no_longer_held" };
|
|
const holder = readNativeLeaseHolder(row.leaseHolder, row.leaseOwner);
|
|
// The proof judged this exact process; the lease has moved on since.
|
|
if (!holder || holder.bootId !== stamp.bootId || holder.pid !== stamp.pid || holder.hostHash !== stamp.processSpace
|
|
|| holder.processStartedAt !== (stamp.processStartedAt ?? null)) {
|
|
return { settled: false, reason: "owner_changed_since_proof" };
|
|
}
|
|
// Work intents before the write they announce (sleep-aware background work).
|
|
if (kind === "native_finalizer_lease") {
|
|
if (row.decisionId) await notifyNativeFinalizationWork(tx, { companyId: row.companyId, runId: row.runId });
|
|
else await notifyNativeResultCommit(tx, { companyId: row.companyId, runId: row.runId, issueId: row.issueId });
|
|
} else {
|
|
await notifyRetainedNativeSessionCleanup(tx, { companyId: row.companyId, runId: row.runId,
|
|
issueId: row.issueId, agentId: row.agentId });
|
|
}
|
|
const updated = await tx.update(nativeRunFinalizations).set({ leaseExpiresAt: now, updatedAt: now })
|
|
.where(and(eq(nativeRunFinalizations.runId, row.runId), eq(nativeRunFinalizations.leaseOwner, row.leaseOwner),
|
|
gt(nativeRunFinalizations.leaseExpiresAt, now),
|
|
sql`${nativeRunFinalizations.leaseHolder}->>'token' = ${row.leaseOwner}`))
|
|
.returning({ runId: nativeRunFinalizations.runId });
|
|
return updated.length ? { settled: true, reason: context.proof.reason } : { settled: false, reason: "lease_no_longer_held" };
|
|
});
|
|
}
|