fix(wallet): bind top-up pending state to mutation lifecycle - #17
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 Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
✨ 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 finished reviewing your PR. |
|
@copilot review |
Reviewed PR on commit |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Group A (money/runtime): - #7: GRANT EXECUTE withdraw_wallet to authenticated + REVOKE anon/public - #6: restore NULL guard on p_amount in withdraw_wallet - #1-#4: reject NULL idempotency keys in all 4 escrow functions Group B (correctness): - #5: document p_idempotency_key API-consistency in submit_milestone - #8,#9: state-machine doc: submitted->released, annotate dead 'approved' - #20: remove 'approved' from MilestoneStatus union in escrow-machine.ts - #16: fix blob URL leak in worker-form.tsx (useEffect + revokeObjectURL) - #17: fix comment attribution CodeAnt -> CodeRabbit in wallet.ts - #18: add .max(500000) to withdrawWalletSchema Group C (dialog hardening): - #10,#11: change-phone-dialog: Zod validation + inFlightRef/mountedRef - #12,#13: delete-account-dialog: Zod validation + inFlightRef/mountedRef - #14: edit-profile-dialog: inFlightRef/mountedRef guards - #15: sign-out-button: useMutation refactor Group D (doc/consistency): - #22: README: Phase 2->4, Edge Functions->pg_cron, approved->released - #21: reconcile phase status README<->AGENTS.md - #23: ADR-0004: Proposed->Accepted - #19: client account page: race-safe requestIdRef + authError capture
…ardening (#20) * feat(wallet): worker withdrawal flow + auto-release cron * fix(wallet): address PR #19 review feedback - withdraw_wallet: add is_worker() check (A), p_amount NULL guard (F), p_idempotency_key NULL guard (C), insert-first/catch unique_violation pattern mirroring topup (D), public. schema qualifier (I), w. table alias to disambiguate available_balance vs RETURNS TABLE column - worker-form: replace as any cast with typed cast to fix pre-existing lint error surfaced during PR #19 pipeline - guard_milestones_status + guard_jobs_status: replace GUC bypass with direct session_user='postgres' check (E) - auto_release_milestones: drop set_config GUC line (E), add RAISE NOTICE on insufficient-locked-balance skip (H) - wallet.ts: mapWalletRpcError helper routing by error.code + exact RAISE EXCEPTION token (G), defense-in-depth is_worker() pre-check in withdrawWalletAction (A), raw error logging before generic fallback - topup-dialog + withdraw-dialog: stable idempotencyKey via useState initializer + useEffect reset on close (B), normalize 4-space to 2-space indent on withdraw-dialog (Copilot #9) * fix(wallet): PR #19 follow-up review (uuid fail-fast + ADR-0005) - uuid.ts: remove Math.random fallback, throw if secure crypto API unavailable; collision resistance is a correctness requirement for wallet idempotency keys (CodeRabbit follow-up #2) - docs/adr/0005: ADR approving in-place modification of auto_release_milestones per Copilot #1 (drop GUC bypass) and CodeRabbit #3 (RAISE NOTICE on silent skip), as required by ADR 0004 - pr19_review_fixes.sql: add ADR reference comment above auto_release_milestones rewrite for discoverability (CodeRabbit follow-up #3) Deferred to PR #20: Sentry.captureException wiring (CodeRabbit follow-up #1) — Sentry SDK installation is the dedicated scope of PR #20 per the roadmap; adding capture calls now would not compile. * fix: address PR #19 CodeAnt security and correctness findings - onboarding/actions.ts: remove PII console.log (fullName, categories, skillTags) that leaked KYC data into server logs on every onboarding attempt (CodeAnt Critical, #discussion_r3280686499) - onboarding/actions.ts: clean up orphaned selfie from kyc bucket when profiles or worker_profiles upsert fails after upload succeeds (CodeAnt Major, #discussion_r3280686511) - client/jobs/[id]/milestones/client-milestones.tsx: scope wallet fetch by profile_id of authenticated user instead of returning arbitrary first visible wallet row; prevents wrong-balance display and funding decisions if RLS ever surfaces multiple wallets (CodeAnt Critical, #discussion_r3280693926) - worker/feed/worker-feed.tsx: gate useInfiniteQuery on auth-ready via enabled:!!user?.id and scope queryKey by user?.id; prevents spurious unauthorized errors on initial mount before auth propagates (CodeAnt Major, #discussion_r3280707356) - worker/jobs/[id]/worker-job-detail.tsx: surface errors from msRes, matRes, appRes in Promise.all instead of silently coercing to empty arrays; CodeAnt flagged matRes specifically but the same pattern affected msRes and appRes (CodeAnt Major, #discussion_r3280707371) * fix: address PR #19 round-4 review findings - _actions/wallet.ts (topup+withdraw catch): replace raw err.message return with fixed generic copy to prevent backend internals (SQL, Supabase, runtime details) from being toasted to the user; server logs retain the full error via console.error (CodeAnt Major, #discussion_r3283709865, #discussion_r3283709876) - client/jobs/[id]/milestones/client-milestones.tsx: move auth resolution out of the fetcher; ClientMilestones now passes user.id into fetchMilestonesData and the useQuery is gated by enabled on user.id with user.id added to queryKey, matching the worker-feed pattern (CodeAnt Major, #discussion_r3283709880) - client-milestones.tsx: surface msRes.error and walletRes.error (with PGRST116 no-rows exception preserving zero-balance fallback) instead of silently rendering empty milestones / zero wallet on query failure (CodeRabbit Major, outside-diff line 91) - client-milestones.tsx + worker/jobs/[id]/milestones/worker-milestones.tsx: migrate idempotency_key generation from crypto.randomUUID to generateUuid so LAN/HTTP origins without secure context do not TypeError on submission (CodeRabbit Major, outside-diff line 208) - onboarding/actions.ts: capture and console.error storage cleanup failures in profileError and workerError branches; orphan accumulation is now observable in server logs pending Sentry wiring (CodeRabbit Nitpick, lines 148-171) - worker/feed/worker-feed.tsx: remove leftover WorkerFeed DEBUG console.log that fired on every render and dumped paginated job data to the browser console; also dropped the now-unused queryStatus destructure that the log was its only consumer (CodeRabbit Nitpick, line 148) * fix(escrow): mapEscrowRpcError + wallet scope + auth gates + migration style Fix 1 - escrow.ts: add mapEscrowRpcError helper (mirrors mapWalletRpcError); replace raw error.message returns and raw outer-catch msg with user-friendly strings + structured console.error across fund/submit/approve/dispute actions. Fix 2 - worker-milestones.tsx: scope wallet query to profile_id, add enabled on user.id and user.id to queryKey in useQuery (CodeRabbit Major + CodeAnt Critical sibling-pattern follow-up to client-milestones.tsx). Fix 3 - client-job-detail.tsx: add enabled on user.id and user.id to queryKey in useQuery to prevent cold-start firing before auth resolves. Fix 4 - 20260518135600_withdraw_wallet.sql: rename unique index to idx_escrow_ledger_withdraw_owner_reference per repo idx_table_purpose naming convention (Copilot Medium). Fix 5 - 20260518181500_withdraw_wallet_fix_ambiguous_column.sql: add public. schema qualifier to function definition and GRANT; change search_path style to set search_path to 'public' for consistency with other SECURITY DEFINER functions (Copilot Med/Low x3). Fix 6 - worker-form.tsx: replace className block/hidden step mounting with conditional render so only the active Step component is mounted; hooks and subscriptions on inactive steps no longer run (CodeAnt Major perf). All form state hoisted in FormProvider so unmount is safe. Fix 7 - 20260521080000_pr19_review_fixes.sql: expand header comment to explicitly document that pakka.allow_milestone_status_change GUC was DROPPED entirely, and enumerate the three allowed guard bypass conditions (SECURITY DEFINER, postgres superuser/pg_cron, is_admin). * fix(withdraw-wallet): INSERT-first idempotency, doc drift, auth hydration UX, count aggregates Fix 1 - 20260521080000_pr19_review_fixes.sql: reorder withdraw_wallet to INSERT into escrow_ledger BEFORE acquiring the wallet FOR UPDATE lock. Race-losers now deterministically hit the function-level EXCEPTION WHEN unique_violation handler instead of racing past the pre-check and hitting insufficient_balance. Removes the now-redundant pre-check SELECT fast-path (the INSERT/unique_violation handles it). Fix 2 - docs/state-machine.md: update two Edge Function cron references to pg_cron (database scheduler) at transition 5 caller and Key Callers table, matching the scheduling mechanism introduced in 20260518225500. Fix 3 - client-milestones.tsx: add !user?.id guard before isLoading and error checks so auth hydration renders MilestonesSkeleton instead of the error UI. Fix 4 - client-job-list.tsx: replace milestones(id)/job_applications(id) full ID-array fetches with milestones_count:milestones(count) / applications_count:job_applications(count) Supabase aggregate syntax; update mapping to read [0]?.count ?? 0 instead of .length. * chore: strip embedded NULL bytes from .gitignore (was treated as binary) * fix(round-7): 7 in-scope fixes for CR/Copilot/CodeAnt round-7 review - adr/0006-nav-shell.md: strip indent from Decision:/Consequences: labels - worker/account/page.tsx: replace null blank-screen with empty-state card + retry - features/withdraw-dialog.tsx: use formatInr(100) instead of hardcoded '₹100' - _actions/escrow.ts: correct mapEscrowRpcError JSDoc (routes by error.message, not SQLSTATE) - worker-milestones.tsx: throw on msRes.error; throw on walletRes.error unless PGRST116 - worker-milestones.tsx: replace err.message leak in handleSubmit catch with fixed user-safe toast - worker/feed/worker-feed.tsx: milestones(id) full-array -> milestones_count:milestones(count) aggregate 16 other findings deferred: - PR #20 (Sentry): escrow.ts 8 capture sites - PR #22 (canonical dialog migration): apply-modal, change-phone-dialog, delete-account-dialog, edit-profile-dialog, worker-form, client-milestones escrow handlers, withdraw-dialog Zod/optimistic - PR #23 (realtime auth-hydration): client-milestones setAuth - Reply-dismiss: client/worker-milestones auth guards (middleware-guarded routes) * fix(round-8): NaN guard, seed wallet locked, Sentry TODO, auth-hydration x3, idempotency-key map x2 - post-job-form.tsx: extract parseFiniteFloat helper; apply to all 5 numeric onChange (budget, lat, lng, milestone amounts, material qty/amount) so transient '-' or '.' inputs write undefined not NaN into form state. - seed-demo-users.ts: correct wallet locked_balance to match seeded ledger: Priya 40k -> 60k (J1-M2 funded), Rohit 0 -> 20k (J2-M1 submitted), Anjali 120k -> 30k (J3-M2 disputed). Comments reference source ledger rows. - worker-milestones.tsx: add // TODO: Sentry.captureException(err) as first line of handleSubmit catch block (canonical grep target for PR #20 wiring). - client-job-detail.tsx / client-milestones.tsx / worker-milestones.tsx: destructure isAuthLoading from useUser(); gate skeleton on (isAuthLoading || !user?.id) before the data/error branch so the hydration boundary no longer renders a false 'Failed to load' error path. - client-milestones.tsx + worker-milestones.tsx: introduce idempotencyKeysRef (Map<string, string>) with getOrCreateIdempotencyKey and clearIdempotencyKey helpers. fund/approve/dispute (client) and handleSubmit (worker) now reuse the same key across retries within a single user intent and rotate only on definitive success. Closes the client side of the idempotency contract; server-side uniqueness enforcement on the escrow RPCs is tracked separately in the Phase 4.5 escrow idempotency PR. Refs round-8 review: CodeAnt CRITICAL x3 (idempotency), CodeAnt HIGH (auth hydration), CodeAnt Major x3, CR Minor. * docs(adr): heading indent + pg_cron drift (round-9) - adr/0001-auth.md, 0002-escrow.md, 0003-dispute.md, 0005-realtime-contract.md, 0007-rpc-pattern.md: strip 2-space indent on Decision/Consequences headings so they render as top-level sections matching adr/template.md. - adr/0004-auto-release.md: same indent fix + rewrite Decision and Consequences to reflect pg_cron (not Edge Function cron) as the actual scheduler; reference migration 20260518225500_schedule_auto_release_cron.sql as source of truth. Round-9 #32/#33/#34 (CR Realtime setAuth on worker-milestones, client-milestones, client-job-detail): no code change. The singleton in src/lib/supabase/client.ts already calls realtime.setAuth() on both eager-prime and onAuthStateChange (resetting to anon key on sign-out, not null). Per-file setAuth wiring is unnecessary. Round-9 #41 + parallel client-milestones/withdraw idempotency findings: no code change. The escrow RPCs (submit_milestone, fund_escrow, approve_milestone, dispute_milestone) do not accept p_idempotency_key in their SQL signatures, so action-layer forwarding has nothing to bind to. RPC-level uniqueness enforcement is tracked in the Phase 4.5 escrow idempotency PR; the per-intent client-side key from round-8 Fix 7/8 is the application-layer half of that contract. Refs round-9 review: Copilot Medium x1 (auto-release doc drift), Copilot Low x5 (ADR heading indent), CR Nitpick x3 (Realtime setAuth - no-op), CodeAnt Major x1 (idempotency forwarding - deferred). * fix(escrow,wallet): honest idempotency comments + PostgrestError type (round-10) Fix 1/2 - worker-milestones.tsx + client-milestones.tsx: rewrite the per-intent idempotency map comment to remove the overpromising "server can deduplicate" claim. New wording: "forward-compatible with server-side RPC idempotency". Grep confirmed exactly 2 occurrences across the codebase; no helper JSDoc affected. Fix 3 - wallet.ts: tighten mapWalletRpcError error parameter from an ad-hoc structural type to PostgrestError. Mirrors the escrow.ts pattern established in round-5. Import added from @supabase/supabase-js. Pure type-tightening: supabase-js v2 (^2.104.1) types PostgrestError.code as string (non-nullable), so existing error.code === "42501" / "22023" comparisons remain valid without narrowing. Both call sites (mapWalletRpcError("topup", err), mapWalletRpcError("withdraw", err)) already pass a PostgrestError. Refs round-10 review: Copilot Medium x3. Round-10 Low (migration bundling, _pr19_review_fixes.sql) reply-dismissed: the consolidation is an intentional review artifact, section headers A/C/D/F/I/E/H already document the per-concern split internally, and retroactively splitting an applied migration on an active branch is destructive. * fix(round-11): auth error handling, auth-hydration redirect (4 files), withdraw_wallet exception scope, ADR cron alignment - worker/account/page.tsx: destructure error from supabase.auth.getUser(); surface a retryable error state on transient auth/network failure instead of redirecting signed-in users to /login. Sentry TODO marker added. - worker-milestones.tsx, client-job-detail.tsx: split the combined isAuthLoading || !user?.id branch into two — skeleton during hydration, router.replace('/login') + return null once auth has resolved with no user. useRouter import added to both. Belt-and-suspenders over the middleware route guard. - client-milestones.tsx: upgrade the post-round-8 split so the !user?.id branch also router.replace('/login') + return null instead of rendering the skeleton indefinitely. Consistent with the two files above. - 20260521080000_pr19_review_fixes.sql (withdraw_wallet): scope the unique_violation handler to only the INSERT INTO public.escrow_ledger via a nested BEGIN..EXCEPTION block. Other UNIQUE violations (present or future) now surface as errors instead of being misclassified as idempotency replays. - adr/0004-auto-release.md: correct the cron expression in the Decision text to 30 20 * * * (20:30 UTC / 02:00 IST, daily) — matches the actual schedule registered in 20260518225500_schedule_auto_release_cron.sql, which is the source of truth. Refs round-11 review: CodeAnt Major x3 (auth error, auth-hydration x2), Copilot Medium x2 (cron mismatch, withdraw_wallet exception scope). Round-11 CodeAnt CRITICAL x1 + Major x1 (idempotency key forwarding on submit/fund) reply-dismissed in PR threads: see Phase 4.5 escrow idempotency PR. * fix(auth): move render-phase redirects to useEffect + worker/account race guard (round-12) Fix 1 - worker-milestones.tsx, client-milestones.tsx, client-job-detail.tsx: move router.replace("/login") out of the render path and into a useEffect([isAuthLoading, user, router]). Render guard collapses back to the combined `isAuthLoading || !user?.id` -> <Skeleton /> shape, so the user sees a skeleton both during auth hydration and during the post-resolution navigation tick. No flash of stale UI; no React render-phase side-effect warning. Dependency array uses `user` (whole object) rather than `user?.id` to avoid the exhaustive-deps lint warning about optional chaining in deps. Closes the regression from round-11 Fix 2/3. Fix 2 - worker/account/page.tsx: clear local profile and worker state (setProfile(null), setWorker(null)) before router.replace("/login") in load()'s !user branch, so stale private data cannot flash between sign-out and navigation. Fix 3 - worker/account/page.tsx: add a requestIdRef race-condition guard to load(). Increments at call start; checks before every setState after each await (post-getUser, post-Promise.all). Concurrent triggers (initial mount, Retry, onSaved, onChanged) can no longer have an older in-flight request overwrite fresher state. Pattern is dependency-free; no AbortController used because supabase-js v2 doesn't support signal forwarding on .from() queries. Refs round-12 review: CodeAnt Major x4 (3 render-phase redirects, 1 stale state, 1 race condition; the render-phase finding was applied proactively to worker-milestones.tsx as well even though CodeAnt only flagged it on the two client files). Round-12 CodeAnt CRITICAL x1 (client-milestones.tsx:238 fund idempotency forwarding) reply-dismissed and conversation resolved in PR thread: see Phase 4.5 escrow idempotency PR. * fix(nav): use explicit job-id hrefs for milestone back-links (ADR-0036) * feat(escrow): wire idempotency keys end-to-end (ADR-0006) * fix(pr20): address CodeRabbit + Copilot review feedback Group A (money/runtime): - #7: GRANT EXECUTE withdraw_wallet to authenticated + REVOKE anon/public - #6: restore NULL guard on p_amount in withdraw_wallet - #1-#4: reject NULL idempotency keys in all 4 escrow functions Group B (correctness): - #5: document p_idempotency_key API-consistency in submit_milestone - #8,#9: state-machine doc: submitted->released, annotate dead 'approved' - #20: remove 'approved' from MilestoneStatus union in escrow-machine.ts - #16: fix blob URL leak in worker-form.tsx (useEffect + revokeObjectURL) - #17: fix comment attribution CodeAnt -> CodeRabbit in wallet.ts - #18: add .max(500000) to withdrawWalletSchema Group C (dialog hardening): - #10,#11: change-phone-dialog: Zod validation + inFlightRef/mountedRef - #12,#13: delete-account-dialog: Zod validation + inFlightRef/mountedRef - #14: edit-profile-dialog: inFlightRef/mountedRef guards - #15: sign-out-button: useMutation refactor Group D (doc/consistency): - #22: README: Phase 2->4, Edge Functions->pg_cron, approved->released - #21: reconcile phase status README<->AGENTS.md - #23: ADR-0004: Proposed->Accepted - #19: client account page: race-safe requestIdRef + authError capture * fix(pr20): row_number dedupe-delete + regen db types + type clients + per-request verify * fix(pr20): H1 shared release reference_id + ON CONFLICT, M3 idempotency error mapper, M1/N1/N5 hardening * fix(pr20): hard-constraint release ledger (remove ON CONFLICT) + per-row isolated auto-release + milestone-aware fund replay (CR-1) * fix(pr20): NULL-safe authz in dispute_milestone + audit NOT IN class + remove scratch files from PR
User description
Context
Post-merge codeant-ai review on #16 flagged that
startTransitionwas being passed an async callback intopup-dialog.tsx.useTransition'sisPendingonly stays true through the synchronous prefix of the callback — it flips false at the firstawait, while the RPC is still in flight. The synchronousinFlightRefgate prevents data corruption (no double-credit, no double-submit), but the UI lies:disabledreleases mid-request.!isPending && setOpen(o)) releases mid-request → user can close/cancel the dialog while the top-up is still in flight.Fix
Replace
useTransitionwithuseMutationfrom@tanstack/react-query.mutation.isPendingis bound to the full async mutationFn lifecycle, including every await.mutationFncallstopupWalletAction({ amount, idempotency_key: crypto.randomUUID() }).onSuccess: always invalidates["wallet"]; gates toast + setState onmountedRef.current(strict-mode safety).onError: gated toast witherr.message.onSettled: resetsinFlightRef.current = false.disabled,onOpenChangeclose lock, button label) now readmutation.isPending.inFlightRefsynchronous guard for the tiny render-lag double-click window.mountedRefstrict-mode reset.Matches the rest of the codebase's mutation pattern.
Verification
pnpm prettier --write✅pnpm build✅ green (only the 2 unrelateduseEffectwarnings onclient/account+worker/account).Refs
topup-dialog.tsxL50-53.CodeAnt-AI Description
Keep wallet top-up locked until the request finishes
What Changed
Impact
✅ Fewer duplicate top-ups✅ Fewer mid-request dialog closures✅ Clearer top-up status messages💡 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.