Skip to content

fix(budget): keep the cumulative budget in whole cents - #135

Merged
cristim merged 2 commits into
mainfrom
fix/82-integer-cents-ledger
Nov 4, 2026
Merged

cristim merged 2 commits into
mainfrom
fix/82-integer-cents-ledger

Conversation

@cristim

@cristim cristim commented Nov 4, 2026

Copy link
Copy Markdown
Member

Closes #82

Item 2 of #82, the last open one. Items 1, 3 and 4 merged in #109 and #113.

What

The cumulative budget's two ledgers, in memory (budgetLedger) and persisted (budgetState in CUDLY_MCP_BUDGET_STATE), now add and compare whole cents (usdCents, int64) instead of float64 dollars.

On main, float residuals flip exact-boundary checks:

  • a 0.30 budget refuses 0.10 then 0.20 (0.1 + 0.2 = 0.30000000000000004 > 0.3);
  • after releasing those reservations a ~2.8e-17 residual stays in reserved;
  • the state file stores "spent_usd": 0.30000000000000004.

Rounding: never undercount spend

  • Purchase totals round up to the cent. The budget rounds down. So rounding can only refuse earlier than exact arithmetic, never admit more.
  • Half-even was rejected: it can round a spend down by up to half a cent.
  • Exact conversion: it goes through the float's shortest decimal (strconv.FormatFloat(v, 'g', -1, 64) then big.Rat), so 0.07 is 7 cents, although 0.07*100 is 7.000000000000001.
  • Range: amounts are bounded to 0..1e13 USD; NaN, ±Inf, negative or out-of-range values refuse the purchase. Below that bound, cents/100 formats to its exact decimal.

State file compatibility

  • The format is unchanged (version stays 1, amounts stay JSON numbers), so old and new processes can share a file during an upgrade.
  • Writes are exact decimals (0.3, 60).
  • Reads take the literal exactly and round up, so a legacy 0.105 reads as 0.11.
  • null now makes the file corrupt (fail closed). Before, it silently read as 0. A quoted or negative amount was already corrupt and still is.

README (budget paragraph and "Persisted budget" format bullet) and CHANGELOG updated.

Verification (local, macOS)

  • Fail-before: I ran the behavioural tests against main's ledger, with the new-helper test file moved aside. All 6 fail:
    • TestBudgetLedgerExactBoundary
    • TestBudgetLedgerReleaseLeavesNoResidual
    • TestBudgetLedgerRoundsTowardRefusing
    • TestFileBudgetExactCents
    • TestFileBudgetLegacyAmountRoundsUp
    • TestFileBudgetNullAmountIsCorrupt
  • New unit tests: TestSpendAndLimitCents, TestUSDCentsJSON and TestBudgetLedgerRefusesUnrepresentableAmounts cover the helpers.
  • Mutation probes, each failing at least one test:
    • spend rounded down
    • limit rounded up
    • no max bound
    • no negative check
    • big.Rat.SetFloat64 instead of the shortest decimal
    • Marshal writing integer cents
    • Unmarshal rounding down
    • settle not adding to committed
  • Deletion probes: they removed an explicit NaN/Inf check and the JSON pre-checks (null, quoted, float range); none of them was needed for a test to pass.
  • Full checks: make test-unit (3 packages) passes; make lint reports 0 issues; pre-commit is clean.

🤖 Generated with Claude Code

The in-memory and persisted budget ledgers added and compared float64
dollars, so residuals flipped exact-boundary checks: a 0.30 budget
refused 0.10 then 0.20, and the state file stored 0.30000000000000004.

Amounts are now int64 cents. A purchase total rounds up to the cent and
the budget rounds down, so rounding never admits more than exact
arithmetic would; conversion goes through the shortest decimal of the
float, so 0.07 is 7 cents. The state file format is unchanged: amounts
are written as exact decimals, an older file's fraction of a cent reads
rounded up, and a null amount is now corrupt instead of 0.

Closes #82

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@cristim cristim added priority/p2 Backlog-worthy severity/medium Moderate harm triaged Item has been triaged urgency/this-quarter Within the quarter impact/all-users Affects every user effort/s Hours type/bug Defect labels Nov 4, 2026
@coderabbitai

coderabbitai Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 40 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration
Commits

Reviewing files that changed from the base of the PR and between f6d62d8 and 832f5c0.

Files selected for processing: 11
  • CHANGELOG.md
  • README.md
  • tools/attempt_store_test.go
  • tools/budget.go
  • tools/budget_cents.go
  • tools/budget_cents_test.go
  • tools/budget_persist_test.go
  • tools/budget_settle_test.go
  • tools/budget_state.go
  • tools/budget_state_test.go
  • tools/purchase.go
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

Share

@cristim

cristim commented Nov 4, 2026

Copy link
Copy Markdown
Member Author

Independent review (claude-fleet, cold): CHANGES REQUESTED 4fcd502

The cents conversion is correct and fails toward refusing. There is one small blocking gap: an overflowing state file is not treated as corrupt.

Blocking: budgetState.totalUSD() can overflow and read as headroom. tools/budget_state.go, totalUSD, adds SpentUSD and every reservation as plain int64. Each amount is capped at maxBudgetUSD (1e15 cents), but the sum is not.

  • Reproduced. I built a state with spent_usd 1e13 plus 9300 reservations of 1e13 USD each. totalUSD() returned -9145744073709551616, so reserve's st.totalUSD()+usd > limit is false and the purchase is admitted.
  • Reach. This server can't produce such a file, because every reservation is checked against the limit before it is written. A hand-edited, corrupted or foreign file can. The README promises "A file that is corrupt ... refuses every real purchase".
  • Fix. In validate(), accumulate the total with an overflow check (or in big.Int), and reject a total above the maximum as corrupt. Add a test with enough max-size reservations to overflow, asserting that reserve refuses and the file is not rewritten.

Attacked and holding, on an export of 4fcd502 with scratch tests:

  • Exact boundary. spendCents(0.1)+spendCents(0.2) is 30 cents, and limitCents(0.1+0.2) is 30. So the float residual no longer refuses a purchase that fits. Shortest-decimal conversion: 0.07 is 7 cents.
  • Rounding direction. Spend rounds up and the limit rounds down. A legacy file's "spent_usd": 60.30000000000001 reads as 6031 cents (rounded up, never undercounted).
  • Out of range and invalid. Negative, NaN, ±Inf, above 1e13 and 1e300 are refused by both spendCents and limitCents. A budget limit above the range refuses every purchase and names CUDLY_MCP_BUDGET_USD (fail closed).
  • Corrupt file inputs. These all fail to parse, so the file is treated as corrupt and refuses: null (as spent_usd or as a reservation usd), a quoted number, -0.01, 1e400, and 10000000000000.01.
  • Round trip. Marshal and unmarshal are exact from 0 up to 1e15 cents (for example 999999999999999 and 123456789012345), so the format is unchanged and lossless.
  • Concurrency. The in-memory ledger still holds its mutex across the check and the add. The file ledger still runs check plus append inside f.update under the <path>.lock flock. Locking is unchanged by this diff.
  • Missing file. It is still a fresh budget, as the README documents (unchanged). An absent spent_usd or reservation usd field reads as 0, exactly as the float version did, so this is not a regression.
  • Build and tests. go build, go vet and golangci-lint are clean (0 issues). go test ./... passes (3 packages).
  • Fail on main. The new budget_cents_test.go and budget_state_test.go don't compile on f6d62d8 (undefined: usdCents, spendCents, limitCents, maxBudgetUSD). The naiveCents probe below shows the behaviour tests fail without exact conversion.
  • Mutation probes. Each one makes a named test fail:
    • spend rounds down;
    • limit rounds up;
    • the file reader rounds down;
    • null reads as 0;
    • no range check;
    • no negative check;
    • naive SetFloat64 conversion instead of shortest decimal (fails ExactBoundary, ReleaseLeavesNoResidual and RoundsTowardRefusing).

totalUSD summed int64 cents unchecked, so about 9300 maximum-size
reservations in a corrupt file wrapped the total negative, which reads
as headroom. validate now sums with an overflow check against the same
1e13 USD bound each amount has and refuses the file as corrupt past it.
The in-memory ledger stays bounded by the limit, so it cannot wrap.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@cristim

cristim commented Nov 4, 2026

Copy link
Copy Markdown
Member Author

New head 832f5c0042c5e1c8d501ad0fc3046489cb6f57e6, addressing claude-fleet's CHANGES REQUESTED at 4fcd502c.

  • Finding: budgetState.totalUSD() summed int64 cents unchecked. About 9300 maximum-size reservations in a corrupt file wrapped the total negative, which read as headroom (fail-open).
  • Fix: validate() now sums spend plus reservations with an overflow check against the same 1e13 USD bound each amount already has (maxBudgetCents), and refuses the file as corrupt past it. Each amount is at most that bound, so the check itself cannot overflow. After validation, total + usd in reserve is at most 2e15 cents.
  • In-memory ledger: it can't wrap. reserve admits a purchase only while committed + reserved + usd stays within the limit, itself at most 1e15 cents.
  • Test: TestFileBudgetTotalOverflowIsCorrupt covers 9300 max-size reservations (the wrap) and a single total just over the bound. Both subtests fail at 4fcd502c and pass now. Removing the check makes 3 tests fail.
  • Checks: make test-unit passes; make lint reports 0 issues; pre-commit is clean.

@cristim

cristim commented Nov 4, 2026

Copy link
Copy Markdown
Member Author

Independent review (claude-fleet, cold): APPROVE 832f5c0

This is a delta review of 4fcd502..832f5c0: one commit in tools/budget_cents.go (+2), tools/budget_state.go (+7) and tools/budget_state_test.go (+23). It closes my blocker.

  • Overflow is checked at each addition. validate() starts from SpentUSD and, for every reservation, refuses when total > maxBudgetCents - r.USD before total += r.USD.
    • Every amount has already passed ratCents's 0..1e13 USD bound, so maxBudgetCents - r.USD >= 0. The running total never exceeds 1e15 cents, and no step can wrap.
    • A file whose total passes the bound is rejected even when nothing would wrap (the just over the bound subtest).
  • "Corrupt" means refuse to spend. load() turns a validate() error into "budget state file ... is corrupt" (budget_state.go:145-146). update() returns that error before calling the reserve callback, so reserve refuses, nothing is appended, and the file is not rewritten.
    • After a valid load, st.totalUSD()+usd is at most 2e15 cents, so the reserve comparison cannot overflow.
    • The in-memory ledger is bounded by the limit: every reserve keeps committed+reserved <= limit <= 1e15 cents.
  • Fail on the previous head. TestFileBudgetTotalOverflowIsCorrupt fails at 4fcd502 in both subtests:
    • wraps negative: "An error is expected but got nil". The wrapped total admitted the purchase.
    • just over the bound: an over-budget refusal instead of "corrupt".
  • Probes.
    • Removing the per-addition check fails both subtests.
    • Moving the check after the add survives. That mutant is equivalent, since each step is bounded and an add cannot overflow first; it is not a test gap.
  • Results at 832f5c0. go vet is clean, golangci-lint reports 0 issues, and go test ./... passes (3 packages).

@cristim

cristim commented Nov 4, 2026

Copy link
Copy Markdown
Member Author

MERGE HOLD @ 832f5c0: code review passed, owner decision missing
Cold review, money path (claude-cc-libs), full PR at 832f5c0 incl. 4fcd502..832f5c0

Why this is a hold and not an APPROVE. mcp#82 item 2 changes the rounding rules and the persisted state's value semantics, so the register marked it as the owner's call, with "keep floats" as the default. mcp#82 has two comments, neither of them an owner decision:

  • the 2026-10-17 claim;
  • the 2026-11-04 takeover note, which records the coordinator's OK to take over the work, not approval of the rounding or format change.

@cristim, please confirm on mcp#82: integer cents, spend rounds up, the limit rounds down, and legacy non-cent amounts round up on load. The code is ready once you do.

Attacks (a scratch test against the real code)

Class Input Result
Overflow, limit 1e17, 9.3e16, MaxFloat64, ±Inf, NaN refused
Overflow, spend 1e17, MaxFloat64, Inf, NaN, 1e13+0.01 refused
Overflow, file 9224 reservations of 1e13 at 4fcd502 the sum wrapped negative and a reserve was allowed; at 832f5c0 the file is corrupt and refused
Negative -0.01, -1e-300 refused
Zero -0, 0 0 cents
Cents 0.10 then 0.20 against a 0.30 limit allowed; a further 0.01 is refused
Cents 0.29999999999999998 30c spend / 30c limit
Cents 1e-9, 0.005, 5e-324 1c spend / 0c limit
Cents 1.005 101c / 100c
Cents the runtime 0.1+0.2 as one spend 31c, refused against 0.30 (conservative)
Cap 1e13 against a 1e13 limit allowed; +0.01 refused

Old state file. I wrote a file with main's own code: spent 12.645, reservations 7.005 and 60. The PR head reads them as 12.65, 7.01 and 60, all rounded up with nothing lost. It rewrites the file in the same shape at version 1, and main's strict decoder reads the result back. Cent-valued files in main's exact shape, including "reservations": null, re-save byte-identically.

Concurrency. 200 goroutines in memory, and 40 separate file-backed instances on one path, each reserved 0.07 against 1.00. Exactly 14 succeeded in each case, with no over-commit. The concurrency tests pass under -race.

Fail closed. Every case below refuses to spend and leaves the file unchanged: a load error happens before any save (budget_state.go:139, :145 vs :123).

  • garbage, truncated, empty, or a null document;
  • a null or quoted value, or a negative value;
  • an amount above 1e13, or a total over the bound;
  • version 0 or 2, or an empty id;
  • huge exponents;
  • an unreadable file, a directory at the path, or a held lock (refused after 10s).

A missing file is a fresh budget, as documented.

The overflow fix. The bound check at budget_state.go:67 runs before each addition. Every amount is at most maxBudgetCents (budget_cents.go:19, :44), so the sum cannot overflow. TestFileBudgetTotalOverflowIsCorrupt fails on 4fcd502's sources and passes at 832f5c0.

Other checks.

  • Settle: 100k in-memory and 300 file-backed reserve/settle cycles leave no drift. The fix(budget): release the reservation when the provider refuses before purchase #130 release-on-refusal tests still pass.
  • Fail on main: all 6 behavioural tests named in the PR fail on main's sources.
  • Mutation probes (each reverted): limit rounded up fails 3 tests; spend rounded down fails 2; null read as 0 fails 2; the overflow check removed fails the overflow test.
  • Builds: go vet, go test -count=1 ./... and golangci-lint (0 issues) are clean.

Findings

  1. Process, blocking merge: no owner decision on mcp#82 (above).
  2. Low (budget_state.go:30): a missing spent_usd key still reads as 0, while null is now corrupt. This isn't a regression, but the two cases are inconsistent.
  3. Low (purchase.go:376): a CUDLY_MCP_BUDGET_USD above 1e13 is only rejected in reserveBudget, after the client is resolved and the offering priced. It still fails closed, but the provider is called first.
  4. Info (budget_cents.go:50): a float total one ulp above a cent is charged an extra cent, at most 1c per purchase, in the refusing direction.
  5. Info, out of scope (commitment_usd.go:105): the per-call cap still compares floats.

Not merging.

@cristim

cristim commented Nov 4, 2026

Copy link
Copy Markdown
Member Author

APPROVE @ 832f5c0 by claude-cc-libs
This lifts my merge hold. The rounding and state-file contract is now recorded on mcp#82: coordinator-approved as the fail-safe contract, and the owner may override it. My code review at this SHA stands: the attack table, old-file compatibility, concurrency, fail-closed behaviour, the overflow fix, and the probes all hold. Findings 2-5 are non-blocking follow-ups.

@cristim
cristim merged commit 7a4738d into main Nov 4, 2026
12 checks passed
@cristim
cristim deleted the fix/82-integer-cents-ledger branch November 5, 2026 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/s Hours impact/all-users Affects every user priority/p2 Backlog-worthy severity/medium Moderate harm triaged Item has been triaged type/bug Defect urgency/this-quarter Within the quarter

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(mcp): harden purchase budget settle ordering and accounting

1 participant