mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-09 07:10:34 +01:00
fix(accounting): settle a never-started run whose recorder bound an empty fence map (#19826)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Budgets hold new work while a native run's final usage receipt is outstanding > - A run that never started a provider has nothing to account. #19700 added a rule that settles such a run at once. > - In restart chaos waves, Retry runs that failed before any provider start still held company admission for up to 10 minutes > - This pull request fixes the condition that stopped the rule from matching > - The benefit: a failed run that provably never started a provider settles at once and never holds other work ## Linked Issues or Issue Description Refs #19700, #19724. **What happened?** An OpenCode Retry run failed at once with `native_session_cleanup_quarantined`. It had no model, no usage receipts, no process start and no provider events, yet it held company admission for the full 600-second closure bound. Two more waves had the same shape, with holds of 309 and 324 seconds. **Root cause** `nativeProviderWorkNeverStarted` (`run-cost-accounting.ts`) required `usage_json.accountingReceiptSourceFences` to be absent. `createRunUsageRecorder` binds every run with that key, as an empty map `{}`, even when no earlier receipt source exists. So the rule never matched a run that had a usage recorder. The run then waited for the closure bound as a run whose receipt was still closing. The restart-evidence check for the unpriced-usage exemption (`budget-exposure.ts`, from #19724) had the same flaw: it treated the presence of the key as restart evidence. **Expected behavior** A failed native run with no receipts, no replaced receipt source and a process that never launched (or was verified stopped) settles at once with no charge. A run whose provider might still be spending keeps holding. ## What Changed - `run-cost-accounting.ts`: the never-started rule treats an empty fence map as "no replaced source". Only a non-empty map shows that provider work began. - `budget-exposure.ts`: restart evidence also requires a non-empty fence map. - `doc/SPEC-implementation.md`: documents the rule. ## Verification - Red then green, `closed-native-receipt-accounting.test.ts`: - "settles a quarantined Retry whose recorder bound an empty fence map at once". This is the exact chaos row: failed, `native_session_cleanup_quarantined`, terminal_failure finalization, receipt sequence 0, empty fence map, no receipts, no process events. - "does not count an empty fence map as restart evidence for a timed-out closure". - Existing tests still pass. A run with a recorded but unverified process still waits for the bound. The restart-derived exemptions still apply to runs with a non-empty fence map or a recovery transition. - Local: `closed-native-receipt-accounting` (51), the budget suites, `heartbeat-cost-accounting`, `adapter-cost-failure-paths`, `native-stop-accounting` and `native-session-executor` all pass. Server `tsc --noEmit` is clean. ## Risks - Low. The change only narrows what counts as a replaced receipt source. A run whose source was really replaced has a non-empty map and behaves as before. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), Claude Code agent with tool use and code execution. ## 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
3112e4c955
commit
8373ae905a
4 files changed
+41
-4
No files matched your search
@@ -1969,7 +1969,11 @@ its receipt source, recovered from a restart, or lost a provider turn on
|
||||
restore) never hard-stops or pauses a company under the blocking policy,
|
||||
whatever its token count; its activity entry and provenance remain the notice. A lost
|
||||
receipt that already carries a provider-reported cost settles at that cost as
|
||||
an estimate marked as a lower bound, not as unpriced. A native
|
||||
an estimate marked as a lower bound, not as unpriced. A failed native run that
|
||||
never started provider work (no usage receipt, no replaced receipt source, and a
|
||||
local process that never launched or was verified stopped) settles at once
|
||||
with no charge and never holds admission; the empty receipt-source fence map
|
||||
that every usage recorder binds is not evidence that provider work began. A native
|
||||
turn that the restored Runner reports as lost on restore can never report its
|
||||
usage. It does not hold the run's receipt open: the receipt closes when the
|
||||
run's other turns do, and its provenance says the observed usage is a lower
|
||||
|
||||
@@ -576,6 +576,35 @@ const support = await getEmbeddedPostgresTestSupport();
|
||||
expect(await f.block()).toBeNull();
|
||||
});
|
||||
|
||||
// s7532 run 23dc4e9d (OpenCode Retry): failed at once with
|
||||
// native_session_cleanup_quarantined before any provider start. Its usage
|
||||
// recorder had bound an empty fence map, so the never-started rule did not
|
||||
// apply and the run held company admission for the full closure bound.
|
||||
it("settles a quarantined Retry whose recorder bound an empty fence map at once", async () => {
|
||||
const f = await neverStartedFixture("never_launched");
|
||||
await db.update(heartbeatRuns).set({ errorCode: "native_session_cleanup_quarantined",
|
||||
usageJson: sql`${heartbeatRuns.usageJson} || ${JSON.stringify({ accountingReceiptSourceId: randomUUID(),
|
||||
accountingReceiptSourceFences: {}, accountingReceiptReady: false })}::jsonb` })
|
||||
.where(eq(heartbeatRuns.id, f.run.id));
|
||||
await db.update(nativeRunFinalizations).set({ failureCode: "native_session_cleanup_quarantined" })
|
||||
.where(eq(nativeRunFinalizations.runId, f.run.id));
|
||||
expect(await reconcileRunCosts(db)).toMatchObject({ accounted: 1 });
|
||||
await flushBudgetEvaluationWork(db);
|
||||
expect((await f.readRun()).costAccountingPending).toBe(false);
|
||||
expect(await f.charges()).toHaveLength(0);
|
||||
expect(await f.block()).toBeNull();
|
||||
});
|
||||
|
||||
it("does not count an empty fence map as restart evidence for a timed-out closure", async () => {
|
||||
const f = await acceptedResultFixture({ stopEvidence: false, finishedMinutesAgo: 11, restart: false });
|
||||
await db.update(budgetPolicies).set({ unpricedUsagePolicy: "block" }).where(eq(budgetPolicies.id, f.policy.id));
|
||||
await db.update(heartbeatRuns).set({ usageJson: sql`${heartbeatRuns.usageJson} || '{"accountingReceiptSourceFences":{}}'::jsonb` })
|
||||
.where(eq(heartbeatRuns.id, f.run.id));
|
||||
expect(await reconcileRunCosts(db)).toMatchObject({ accounted: 1 });
|
||||
await flushBudgetEvaluationWork(db);
|
||||
expect((await f.block())?.reason).toMatch(/no reliable price|budget hard-stop/);
|
||||
});
|
||||
|
||||
it("keeps waiting for the bound when the provider process was never verified stopped", async () => {
|
||||
const f = await neverStartedFixture("unverified");
|
||||
expect(await reconcileRunCosts(db)).toMatchObject({ accounted: 0 });
|
||||
|
||||
@@ -163,7 +163,9 @@ async function readObservedSpends(db: Db, policies: SpendPolicy[], now: Date, ex
|
||||
// restart, or lost a provider turn on restore. Its activity entry and
|
||||
// provenance remain the board's notice.
|
||||
const finalCharge = sql`${costEvents.idempotencyKey} = 'heartbeat:' || ${costEvents.heartbeatRunId}::text || ':final'`;
|
||||
const restartDerived = (alias: string) => sql.raw(`(${alias}.usage_json ? 'accountingReceiptSourceFences'
|
||||
// Every usage recorder binds an empty fence map; only a replaced receipt
|
||||
// source (a non-empty map) is restart evidence.
|
||||
const restartDerived = (alias: string) => sql.raw(`(coalesce(${alias}.usage_json->'accountingReceiptSourceFences', '{}'::jsonb) <> '{}'::jsonb
|
||||
or coalesce(${alias}.usage_json->'pricingProvenance'->>'evidence', '') like '%lost on restore%'
|
||||
or exists (select 1 from heartbeat_run_events restart_event where restart_event.run_id = ${alias}.id
|
||||
and restart_event.company_id = ${alias}.company_id and restart_event.event_type = 'native.recovery.transition'
|
||||
|
||||
@@ -102,12 +102,14 @@ const closedNativeReceiptSource = sql`${heartbeatRuns.runtimeMode} = 'native'
|
||||
* settled, not one usage receipt was ever journaled for the run (a provider
|
||||
* turn journals one as soon as it starts), and its local process either never
|
||||
* launched or was verified stopped. Settle it at once, like a pre-provider
|
||||
* failure, instead of holding company work. */
|
||||
* failure, instead of holding company work. A usage recorder binds every run
|
||||
* with an empty fence map; only a replaced receipt source (a non-empty map)
|
||||
* shows that provider work began. */
|
||||
const nativeProviderWorkNeverStarted = sql`${heartbeatRuns.runtimeMode} = 'native'
|
||||
and ${heartbeatRuns.status} in ('failed', 'cancelled', 'timed_out', 'interrupted')
|
||||
and ${settledNativeCoordinator}
|
||||
and coalesce((${heartbeatRuns.usageJson}->>'accountingReceiptSequence')::int, 0) = 0
|
||||
and ${heartbeatRuns.usageJson}->'accountingReceiptSourceFences' is null
|
||||
and coalesce(${heartbeatRuns.usageJson}->'accountingReceiptSourceFences', '{}'::jsonb) = '{}'::jsonb
|
||||
and not exists (select 1 from ${runUsageReceipts} journaled_receipt
|
||||
where journaled_receipt.company_id = ${heartbeatRuns.companyId} and journaled_receipt.run_id = ${heartbeatRuns.id})
|
||||
and ${heartbeatRuns.processPid} is null and ${heartbeatRuns.processGroupId} is null
|
||||
|
||||
Reference in new issue
Block a user