Skip to content

feat(mcp): cap cumulative real-purchase spend per process - #80

Merged
cristim merged 1 commit into
mainfrom
feat/51-purchase-budget
Oct 17, 2026
Merged

cristim merged 1 commit into
mainfrom
feat/51-purchase-budget

Conversation

@cristim

@cristim cristim commented Oct 17, 2026

Copy link
Copy Markdown
Member

Summary

This is part 3 of 3 for #51, after #66 and #69 (both merged). It replaces #77, which closed automatically when the #69 base branch was deleted after merge.

The per-call USD cap (#69) still let a model repeat a purchase with a fresh idempotency_nonce, region or profile and spend again. Real purchases now also require CUDLY_MCP_BUDGET_USD, a ceiling on the total USD this server process commits.

  • Before credentials: authorizeRealPurchase reads the budget, and an unset or invalid value refuses before ResolveClient. It now returns purchaseLimits{usd, budget}.
  • Reservation: after enforceUSDCap prices the purchase, its total is reserved against the budget under a mutex, so concurrent calls cannot both pass. One stderr line is logged per reservation.
  • Settle: purchaseAndSettle makes the PurchaseCommitment call, logs, audits and settles. Once the purchase is invoked, the reservation stays counted on every error, including an unknown outcome, and on every non-no-op result. Only a clean no-op, or a commitment the library adopted (ExistingCommitment), releases it. Refusals before the purchase call never count.
  • Scope of the budget: it lives in memory. It resets on restart, is not shared between processes or MCP clients, and cannot see purchases made elsewhere. The README says so, and the error message names the restart caveat. A persisted budget is not implemented. The owner's question on it went unanswered by its deadline, so the per-process default applies; persistence is tracked in feat(mcp): persist the cumulative purchase budget across restarts and processes #74.
  • Operator surface:
    • mcpb/manifest.json: env var in all 3 platform blocks, plus a budget_usd user setting.
    • server.json
    • README spend-cap paragraph and CHANGELOG Breaking / Added entries.
    • The shared unset-cap message now says "to a positive value".

Closes #51. With #66, #69 and this PR, scope items 1 and 2 are implemented. Item 3 (priced RI/CUD preview) was an optional owner decision and is not done. GCP re-enable is #67, auditing refusals is #68 and a persisted budget is #74.

Verification (local, macOS)

Fake-cloud and unit evidence only. No real purchase or cloud call is made. The full suite ran at 909549f; after rebasing onto main at 8c9b39f (range-diff identical; #69 squash tree equals its reviewed head 928c95c), gofmt, build, vet and go test -race were re-run at 8e58a54 and pass.

  • gofmt, go vet ./... and make build pass.
  • go test -race -count=1 ./... passes in all 3 packages. golangci-lint and gosec, both run with GOTOOLCHAIN=go1.26.9, report 0 issues.
  • gocyclo -over 10 lists no non-test code. markdownlint passes and both JSON files parse.
  • Regression (the feat(mcp): USD spend cap on the execute path and a cumulative purchase budget #51 repeat-call hole): TestBudgetRepeatCallWithFreshNonceRefused makes two identical $60 purchases with different nonces under a $100 budget. With the reserve call disabled both execute and the test fails. With it, the second is refused and no purchase call is made for it.
  • Other tests:
    • concurrency under -race (exactly 3 of 10 reservations succeed)
    • a failed purchase stays counted
    • a clean no-op is released
    • an existing commitment is released
    • an amount equal to the limit is allowed
    • an unset budget refuses before resolve
    • env parsing
  • Mutation probes (scratch copy, restored): each of these fails tests:
    • disabling reserve
    • settle always releasing
    • > changed to >= in reserve
    • dropping the ExistingCommitment release

Review

An independent reviewer (Sonnet, high effort, non-author) did two passes. It walked every (err, Success, Error, ExistingCommitment) combination and every path between reserve and settle; there are no leaks and no double settle. It also checked the singleton against parallel tests and found no race.

Pass 1 found:

  • A detached doc comment: fixed.
  • A missing equal-to-limit test, because the >= mutation survived: added, and now caught.
  • Unset-message wording that was wrong for the integer count cap: fixed.

Pass 2 found no actionable findings. CodeRabbit is not pinged because the org's CR quota is exhausted.

🤖 Generated with Claude Code

The per-call USD cap still let a model repeat a purchase with a fresh
idempotency_nonce, region or profile and spend again. Real purchases
now also need CUDLY_MCP_BUDGET_USD, a ceiling on the total USD this
server process commits. An unset or invalid value refuses before
credentials are resolved.

After the USD cap prices a purchase, its total is reserved against the
budget under a mutex, so concurrent calls cannot both pass. Once
PurchaseCommitment has been invoked the reservation stays counted on
every error and every non-no-op result; only a clean no-op or a
commitment the library adopted is released. The budget lives in
memory: it resets on restart, is not shared between processes and
cannot see purchases made elsewhere, which the README states.

Refs #51

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@cristim cristim added urgency/this-sprint Within the current sprint priority/p1 Next up; this sprint severity/high Significant harm impact/all-users Affects every user effort/m Days type/feat New capability triaged Item has been triaged labels Oct 17, 2026
@coderabbitai

coderabbitai Bot commented Oct 17, 2026

Copy link
Copy Markdown

Warning

Review limit reached

  • Run on-demand review

This review includes 11 billable files and costs up to $2.75.

  • Ask an admin to make reviews automatic

Open in CodeRabbit

Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing.

Or wait 9 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available. Your 74 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository: LeanerCloud/cloud-commitments-mcp/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Essentials
  • Run ID: ddab7757-4603-4ce0-8b01-de093266d476
📥 Commits

Reviewing files that changed from the base of the PR and between 8c9b39f and 8e58a54.

📒 Files selected for processing: 11
  • CHANGELOG.md
  • README.md
  • cmd/cudly-mcp/gcp_purchase_protocol_test.go
  • cmd/cudly-mcp/main_test.go
  • mcpb/manifest.json
  • server.json
  • tools/budget.go
  • tools/budget_test.go
  • tools/purchase.go
  • tools/purchase_test.go
  • tools/spend_caps.go
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@cristim

cristim commented Oct 17, 2026

Copy link
Copy Markdown
Member Author

Merge-driver verdict: CLEAN at 8e58a5406503acb0ebc5c73ae57cc74022dacc97

An independent adversarial reviewer (Sonnet, high effort, cold brief, not the author) reviewed the full diff. It found no blocking findings.

Verified:

  • Concurrency. reserve (tools/budget.go:30-40) holds the mutex across both the check and the hold. settle moves the amount from reserved to committed in a single lock hold. TestBudgetConcurrentReservations runs 10 goroutines of 30 against a budget of 100 and gets exactly 3 successes.
  • Release semantics.
    • No early return exists between reserve and settle.
    • Every error is counted: timeout, context cancel and unknown outcome.
    • Success is counted, and so is a result.Error set with a nil err.
    • Only a clean no-op or an ExistingCommitment releases the reservation.
    • A panic in PurchaseCommitment leaves the reservation held, which fails closed.
  • Fail-closed configuration. An unset, empty, unparsable, NaN, Inf, zero or negative budget refuses inside authorizeRealPurchase, before ResolveClient. Previews never read it.
  • Idempotent replays. A replay that returns ExistingCommitment counts zero, so nothing is double-counted.
  • Operator surface. CUDLY_MCP_BUDGET_USD is consistent across spend_caps.go, manifest.json (all 3 blocks), server.json, README and CHANGELOG.

Mutation probes (6), all killed:

  1. Releasing on an unknown-outcome error.
  2. Dropping the mutex. This is caught by -race, which CI's unit-test job runs (ci.yml:234).
  3. Changing > to >=.
  4. Skipping the reserve.
  5. Ignoring ExistingCommitment.
  6. Never committing on settle.

Local verification on macOS (real runs, GOTOOLCHAIN=go1.26.9, mock provider, no cloud calls):

  • go build ./... and go vet ./... pass.
  • go test -race -count=1 ./... passes: 817 tests.
  • -race -count=20 -run TestBudget passes: 160 runs.
  • Real-account coverage gap: no live purchase was exercised, by design.

Non-blocking follow-ups:

  • purchaseSpent checks ExistingCommitment before err. Checking err first would be strictly safer for a hypothetical {ExistingCommitment, err} result.
  • The ledger is float-based, so an exact boundary case can be off by one ulp. An integer-cents ledger would remove that.
  • There is no test for the panic path or for context cancellation with a budget set.
  • The manifest has min: 1 while the server accepts any value above 0.

Gate items:

@cristim
cristim merged commit 987bbf5 into main Oct 17, 2026
15 checks passed
@cristim
cristim deleted the feat/51-purchase-budget branch October 17, 2026 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/m Days impact/all-users Affects every user priority/p1 Next up; this sprint severity/high Significant harm triaged Item has been triaged type/feat New capability urgency/this-sprint Within the current sprint

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(mcp): USD spend cap on the execute path and a cumulative purchase budget

1 participant