fix(responses): record the canonical spend pool for request usage - #6370

No actionable comments were generated in the recent review. 🎉

sequenceDiagram
  participant HttpAdmission
  participant RequestSendBudget
  participant SpendReservationLedger
  participant Provider
  HttpAdmission->>SpendReservationLedger: Check pool continuity and ceiling
  SpendReservationLedger-->>HttpAdmission: Allow admission or return refusal
  RequestSendBudget->>SpendReservationLedger: Reserve send and obtain proof
  RequestSendBudget->>Provider: Dispatch routed request
  Provider-->>RequestSendBudget: Report physical send using permit
  RequestSendBudget->>SpendReservationLedger: Confirm the matching reservation

Merge Risk: 🟡 Moderate · up to 7f753

Upgrades with historical spend may refuse otherwise eligible provider requests until an operator maps the old alias. Resolve or explicitly accept that migration requirement before merging.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 4201e

The change strengthens shared provider spending controls. Remaining risk is operational: unverifiable historical usage deliberately blocks requests, and rollback must preserve the accounting rules and current spending history. No newly exploitable path was established in the examined flows.

Retained concerns
No architecture-level concerns identified.

• Fix all pre-merge checks with AI

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

_{Comment @coderabbitai help to get the list of available commands.}

✅ Deterministic PR hygiene checks passed.

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Historical continuity is implemented in 44b3e14.

• Explicit salted alias mappings aggregate original settled, reserved and unresolved balances once. Unknown history fails pool-enforced admission closed, including rootless preflight; no real ledger or installed app was modified.
• v1 checkpoints retain salted identity evidence. Supported rollback retains both canonical route attribution and the compatible reader/writer with the latest journal and same salt. Unmodified-old-binary rollback is unsupported.
• Already-exhausted canonical pools are refused before passthrough. This remains a snapshot check; atomic admission for crossing/concurrent post-reported sends is not claimed.
• 184 focused local tests passed (1,470 assertions), along with typecheck, structure/privacy/file-size checks and the 545-page docs build. The broader local test:changed run remains unverified, as documented in Verification.
• Exact-HEAD Cross-platform CI and React Doctor succeeded. Skipped platform jobs remain skipped.

The prior automated review summaries name f7a50dc; they are not represented as reviews of this new commit. Draft status is retained while current-head review, independent maintainer/security approval and any required sponsorship remain pending.

Maintainer review at head 22027eb28bf0f6c545bfa06f69693080ea2e9f07. Deferring this from the next release. The privacy side looks right: journal aliases stay salted, salt and journal file handling is unchanged, refusal text names no alias, and a malformed spendPoolAliases is rejected on write and fails closed on load. The blocker is upgrade behavior.

• P1 – upgrade lockout for pool-ceiling users (src/lib/spend-pool-continuity.ts:89, src/lib/spend-reservation-ledger.ts:713 and :1065, src/server/workflow-refusal.ts:117). An alias with existing spend history is never bound automatically. That includes the plain canonical provider alias (for example openai), not only the old account-labeled ones. So an install upgraded from 2.75.0 with spend.pool.maxTokens set and retained history gets workflow_pool_history_unresolved (local 429) on every inference request, including requests with no workflow root. The only way out is a hand-written spendPoolAliases entry, and writing one means computing the salted alias yourself, because no command lists or derives these values.
• P2 – the lockout does not age out (spend-reservation-ledger.ts:915). Unresolved pool history is never pruned, so a user who adds a pool ceiling later hits the same refusal.
• P3 – one corrupt line now blocks compaction for good (spend-reservation-ledger.ts:861, :988). The journal then grows without bound and ceilings stay refused. Before this change, compaction healed it.

Users with no ceilings, or only root/identity ceilings, are unaffected (:1040, :1199).

What would make this mergeable: bind an alias automatically when it already equals its canonical provider (or when the mapping can be proven from the journal alone), and give operators a supported way out for unresolved history, such as ocx output that lists unresolved pool aliases with a mapping command, or a documented escape hatch. Please also restore compaction self-healing, or explain why it has to stop. Hosted CI for this head was still running when this was written, so I make no claim about it.

Review fixes are published in 22027eb (tree 259d8e44d6130059c9d8b4b417f7c09824817f18), preserving the prior history. CodeRabbit marked all three reported findings addressed and resolved their threads.

Cross-platform CI and React Doctor now both succeed for this exact HEAD. The CI run completed 17 successful and 8 skipped jobs; skipped jobs are not execution evidence. Its checkout was PR merge 1d3907a2, which includes this HEAD.

Hosted logs show the new graph/permit regressions passing in Linux shard 2 and the locally failing compaction fetch cases passing in Linux shard 3. The local combined command remains recorded as failed/incomplete with seven observed fixture failures; it was not retried or relabeled as passing.

The body now separates current-head evidence from earlier validation and retains the rollback and report-only transport limitations. This CI result and resolution of the three findings do not replace independent maintainer/security approval.

@lidge-jun, I reproduced the upgrade lockout and non-expiring unresolved history at 22027eb and agree these release concerns remain open despite the CI pass. A mapping UI alone would not make an ordinary upgrade work automatically.

Exact salted-name equality cannot distinguish an old account label from a provider ID with the same spelling. One possible alternative is to retain unresolved spend without assigning ownership and conservatively include it in every candidate pool's budget and retention decisions. That could admit demonstrably under-budget reservations without discarding history, but would need explicit tests and would not fix the existing post-reported passthrough/concurrency limitations.

For corrupted journals, the previous compaction drops evidence without recovering potentially missing spend; I would preserve the evidence and persistent refusal state while compacting valid records.

Would you prefer the conservative unresolved-spend approach for the upgrade path, with explicit mapping as a recovery aid, or a narrower revision that requires proven historical bindings? I have not implemented either expansion and will keep the release hold in place.

@coderabbitai review

Current source checkpoint at 7f75339: the follow-up is substantive, not just pool-group performance. In spend-reservation-ledger, preparePoolContinuity no longer unconditionally denies because positive unbound history exists; pool views conservatively include that history once per candidate, and ambiguous new routed spend uses a separate pool-current hash domain. Invalid/conflicting continuity evidence and failed durable publication still refuse. The unchanged self-binding condition by itself is therefore no longer a complete current explanation of the earlier upgrade lockout.

This is static delta inspection, not independent execution of the migration/retention/rollback/ambiguous-resend cases or acceptance of the accounting policy. @lidge-jun's explicit release/design hold remains until the owner withdraws it against current evidence. Exact-head CI37215477998 and React Doctor passed, but do not substitute for that decision or the required independent boundary review. Please refresh the description's published source-head receipt (currently92bc6b...) to this head and retain coverage limits. No installed ledger, account, provider, scan or merge touched by me.

Historical checkpoint:
Scoped current-head check at 2afa62a: the unresolved-history upgrade hold has not been cleared by the pool-group performance change. createPoolContinuity.prepare still creates a self-binding only when the requested alias has no historical entry, so existing spend is not automatically bound on upgrade. The retention/admission behavior needs an explicit accepted migration contract, not merely green CI or a mapping UI. The conservative-unresolved-spend versus proven-binding choice in your follow-up remains an owner design decision; I am not choosing or implementing an expanded accounting policy on another author’s branch. Please retain @lidge-jun’s release hold and add upgrade/retention/corrupt-journal proof for whichever strategy is accepted. This is a source checkpoint, not a new ledger reproduction, a full 43-file review, or sponsorship/security approval. No installed ledger or app was touched.

@coderabbitai review

Please review the unreviewed delta through published head 7f75339fded80016342de09222d3bc8dd44e8aba. The paused walkthrough still identifies coverage through 4201e9b0a3f4e25ad0d97f100af39f474ffe7cdf, so its successful status is not current-head review evidence.

Author follow-up on 2026-10-06: all seven existing inline threads are already resolved. Cross-platform CI 37215477998 and React Doctor 37215478063 completed successfully for the current head. No unchanged passing check was rerun.

Please assess the current conservative per-candidate treatment of unresolved positive history, exact permit/receipt ownership and persistence/rollback contracts. The historical blanket-refusal and canonical-startup suggestions are not approval of a different policy. The owner's upgrade/migration policy hold remains open; the latest maintainer checkpoint is not an approval or an explicit withdrawal of that hold. The owner is already a requested reviewer, so I have not sent a duplicate owner review request. No journal, balance, provider credential, installed state or integration policy was changed.