fix(escrow): add GET DIAGNOSTICS row_count guards to auto_release_milestones - #23
Conversation
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
More reviews will be available in 27 minutes and 10 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR contains two independent changes: a ChangesTemporary JSON files configuration
Wallet update diagnostics guard
🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
codeant-ai
Bot
added
the
size:L
|
CodeAnt AI finished reviewing your PR. |
Greptile SummaryThis migration adds
Confidence Score: 5/5Safe to merge — the core escrow logic is correct, the wallet-not-found exception is now properly caught per-milestone, and no existing migration is modified. The SQL function change is a targeted, forward-only CREATE OR REPLACE that adds the missing exception handler and row_count guards without touching any other code path. The wallet debit/credit logic, ledger ordering, and permission lockdown are unchanged. Both findings are non-blocking observations that do not affect correctness or safety. No files require special attention — the migration is the only substantive change and its logic is sound. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[pg_cron fires auto_release_milestones] --> B[Cursor: submitted milestones past auto_release_at\nFOR UPDATE OF m SKIP LOCKED]
B --> C{More rows?}
C -- No --> Z[Return v_count]
C -- Yes --> D[BEGIN subtransaction]
D --> E[PERFORM wallets FOR UPDATE\nlock both in profile_id order]
E --> F{Client wallet exists?}
F -- No --> G[RAISE P0002 wallet_not_found]
F -- Yes --> H{Client locked_balance >= amount?}
H -- No --> I[RAISE NOTICE: underfunded, continue loop]
I --> C
H -- Yes --> J[UPDATE wallets DEBIT client locked_balance]
J --> K{GET DIAGNOSTICS v_rows = 1?}
K -- No, dead path --> G
K -- Yes --> L[UPDATE wallets CREDIT worker available_balance]
L --> M{GET DIAGNOSTICS v_rows = 1?}
M -- No, worker wallet missing --> G
M -- Yes --> N[UPDATE milestones status=released]
N --> O[INSERT escrow_ledger]
O --> P{All milestones released or refunded?}
P -- Yes --> Q[UPDATE jobs status=completed]
P -- No --> R[INSERT notifications x2]
Q --> R
R --> S[v_count + 1, END subtransaction]
S --> C
G --> T{EXCEPTION handler}
T -- unique_violation --> U[RAISE WARNING: duplicate blocked]
T -- P0002 --> V[RAISE WARNING: wallet not found]
U --> C
V --> C
Reviews (3): Last reviewed commit: "fix(escrow): add GET DIAGNOSTICS row_cou..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@supabase/migrations/20260604060000_auto_release_row_count_guard.sql`:
- Around line 49-54: The current IF uses an EXISTS check on public.wallets
(select 1 ... where w.profile_id = v_milestone.client_id and w.locked_balance >=
v_milestone.amount) which conflates "wallet missing" and "insufficient
locked_balance" and thus never triggers the new wallet_not_found / row_count
guard; change the logic to first check for the existence of a wallet row for
v_milestone.client_id (e.g., EXISTS(select 1 from public.wallets w where
w.profile_id = v_milestone.client_id)) and, if missing, raise/route to the
wallet_not_found / P0002 path, otherwise check locked_balance >=
v_milestone.amount and only then treat as underfunded, ensuring the row_count
guard executes when no wallet row exists.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4a9968fa-ac80-4522-9bea-1a3f02082e2c
📒 Files selected for processing (2)
.gitignoresupabase/migrations/20260604060000_auto_release_row_count_guard.sql
…estones
The two wallet UPDATEs in auto_release_milestones (client debit, worker
credit) lacked row_count verification. If either wallet row were missing,
the UPDATE would silently affect 0 rows — money would vanish from the
client's locked balance without arriving at the worker.
Every other money-path RPC (fund_escrow, approve_milestone, withdraw_wallet,
topup_wallet) already has this guard. This forward-only migration brings
auto_release into line.
Additionally:
- The per-milestone EXCEPTION handler now catches SQLSTATE P0002
(wallet_not_found) so a missing wallet skips that milestone and
continues the loop, instead of aborting the entire pg_cron batch.
- The client-wallet check is split: first verify the wallet row exists
(route to P0002 if missing), then check locked_balance >= amount
(route to the underfunded skip path if insufficient).
Fixes: audit finding F1 (MEDIUM)
Addresses: Greptile (batch-abort blast radius), CodeRabbit (client-wallet conflation)
Tested: scratch/test_f1_isolation.ts (multi-milestone: A released, B skipped, count=1)
scratch/test_f1_guard.ts (single missing wallet: count=0, balance unchanged)
scratch/test_concurrent.ts (happy path: money moves exactly once)
d659881 to
655cb56
Compare
|
@greptile-apps review |
|
@coderabbitai review |
✅ Action performedReview finished.
|
1 participant
User description
Problem
Audit finding F1 (MEDIUM): The two wallet
UPDATEs inauto_release_milestones(client debit, worker credit) lackedGET DIAGNOSTICS row_countverification.If either wallet row were missing, the
UPDATEwould silently affect 0 rows — money would vanish from the client's locked balance without arriving at the worker.Every other money-path RPC (
fund_escrow,approve_milestone,withdraw_wallet,topup_wallet) already has this guard.Fix
New forward-only migration
20260604060000_auto_release_row_count_guard.sqlthatCREATE OR REPLACEsauto_release_milestones()with:Everything else (FOR UPDATE, status guard, ledger insert order, REVOKE/GRANT) is identical to the prior version.
Validation
tsc --noEmit— cleanvitest run— 22/22 passnpx supabase db push— migration applied to devscratch/test_concurrent.ts— approve + auto-release case: money moves exactly once, 1 ledger row, correct balancesDiff
1 file changed:
supabase/migrations/20260604060000_auto_release_row_count_guard.sql(+146)No existing migrations modified.
CodeAnt-AI Description
Protect automatic milestone release from silently losing money
What Changed
Impact
✅ Fewer lost escrow transfers✅ Clearer auto-release failures✅ Safer milestone payouts💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.
Summary by CodeRabbit
Chores
Improvements