From 8373ae905a2d689563983f741f8d11a677222eeb Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Sat, 7 Nov 2026 08:15:49 -0600 Subject: [PATCH] fix(accounting): settle a never-started run whose recorder bound an empty fence map (#19826) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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 --- doc/SPEC-implementation.md | 6 +++- .../closed-native-receipt-accounting.test.ts | 29 +++++++++++++++++++ server/src/services/budget-exposure.ts | 4 ++- server/src/services/run-cost-accounting.ts | 6 ++-- 4 files changed, 41 insertions(+), 4 deletions(-) diff --git a/doc/SPEC-implementation.md b/doc/SPEC-implementation.md index 61e682058e5..82fa68c2de2 100644 --- a/doc/SPEC-implementation.md +++ b/doc/SPEC-implementation.md @@ -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 diff --git a/server/src/__tests__/closed-native-receipt-accounting.test.ts b/server/src/__tests__/closed-native-receipt-accounting.test.ts index 2240e310e27..b2c2ecce72e 100644 --- a/server/src/__tests__/closed-native-receipt-accounting.test.ts +++ b/server/src/__tests__/closed-native-receipt-accounting.test.ts @@ -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 }); diff --git a/server/src/services/budget-exposure.ts b/server/src/services/budget-exposure.ts index da147c5ce13..1ac270e5f78 100644 --- a/server/src/services/budget-exposure.ts +++ b/server/src/services/budget-exposure.ts @@ -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' diff --git a/server/src/services/run-cost-accounting.ts b/server/src/services/run-cost-accounting.ts index cb4244dc60e..fc244d914b3 100644 --- a/server/src/services/run-cost-accounting.ts +++ b/server/src/services/run-cost-accounting.ts @@ -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