Repository navigation
fix(exchange): atomically reserve RI exchange daily headroom - #659
Conversation
|
Warning Review limit reached
This review includes 7 billable files and costs up to $1.75.
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. View limit detailsLimit 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. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (7)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (7)
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. 📝 WalkthroughWalkthroughThe 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. ChangesRI exchange reservation
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
Merge Risk: ⚪ Minimal · up to 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)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai full review |
|
1591f10 to
0bf718f
Compare
|
@coderabbitai full review |
|
|
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. |
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