fix(accounting): keep budget admission and usage handoff alive under run-row updates (#20484)

## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - Before a run starts provider work, Paperclip reserves its budget and
binds a usage recorder. Both write the run's `heartbeat_runs` row.
> - Run-log appends, output progress and lifecycle events update that
same row while the run prepares and streams.
> - #20385 fixed this race for the standalone usage-receipt write only.
Budget admission and the recorder handoff still take a repeatable-read
snapshot before they lock the run row.
> - Each concurrent row update makes that snapshot stale. Four retries
run out with SQLSTATE `40001`, and the run ends `adapter_failed` with
raw SQL as its error.
> - This pull request applies the #20385 pattern (READ COMMITTED under
the existing locks) to those writers and to the final dispatch gate. It
also gives exhausted retries a bounded cause.
> - The benefit is that busy runs start and account normally, and a
remaining failure names its cause.

## Linked Issues or Issue Description

Refs #20385. Builds on #20483 (bounded failure cause and
`withoutSqlStatementText`).

**What happened?**

The restart-chaos L1 workload with `--db-pressure 4` (harness writers
that update each running run row, one row per statement, like a fast
stream) failed on master. In one wave, 0 of 4 tasks passed. Every first
run failed in `reserveRunBudget` before spawn. In an earlier wave, a run
failed in `createRunUsageRecorder`. The stack in both cases is `could
not serialize access due to concurrent update` (`40001`) on the run-row
`FOR UPDATE`. The board `runError` showed the full `select ... from
heartbeat_runs ... for update` statement. The server log line `heartbeat
execution failed` dropped the database cause.

Realism: the harness writers are synthetic, but the product has the same
writers on the same row. `appendHeartbeatRunEvent` increments
`next_event_seq`, output progress updates `last_output_*`, and lifecycle
events land while a run prepares. On a hosted database the window
between snapshot and row lock is wider than locally, so a busy run
reaches the same race without the harness.

**Expected behavior**

Budget admission and the recorder handoff commit while other writers
update the run row. Budget invariants stay intact: one reservation per
run, a reached hard stop still refuses work, and committed holds count
against later admissions. Usage receipts are journaled once. If retries
still run out, the run shows a bounded cause without SQL, and the server
log records the SQLSTATE.

**Steps to reproduce**

Run `server/src/services/run-accounting-contention.test.ts` against the
parent commit. The deterministic cases commit a real
`appendHeartbeatRunEvent` after the writer's transaction checks the
company and before it locks the run. Six of the eight cases fail with
`40001`.

## What Changed

- `reserveRunBudget` runs at READ COMMITTED. All mutable reads follow
the shared admission fences, the agent `FOR SHARE` lock and the run-row
`FOR UPDATE`. Exposure stays one SQL statement (hold-to-ledger atomic).
The `budget_reservations_run_idx` unique index and the run lock still
prevent a second hold.
- The final dispatch gate's accounting phase
(`dispatchResolvedInteractionIfCurrent` through `withIssueThenRunLocks`
in `run-dispatch/adapters/postgres.ts`) runs at READ COMMITTED. Its
ownership phase already read the same staleness facts at READ COMMITTED
under the same issue and run locks. Retry promotion and stale-run
cancellation keep their repeatable-read snapshot. The first L1 proof
wave found this writer exhausting `40001` once the other three were
fixed.
- `createRunUsageRecorder` runs its receipt-source handoff and its
capture-failure fence at READ COMMITTED. Both lock the run row first,
the same as `persistUsageReceipt` after #20385.
- `withOperationAccountingTransaction` raises
`AccountingContentionError` when retries run out. It has a bounded
message, `code` keeps the SQLSTATE (`40001`/`40P01`), `cause` keeps the
database error, and a warning logs both.
- `failRun` shows that bounded message. The code stays `adapter_failed`,
so chat and recovery retry paths still apply. #20483's `failureCause`
records `40001`. All other SQL text handling comes from #20483
(`withoutSqlStatementText`), unchanged here.
- `doc/DATABASE.md` documents the isolation choice and the failure
contract.

### Inventory of REPEATABLE READ / SERIALIZABLE transactions that write
run-scoped rows

| Writer | Status | Notes |
| --- | --- | --- |
| `persistUsageReceipt` (`usage-receipts.ts`) | Fixed in #20385 | READ
COMMITTED under the run lock. |
| `reserveRunBudget` (`budget-reservations.ts`) | Fixed here | Seen in
waves (`budget-reservations.ts:90`). |
| `createRunUsageRecorder` handoff (`usage-receipts.ts`) | Fixed here |
Seen in waves (`usage-receipts.ts:572`). |
| `createRunUsageRecorder` capture-failure fence (`persistFailure`) |
Fixed here | Same shape: run lock first, runs mid-stream. |
| Native finalizer `claimCoordinator` (`native-run-finalizer.ts`) | Safe
| A default READ COMMITTED transaction with `FOR UPDATE` on the
coordinator row. It cannot raise `40001`. The wave errors there were
`native_finalization_missing`, a symptom of the failed reservation (no
coordinator existed yet). |
| Final dispatch gate, accounting phase
(`dispatchResolvedInteractionIfCurrent`) | Fixed here | Seen in proof
wave `w-20261110T075857-s20386` (runs `140ddb10`, `c028c6c4`,
`fea0ee5a`). Its ownership phase already used READ COMMITTED under the
same locks with the same staleness reads. |
| Retry promotion and stale-run cancellation (`promoteOrCancelDueRetry`,
`cancelStaleQueuedRun`, same lock helper) | Follow-up | Kept at
repeatable read: `loadGateFacts` combines wakes, comments, dependencies,
tree holds and repair state in one decision. They act on queued or
scheduled runs, not streaming ones. Not seen failing in waves. If
retries run out, they now fail with the bounded cause. |
| Run settlement `accountRunCost` (`run-cost-accounting.ts`) | Follow-up
| Seen with `40001` in a wave log. Leaves the run pending;
`reconcileRunCosts` retries it durably. Multi-aggregate (coordinator,
receipts, images, closure). |
| Tool cost events (`paperclip-runner-tool-authority.ts`, `costs.ts`) |
Safe | Insert `cost_events` only; the run row is referenced, not locked
or updated. |
| Budget enforcement and evaluation (`budgets.ts`), billing
reconciliation, accounting-integrity repair | Safe | Lock policy,
invoice or company rows, not a streaming run row. Run reads are plain
selects. |
| Browser use, decisions, prompt-injection, fast responses | Safe |
Write their own invocation/reservation rows, not `heartbeat_runs`. |
| Read-only RR snapshots (`withAccountingReadSnapshot`, idle-sleep, tool
gateway, customer success, `routes/decisions.ts`) | Safe | Read only, or
no run-scoped writes. |

## Verification

- Red on the parent commit, green on this branch:
`server/src/services/run-accounting-contention.test.ts` (real embedded
PostgreSQL).
- `reserveRunBudget > reserves once when run events commit between its
company check and its run lock` (red: `40001`)
- `reserveRunBudget > reserves every run while back-to-back writers
update the run row` (red: `40001`)
- `reserveRunBudget > counts every committed hold, so successive
admissions never exceed the budget` (red)
- `reserveRunBudget > still refuses a run once the hard stop is reached`
(red: `40001` instead of `409`)
- `createRunUsageRecorder > binds its receipt source when run events
commit between its company check and its run lock` (red: `40001`; green
asserts the spooled receipt is journaled exactly once)
- `createRunUsageRecorder > binds while back-to-back writers update the
run row` (red: `40001`)
- `reserveRunBudget > never creates a second reservation for one run
when two dispatches race` (invariant; passes before and after)
- `final dispatch gate > admits and hands off a reserved run when run
events commit between its company check and its run lock` (red:
`AccountingContentionError` after 4 × `40001`)
- `createRunUsageRecorder > persists its capture-failure fence once when
run events commit between its company check and its run lock`
- `final dispatch gate > admits while back-to-back writers update the
run row` (red: `40001`)
- `exhausted retries > fail with a bounded, SQL-free cause that keeps
the SQLSTATE` (new contract)
- `run-completion.test.ts > saves a bounded cause and keeps the
retryable generic code when accounting retries are exhausted` (saved
code, message and `failureCause`, and log fields, through `failRun`)
- Related suites pass: `usage-receipts-read`,
`admission-independent-of-cleanup`, `budget-live-attempts`,
`idle-accounting-work`, `budget-enforcement-scoping`,
`closed-native-receipt-accounting`, `budget-evaluation-work`,
`run-dispatch/adapters/postgres`, `heartbeat/run-completion` (all 126
run-completion cases pass; its `afterAll` embedded-PostgreSQL teardown
exceeded 30 s on this loaded host, the known local flake).
- `pnpm --filter @paperclipai/server typecheck` passes.
- Latency of `reserveRunBudget` on the normal path (no contention,
embedded PostgreSQL, 350 measured reservations per run, 3 runs each):
before p50 14.7 / 15.0 / 18.6 ms, p95 25.6 / 31.4 / 39.4 ms; after p50
14.2 / 14.0 / 16.6 ms, p95 29.7 / 24.0 / 29.3 ms. No added latency: the
statements are the same, only the isolation level changes. Under
contention it removes the retry backoff.
- Live restart-chaos L1 waves (`--seed 20386 --db-pressure 4`, codex and
claude, K1):
- Master before this change: `w-20261110T050818-s20386`, 0/4. Every
first run failed in `reserveRunBudget` with `40001`.
- Head `90530789b0` (first three writers fixed):
`w-20261110T075857-s20386`, 2/4. Both codex tasks passed. Both claude
failures (t02, t03) trace to the dispatch gate, now fixed. Those runs
already showed the bounded cause, with no SQL on the board. The failure
was logged with `sqlState` 40001. No `reserveRunBudget` or recorder
`40001` remained.
- Head `d5ed21926f` (dispatch gate fixed): `w-20261110T090531-s20386`.
All 4 tasks reached done with no retry and R1–R6 passed. There were no
failed runs and zero `40001` in the server log, under 310k pressure
updates. Each task still failed the T-oracle `company_spend_settled`
with `pending_runs=1`. That pending run is t02's continuation run
`601ae80a`, cancelled before provider work as `issue_terminal_status`,
whose accounting debt was not yet settled when graded about 15 s later.
It is not a contention failure. Caveat: I edited the worktree during
this wave (`worktree_changed_mid_wave`), so a clean wave on the final
head follows.
- Final head `9f1c480717`, clean worktree: `w-20261110T094417-s20386`.
All 4 tasks reached done with no retry, and R1–R6 passed for each. There
were no failed runs and zero serialization failures in the server log,
under about 523k pressure updates. Each task still fails only the
T-oracle `company_spend_settled` (`pending_runs=1`). It is the same
cancelled pre-provider continuation run (`issue_terminal_status`), whose
accounting debt was still unsettled at grading. That is a separate
settlement-timing follow-up, not database contention. Master was 0/4 on
the same seed.

## Risks

- READ COMMITTED gives each statement its own snapshot. Admission reads
policy, scope and exposure in separate statements. The shared
company/project fences and the agent share lock exclude concurrent pause
and policy edits, and exposure stays one statement, so each read is at
least as fresh as the old snapshot.
- Concurrent admissions of different runs stay best-effort, as
`doc/SPEC-implementation.md` already states ("concurrent starts can
exceed the same observed balance"). This change does not widen that
window: each statement snapshot is newer than the old transaction
snapshot. It does not add a company mutex.
- A run that fails after exhausted retries now records
`accounting_contention` instead of `adapter_failed`. Retry rules that
key on `adapter_failed` spawn-like messages did not match this failure
before, so no automatic retry changes.
- No schema, migration or persisted-format change.

## Model Used

- Claude Opus 5.5 (Anthropic), model ID `claude-opus-5-5`, used through
Claude Code with shell, file-edit and code-execution tools.

## 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
- [x] All Paperclip CI gates are green
- [x] 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:
DottaandClaude Opus 5.5 authored and GitHub committed 2026-11-10 03:47:48 -06:00
1 parent 914e3ae14f
commit 0b20ea6fe9
8 files changed
+414 -12

No files matched your search

@@ -227,6 +227,7 @@ export function createPostgresRunDispatchAdapter(
onMissing: () => T,
operation: (tx: Db, run: HeartbeatRun, publications: ActivityPublication[]) => Promise<T>,
mode: "accounting" | "ownership",
isolation: "snapshot" | "read committed" = "snapshot",
): Promise<T> {
// Queue editing and claiming already use issue -> wake -> run. Read the
// immutable issue reference without a lock first, then acquire issue ->
@@ -282,7 +283,8 @@ export function createPostgresRunDispatchAdapter(
return { kind: "value" as const, value: await operation(typedTx, run, publications) };
};
const result: { kind: "missing" | "retry" } | { kind: "value"; value: T } = mode === "accounting"
? await withOperationAccountingTransaction(db, input.companyId, locked)
? await withOperationAccountingTransaction(db, input.companyId, locked,
isolation === "read committed" ? { isolationLevel: "read committed" } : {})
: await db.transaction(tx => locked(tx as unknown as Db, []));
if (result.kind === "missing") return onMissing();
@@ -294,12 +296,21 @@ export function createPostgresRunDispatchAdapter(
);
}
/** Accounting phase under issue -> run locks. Gate decisions that combine
* many facts (retry promotion, stale-run cancellation) keep one repeatable-
* read snapshot. The final dispatch gate passes READ COMMITTED: run-log
* appends and output progress update a running row while it waits, and a
* snapshot taken before the run lock went stale on each one until every
* retry was spent. Its mutable reads all follow the admission fences and the
* issue/run locks, budget exposure is one statement, and its ownership phase
* re-reads the staleness facts at READ COMMITTED under the same locks. */
function withIssueThenRunLocks<T>(
input: { runId: string; companyId: string; recoveryLeaseOwner?: string; enforceContinuationOwnership?: boolean },
onMissing: () => T,
operation: (tx: Db, run: HeartbeatRun, publications: ActivityPublication[]) => Promise<T>,
isolation: "snapshot" | "read committed" = "snapshot",
) {
return withRunOwnershipLocks(input, onMissing, operation, "accounting");
return withRunOwnershipLocks(input, onMissing, operation, "accounting", isolation);
}
async function loadGateFacts(
@@ -1474,6 +1485,7 @@ export function createPostgresRunDispatchAdapter(
);
},
dispatchLockedRun,
"read committed",
);
if (!admission.dispatched) return admission;
// Accounting has committed. Close the separate issue/run ownership race