mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-09 07:10:34 +01:00
fix(accounting): close a remote run's lost receipt once its sandboxes are terminated (#19606)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Budget admission waits while completed runs await accounting; #19497 made that a hold on new work, never a hard stop > - #19497 also closes a lost usage receipt once a local provider is verified stopped, so the hold ends > - A remote (Daytona) run has no local stop event and keeps its recorded remote process identity, so its lost receipt never closed. The company then held all new work until a receipt that can no longer arrive > - This pull request closes a remote run's lost receipt once every sandbox it used carries a provider termination receipt > - The benefit is that a restart that kills a Daytona turn no longer stops a company's new work ## Linked Issues or Issue Description Follow-up to #19497. Refs #19544 (Daytona chaos harness). **What happened?** In a Daytona restart wave, run `a261c30b` failed after native recovery was blocked. Its usage receipt stayed incomplete (`accountingReceiptReady: false`). Both of its sandbox leases were released with a `destroyed` termination receipt, but accounting stayed pending. Admission then refused new company work with "New work must wait while a native run recovers its unfinished accounting". **What did you expect?** Once every sandbox of the run is stopped or destroyed, no runner can report usage for it. The known part of its usage should settle as an unpriced charge, as it does for a verified local stop, and new work should be admitted. **Steps to reproduce** 1. Run a native task on a Daytona environment with a hard-stop budget. 2. Kill the control plane during the provider turn so the turn's receipt is lost and native recovery ends in terminal failure. 3. After the sandbox lease is released with a termination receipt, observe that the run stays `cost_accounting_pending` and new work is held. ## What Changed - `run-cost-accounting.ts`: the closed-receipt predicate gains a remote branch. A native run in terminal failure without a result (failed, cancelled, timed out or interrupted) is closed when it has at least one environment lease and every lease carries a `paperclip.remote-termination.v1` receipt bound to that lease, run, company, provider and provider lease ID, with state `stopped` or `destroyed`, after a successful release. This mirrors `hasRemoteTerminationReceipt`. One SQL predicate still serves the reconciler scan and settlement. - The local branch is unchanged: a failed run with a verified local stop event and a cleared process identity. - A sandbox that may still run (no receipt, lease still active), a receipt bound to another sandbox, or a run with no lease keeps its debt pending. - `budget-enforcement-worker.ts`: the accounting worker also wakes when environment-cleanup or retained-session-cleanup work commits. Those are the signals the lease writers already send when they release or stop a sandbox. - `doc/SPEC-implementation.md`: documents the remote closure rule. ## Verification - `closed-native-receipt-accounting.test.ts` (embedded Postgres): - A failed and a cancelled remote run, with one destroyed and one stopped sandbox, settle as one unpriced charge, settle the reservation, and clear admission. These are red on master. - Debt stays pending for an active sandbox with no receipt, for one of two sandboxes without a receipt, for a receipt bound to another sandbox, and for a run with no lease. - `budget-enforcement-worker.test.ts`: the accounting worker wakes on environment-cleanup and retained-session-cleanup commits, and stops waking after `stop()`. These are red on master. - Existing suites pass locally: native-stop-accounting, budget-enforcement-scoping, heartbeat-cost-accounting, run-cost-accounting, cost-data-quality, budgets-service. Server typecheck passes. - Live Daytona restart wave: results in the comments. ## Risks - A cancelled remote run now closes once its sandboxes are terminated. Stop recovery could otherwise reattach a recorder, but a stopped or destroyed sandbox has no runner to reattach to. Local cancelled runs keep their drain. - Workspace export can restart a stopped sandbox and removes its termination receipt first. A receipt that arrives after settlement still posts through late-receipt reconciliation. - Reusable-lease chains (`resolveRemoteTerminationLease`) and recovered-cleanup stop receipts are not mirrored, so those runs stay pending. That is the conservative side. - Low query cost: the predicate runs only for pending terminal runs. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file editing, embedded Postgres tests, live restart-chaos harness). ## 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>
This commit is contained in:
1 parent
b8d94b3ad1
commit
6d1ecfc420
5 files changed
+122
-19
No files matched your search
@@ -1884,7 +1884,10 @@ to block budget admission.
|
||||
After native recovery reaches terminal failure, an incomplete receipt holds new
|
||||
work as recovering accounting; it does not open a hard-stop incident. When the
|
||||
run no longer records a provider process (recovery clears it only after a
|
||||
verified stop), no producer can deliver the receipt. Accounting then records
|
||||
verified stop), no producer can deliver the receipt. A remote run reaches the
|
||||
same point when every sandbox allocation it used carries a provider
|
||||
termination receipt (stopped or destroyed); a sandbox that may still run keeps
|
||||
the debt pending. Accounting then records
|
||||
its known part as an unpriced charge, so the unpriced-usage policy governs
|
||||
admission. A later receipt still posts through late-receipt reconciliation.
|
||||
A controller that detaches for server restart loses checkpoint and completion
|
||||
|
||||
@@ -147,6 +147,25 @@ describe("budget accounting delivery", () => {
|
||||
expect(work.accounting).toHaveBeenCalledTimes(3);
|
||||
});
|
||||
|
||||
// A sandbox stop or lease release can close a remote run's lost receipt.
|
||||
it.each([DELIVERY_QUEUES.environmentCleanup, DELIVERY_QUEUES.retainedNativeSessionCleanup])(
|
||||
"wakes accounting when %s work commits", async queue => {
|
||||
const s = setup();
|
||||
const recover = vi.fn(async () => {});
|
||||
const worker = registerCostAccountingWork(s.db, s.coordinator, recover);
|
||||
subscriptions.push(worker.stop);
|
||||
await worker.ready;
|
||||
const before = recover.mock.calls.length;
|
||||
await s.owner.transaction(tx => notifyDeliveryWork(tx as object, queue));
|
||||
await vi.advanceTimersByTimeAsync(1);
|
||||
expect(recover.mock.calls.length).toBeGreaterThan(before);
|
||||
worker.stop();
|
||||
const stopped = recover.mock.calls.length;
|
||||
await s.owner.transaction(tx => notifyDeliveryWork(tx as object, queue));
|
||||
await vi.advanceTimersByTimeAsync(1);
|
||||
expect(recover).toHaveBeenCalledTimes(stopped);
|
||||
});
|
||||
|
||||
it("replays durable spool work after a pre-transaction failure and releases its local wake on stop", async () => {
|
||||
const s = setup();
|
||||
const recover = vi.fn(async () => {});
|
||||
|
||||
@@ -2,7 +2,8 @@ import { randomUUID } from "node:crypto";
|
||||
import { and, eq, sql } from "drizzle-orm";
|
||||
import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest";
|
||||
import {
|
||||
agents, budgetIncidents, budgetPolicies, budgetReservations, companies, costEvents, createDb, heartbeatRunEvents, heartbeatRuns, issues,
|
||||
agents, budgetIncidents, budgetPolicies, budgetReservations, companies, costEvents, createDb, environmentLeases, environments,
|
||||
heartbeatRunEvents, heartbeatRuns, issues,
|
||||
nativeRunFinalizations, type Db,
|
||||
} from "@paperclipai/db";
|
||||
import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "./helpers/embedded-postgres.js";
|
||||
@@ -210,6 +211,57 @@ const support = await getEmbeddedPostgresTestSupport();
|
||||
expect(policy.id).toBe(f.policy.id);
|
||||
});
|
||||
|
||||
// Daytona wave: a remote run's lost receipt held all new company work even
|
||||
// after its sandbox was destroyed. A remote run has no local stop event and
|
||||
// keeps its recorded remote process identity.
|
||||
async function remoteFixture(options: { status?: "failed" | "cancelled"; leases?: Array<{
|
||||
receipt?: "destroyed" | "stopped" | "none" | "foreign"; released?: boolean;
|
||||
}> } = {}) {
|
||||
const f = await fixture({ processPid: 443, stopEvidence: false });
|
||||
if (options.status) await db.update(heartbeatRuns).set({ status: options.status }).where(eq(heartbeatRuns.id, f.run.id));
|
||||
const [environment] = await db.insert(environments).values({ name: `Daytona ${f.run.id}`, driver: "sandbox",
|
||||
config: { provider: "daytona" } }).returning();
|
||||
for (const lease of options.leases ?? [{ receipt: "destroyed" }]) {
|
||||
const id = randomUUID(), providerLeaseId = randomUUID(), released = lease.released !== false;
|
||||
const receipt = lease.receipt ?? "destroyed";
|
||||
await db.insert(environmentLeases).values({ id, environmentId: environment.id, companyId: f.companyId,
|
||||
issueId: f.issue.id, heartbeatRunId: f.run.id, provider: "daytona", providerLeaseId, leasePolicy: "ephemeral",
|
||||
status: released ? "expired" : "active", cleanupStatus: released ? "success" : null,
|
||||
releasedAt: released ? new Date() : null,
|
||||
metadata: receipt === "none" ? {} : { remoteExecutionTermination: { schema: "paperclip.remote-termination.v1",
|
||||
companyId: f.companyId, runId: f.run.id, leaseId: id, provider: "daytona",
|
||||
providerLeaseId: receipt === "foreign" ? randomUUID() : providerLeaseId,
|
||||
state: receipt === "foreign" ? "destroyed" : receipt, confirmedAt: new Date().toISOString() } } });
|
||||
}
|
||||
return f;
|
||||
}
|
||||
|
||||
it.each(["failed", "cancelled"] as const)("settles a %s remote run once every sandbox has a termination receipt", async status => {
|
||||
const f = await remoteFixture({ status, leases: [{ receipt: "destroyed" }, { receipt: "stopped" }] });
|
||||
expect(await f.block()).not.toBeNull();
|
||||
expect(await reconcileRunCosts(db)).toMatchObject({ accounted: 1 });
|
||||
await flushBudgetEvaluationWork(db);
|
||||
expect((await f.readRun()).costAccountingPending).toBe(false);
|
||||
expect(await f.charges()).toMatchObject([{ costStatus: "unpriced" }]);
|
||||
expect((await f.reservation()).state).toBe("settled");
|
||||
expect(await f.block()).toBeNull();
|
||||
});
|
||||
|
||||
it.each([
|
||||
["a sandbox that may still run", [{ receipt: "none", released: false }]],
|
||||
// Older plugins release successfully without returning a receipt.
|
||||
["a released sandbox without a termination receipt", [{ receipt: "none" }]],
|
||||
["one of two sandboxes without a receipt", [{ receipt: "destroyed" }, { receipt: "none", released: false }]],
|
||||
["a receipt for another sandbox", [{ receipt: "foreign" }]],
|
||||
["no sandbox at all", []],
|
||||
] as const)("keeps a remote run's debt pending with %s", async (_label, leases) => {
|
||||
const f = await remoteFixture({ leases: [...leases] });
|
||||
expect(await reconcileRunCosts(db)).toMatchObject({ accounted: 0 });
|
||||
expect(await accountRunCost(db, f.run.id)).toBe(false);
|
||||
expect((await f.readRun()).costAccountingPending).toBe(true);
|
||||
expect(await f.charges()).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("keeps the debt pending while native recovery can still produce a result", async () => {
|
||||
const f = await fixture({ phase: "observed" });
|
||||
expect(await reconcileRunCosts(db)).toMatchObject({ accounted: 0 });
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import type { Db } from "@paperclipai/db";
|
||||
import { withBudgetSpan } from "./budget-tracing.js";
|
||||
import type { createDeliveryWorkCoordinator } from "./delivery-work-coordinator.js";
|
||||
import { DELIVERY_QUEUES } from "./delivery-work-notifications.js";
|
||||
import { DELIVERY_QUEUES, subscribeDeliveryWork } from "./delivery-work-notifications.js";
|
||||
import {
|
||||
deliverBudgetEnforcement,
|
||||
hasPendingBudgetEnforcement,
|
||||
@@ -87,8 +87,13 @@ export function registerCostAccountingWork(db: Db, coordinator: Coordinator, rec
|
||||
if (event.type === "heartbeat.run.status" &&
|
||||
["succeeded", "failed", "timed_out", "cancelled", "interrupted"].includes(String(event.payload.status))) worker.wake();
|
||||
});
|
||||
// A released or stopped sandbox can close a terminal run's lost receipt.
|
||||
// Those lease writers signal environment or retained-session cleanup.
|
||||
const unsubscribeLeases = [DELIVERY_QUEUES.environmentCleanup, DELIVERY_QUEUES.retainedNativeSessionCleanup]
|
||||
.map(queue => subscribeDeliveryWork(db, queue, worker.wake));
|
||||
const stop = () => {
|
||||
unsubscribe();
|
||||
for (const unsubscribeLease of unsubscribeLeases) unsubscribeLease();
|
||||
if (accountingWakes.get(db) === worker.wake) accountingWakes.delete(db);
|
||||
};
|
||||
return { ...worker, stop };
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import { notifyStrandedAgentStatus } from "./live-run-recovery-notifications.js";
|
||||
import { usdToUnits, unitsToCents } from "@paperclipai/shared";
|
||||
import { and, asc, eq, inArray, or, sql } from "drizzle-orm";
|
||||
import { budgetReservations, agents, heartbeatRunEvents, heartbeatRuns, nativeRunFinalizations, issues, projects, decisionInvocations, promptInjectionInvocations, type Db } from "@paperclipai/db";
|
||||
import { budgetReservations, agents, environmentLeases, heartbeatRunEvents, heartbeatRuns, nativeRunFinalizations, issues, projects, decisionInvocations, promptInjectionInvocations, type Db } from "@paperclipai/db";
|
||||
import { withOperationAccountingTransaction } from "./accounting-transaction.js";
|
||||
import { withBudgetSpan } from "./budget-tracing.js";
|
||||
import { conflict } from "../errors.js";
|
||||
@@ -39,25 +39,49 @@ async function readAccountingClosure(db: Db, runId: string) {
|
||||
return closure;
|
||||
}
|
||||
|
||||
/** A failed native run's receipt source is closed only with positive stop
|
||||
* evidence: recovery ended in terminal failure, the run no longer records a
|
||||
* process identity, and its latest server-authored process event is the
|
||||
* verified local stop. A deadline failure whose process still runs keeps its
|
||||
* identity and has no stop event, so it stays pending. Missing identity alone is not proof. A cancelled run
|
||||
* keeps its accounting drain: Stop recovery can still deliver the final
|
||||
* receipt through the original or a reattached recorder. Remote runs keep
|
||||
* their debt until a receipt arrives. One predicate serves the reconciler
|
||||
/** Every remote allocation of the run carries a provider termination receipt
|
||||
* bound to that exact lease and run (mirrors hasRemoteTerminationReceipt): the
|
||||
* sandbox is stopped or destroyed, so no runner there can report usage. The
|
||||
* per-lease check uses "is not true": a missing receipt makes it NULL, which
|
||||
* must never count as proof. */
|
||||
const remoteExecutionTerminated = sql`exists (select 1 from ${environmentLeases} remote_lease
|
||||
where remote_lease.company_id = ${heartbeatRuns.companyId} and remote_lease.heartbeat_run_id = ${heartbeatRuns.id})
|
||||
and not exists (select 1 from ${environmentLeases} remote_lease
|
||||
where remote_lease.company_id = ${heartbeatRuns.companyId} and remote_lease.heartbeat_run_id = ${heartbeatRuns.id}
|
||||
and (remote_lease.provider is not null and remote_lease.provider <> 'local'
|
||||
and remote_lease.provider_lease_id is not null and remote_lease.released_at is not null
|
||||
and remote_lease.cleanup_status = 'success' and remote_lease.status in ('released', 'expired', 'failed')
|
||||
and remote_lease.metadata->'remoteExecutionTermination'->>'schema' = 'paperclip.remote-termination.v1'
|
||||
and remote_lease.metadata->'remoteExecutionTermination'->>'companyId' = remote_lease.company_id::text
|
||||
and remote_lease.metadata->'remoteExecutionTermination'->>'runId' = remote_lease.heartbeat_run_id::text
|
||||
and remote_lease.metadata->'remoteExecutionTermination'->>'leaseId' = remote_lease.id::text
|
||||
and remote_lease.metadata->'remoteExecutionTermination'->>'provider' = remote_lease.provider
|
||||
and remote_lease.metadata->'remoteExecutionTermination'->>'providerLeaseId' = remote_lease.provider_lease_id
|
||||
and remote_lease.metadata->'remoteExecutionTermination'->>'state' in ('stopped', 'destroyed')) is not true)`;
|
||||
|
||||
/** A native run's receipt source is closed only with positive stop evidence,
|
||||
* after recovery ended in terminal failure without a result:
|
||||
* - Local: the run no longer records a process identity and its latest
|
||||
* server-authored process event is the verified local stop. Only a failed
|
||||
* run closes this way; a cancelled local run keeps its accounting drain,
|
||||
* since Stop recovery can still reattach a recorder.
|
||||
* - Remote: every allocation has a termination receipt for its sandbox, so
|
||||
* neither the original nor a reattached runner can report usage. That proof
|
||||
* does not clear the recorded remote process identity.
|
||||
* Missing identity alone is never proof. One predicate serves the reconciler
|
||||
* scan and settlement so they cannot disagree. */
|
||||
const closedNativeReceiptSource = sql`${heartbeatRuns.runtimeMode} = 'native' and ${heartbeatRuns.status} = 'failed'
|
||||
and ${heartbeatRuns.processPid} is null and ${heartbeatRuns.processGroupId} is null
|
||||
const closedNativeReceiptSource = sql`${heartbeatRuns.runtimeMode} = 'native'
|
||||
and exists (select 1 from ${nativeRunFinalizations} where ${nativeRunFinalizations.runId} = ${heartbeatRuns.id}
|
||||
and ${nativeRunFinalizations.companyId} = ${heartbeatRuns.companyId}
|
||||
and ${nativeRunFinalizations.resultId} is null and ${nativeRunFinalizations.phase} = 'terminal_failure')
|
||||
and (select process_event.event_type from ${heartbeatRunEvents} process_event
|
||||
where process_event.company_id = ${heartbeatRuns.companyId} and process_event.run_id = ${heartbeatRuns.id}
|
||||
and process_event.source_event_id is null
|
||||
and process_event.event_type in ('native.process_start_requested', 'native.process_identity_recorded', 'native.local_process_stopped')
|
||||
order by process_event.seq desc limit 1) = 'native.local_process_stopped'`;
|
||||
and ((${heartbeatRuns.status} = 'failed'
|
||||
and ${heartbeatRuns.processPid} is null and ${heartbeatRuns.processGroupId} is null
|
||||
and (select process_event.event_type from ${heartbeatRunEvents} process_event
|
||||
where process_event.company_id = ${heartbeatRuns.companyId} and process_event.run_id = ${heartbeatRuns.id}
|
||||
and process_event.source_event_id is null
|
||||
and process_event.event_type in ('native.process_start_requested', 'native.process_identity_recorded', 'native.local_process_stopped')
|
||||
order by process_event.seq desc limit 1) = 'native.local_process_stopped')
|
||||
or (${heartbeatRuns.status} in ('failed', 'cancelled', 'timed_out', 'interrupted') and ${remoteExecutionTerminated}))`;
|
||||
|
||||
async function nativeReceiptSourceClosed(db: Db, runId: string) {
|
||||
const [row] = await db.select({ closed: sql<boolean>`${closedNativeReceiptSource}` }).from(heartbeatRuns)
|
||||
|
||||
Reference in new issue
Block a user