Skip to content

fix(exchange): atomically reserve RI exchange daily headroom - #659

Merged
cristim merged 1 commit into
mainfrom
fix/atomic-auto-exchange-reservation
Oct 8, 2026
Merged

cristim merged 1 commit into
mainfrom
fix/atomic-auto-exchange-reservation

Conversation

@cristim

@cristim cristim commented Oct 7, 2026 •

Copy link
Copy Markdown
Member

Two concurrent AUTO exchanges could each read the same daily headroom before either recorded spending. This adds an atomic PostgreSQL reservation: a global transaction advisory lock guards the UTC-day spend calculation, and the full execution ceiling is inserted as a processing payment before the provider can execute. Failed inserts or commits leave the caller's record unchanged.

This PR adds the store contract and implementation. The adapter and runner use it in the follow-up platform PR. Refs LeanerCloud/cloud-commitments-go#41.

Verification: the Go 1.26.6 full -race -short ./... suite, pinned golangci-lint v2.10.1, and all applicable normal commit hooks passed. An independent real PostgreSQL test previously proved two-store serialization, UTC boundary accounting, and advisory-lock blocking before a validation-only extraction; that extraction did not change the transaction body. A fresh database rerun was unavailable because the local Docker Desktop fixture stopped. No cloud execution or purchase was attempted.

Summary by CodeRabbit

  • New Features
    • Exchange payments can now be reserved within configured per-exchange and daily spending limits. Reservations account for existing daily spend and remaining headroom, helping prevent concurrent exchanges from exceeding the daily limit.
    • Reservations use the lower of the per-exchange limit and the available daily headroom. Invalid or out-of-range amounts are rejected.

@cristim cristim added triaged Item has been triaged priority/p1 Next up; this sprint severity/high Significant harm urgency/this-quarter Within the quarter impact/many Affects most users effort/m Days type/bug Defect labels Oct 7, 2026
@coderabbitai

coderabbitai Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Warning

Review limit reached

  • Run on-demand review

This review includes 7 billable files and costs up to $1.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 3 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 76 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-platform/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Essentials
  • Run ID: cbcfcd4b-29f0-477a-a589-2221392bdd0e
📥 Commits

Reviewing files that changed from the base of the PR and between d3c5bb8 and 0bf718f.

📒 Files selected for processing (7)
  • internal/analytics/collector_test.go
  • internal/config/interfaces.go
  • internal/config/store_postgres.go
  • internal/config/store_postgres_pgxmock_test.go
  • internal/config/store_postgres_ri_exchange_reservation_integration_test.go
  • internal/mocks/stores.go
  • internal/server/test_helpers_test.go

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: LeanerCloud/cloud-commitments-platform/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Essentials
  • Run ID: a43463a0-015d-42a5-b953-a03485a00f4c
📥 Commits

Reviewing files that changed from the base of the PR and between d3c5bb8 and 0bf718f.

📒 Files selected for processing (7)
  • internal/analytics/collector_test.go
  • internal/config/interfaces.go
  • internal/config/store_postgres.go
  • internal/config/store_postgres_pgxmock_test.go
  • internal/config/store_postgres_ri_exchange_reservation_integration_test.go
  • internal/mocks/stores.go
  • internal/server/test_helpers_test.go

Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.


📝 Walkthrough

Walkthrough

The config store interface and PostgreSQL store now support RI exchange reservations against daily and per-exchange USD caps. The store uses exact decimal amounts and a transaction-scoped advisory lock. Unit and integration tests cover reservation outcomes, failures, and concurrent stores.

Changes

RI exchange reservation

Layer / File(s) Summary
Transactional reservation
internal/config/interfaces.go, internal/config/store_postgres.go
The store interface adds ReserveRIExchange. PostgreSQL runs the insert through a shared query executor, serializes reservations with a transaction-scoped advisory lock, calculates the available ceiling from daily spend and both caps, and uses explicit UTC bounds in the shared daily-spend query.
Reservation tests and store mocks
internal/config/store_postgres_pgxmock_test.go, internal/config/store_postgres_ri_exchange_reservation_integration_test.go, internal/mocks/stores.go, internal/analytics/collector_test.go, internal/server/test_helpers_test.go
Tests cover reservation success, cap rejection, transaction failures, decimal parsing, UTC spend accounting, concurrent stores, and lock waiting. Store mocks implement the added method.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant PostgresStore
  participant PostgreSQL
  Caller->>PostgresStore: Submit record and USD caps
  PostgresStore->>PostgreSQL: Begin transaction and acquire daily-spend advisory lock
  PostgresStore->>PostgreSQL: Read daily spend
  PostgresStore->>PostgreSQL: Insert record with reservation ceiling
  PostgresStore->>PostgreSQL: Commit transaction
  PostgresStore->>Caller: Return reservation ceiling or error
Loading

Merge Risk: ⚪ Minimal · up to 0bf71

This change adds an atomic way to reserve daily RI exchange spending headroom so that concurrent automatic exchanges cannot overspend the daily cap. No production code calls it yet; the callers arrive in a follow-up change. No merge-blocking issues were identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 7 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: atomically reserving RI exchange daily headroom.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · 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 7, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor
⚠️ Action not completed

Review rate limited.


Your included review limit is currently reached under our Fair Usage Limits Policy. Your current included review allowance is based on your included PR review attempts over the past 7 days. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 54 minutes.

@cristim
cristim force-pushed the fix/atomic-auto-exchange-reservation branch from 1591f10 to 0bf718f Compare October 8, 2026 01:57
@cristim

cristim commented Oct 8, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor
⚠️ Action not completed

Review rate limited.


Your included review limit is currently reached under our Fair Usage Limits Policy. Your current included review allowance is based on your included PR review attempts over the past 7 days. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 3 minutes.

@cristim

cristim commented Oct 8, 2026

Copy link
Copy Markdown
Member Author

Independent adversarial review and merge evidence

Reviewed head: 0bf718f, rebased onto d3c5bb8. Independent capable Codex full-diff review found no actionable findings. All seven reservation patches retain the reviewed 400-line change; unrelated ladder and Azure-secret changes are preserved.

Fresh independent Go 1.26.6 build, full go test -count=1 -race -short ./..., and CI-pinned golangci-lint 2.10.1 passed on this exact head. Tested Git blobs match the reviewed source. Logs are preserved at /tmp/claude/platform659-rebased-0bf718f-{build,race,lint}.log.

Real PostgresStore methods with transaction mocks were exercised for full-ceiling INSERT amounts: initial900 reserves1000, per-exchange cap500, remaining daily50, exact micro-dollar flooring, maximum DECIMAL, and rejection before Begin for invalid inputs. All five ceiling probes failed on the deliberate initial-payment mutation and passed on the reviewed implementation. Production store and regression blobs are byte-identical after rebase. Concrete pgx transaction and PostgreSQL transaction-lock/read-committed semantics were independently checked.

Evidence is realistic mock-based local verification. The committed dedicated PostgreSQL integration regression compiled but skipped without its database fixture; no fresh real-database or live AWS acceptance is claimed. The earlier reported database pass is not counted as audited fresh proof. Caller adapter wiring and package pin follow in a separate PR. Manual approval concurrency, UTC rollover holds, and ambiguous AWS acceptance remain tracked by platform656, platform657, and platform100. Go41 remains open.

Exact-head CI workflows37715455855 and37715455939 completed successfully. The merge guard rechecks all checks/statuses, unchanged reviewed head and authoritative main, main target, clean mergeability and normal protections immediately before merging. Current CodeRabbit summary names this exact head, includes a recent no-actionables section, and still reports the review limit. The latest full-review request explicitly reports rate limiting at #659 (comment) ; formal review records and inline findings are empty. Do not infer a completed formal full review from the summary alone. The generic80-percent docstring warning is justified under the repository's sparse-comment policy: private helpers, mocks and tests do not need boilerplate comments describing their names; no actionable code finding was returned.

CR waived: quota, adversarial review + local verification + green CI

Record the actual merge commit in worker2's CR retrospective ledger and fleet bus after merge; request a retrospective review when quota returns and resolve any actionables in follow-up PRs. No protection bypass, manual deployment, cloud purchase, or branch/worktree deletion is authorized or performed by this merge procedure.

@cristim
cristim merged commit d8127f8 into main Oct 8, 2026
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/m Days impact/many Affects most users priority/p1 Next up; this sprint severity/high Significant 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.

1 participant