perf: read-path scale hardening — RCA-driven backend fixes + an authz leak found on the way#857
Merged
Merged
Conversation
…edits The refund cascade's WALLET leg branch credited the org wallet with no audit artifact. Org-wallet-funded B2C bookings (Payment.organizationId null, wallet owned by an org) were fully invisible to the org audit surface; org-tagged payments only got the payment-level INVOICE row, which the wallet-filtered view misses. walletCredit now returns the billing account's ownerOrgId (already selected, zero extra queries) and the cascade writes a category:WALLET / action:WALLET_REFUND row per wallet leg, org resolved via payment.organizationId ?? ownerOrgId, with balanceAfterPaise in details. Step 8's INVOICE row is unchanged — org-tagged wallet payments intentionally get both the funding-level and payment-level rows. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…/pending/[paymentId] A user who abandoned checkout had no way to release their own tentative hold — they waited out the 24h cleanup window (#833 residue). New cancelPendingCheckout core (Serializable tx, withSerializableRetry): - CAS claim PENDING→EXPIRED via guarded updateMany — the cancel-vs-webhook race closer per the #836/#838 doctrine; loser gets 409 - referral credits restored (reverseCreditsForPayment, same step as the abandoned-payments cron) - tentative slots released with per-type scoping mirroring the webhook's confirm scope: class = caller's slots across all session appointments, webinar = caller's slot only, consultation/subscription = all tentative - parent → CANCELLED via the CAS transition map with a NARROW fromIn (PENDING/APPROVED_PENDING_PAYMENT) — never cancels a parent another payment already confirmed; IllegalTransitionError rolls everything back - post-commit best-effort gateway cancel (never un-cancels on failure) Route: DELETE with getSession + new cancelPendingLimiter (10/min, own bucket so cancelling never consumes checkout quota). Dashboard: Cancel button beside the Processing pill on gateway_pending items in PendingPaymentsWidget. Tests: 10 unit cases + chaos scenario test-cancel-pending-vs-webhook (cancel-vs-capture-webhook with consistent end-state assertions incl. the documented late-capture orphan, plus cancel-vs-cancel double-submit leg). Known residual (follow-up issue): a genuine late capture can resurrect an EXPIRED payment to SUCCEEDED (handlers.ts confirm path) — orphan is flagged by the reconciler for refund; never a half-confirmed booking. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…delete scripts copy Two implementations of the most money-critical path was drift waiting to happen. The weekly GH job (jobs/payouts/process-payouts.ts) now drives lib/payments/payouts directly; scripts/payouts/process-payouts.ts is deleted (no shim — zero external importers besides the jobs wrapper and the test, both re-pointed). Closes the TODO #620 migration note. Deliberate behavior changes for the weekly job (decided on #850 review — the issue's 'no behaviour change' claim was wrong): - TDS 194-O withholding now applies (canonical compliance engine, TDS_ENGINE flag semantics unchanged) - Redis lock:payout_processing + retryCount < MAX filter now apply - org payouts (PENDING→PROCESSING advancement) ride the weekly job via new org-payout-service.processPendingOrgPayouts() Hardening ported INTO the service during consolidation: - #776 CAS claim: updateMany {id, status: APPROVED} — the service's plain update() could double-submit when two runners raced with Redis down; losers return skipped:true and never touch the gateway - silent-skip guard in the jobs wrapper: empty result + APPROVED payouts waiting (lock held / Redis down) now exits 1 instead of a green no-op - workflow env fix: the service reads RAZORPAYX_KEY_SECRET || RAZORPAY_SECRET — process-payouts.yml only set RAZORPAY_KEY_SECRET, which would have mass-failed every payout post-migration; RAZORPAY_SECRET is now mapped from the same secret - package.json scripts:process-payouts pointed at a nonexistent file; re-pointed at the jobs wrapper Test retargeted to the service path: both #785 quarantine invariants (post-gateway fail → PROCESSING + earnings linked; pre-gateway fail → FAILED + unlinked) plus a new CAS-skip case (gateway never called). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…pooler timeouts Both waitlist scripts imported CronLockHeldError but never called withCronLock — the catch blocks were dead code and concurrent runs could overlap. The scripts now wrap their bodies in withCronLock(<same names as the jobs/waitlist wrappers>, failMode 'open') so every entry point mutually excludes. The recurring scheduled failures (June 6/7/10) were Prisma ETIMEDOUTs on the FIRST query from GitHub runners against the Supabase pooler — Prisma 7 surfaces them as KnownRequestError code 'ETIMEDOUT'. New lib/db/connect-retry.ts (withDbConnectRetry, sibling of serializable-retry) retries only connectivity-shaped failures (ETIMEDOUT/ECONNRESET/ECONNREFUSED/P1001/P1002/init errors) with 2s/4s/8s backoff + jitter; business errors propagate immediately. Re-running the bodies is safe: reminderSentAt bounds re-sends, expiry processing is CAS-guarded. The other half of these issues — the module-load crash on missing UPSTASH_* env — was already fixed by 433091e (PR #851). Note: jobs/waitlist/* wrappers remain orphaned dead code (workflows call the scripts directly); flagged for a follow-up cleanup, not deleted here. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ated by this train TESTING_PLAN.md: route matrix with complex params for the four fixes (#849 cancel-pending authz/CAS matrix, #835 wallet-refund audit asserts, #850 payout job incl. concurrent-dispatch and silent-skip checks, #814/#821 cron lock-held/retry behavior), a 5-agent parallel execution protocol with deliberately colliding pairs, and per-scenario fixture + verification snippets (cancel-vs-cancel, cancel-vs-webhook, last-seat storm, waitlist-expire-vs-book, idempotency replay). Doc syncs: tentative TTL 7d→24h (#833) + self-serve release pointer in the cron doc; payout script paths re-pointed at jobs/payouts + lib/payments/payouts after the #850 deletion. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…s-core shape Convention recheck across all 54 cron workflows: every one calls a jobs/<domain>/<name>.ts entry except the two waitlist workflows, which invoked scripts/ directly while jobs/waitlist/* held full DUPLICATE implementations with zero callers — already drifted (the duplicate sent the event id where the script sends the plan id in reminder links). Restored the dominant shape (the cleanup-tentative-slots idiom): - scripts/waitlist/* are now import-only cores exporting sendExpirationReminders() / processWaitlistExpirations() — lock + connect-retry inside, structured results out, no side effects on import - jobs/waitlist/* are thin GH wrappers: abortIfMaintenance, GH outputs, CronLockHeldError clean-skip, exit codes - both workflows re-pointed at the jobs wrappers For the payout job (#850): rechecked and left as-is — jobs→lib-service with no scripts layer is the established NEWER convention (15 precedents: billing/*, compliance/*, contracts/*, stream/*, meetings/*) and the old wrapper's TODO #620 explicitly requested that migration; the scripts copy there was a divergent duplicate of lib, not the core. Also: master-runner mkdirs its gitignored results/reports dir instead of failing the whole run after every scenario passed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… cancel-pending Found by the multi-agent race validation (TESTING_PLAN §4.1): 8+ truly concurrent DELETEs on one payment produced a single 500 — the Serializable loser exhausted withSerializableRetry's 3 attempts and the final write conflict (Prisma 7 client-engine 'TransactionWriteConflict' / P2034) escaped the route's catch as a server error. A concurrent writer on this payment is a conflict outcome by definition — the winner's verdict stands and the client refetches — so the route now maps the conflict shapes to 409 alongside IllegalTransitionError. Re-validated: 12 concurrent cancels → exactly one 200, nine 409, two 429 (per-user limiter), zero 5xx; payment EXPIRED once, slots released, parent CANCELLED. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… requests bundle + composite indexes Read-path scale hardening for 100s-1000s of concurrent users (PR-2 of the train; findings from the scale audit, validated against live code): - slots/availability (#309 live half): removeBookedSlots no longer routes through Appointment with an UNFILTERED include (a 200-slot subscription returned all 200 rows + full appointment scalars per picker request) — the include is filtered to candidate times and nothing else is selected; Set lookup replaces O(n×m) Array.includes. Plus Cache-Control s-maxage=15/swr=60: a popular consultant's day now costs one origin hit per 15s instead of one per viewer; staleness is safe because checkout re-validates at confirm time (#788/#827). - participants/{class,webinar}: GET responses bounded — display-field user selects (the old user:true shipped every User scalar per participant per poll), waitlist capped at 500 with a truncated flag; DELETE no longer loads the entire roster to remove one participant (ownership check + targeted slot query); participantReadLimiter (30/min) on GET and eventMutationLimiter on DELETE — both routes previously had NO limiter. - requests tab: the /api/dashboard/consultant/[id]/requests bundle — six unbounded datasets with 4-level includes, prefetched on EVERY dashboard load — had ZERO data consumers (the page bound it to _requestsData and never read it; RequestSlotAllocationTab self-fetches the paginated /api/events/* endpoints and owns its loading states). Endpoint deleted with its fetcher/query/prefetch wiring; the page renders the tab immediately, removing a duplicate loading phase. The tab's real data source gets display-field user selects (GET+PATCH, consultations + subscriptions). - schema: composite indexes (consultationPlanId, requestStatus, requestedAt) + subscription mirror — serve the requests-tab hot path ORDER BY requestedAt DESC; pushed (additive). Search route already had s-maxage=60 (audit claim was stale — no change). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…file id
Found live during PR-2 route verification: Prisma IGNORES undefined in
where clauses, so a CONSULTANT session whose denormalized
User.consultantProfileId column was null got
`consultantProfile: { id: undefined }` — the ownership filter silently
vanished and GET /api/events/{consultations,subscriptions} served EVERY
consultant's requests (requester PII included). Verified leaked cross-
consultant rows with a real session before fixing.
Blast radius in dev was near-total: the seed created profiles via nested
create but never wrote the denormalized session-facing columns — 29/30
consultants and 40 consultees had null columns, so virtually every seeded
consultant session hit the unfiltered path. Runtime onboarding does set
the columns, so production exposure is limited to users whose columns
desynced.
Fixes:
- both collection GETs now use the participants-route idiom
(?? "__none__") so a missing profile id matches nothing instead of
everything
- seed writes the User.*ProfileId columns after profile creation
- dev rows repaired (76 users synced from their profile rows)
The item routes were already fail-closed (ownership compares against the
id, and undefined === x is false).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…imming, shared trending cache, waitlist index RCA-driven (3-agent investigation, findings verified against live routes — two agent claims were refuted by measurement: events count() was already parallelized, and the search route already had s-maxage): - planner: getParticipantCounts re-queried webinars the route had ALREADY fetched with slots+users — webinar counts now computed in-memory from the fetched rows (FIX #556 semantics preserved: non-tentative only, deduped, host excluded); classes keep their one batched query (their include carries no slots). The duplicate consultantProfile fetch is gone (reuses the org-scope lookup). Slot users slimmed to display fields — the full User row per attendee was the payload's biggest over-fetch. Verified live: 200, counts intact. - events/{consultations,subscriptions}: appointment.payment include slimmed from the full row (30+ cols incl. FX decimals) to {id, paymentStatus, amount, currency} at all 4 sites — no consumer reads more (verified by grep over both dashboards). - explore trending: the rank step (ALL marketplace plans × 30-day slot rows, sorted in JS) ran once per REQUEST under React.cache — unstable_cache now shares one computation across requests for 60s; cached value is just the ranked id array. - Waitlist (status, expiresAt) composite index — the reminder/expiration crons range-scan NOTIFIED by expiry window; pushed (additive). Ops (no code): PG_POOL_MAX=1 set in Netlify production env — serverless functions each opened a default 10-connection pool; at Netlify's concurrency that exhausts Supabase's connection cap (the #852 knob). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughConsolidates payout and waitlist jobs around canonical libs, adds cancelPendingCheckout with a guarded DELETE API and rate limiting, tightens Prisma selects and participant endpoints, records wallet refund org audit rows, introduces DB connect-retry utilities and tests, removes old payout scripts, and adds extensive tests and a multi-agent testing plan. ChangesJobs & Workflows
Waitlist cores
Payout Library
DB connect retry
Pending Checkout Cancellation
API selects & participant endpoints
Dashboard & explore programs
Wallet refund auditing & seeding
Prisma indexes
Misc & docs
Estimated code review effort
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a self-serve cancel-pending checkout endpoint, consolidates the payout jobs into the canonical service with TDS and locking, refactors waitlist crons to use a single source with connection retries, and optimizes various read paths by reducing over-fetching and caching trending programs. The review feedback highlights opportunities to maximize cache hits by caching the full trending lists rather than caching per-limit, to execute participant slot disconnections concurrently using prisma.$transaction instead of sequential loops, and to avoid mutating outer-scope variables inside retriable transaction blocks to prevent retry bugs.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
✅ Deploy Preview for familiarise ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
app/api/events/subscriptions/route.ts (1)
89-98:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInconsistent user select shape for
requestedBy.userbetween GET and PATCH.The GET handler's
requestedBy.userselect (lines 91-98) returns only{ id, name, email, image }, while the PATCH handler (line 175, 249) returns{ id, name, email, image, role, phone }. This creates inconsistent response shapes for the same nested entity. The consultations route uses the full select consistently for both profiles.Consider aligning with the PATCH shape for consistency:
Proposed fix
requestedBy: { include: { - user: { - select: { - id: true, - name: true, - email: true, - image: true, - }, - }, + user: { select: { id: true, name: true, email: true, image: true, role: true, phone: true } }, }, },🤖 Prompt for 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. In `@app/api/events/subscriptions/route.ts` around lines 89 - 98, The GET handler's requestedBy.user select in route.ts returns a reduced shape ({id,name,email,image}) while the PATCH handlers return the fuller shape ({id,name,email,image,role,phone}), causing inconsistent response shapes; update the GET handler's requestedBy.include.user.select to include role and phone to match the PATCH response (ensure the same select shape is used for requestedBy.user across the GET and PATCH handlers in this file, e.g., in the GET function and the PATCH/update handlers) so clients receive a consistent nested user object.__tests__/payments/refund-operation.test.ts (1)
702-706: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winSeed
ownerOrgIdon all wallet-funded test billing accounts.Two wallet-funded fixtures still omit
ownerOrgId; that creates unrealistic states for the new wallet-owner attribution path and can hide attribution regressions.Suggested fixture tightening
@@ state.billingAccounts.set("ba-mc", { id: "ba-mc", walletBalance: 50000, currency: "INR", + ownerOrgId: "org-1", }); @@ state.billingAccounts.set("ba-neg", { id: "ba-neg", walletBalance: 50000, currency: "INR", + ownerOrgId: "org-1", });Also applies to: 1043-1047
🤖 Prompt for 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. In `@__tests__/payments/refund-operation.test.ts` around lines 702 - 706, The test fixture for billing accounts is missing ownerOrgId for wallet-funded accounts; update the calls to state.billingAccounts.set (e.g., the entry with id "ba-mc" and the other occurrence around lines 1043-1047) to include an ownerOrgId field (set to a realistic org id used in tests, e.g., "org-..." or the same org used elsewhere in the test) so wallet-funded billingAccount fixtures always include ownerOrgId for correct wallet-owner attribution.app/dashboard/consultee/[consulteeId]/(features)/home/PendingPaymentsWidget.tsx (1)
67-69:⚠️ Potential issue | 🟠 Major | ⚡ Quick winClear stale error state after a successful refetch.
On Line 68, successful responses update
pendingPaymentsbut never clearerror. After one transient failure, the widget can stay stuck in the error branch even when later polls succeed.Suggested fix
const data = await response.json(); + setError(null); setPendingPayments(data.pendingPayments || []);🤖 Prompt for 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. In `@app/dashboard/consultee/`[consulteeId]/(features)/home/PendingPaymentsWidget.tsx around lines 67 - 69, The fetch success path updates pendingPayments but doesn't clear the error state, so after a transient fetch failure the component can remain in the error branch; in the success block (where setPendingPayments(data.pendingPayments || [] ) is called) also call the state setter that clears the error (e.g., setError(null) or setFetchError(undefined)) to reset any previous error, ensuring the component can render the successful state—locate the fetch logic and relevant state setters (setPendingPayments and the error state setter) in PendingPaymentsWidget and add the clear-error call immediately after updating pendingPayments.
🤖 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 `@__tests__/payments/cancel-pending-checkout.test.ts`:
- Around line 281-410: The tests cover consultation, webinar, and class flows
but miss the subscription-parent branch of cancelPendingCheckout; add a new test
that seeds a payment with subscriptionId (e.g., paymentId "pay-s", userId
"user-1", paymentStatus "PENDING", subscriptionId set) and then calls
cancelPendingCheckout({ paymentId: "pay-s", userId: "user-1" }); assert that
transitionSubscriptionRequest was invoked with the subscription id (or
appropriate args), the payment's paymentStatus becomes "EXPIRED" (or expected
final status), cancelPaymentIntent behavior is handled (mock reject/resolve
variants), and no unrelated slots are removed; reference cancelPendingCheckout
and transitionSubscriptionRequest in the test to ensure the guarded
subscription-parent transition path is covered.
In `@jobs/payouts/process-payouts.ts`:
- Around line 123-127: When merging orgResult.errors into result in the block
that currently logs org errors (the orgResult.errors.forEach(...) branch), also
increment result.failed by the number of org errors so job signals remain
consistent; e.g. add result.failed += orgResult.errors.length (or increment per
error) alongside result.errors.push(...orgResult.errors) and result.success =
false in the same scope of the processPayouts handling code.
In `@jobs/waitlist/process-expired-notifications.ts`:
- Line 75: The top-level call main() is unhandled and may produce an unhandled
promise rejection; update the invocation to explicitly handle errors by chaining
a .catch(err => { processLogger?.error('process-expired-notifications failed',
err); process.exit(1); }) and (optionally) a .finally(() => process.exit(0)) or
ensure cleanup, referencing the existing main() function in this file; this
ensures any rejection from main() is logged and the process exits with an
appropriate non-zero code.
In `@lib/data/explore-programs.ts`:
- Around line 72-115: The cache key for unstable_cache in
getTrendingClassPlanIds (and similarly getTrendingWebinarPlanIds) omits the
limit argument causing stale results across different limits; fix by making the
cached call parameterized — either wrap the unstable_cache inside a non-cached
function so you call unstable_cache with a key that includes String(limit)
(e.g., ["trending-class-plan-ids", String(limit)]) or fetch a single large max
result inside the cache and then slice to the requested limit after the cached
call; update the unstable_cache invocation(s) accordingly to include the limit
in the key or use the fixed-large-fetch-and-slice approach so different limit
values return correct results.
In `@lib/payments/operations/refund.ts`:
- Around line 393-409: The WALLET ledger posting still uses
payment.organizationId only, causing mismatched attribution for org-funded B2C
refunds; update the WALLET ledger posting creation to use the same fallback as
the audit row by setting organizationId to payment.organizationId ??
credit.ownerOrgId (replace occurrences where ledger postings for type "WALLET"
are created — e.g., the Step 9 WALLET ledger posting and the similar block
around lines 760-773), ensuring ledger entries use credit.ownerOrgId when
payment.organizationId is null.
In `@lib/payments/payouts/org-payout-service.ts`:
- Around line 804-806: processOrgPayout can return "PROCESSING" both when this
worker successfully claimed the payout and when another worker already holds the
claim, so incrementing result.advanced on out.status === "PROCESSING"
overcounts; update processOrgPayout to include a deterministic claim indicator
(e.g., add a boolean property like claimed or a distinct status
"CLAIMED_BY_THIS_WORKER") and then change the caller (the code checking out from
processOrgPayout and touching result.advanced) to increment only when
out.claimed === true (or out.status === "CLAIMED_BY_THIS_WORKER"), leaving
no-op/other-worker paths uncounted.
In `@prisma/seedFiles/1a-create-users.ts`:
- Around line 630-638: The seed code updates denormalized profile IDs via
prisma.user.update but then still pushes the original pre-update user object
(see users.push(user)), causing callers of createUsers() to receive stale null
profile IDs; fix by using the updated user returned from prisma.user.update (or
re-fetching the user) and push that updated object into the users array so the
output reflects the DB changes and includes
consultantProfileId/consulteeProfileId/staffProfileId/adminProfileId.
In `@TESTING_PLAN.md`:
- Around line 224-225: Replace the informal term "ACKed" in the invariant text
with the clearer word "acknowledged" (e.g., change "webhook ACKed 2xx always" to
"webhook acknowledged 2xx always"); ensure any other occurrences of "ACKed" in
TESTING_PLAN.md use "acknowledged" for consistent, clearer documentation
language.
In
`@tests/typescript/race-conditions/scenarios/07-real-api-booking/test-cancel-pending-vs-webhook.ts`:
- Around line 247-251: The current test assertion only checks tentativeLeft ===
0 for the "webhook win" branch; update the check invocation (the call to check
with message "leg1/B: webhook win confirmed the booking (no tentative residue)")
to also assert the parent booking's status is not "CANCELLED" (e.g., include a
condition like parent?.status !== "CANCELLED" or an explicit status check) so
the invariant ensures no contradictory terminal parent state; keep the existing
tentativeLeft, confirmed and parent diagnostics in the object passed to check.
- Around line 324-330: The update is overwriting real DB state by hard-resetting
cancellationNotes/cancelledAt to null; instead restore the original values from
the saved snapshot (use the snapshot or the original consultation object) when
calling prisma.consultation.update so you set cancellationNotes:
consultationSnapshot.cancellationNotes and cancelledAt:
consultationSnapshot.cancelledAt (or consultation.cancellationNotes /
consultation.cancelledAt) rather than null, preserving pre-existing data in
shared dev fixtures.
---
Outside diff comments:
In `@__tests__/payments/refund-operation.test.ts`:
- Around line 702-706: The test fixture for billing accounts is missing
ownerOrgId for wallet-funded accounts; update the calls to
state.billingAccounts.set (e.g., the entry with id "ba-mc" and the other
occurrence around lines 1043-1047) to include an ownerOrgId field (set to a
realistic org id used in tests, e.g., "org-..." or the same org used elsewhere
in the test) so wallet-funded billingAccount fixtures always include ownerOrgId
for correct wallet-owner attribution.
In `@app/api/events/subscriptions/route.ts`:
- Around line 89-98: The GET handler's requestedBy.user select in route.ts
returns a reduced shape ({id,name,email,image}) while the PATCH handlers return
the fuller shape ({id,name,email,image,role,phone}), causing inconsistent
response shapes; update the GET handler's requestedBy.include.user.select to
include role and phone to match the PATCH response (ensure the same select shape
is used for requestedBy.user across the GET and PATCH handlers in this file,
e.g., in the GET function and the PATCH/update handlers) so clients receive a
consistent nested user object.
In
`@app/dashboard/consultee/`[consulteeId]/(features)/home/PendingPaymentsWidget.tsx:
- Around line 67-69: The fetch success path updates pendingPayments but doesn't
clear the error state, so after a transient fetch failure the component can
remain in the error branch; in the success block (where
setPendingPayments(data.pendingPayments || [] ) is called) also call the state
setter that clears the error (e.g., setError(null) or setFetchError(undefined))
to reset any previous error, ensuring the component can render the successful
state—locate the fetch logic and relevant state setters (setPendingPayments and
the error state setter) in PendingPaymentsWidget and add the clear-error call
immediately after updating pendingPayments.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 43a2afb7-2090-4002-b6c1-af36c4c231cd
📒 Files selected for processing (44)
.github/workflows/process-payouts.yml.github/workflows/process-waitlist-expirations.yml.github/workflows/send-waitlist-reminders.yml.gitignoreTESTING_PLAN.md__tests__/enterprise/process-payouts-false-failed.test.ts__tests__/lib/connect-retry.test.ts__tests__/payments/cancel-pending-checkout.test.ts__tests__/payments/refund-operation.test.tsapp/api/checkout/pending/[paymentId]/route.tsapp/api/dashboard/consultant/[consultantId]/planner/route.tsapp/api/dashboard/consultant/[consultantId]/requests/route.tsapp/api/events/consultations/route.tsapp/api/events/subscriptions/route.tsapp/api/participants/class/[classId]/route.tsapp/api/participants/webinar/[webinarId]/route.tsapp/api/slots/availability/[consultantId]/route.tsapp/dashboard/consultant/[consultantId]/(features)/requests/page.tsxapp/dashboard/consultee/[consulteeId]/(features)/home/PendingPaymentsWidget.tsxdocs/booking/13-cron-jobs-and-background-tasks.mddocs/payments/payouts/06-configuration.mddocs/prisma/prisma-7-migration.mdhooks/useConsultantPrefetchDashboard.tsjobs/payouts/process-payouts.tsjobs/waitlist/process-expired-notifications.tsjobs/waitlist/send-expiration-reminders.tslib/api/organizations/wallet.tslib/dashboard-queries.tslib/data/explore-programs.tslib/db/connect-retry.tslib/payments/operations/cancel-pending.tslib/payments/operations/refund.tslib/payments/payouts/index.tslib/payments/payouts/org-payout-service.tslib/payments/payouts/payout-service.tslib/rate-limit.tspackage.jsonprisma/schema.prismaprisma/seedFiles/1a-create-users.tsscripts/payouts/process-payouts.tsscripts/waitlist/process-expired-notifications.tsscripts/waitlist/send-expiration-reminders.tstests/typescript/race-conditions/master-runner.tstests/typescript/race-conditions/scenarios/07-real-api-booking/test-cancel-pending-vs-webhook.ts
💤 Files with no reviewable changes (3)
- scripts/payouts/process-payouts.ts
- app/api/dashboard/consultant/[consultantId]/requests/route.ts
- lib/dashboard-queries.ts
…bution, send-result handling, cron retry boundary Legit findings from the Gemini/CodeRabbit review pass, each verified against current code before fixing (full triage table on the PR): - cancel-pending: gatewayCancel now returned FROM the Serializable tx — the outer-scope mutation could leak a stale value when an aborted attempt retried into an early-exit path, firing a gateway cancel for an order this run did not expire - refund step 9: the WALLET ledger posting now attributes to payment.organizationId ?? wallet-owner org (captured from walletCredit) — keying off the payment tag alone posted org-wallet-funded B2C refunds to a null-org WALLET account, guaranteeing WALLET_BALANCE_DRIFT at reconcile (the same attribution rule the #835 audit row already uses) - waitlist cores: the connect-retry now wraps ONLY a SELECT 1 warm-up — retrying the whole body could replay non-idempotent email sends; and both cores now branch on the senders' { success:false } contract instead of counting every awaited call as delivered (a failed reminder no longer stamps reminderSentAt and silently drops forever) - processOrgPayout: claimed flag surfaced through the public return so batch callers count real PENDING→PROCESSING claims, not another worker's status echo; jobs wrapper gains org_errors output so success=false is always explained (failed stays consultant-only) - jobs/waitlist wrappers: main().catch parity with the payouts wrapper - handle-stuck-payouts.yml + reconcile-payout-status.yml: RAZORPAY_SECRET mapping added — same env-name trap as #850's process-payouts fix (the payouts client never reads RAZORPAY_KEY_SECRET; the job-level gate passing alone left the client with an empty secret) - chaos scenario: fixture restoration moved into finally (early throws left the shared fixture dirty), cancellation fields restore from snapshot instead of hard-nulling, webhook-win leg asserts the parent was not cancelled; subscription-parent unit coverage added - connect-retry comment corrected (3 attempts = two sleeps); TESTING_PLAN env-regression row tightened; 'ACKed' → 'acknowledged' Declined as wrong (justifications on the threads): nullable-paymentIntent guard (schema: non-null @unique), null-targetOrgId guard (ownerOrgId is required + walletCredit throws P2025 first), cancelPaymentIntent 'reason' param claim (actual signature takes the gateway). tsc clean; 1357/1357 jest (incl. updated processOrgPayout shape assertions and 2 new subscription-parent cases). Part of #854 / #857. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…read-path-scale-hardening
…isconnects, seed object sync - trending rank caches the FULL ranked id list and callers slice to their limit — unstable_cache keys include fn args, so the limit parameter created one cache entry (and one full scan) per distinct limit. (The contradictory bot suggestions both land here: Gemini's per-limit-entry point was right; CodeRabbit's stale-cross-limit claim was wrong since args ARE keyed — moot either way with no arg.) - participants DELETE: per-slot sequential updates → one prisma.$transaction batch (atomic; a mid-loop failure could partially remove a participant) - seed: the in-memory user object now mirrors the denormalized profile-id column update — downstream seed files received it with stale nulls tsc clean, 1357/1357 jest. Part of #857. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/api/participants/class/[classId]/route.ts (1)
191-193:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
slotsAvailablehardcoded to 1 in both routes may under-notify waitlist.Both class and webinar DELETE handlers pass
slotsAvailable: 1tohandleSlotOpening, butuserSlots.lengthcould be > 1 if the removed participant occupied multiple slots. The shared root cause is that neither route accounts for multi-slot participants.Per
lib/waitlist/slot-handler.ts,handleSlotOpeningnotifies up toslotsAvailablequeued users. If a participant held N slots, only 1 waitlist user is notified instead of N.If the data model allows a single user to occupy multiple slots in the same appointment, both routes should pass
userSlots.lengthinstead of1.🤖 Prompt for 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. In `@app/api/participants/class/`[classId]/route.ts around lines 191 - 193, The DELETE handlers currently call handleSlotOpening with slotsAvailable: 1 which under-notifies when a removed participant occupied multiple slots; change both places where participantRemoved is true to compute slotsAvailable from the removed participant's slot count (use userSlots.length or the equivalent variable that holds how many slots the participant held) and pass that value into handleSlotOpening({ classId, slotsAvailable: userSlots.length }) (and the analogous call in the webinar route) so handleSlotOpening can notify up to the correct number of waitlist users.
🤖 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.
Outside diff comments:
In `@app/api/participants/class/`[classId]/route.ts:
- Around line 191-193: The DELETE handlers currently call handleSlotOpening with
slotsAvailable: 1 which under-notifies when a removed participant occupied
multiple slots; change both places where participantRemoved is true to compute
slotsAvailable from the removed participant's slot count (use userSlots.length
or the equivalent variable that holds how many slots the participant held) and
pass that value into handleSlotOpening({ classId, slotsAvailable:
userSlots.length }) (and the analogous call in the webinar route) so
handleSlotOpening can notify up to the correct number of waitlist users.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: adff8c5d-612a-4674-bace-cc1fd6c0d691
📒 Files selected for processing (19)
.github/workflows/handle-stuck-payouts.yml.github/workflows/reconcile-payout-status.ymlTESTING_PLAN.md__tests__/enterprise/live-payout-submission.test.ts__tests__/payments/cancel-pending-checkout.test.tsapp/api/participants/class/[classId]/route.tsapp/api/participants/webinar/[webinarId]/route.tsjobs/payouts/process-payouts.tsjobs/waitlist/process-expired-notifications.tsjobs/waitlist/send-expiration-reminders.tslib/data/explore-programs.tslib/db/connect-retry.tslib/payments/operations/cancel-pending.tslib/payments/operations/refund.tslib/payments/payouts/org-payout-service.tsprisma/seedFiles/1a-create-users.tsscripts/waitlist/process-expired-notifications.tsscripts/waitlist/send-expiration-reminders.tstests/typescript/race-conditions/scenarios/07-real-api-booking/test-cancel-pending-vs-webhook.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
PR-2 of the train (stacked on #854). Read-path fixes for 100s–1000s of concurrent users, driven by a 3-agent RCA over frontend/backend/DB — every agent claim was re-verified against live routes before acting (two were refuted: events
count()was already parallelized; search already hads-maxage).Security fix found during live verification (commit
fcf52b62^..)Ownership filter silently dropped for sessions with a missing profile id. Prisma ignores
undefinedin where clauses, soconsultantProfile: { id: undefined }matched every consultant's consultations/subscriptions on the collection GETs — verified leaking cross-consultant rows (requester PII) with a real session. Root cause chain: the seed never wrote the denormalizedUser.*ProfileIdcolumns the session layer reads (29/30 consultants + 40 consultees desynced in dev). Fixed with the participants-route?? "__none__"idiom on both GETs, the seed now syncs the columns, and the 76 dev rows were repaired. Item routes were already fail-closed.Perf fixes
/api/dashboard/consultant/[id]/requests— six unbounded datasets with 4-level includes, prefetched on every dashboard load, zero data consumers (page bound it to_requestsData, never read; the tab self-fetches paginated/api/events/*). Page renders the tab immediately — one loading phase instead of two.removeBookedSlotsinclude filtered to candidate times (was unbounded — 200-slot subscriptions returned all 200 rows per picker request);Cache-Control: s-maxage=15, swr=60— a popular consultant's day costs one origin hit per 15s.user: true→ display selects (GET+PATCH),paymentslimmed to{id, paymentStatus, amount, currency}at all 4 sites.unstable_cache60s (was per-request).Consultation(consultationPlanId, requestStatus, requestedAt)+ Subscription mirror (requests-tab hot path),Waitlist(status, expiresAt)(cron range scans).Ops (done, no code)
PG_POOL_MAX=1set in Netlify production env — each serverless function opened a default 10-connection pool; at Netlify concurrency that exhausts Supabase's cap (the #852 knob exists for exactly this).Verification
tsc clean, 1355/1355 jest, lint adds no errors; planner/events/availability smoke-tested live (200s, shapes intact, counts correct); authz leak re-tested closed (0 rows for the degenerate session).
Follow-up
PR-2.5 (next): frontend cache-first tab smoothness from the same RCA — refetchOnWindowFocus default, keepPreviousData, Stream SDK out of layouts, poller consolidation.
Part of the post-#837 hardening train. Touches the live half of #309.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
Tests