mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-15 10:21:41 +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>
139 lines
6.1 KiB
TypeScript
139 lines
6.1 KiB
TypeScript
import { sql } from "drizzle-orm";
|
|
import {
|
|
check,
|
|
foreignKey,
|
|
pgTable,
|
|
uuid,
|
|
text,
|
|
timestamp,
|
|
integer,
|
|
index,
|
|
jsonb,
|
|
} from "drizzle-orm/pg-core";
|
|
import { companies } from "./companies.js";
|
|
import { issues } from "./issues.js";
|
|
import { heartbeatRuns } from "./heartbeat_runs.js";
|
|
import { nativeRunResults } from "./native_run_results.js";
|
|
import { statusDecisions } from "./status_decisions.js";
|
|
import { workAssessments } from "./work_assessments.js";
|
|
|
|
export const nativeRunFinalizations = pgTable(
|
|
"native_run_finalizations",
|
|
{
|
|
runId: uuid("run_id").primaryKey(),
|
|
companyId: uuid("company_id").notNull().references(() => companies.id),
|
|
issueId: uuid("issue_id").notNull(),
|
|
phase: text("phase").notNull(),
|
|
attempt: integer("attempt").notNull().default(0),
|
|
leaseOwner: text("lease_owner"),
|
|
leaseExpiresAt: timestamp("lease_expires_at", { withTimezone: true }),
|
|
// Server process that took a token-only lease (finalizer or cleanup):
|
|
// {token, bootId, pid, hostHash, processStartedAt}. It names the holder only
|
|
// while `token` equals lease_owner, so a later lease never inherits it. The
|
|
// controller_* columns stay the executor's. Null on rows saved before it existed.
|
|
leaseHolder: jsonb("lease_holder").$type<Record<string, unknown>>(),
|
|
controllerBootId: text("controller_boot_id"),
|
|
controllerPid: integer("controller_pid"),
|
|
// PID space of controllerPid (controller-process-identity hostHash). Null
|
|
// on rows saved before it existed.
|
|
controllerHostHash: text("controller_host_hash"),
|
|
controllerProcessStartedAt: timestamp("controller_process_started_at", {
|
|
withTimezone: true,
|
|
}),
|
|
controllerGeneration: integer("controller_generation").notNull().default(0),
|
|
recoveryState: text("recovery_state"),
|
|
recoveryRequestId: text("recovery_request_id"),
|
|
recoveryHistory: jsonb("recovery_history")
|
|
.$type<Array<Record<string, unknown>>>()
|
|
.notNull()
|
|
.default(sql`'[]'::jsonb`),
|
|
resultId: uuid("result_id"),
|
|
assessmentId: uuid("assessment_id"),
|
|
decisionId: uuid("decision_id"),
|
|
failureCode: text("failure_code"),
|
|
failureDetail: jsonb("failure_detail").$type<Record<string, unknown>>(),
|
|
controlDeadlineAt: timestamp("control_deadline_at", { withTimezone: true }),
|
|
nextAttemptAt: timestamp("next_attempt_at", { withTimezone: true }),
|
|
createdAt: timestamp("created_at", { withTimezone: true }).notNull().defaultNow(),
|
|
updatedAt: timestamp("updated_at", { withTimezone: true }).notNull().defaultNow(),
|
|
},
|
|
(table) => ({
|
|
pendingEffectOwnerIdx: index("native_run_finalizations_pending_effect_owner_idx")
|
|
.on(table.companyId, table.issueId, table.decisionId)
|
|
.where(sql`${table.decisionId} is not null AND ${table.resultId} is not null AND ${table.phase} <> 'terminal_failure'`),
|
|
unsafeExportRecoveryIdx: index("native_run_finalizations_unsafe_export_recovery_idx")
|
|
.on(table.companyId, table.issueId, table.runId)
|
|
.where(sql`${table.phase} = 'terminal_failure' AND ${table.failureCode} = 'native_workspace_sync_out_unsafe_archive'`),
|
|
// The `native_accepted_result_lease` ownership kind: outstanding executor
|
|
// leases on an accepted result that is not final yet (reconciler listing).
|
|
// The `native_finalizer_lease` and `native_cleanup_lease` ownership kinds
|
|
// (reconciler listing and settle watch). Both are sparse.
|
|
finalizerLeaseIdx: index("native_run_finalizations_finalizer_lease_idx")
|
|
.on(table.leaseExpiresAt)
|
|
.where(sql`${table.leaseOwner} like 'native-finalizer:%'`),
|
|
cleanupLeaseIdx: index("native_run_finalizations_cleanup_lease_idx")
|
|
.on(table.leaseExpiresAt)
|
|
.where(sql`${table.leaseOwner} like 'native-cleanup:%'`),
|
|
acceptedResultLeaseIdx: index("native_run_finalizations_accepted_result_lease_idx")
|
|
.on(table.leaseExpiresAt)
|
|
.where(sql`${table.resultId} is not null AND ${table.leaseOwner} is not null AND ${table.phase} not in ('committed', 'terminal_failure', 'applied')`),
|
|
controlDeadlineIdx: index("native_run_finalizations_control_deadline_idx")
|
|
.on(table.controlDeadlineAt).where(sql`${table.controlDeadlineAt} is not null`),
|
|
issueCompanyFk: foreignKey({
|
|
columns: [table.companyId, table.issueId],
|
|
foreignColumns: [issues.companyId, issues.id],
|
|
name: "native_run_finalizations_issue_company_fk",
|
|
}),
|
|
runOwnerFk: foreignKey({
|
|
columns: [table.companyId, table.issueId, table.runId],
|
|
foreignColumns: [heartbeatRuns.companyId, heartbeatRuns.nativeIssueId, heartbeatRuns.id],
|
|
name: "native_run_finalizations_run_owner_fk",
|
|
}),
|
|
resultOwnerFk: foreignKey({
|
|
columns: [table.companyId, table.issueId, table.runId, table.resultId],
|
|
foreignColumns: [
|
|
nativeRunResults.companyId,
|
|
nativeRunResults.issueId,
|
|
nativeRunResults.runId,
|
|
nativeRunResults.id,
|
|
],
|
|
name: "native_run_finalizations_result_owner_fk",
|
|
}),
|
|
assessmentOwnerFk: foreignKey({
|
|
columns: [table.companyId, table.issueId, table.runId, table.assessmentId],
|
|
foreignColumns: [
|
|
workAssessments.companyId,
|
|
workAssessments.issueId,
|
|
workAssessments.runId,
|
|
workAssessments.id,
|
|
],
|
|
name: "native_run_finalizations_assessment_owner_fk",
|
|
}),
|
|
decisionOwnerFk: foreignKey({
|
|
columns: [
|
|
table.companyId,
|
|
table.issueId,
|
|
table.runId,
|
|
table.assessmentId,
|
|
table.decisionId,
|
|
],
|
|
foreignColumns: [
|
|
statusDecisions.companyId,
|
|
statusDecisions.issueId,
|
|
statusDecisions.runId,
|
|
statusDecisions.assessmentId,
|
|
statusDecisions.id,
|
|
],
|
|
name: "native_run_finalizations_decision_owner_fk",
|
|
}),
|
|
assessmentRequiresResultCheck: check(
|
|
"native_run_finalizations_assessment_requires_result_check",
|
|
sql`${table.assessmentId} is null or ${table.resultId} is not null`,
|
|
),
|
|
decisionRequiresAssessmentCheck: check(
|
|
"native_run_finalizations_decision_requires_assessment_check",
|
|
sql`${table.decisionId} is null or ${table.assessmentId} is not null`,
|
|
),
|
|
}),
|
|
);
|