mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-06 23:20:53 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The server owns execution, Stop delivery, and usage settlement. > - A Stop can remain pending when the controller restarts. > - Recovery must reach the existing session without starting more paid work. > - Startup and budget fences currently block that cleanup, and interrupted runs leave the restart selection. > - This pull request gives exact audited Stop cleanup a shared authority check and records its real acknowledgement. > - The benefit is safe cleanup while usage debt remains honest. ## Linked Issues or Issue Description **What happened?** A pending native run-only Stop can retain an unfinished execution owner after controller loss. Its startup cancellation fence prevents recovery admission. If teardown marks the run interrupted and clears its task lock, restart selection also omits it. A recovered session can receive cancellation without the durable acknowledgement that the normal Stop request would save. **Expected behavior** Recover only the existing remote session for the audited pending Stop. Keep fresh provider work blocked, including during budget pause. Keep incomplete usage pending until a real complete receipt arrives. **Steps to reproduce** 1. Start a native run with a held reservation and an authenticated remote checkpoint. 2. Persist a run-only Stop and its startup and execution cancellation fences before acknowledgement. 3. Lose the controller. Leave the coordinator unfinished, mark the run interrupted, and clear its task lock. 4. Attempt restart recovery and observe the missing claim or cancellation admission rejection. Related work: #14733 binds caller Stop requests. #18658 handles plugin startup cleanup obligations. #18035 proposes broader accounting changes. This change repairs existing-session Stop recovery and preserves current accounting guards. ## What Changed - Add one shared check for the exact run, company, issue, pending Stop, intent audit, live recovery lease, generation, and remote reattachment disposition. - Include eligible interrupted Stop owners in restart selection. Keep generic interruptions and another task execution owner fenced. - Reuse only the original held reservation for cleanup. Keep budget pause effective for fresh, bootstrap, dead-runner, and unproven recovery work. - Apply the same cleanup authority at heartbeat runtime selection and native entry. Skip attachment staging and warm-owner retirement only for that exact cleanup. Revalidate at session publication and fail closed if the proof changed. - Send the existing Stop through the audited cancellation producer before turn submission. Close the coordinator only after acknowledgement, including system-owned run-only Stops. - Give verified cleanup a transport purpose that retains pending semantic inputs without replay, dispatch, or a semantic reply. Add an explicit Core retain-only intake mode; preserve authenticated identity, schema, digest, sequence and replay checks. Ordinary execution still requires its dispatcher. - Add PostgreSQL, native protocol, and startup-fence regressions. Update the execution contract and recovery maps. ## Verification - The combined Stop regression fails on unchanged production at the cancellation reservation guard. Interrupted restart selection also fails before the repair while its running sibling passes. - An assembled `heartbeatService.recoverNativeRunsAfterRestart` regression exposes the earlier runtime-selection fence with the rest of the repair present. The fixed path reaches selector/claim, runtime selection, original held-budget reuse during pause, dispatch, real native protocol, audited cancellation, and heartbeat settlement. It starts no provider turn, acquires no new sandbox, and does not reclaim the task lock. - Fail-if-called attachment staging and turn boundaries remain untouched. A retained warm-owner regression prevents retirement. Invalidation at session publication fails closed. Actual backend/driver constructor tests verify cleanup-purpose forwarding, absent default options, and denial under foreign ownership. - Missing usage retains accounting debt and the hold. A later complete receipt records one 25-cent charge and settles that same hold. Concurrent duplicate cleanup is denied. Both user-owned and system-owned Stops pass. - Real transport tests: 9 pass on Node 24.22.1. They preserve ordinary saved-tool and completion recovery, prove the authority-denial race before cancellation, and qualify cleanup with pending ordinary/completion inputs and either handler ordering. - Real authenticated Core intake: 11 pass. Callback-present and callback-absent retain-only cases persist and acknowledge unanswered input without dispatch, recovery or result. Default missing-dispatcher rejection and digest-integrity/suspension guards remain effective. Omitting the callback alone first failed by disconnecting; the explicit retain-only mode fixes that defect. - Repository `pnpm -r typecheck`, the server/runner-vendor build, map checks, and module boundary checks passed locally before integrating the latest retained-startup change. That integration passes 11 native startup/cleanup compatibility cases and 3 assembled recovery/accounting cases. The final broad affected-suite attempt was stopped without a verdict; it is incomplete. Full repository tests and build are delegated to current-head GitHub CI. - Review exposed an additional real-process close failure with unanswered input. The corrected path requires a fresh exact provider-stop receipt confirming exit, followed by durable event drain and exact runner suspension. All 11 focused transport cases pass, including missing/false exit-proof negatives and ordinary recovery siblings. Runner TypeScript and map validation pass after this correction. - The assembled test controls provider, workspace-transfer and GitHub bootstrap I/O. It does not qualify hosted credentials, physical provider termination, workspace preservation, or historical receipt recovery. Local socket-permission setup failures are separate from semantic regression results. No hosted replay has run on this repair. - CI exposed an ordinary factory-call compatibility regression and an obsolete authority-read test fixture. The repair preserves single-argument ordinary calls and updates the race fixture without changing its Stop assertions. The complete native executor suite now passes all 1,049 tests; the ordinary factory boundary and all five Stop-accounting cases pass. A separate local environment-default case twice exceeded its five-second wait while other work ran; its prior-head CI case passed. The same unchanged case passed an isolated diagnostic without changing its assertion or timeout; the precise contention source remains unproven. - The final fixture edit initially used the wrong overloaded promise type. CI rejected it; the annotation-only correction passes the exact `pnpm run typecheck:build-gaps` entry point locally, including server service tests and remaining workspaces. No runtime assertion changed. - Final exact head `007fd4d04a2d6338c6dc073e97ab470a277fc3d0`: required `ci / verify` and `ci / e2e`, build, typecheck and test groups pass. Greptile is 5/5 with both findings resolved. Three expected checks are skipped; the optional native check is neutral, while its final current-head comment gives 5/5 from source inspection without a checkout or test execution. The first server-shard attempt lost its self-hosted runner connection; one scoped rerun passed. Its original annotation and unavailable-log evidence are retained. ## Risks - This changes execution authority. Cleanup requires an audited, exactly bound pending Stop, an unfinished coordinator, a live remote reattachment claim, and the original held reservation. It never authorizes a replacement provider turn. - Missing checkpoint, original held reservation, termination, or usage evidence can keep cleanup or accounting pending. The repair does not manufacture acknowledgement, replies, receipts, or zero cost. - Retain-only intake is an internal trusted constructor option. It changes no agent tool permission. Ordinary callback requirements and semantic dispatch remain unchanged. - No schema, public API, provider profile, or deployment setting changes are required. ## Model Used OpenAI Codex, `gpt-6.1-sol`, Extra High reasoning, with source inspection, code execution, and regression testing. The context-window capacity is not exposed in this session. ## 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 described the issue in-PR following the bug report template - [x] I have not referenced internal/instance-local Paperclip issues or links - [x] My branch name describes the change and contains no internal ticket ID - [x] I have run affected 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 risks - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open findings - [x] I will address all reviewer comments before merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
139 lines
10 KiB
TypeScript
139 lines
10 KiB
TypeScript
import { and, eq, sql } from "drizzle-orm";
|
|
import { agents, budgetReservations, heartbeatRuns, nativeRunFinalizations, type Db } from "@paperclipai/db";
|
|
import { centsToUnits, unitsToCents } from "@paperclipai/shared";
|
|
import { conflict, notFound } from "../errors.js";
|
|
import { withAccountingTransaction } from "./accounting-transaction.js";
|
|
import { budgetServiceInTransaction } from "./budgets.js";
|
|
import { withBudgetSpan } from "./budget-tracing.js";
|
|
import type { ActivityPublication } from "./activity-log.js";
|
|
import { readClaimedNativeStopRecovery } from "./native-runtime/native-cancellation-request.js";
|
|
|
|
/** Native finalization takes its coordinator before issue/run locks. Recovery
|
|
* admission uses the same order after the company accounting lock, so its
|
|
* later ownership checks never wait for a coordinator while holding a run. */
|
|
export async function lockRunBudgetRecoveryOwnerInTransaction(
|
|
tx: Db, companyId: string, runId: string, recoveryLeaseOwner?: string,
|
|
) {
|
|
if (!recoveryLeaseOwner) return;
|
|
const [coordinator] = await tx.select({ leaseOwner: nativeRunFinalizations.leaseOwner, leaseExpiresAt: nativeRunFinalizations.leaseExpiresAt }).from(nativeRunFinalizations)
|
|
.where(and(eq(nativeRunFinalizations.companyId, companyId), eq(nativeRunFinalizations.runId, runId))).for("update");
|
|
if (!coordinator || coordinator.leaseOwner !== recoveryLeaseOwner || !coordinator.leaseExpiresAt || coordinator.leaseExpiresAt.getTime() <= Date.now())
|
|
throw conflict("Native recovery no longer owns provider dispatch");
|
|
}
|
|
|
|
/** Called by the final dispatch gate with the company and run already locked.
|
|
* Admission may have waited through preparation; that earlier reservation is
|
|
* evidence of ownership, not permission to bypass a newer charge or pause.
|
|
* Recovery callers prelock their coordinator before taking issue/run locks. */
|
|
export async function recheckRunBudgetReservationInTransaction(
|
|
tx: Db,
|
|
publications: ActivityPublication[],
|
|
run: typeof heartbeatRuns.$inferSelect,
|
|
reservationId: string,
|
|
recoveryLeaseOwner?: string,
|
|
): Promise<string | null> {
|
|
const stopRecovery = await readClaimedNativeStopRecovery(tx, run, recoveryLeaseOwner);
|
|
if ((run.resultJson?.cancellation || run.resultJson?.startupCancellation) && !stopRecovery)
|
|
return "Run cancellation already requested";
|
|
if (run.costAccountedAt || !["queued", "running"].includes(run.status))
|
|
return "Run can no longer start provider work";
|
|
const [reservation] = await tx.select().from(budgetReservations).where(and(
|
|
eq(budgetReservations.id, reservationId), eq(budgetReservations.companyId, run.companyId),
|
|
eq(budgetReservations.runId, run.id), eq(budgetReservations.agentId, run.agentId),
|
|
eq(budgetReservations.state, "held"),
|
|
)).for("update");
|
|
if (!reservation) return "Provider dispatch no longer owns a held budget reservation";
|
|
if (recoveryLeaseOwner) {
|
|
const [owner] = run.runtimeMode === "native" ? await tx.select({ runId: nativeRunFinalizations.runId })
|
|
.from(nativeRunFinalizations).where(and(eq(nativeRunFinalizations.companyId, run.companyId),
|
|
eq(nativeRunFinalizations.runId, run.id), eq(nativeRunFinalizations.leaseOwner, recoveryLeaseOwner),
|
|
sql`${nativeRunFinalizations.leaseExpiresAt} > clock_timestamp()`)) : [];
|
|
if (!owner) return "Native recovery no longer owns provider dispatch";
|
|
}
|
|
if (stopRecovery) return null;
|
|
const { block, policies, observations } = await budgetServiceInTransaction(tx, publications)
|
|
.evaluateAdmission(run.companyId, run.agentId, { projectId: reservation.projectId });
|
|
if (block) return block.reason;
|
|
const [agent] = await tx.select({ status: agents.status, lifecycleState: agents.lifecycleState }).from(agents)
|
|
.where(and(eq(agents.companyId, run.companyId), eq(agents.id, run.agentId)));
|
|
if (!agent || agent.lifecycleState !== "ready" || ["paused", "terminated", "pending_approval"].includes(agent.status))
|
|
return "Agent is paused or its lifecycle is not ready for new work";
|
|
for (const [index, policy] of policies.entries()) {
|
|
const observed = observations[index];
|
|
// This operation's hold is already included in exposure. Do not propose it
|
|
// twice, or reject a nonzero reservation that exactly fills the budget.
|
|
const committed = centsToUnits(observed.ledgerCents) + centsToUnits(observed.effectiveHeldCents);
|
|
const limit = centsToUnits(policy.amount);
|
|
if (committed > limit || (centsToUnits(reservation.amountCents) === 0n && committed >= limit))
|
|
return "Available budget is reserved by unfinished runs";
|
|
}
|
|
return null;
|
|
}
|
|
|
|
/** Reserve before dispatch, under the same company lock as charges and policy
|
|
* changes. Estimates constrain admission; they cannot cap a provider's bill.
|
|
* A reservation survives timeouts and restarts until accounting proves closure. */
|
|
export async function reserveRunBudget(db: Db, companyId: string, runId: string, projectId: string | null, ledgerScope: Record<string, unknown> = {}, recoveryLeaseOwner?: string) {
|
|
const result = await withBudgetSpan("budget.admission.reserve", () => withAccountingTransaction(db, companyId, async (tx, publications) => {
|
|
await lockRunBudgetRecoveryOwnerInTransaction(tx, companyId, runId, recoveryLeaseOwner);
|
|
const [run] = await tx.select().from(heartbeatRuns).where(and(eq(heartbeatRuns.id, runId), eq(heartbeatRuns.companyId, companyId))).for("update");
|
|
if (!run) throw notFound("Run not found");
|
|
const stopRecovery = await readClaimedNativeStopRecovery(tx, run, recoveryLeaseOwner);
|
|
if ((run.resultJson?.cancellation || run.resultJson?.startupCancellation) && !stopRecovery) throw conflict("Run cancellation already requested");
|
|
if (run.costAccountedAt || !["queued", "running"].includes(run.status)) throw conflict("Run can no longer start provider work");
|
|
const [existing] = await tx.select().from(budgetReservations).where(and(eq(budgetReservations.companyId, companyId), eq(budgetReservations.runId, runId)));
|
|
if (existing) {
|
|
// The native coordinator fences recovery ownership. Reuse the original
|
|
// hold only for that live owner, never for an ordinary duplicate dispatch.
|
|
const [owner] = recoveryLeaseOwner && run.runtimeMode === "native" && existing.state === "held"
|
|
&& existing.projectId === projectId ? await tx.select({ runId: nativeRunFinalizations.runId })
|
|
.from(nativeRunFinalizations).where(and(eq(nativeRunFinalizations.companyId, companyId),
|
|
eq(nativeRunFinalizations.runId, runId), eq(nativeRunFinalizations.leaseOwner, recoveryLeaseOwner),
|
|
sql`${nativeRunFinalizations.leaseExpiresAt} > clock_timestamp()`)) : [];
|
|
if (owner) {
|
|
if (stopRecovery) return { ...existing, reused: true };
|
|
// Recovery has already made this run queued/running, so its own pending
|
|
// accounting is excluded. Current overruns, other unfinished runs and
|
|
// scope pauses still apply before any more paid work starts.
|
|
const block = await budgetServiceInTransaction(tx, publications).getInvocationBlock(companyId, run.agentId, { projectId });
|
|
if (block) return { admissionBlockReason: block.reason };
|
|
const [agent] = await tx.select({ status: agents.status, lifecycleState: agents.lifecycleState }).from(agents)
|
|
.where(and(eq(agents.companyId, companyId), eq(agents.id, run.agentId)));
|
|
if (!agent || agent.lifecycleState !== "ready" || ["paused", "terminated", "pending_approval"].includes(agent.status))
|
|
return { admissionBlockReason: "Agent is paused and cannot start new work" };
|
|
return { ...existing, reused: true };
|
|
}
|
|
throw conflict("Provider dispatch has already reserved this run");
|
|
}
|
|
if (stopRecovery) throw conflict("Native Stop recovery requires the original held reservation");
|
|
const { block, policies, observations } = await budgetServiceInTransaction(tx, publications).evaluateAdmission(companyId, run.agentId, { projectId });
|
|
if (block) return { admissionBlockReason: block.reason };
|
|
// A budget release can start asynchronous resume hooks. Capacity alone
|
|
// does not authorize dispatch before that lifecycle operation is ready.
|
|
const [agent] = await tx.select({ status: agents.status, lifecycleState: agents.lifecycleState }).from(agents)
|
|
.where(and(eq(agents.companyId, companyId), eq(agents.id, run.agentId)));
|
|
if (!agent || agent.lifecycleState !== "ready" || ["paused", "terminated", "pending_approval"].includes(agent.status))
|
|
return { admissionBlockReason: "Agent is paused or its lifecycle is not ready for new work" };
|
|
const amount = policies.reduce((max, policy) => { const next = centsToUnits(policy.reservationCents); return next > max ? next : max; }, 0n);
|
|
for (const [index, policy] of policies.entries()) {
|
|
const observed = observations[index];
|
|
// Even zero-estimate runs cannot enter a scope whose available capacity
|
|
// is completely reserved. Outstanding reservations carry across months.
|
|
const committed = centsToUnits(observed.ledgerCents) + centsToUnits(observed.effectiveHeldCents);
|
|
const limit = centsToUnits(policy.amount);
|
|
if (committed >= limit || committed + amount > limit) return { admissionBlockReason: "Available budget is reserved by unfinished runs" };
|
|
}
|
|
// A zero-valued row also fences duplicate dispatch when estimates are off.
|
|
const [reservation] = await tx.insert(budgetReservations).values({ companyId, runId, agentId: run.agentId, projectId,
|
|
amountCents: unitsToCents(amount), providerStartedAt: new Date() }).returning();
|
|
await tx.update(heartbeatRuns).set({ costAccountingPending: true,
|
|
usageJson: sql`coalesce(${heartbeatRuns.usageJson}, '{}'::jsonb) || ${JSON.stringify({ accountingReceiptReady: false, ledgerScope: { ...(run.usageJson?.ledgerScope as object ?? {}), ...ledgerScope, projectId } })}::jsonb`,
|
|
}).where(eq(heartbeatRuns.id, runId));
|
|
return { ...reservation, reused: false };
|
|
}));
|
|
// Commit required block/resume transitions even when dispatch is denied.
|
|
// Throwing inside the transaction would discard those durable obligations.
|
|
if ("admissionBlockReason" in result) throw conflict(result.admissionBlockReason ?? "Budget admission blocked");
|
|
return result;
|
|
}
|