Skip to content

perf(dashboard): cache-first tab smoothness — focus-refetch default, keepPreviousData, poller hygiene#858

Merged
teetangh merged 15 commits into
devfrom
perf/dashboard-tab-smoothness
Jun 11, 2026
Merged

perf(dashboard): cache-first tab smoothness — focus-refetch default, keepPreviousData, poller hygiene#858
teetangh merged 15 commits into
devfrom
perf/dashboard-tab-smoothness

Conversation

@teetangh

@teetangh teetangh commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

What

PR-2.5 of the train (stacked on #857) — the frontend half of the explore/dashboard slowness RCA, per the agreed cache-first approach. The finding: tab-to-tab lag was mostly query config, not routing — App Router already persists the dashboard layout across tab switches, so the fix is letting the warm React Query cache actually serve.

  • refetchOnWindowFocus defaults to false (was globally true): every window focus refired every mounted query — returning to the browser tab felt like a full reload. The payment-status surface that justified the old default keeps freshness explicitly: PendingPaymentsWidget opts back in and polls; the admin approval/home pages already poll via refetchInterval.
  • PendingPaymentsWidget → React Query: the manual fetch + setInterval(120s) restarted from zero on every tab-switch remount and kept firing in unfocused windows. Now useQuery + refetchInterval — deduped via the shared cache, paused while unfocused, cancel action invalidates the key.
  • Requests-tab poller skips ticks while document.hidden.
  • keepPreviousData on all org-scope-filtered queries (consultant appointments/trials/classes/webinars, planner, consultee appointments) — filter changes show the previous list instead of a skeleton flash; mirrors the documents-page idiom (feat(documents): Add pagination to document dashboards #346).
  • Stream video CSS co-located in StreamProvider (was imported by both dashboard layouts).

Deliberately NOT in this PR

  • Stream SDK JS splitting (~771KB)StreamProvider's module-level imports still ship with the dashboard; that's perf: bundle analysis results — Stream.io SDK at 771KB, 5 quick wins, phased optimization plan #639's phased work. A naive dynamic() on a provider that wraps children would blank the dashboard until the chunk loads — it needs the context-shell refactor.
  • Tab re-architecture (parallel routes / client tabs) — per the RCA decision, measure after these quick wins; layouts already persist.
  • Sidebar prefetch — already present (prefetch + hover-prefetch), verified.

Verification

tsc clean, 1355/1355 jest, lint adds no errors; explore + dashboard pages smoke-tested 200 live.

Part of the post-#837 hardening train (RCA in #857's description).

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Users can cancel pending payments from the Pending Payments widget (Cancel button; shows conflict/notice when cancellation races or is no longer possible).
  • Performance & UX Improvements

    • Dashboard and planner lists preserve prior results while loading to reduce UI flashing.
    • Reduced API payloads by returning only needed user/payment fields.
    • Trending programs ranking cached for 60s; DB indexes added for faster lookups.
  • Reliability & Bug Fixes

    • Improved retry/resilience for DB connections and more robust waitlist/payout background processing.
  • Documentation

    • Tentative-slot hold window changed from 7 days to 24 hours.

teetangh and others added 11 commits June 11, 2026 14:03
…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>
…keepPreviousData, poller hygiene

Frontend half of the perf RCA (PR-2.5). The tab-to-tab lag was mostly
query config, not routing — App Router already persists the dashboard
layout across tab switches, so the wins come from letting the warm React
Query cache actually serve:

- refetchOnWindowFocus now defaults to FALSE: the old global true refired
  every mounted query on each window focus — returning to the browser tab
  felt like a full reload. The payment-status surface that justified the
  old default keeps freshness explicitly (PendingPaymentsWidget opts back
  in + polls; the admin approval/home pages already poll via
  refetchInterval).

- PendingPaymentsWidget: manual fetch + setInterval(120s) replaced with
  useQuery + refetchInterval — deduped across tab-switch remounts via the
  shared cache, paused while the window is unfocused
  (refetchIntervalInBackground=false), cancel action invalidates the key.

- RequestSlotAllocationTab poller skips ticks while document.hidden.

- keepPreviousData on the org-scope-filtered queries (consultant
  appointments + trials + classes + webinars, planner, consultee
  appointments) — filter changes show the previous list instead of a
  skeleton flash; mirrors the documents-page idiom (#346).

- Stream video CSS co-located in StreamProvider (was imported by both
  dashboard layouts). NOTE: the Stream SDK JS (~771KB) still loads with
  the dashboard via StreamProvider's module-level imports — splitting
  that is #639's phased work; a naive dynamic() on a provider that wraps
  children would blank the dashboard until the chunk loads.

Sidebar already had prefetch + hover-prefetch — no change needed.

tsc clean, 1355/1355 jest, lint adds no errors; explore + dashboard
pages smoke-tested 200 against the dev server.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Consolidates payout/workers to canonical services, adds cancel-pending API and DB retry helpers, tightens Prisma queries/authorization, improves frontend query UX and polling, records wallet refund audit attribution, adds indexes and caching, and expands tests/docs for multi-agent race validation.

Changes

Booking & Finance Hardening

Layer / File(s) Summary
Payout Job Refactoring & CAS Claim Pattern
jobs/payouts/process-payouts.ts, lib/payments/payouts/*, scripts/payouts/process-payouts.ts (deleted), .github/workflows/process-payouts.yml, package.json, docs/payments/payouts/*, docs/prisma/prisma-7-migration.md, __tests__/enterprise/process-payouts-false-failed.test.ts
Move payout processing from deleted scripts/ wrapper to jobs/ job entrypoint driving lib/payments/payouts. Add atomic update-claim (APPROVED→PROCESSING) to avoid double-processing, export new OrgProcessingResult and PayoutResult.skipped, update job outputs and tests.
Waitlist Job Refactoring to Importable Cores
scripts/waitlist/process-expired-notifications.ts, scripts/waitlist/send-expiration-reminders.ts, jobs/waitlist/process-expired-notifications.ts, jobs/waitlist/send-expiration-reminders.ts, .github/workflows/process-waitlist-expirations.yml, .github/workflows/send-waitlist-reminders.yml
Refactor waitlist scripts into importable cores (typed results, disconnect helper) and convert jobs to GitHub Actions wrappers that call cores, write GITHUB_OUTPUT, handle CronLockHeldError, and exit appropriately.
Pending Payment Cancellation Feature
app/api/checkout/pending/[paymentId]/route.ts, lib/payments/operations/cancel-pending.ts, lib/rate-limit.ts, __tests__/payments/cancel-pending-checkout.test.ts, tests/typescript/race-conditions/.../test-cancel-pending-vs-webhook.ts
Add cancelPendingCheckout with Serializable transaction and CAS update, tentative-slot deletions scoped by appointment type, optional consultation/subscription cancellation, and best-effort post-commit gateway cancellation. Provide DELETE route translating outcome codes to 200/404/409. Add cancelPendingLimiter and UI cancel flow plus tests including race scenarios.
DB Connect Retry & Wallet Audit Logging
lib/db/connect-retry.ts, lib/api/organizations/wallet.ts, lib/payments/operations/refund.ts, __tests__/lib/connect-retry.test.ts, __tests__/payments/refund-operation.test.ts
Add isDbConnectError and withDbConnectRetry with exponential backoff+jitter and tests. Extend walletCredit to return ownerOrgId; emit WALLET orgAuditLog rows for wallet refund legs and attribute ledger postings with payment.organizationId ?? walletOwnerOrgId.
API Endpoint Query & Authorization Hardening
app/api/events/consultations/route.ts, app/api/events/subscriptions/route.ts, app/api/participants/class/[classId]/route.ts, app/api/participants/webinar/[webinarId]/route.ts, app/api/dashboard/consultant/[consultantId]/planner/route.ts, app/api/slots/availability/[consultantId]/route.ts
Ensure non-privileged ownership filters use sentinel __none__ to avoid dropped constraints; replace broad include: { user: true } with explicit select projections; add participant read/mutation limiters and waitlist caps; refactor planner participant counts and optimize booked-slot checks; add short-lived cache headers for availability.
Frontend UX: React Query, Polling, and Styles
app/dashboard/.../appointments/page.tsx, planner/page.tsx, requests/page.tsx, RequestSlotAllocationTab.tsx, PendingPaymentsWidget.tsx, providers/ReactQueryProvider.tsx, providers/StreamProvider.tsx
Use keepPreviousData to reduce UI flashing during scope changes, make polling visibility-aware, refactor pending-payments to React Query with cancel button (DELETE API), disable global refetch-on-window-focus, and move Stream Video CSS import into provider.
Caching, Indexing, Seeding & Query Surface
lib/data/explore-programs.ts, prisma/schema.prisma, prisma/seedFiles/1a-create-users.ts, lib/dashboard-queries.ts, hooks/useConsultantPrefetchDashboard.ts
Add cached trending helpers via unstable_cache (60s), add composite Prisma indexes for Consultation/Subscription/Waitlist hot paths, denormalize *ProfileId in seeds, and remove consultant requests fetcher/prefetch from centralized dashboard queries.
Testing & Documentation
__tests__/*, tests/typescript/race-conditions/*, TESTING_PLAN.md, docs/booking/13-cron-jobs-and-background-tasks.md, .gitignore
Add/adjust many unit and E2E tests (connect-retry, cancelPendingCheckout, refund audit assertions, payout tests, race harness test), ensure master runner creates reports dir, add TESTING_PLAN for multi-agent validation, and update tentative-slot TTL doc (7 days → 24 hours).

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related issues

"🐇 I hopped through jobs and queues today,
Payouts claim their turn, no double-pay.
Waitlists tidy, reminders sing on time,
Cancels race fair, and tests chase the rhyme.
A tiny carrot of logs — all green and fine."

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch perf/dashboard-tab-smoothness

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a self-service cancellation mechanism for pending checkouts via a new DELETE endpoint, adds robust payout processing improvements including TDS withholding and Redis locking, and optimizes several dashboard data-fetching paths. Key changes include the addition of a cancel-pending checkout operation, migration of payout processing to a canonical service, and performance enhancements such as query batching and improved caching strategies. No review comments were provided, so no filtering was required.

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.

@netlify

netlify Bot commented Jun 11, 2026

Copy link
Copy Markdown

Deploy Preview for familiarise ready!

Name Link
🔨 Latest commit b51fb19
🔍 Latest deploy log https://app.netlify.com/projects/familiarise/deploys/6a2aaf530f27b7000759112c
😎 Deploy Preview https://deploy-preview-858--familiarise.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 37 (🔴 down 15 from production)
Accessibility: 90 (no change from production)
Best Practices: 83 (no change from production)
SEO: 83 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify project configuration.

teetangh and others added 2 commits June 11, 2026 18:11
…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>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 9

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
app/dashboard/consultant/[consultantId]/(features)/requests/RequestSlotAllocationTab.tsx (1)

331-344: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Refresh immediately when the tab becomes visible again.

Line 340 skips hidden ticks, but there is no catch-up fetch on visibility restore, so data can stay stale for up to the full poll interval. Add a visibilitychange listener that triggers fetchData() when document.hidden flips to false.

Proposed fix
   useEffect(() => {
     fetchData();
     // Set up polling for real-time updates
     const REQUEST_POLL_INTERVAL = parseInt(
       process.env.NEXT_PUBLIC_REQUEST_POLL_INTERVAL ?? "300000",
     ); // 5 minutes
     // Perf RCA: skip the tick while the tab is hidden — the old interval
     // kept hitting the API from backgrounded tabs.
     const interval = setInterval(() => {
       if (!document.hidden) fetchData();
     }, REQUEST_POLL_INTERVAL);
+
+    const handleVisibilityChange = () => {
+      if (!document.hidden) {
+        fetchData();
+      }
+    };
+    document.addEventListener("visibilitychange", handleVisibilityChange);
 
-    return () => clearInterval(interval);
+    return () => {
+      clearInterval(interval);
+      document.removeEventListener("visibilitychange", handleVisibilityChange);
+    };
   }, [fetchData]);
🤖 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/consultant/`[consultantId]/(features)/requests/RequestSlotAllocationTab.tsx
around lines 331 - 344, The effect sets up a polling interval
(REQUEST_POLL_INTERVAL) and skips ticks when document.hidden, but it doesn't
refresh immediately when the tab becomes visible; update the useEffect that
references fetchData to also add a 'visibilitychange' listener which calls
fetchData() when document.hidden === false, and ensure you remove that listener
in the returned cleanup along with clearing the interval; modify the effect that
defines REQUEST_POLL_INTERVAL, interval, and uses fetchData to register and
deregister this visibilitychange handler so data is fetched on visibility
restore.
🤖 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__/enterprise/process-payouts-false-failed.test.ts`:
- Around line 153-155: The test currently asserts results[0] has payoutId "po_1"
and skipped: true but omits success: false; update the assertion for the
CAS-lost/skip branch to also assert success: false (either by expanding the
toMatchObject to include success: false or adding a separate
expect(results[0].success).toBe(false)), targeting the existing results array
and the entry with payoutId "po_1".

In `@app/api/participants/class/`[classId]/route.ts:
- Around line 174-183: The loop that disconnects a user from slots (iterating
over userSlots and calling prisma.slotOfAppointment.update) is non-atomic and
can leave partial state if an update fails; change the sequential updates inside
the DELETE handler (the code that currently calls
prisma.slotOfAppointment.update for each slot) to a single atomic call using
prisma.$transaction([...]) by mapping each slot to its corresponding update
operation and passing that array into prisma.$transaction so all disconnects
succeed or all rollback; apply the same change in both the class DELETE handler
(where userSlots is used) and the webinar DELETE handler.

In `@app/dashboard/consultant/`[consultantId]/(features)/requests/page.tsx:
- Around line 27-34: The OrgContextFilter currently has no effect because its
orgScope is not consumed by the tab’s API reads; either hide/disable the filter
until backend support exists or wire orgScope through the tab’s fetch path:
Option A (quick): remove or conditionally render OrgContextFilter (the component
using useOrgScope) on the page so consultants with multiple orgs don't see a
non-functional control; Option B (preferred long-term): add an orgScope
parameter to RequestSlotAllocationTab (and any child fetch helpers) and append
it as a query param to the /api/events/* requests so the tab’s data requests
honor useOrgScope — implement one of these two fixes and leave a TODO
referencing RequestSlotAllocationTab, OrgContextFilter, useOrgScope and the
/api/events/* endpoints.

In `@lib/payments/operations/cancel-pending.ts`:
- Line 10: The file imports cancelPaymentIntent from a scripts/ path causing a
backward dependency; move the gateway cancellation logic into a shared lib
module (e.g., create lib/payments/gateway/cancel-payment.ts or similar)
exporting cancelPaymentIntent, update the import in
lib/payments/operations/cancel-pending.ts to import from that new lib module,
and change the cleanup script (scripts/payments/cleanup-abandoned-payments) to
import cancelPaymentIntent from the same new shared lib module so both consumers
use lib/payments/gateway::cancelPaymentIntent.

In `@lib/payments/payouts/org-payout-service.ts`:
- Around line 796-806: processPendingOrgPayouts currently only selects
organizationPayout rows with status "PENDING", so payouts left in "PROCESSING"
after transient gateway submission failures are never retried; update the driver
in processPendingOrgPayouts to also fetch eligible "PROCESSING" rows (e.g.,
query prisma.organizationPayout.findMany for status IN ["PENDING","PROCESSING"]
or run a second query for PROCESSING where gatewayOutcome is null/non-terminal
or lastAttempt is older than a retry threshold), ensure the selection uses the
same select shape (id) and increments result.scanned accordingly, and keep using
processOrgPayout(p.id) so transient failures can be retried without manual
intervention.

In `@lib/payments/payouts/payout-service.ts`:
- Around line 61-66: processApprovedPayouts returns CAS-lost outcomes as {
success:false, skipped:true } but callers compute failures with results.filter(r
=> !r.success) which counts skipped items as failures; update the aggregations
in the API entrypoints that consume processApprovedPayouts (the route handlers
that compute failed/success summaries) to treat skipped payouts as neutral by
changing failure logic to something like failed = results.filter(r => !r.success
&& !r.skipped) and derive overall success accordingly, and add a
unit/integration test covering ticket `#776` to ensure skipped:true does not mark
the run as failed; look for usages of processApprovedPayouts and the results
variable in the route handlers and adjust their summarize/aggregation logic
(mirroring jobs/payouts/process-payouts.ts behavior).

In `@prisma/seedFiles/1a-create-users.ts`:
- Around line 624-638: The per-user seeding must be atomic: wrap the user
creation, the subsequent prisma.user.update that syncs profile IDs, and related
account.create calls into a single Prisma transaction so either all writes
succeed or none persist. Replace the separate calls around user.create /
prisma.user.update / account.create with a prisma.$transaction block and use the
transaction client (tx.user.create, tx.user.update, tx.account.create) for those
operations; preserve existing error handling by letting the transaction throw on
failure so partial writes are rolled back.

In
`@tests/typescript/race-conditions/scenarios/07-real-api-booking/test-cancel-pending-vs-webhook.ts`:
- Around line 247-251: The current assertion only checks tentativeLeft === 0
which can be true in cancel-lost cases; update the webhook-win assertion in the
test (the check call that uses tentativeLeft, confirmed, parent) to explicitly
assert webhook-win invariants: ensure confirmed indicates at least one confirmed
slot (e.g., confirmed > 0 or confirmed contains the expected slot) and ensure
parent is not the CANCELLED state (parent !== "CANCELLED"), in addition to
tentativeLeft === 0, so the test cannot pass from deleted/cancelled residues.
- Around line 78-355: The test mutates DB state early but only restores it at
the end; wrap the parts that change state (from after snapshotting originalSlots
through both race legs and until before finish("cancel-pending-vs-webhook")) in
a try block and move the restore logic (the prisma.payment.deleteMany and
prisma.consultation.update and the slotOfAppointment.upsert loop that uses
originalSlots) into a finally block so it always runs; keep existing variables
(appointment, consultation, originalSlots, leg1, leg2) in scope, await the
restore operations in the finally, and ensure the finally handles leg1 possibly
being null when building the deleteMany id list.

---

Outside diff comments:
In
`@app/dashboard/consultant/`[consultantId]/(features)/requests/RequestSlotAllocationTab.tsx:
- Around line 331-344: The effect sets up a polling interval
(REQUEST_POLL_INTERVAL) and skips ticks when document.hidden, but it doesn't
refresh immediately when the tab becomes visible; update the useEffect that
references fetchData to also add a 'visibilitychange' listener which calls
fetchData() when document.hidden === false, and ensure you remove that listener
in the returned cleanup along with clearing the interval; modify the effect that
defines REQUEST_POLL_INTERVAL, interval, and uses fetchData to register and
deregister this visibilitychange handler so data is fetched on visibility
restore.
🪄 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: 8104ccb9-0e65-4abb-aec7-a40f9c7123a0

📥 Commits

Reviewing files that changed from the base of the PR and between fcf52b6 and 93a0941.

📒 Files selected for processing (52)
  • .github/workflows/process-payouts.yml
  • .github/workflows/process-waitlist-expirations.yml
  • .github/workflows/send-waitlist-reminders.yml
  • .gitignore
  • TESTING_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.ts
  • app/api/checkout/pending/[paymentId]/route.ts
  • app/api/dashboard/consultant/[consultantId]/planner/route.ts
  • app/api/dashboard/consultant/[consultantId]/requests/route.ts
  • app/api/events/consultations/route.ts
  • app/api/events/subscriptions/route.ts
  • app/api/participants/class/[classId]/route.ts
  • app/api/participants/webinar/[webinarId]/route.ts
  • app/api/slots/availability/[consultantId]/route.ts
  • app/dashboard/consultant/[consultantId]/(features)/appointments/page.tsx
  • app/dashboard/consultant/[consultantId]/(features)/planner/page.tsx
  • app/dashboard/consultant/[consultantId]/(features)/requests/RequestSlotAllocationTab.tsx
  • app/dashboard/consultant/[consultantId]/(features)/requests/page.tsx
  • app/dashboard/consultant/[consultantId]/layout.tsx
  • app/dashboard/consultee/[consulteeId]/(features)/appointments/page.tsx
  • app/dashboard/consultee/[consulteeId]/(features)/home/PendingPaymentsWidget.tsx
  • app/dashboard/consultee/[consulteeId]/layout.tsx
  • docs/booking/13-cron-jobs-and-background-tasks.md
  • docs/payments/payouts/06-configuration.md
  • docs/prisma/prisma-7-migration.md
  • hooks/useConsultantPrefetchDashboard.ts
  • jobs/payouts/process-payouts.ts
  • jobs/waitlist/process-expired-notifications.ts
  • jobs/waitlist/send-expiration-reminders.ts
  • lib/api/organizations/wallet.ts
  • lib/dashboard-queries.ts
  • lib/data/explore-programs.ts
  • lib/db/connect-retry.ts
  • lib/payments/operations/cancel-pending.ts
  • lib/payments/operations/refund.ts
  • lib/payments/payouts/index.ts
  • lib/payments/payouts/org-payout-service.ts
  • lib/payments/payouts/payout-service.ts
  • lib/rate-limit.ts
  • package.json
  • prisma/schema.prisma
  • prisma/seedFiles/1a-create-users.ts
  • providers/ReactQueryProvider.tsx
  • providers/StreamProvider.tsx
  • scripts/payouts/process-payouts.ts
  • scripts/waitlist/process-expired-notifications.ts
  • scripts/waitlist/send-expiration-reminders.ts
  • tests/typescript/race-conditions/master-runner.ts
  • tests/typescript/race-conditions/scenarios/07-real-api-booking/test-cancel-pending-vs-webhook.ts
💤 Files with no reviewable changes (5)
  • app/dashboard/consultee/[consulteeId]/layout.tsx
  • app/dashboard/consultant/[consultantId]/layout.tsx
  • scripts/payouts/process-payouts.ts
  • app/api/dashboard/consultant/[consultantId]/requests/route.ts
  • lib/dashboard-queries.ts

Comment on lines +153 to +155
expect(results).toHaveLength(1);
expect(results[0]).toMatchObject({ payoutId: "po_1", skipped: true });
expect(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Assert success: false in the CAS-lost test contract.

The skipped-path test currently validates skipped: true but not success: false, so a regression in result shape could slip through.

Suggested test tightening
-    expect(results[0]).toMatchObject({ payoutId: "po_1", skipped: true });
+    expect(results[0]).toMatchObject({
+      payoutId: "po_1",
+      success: false,
+      skipped: true,
+    });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
expect(results).toHaveLength(1);
expect(results[0]).toMatchObject({ payoutId: "po_1", skipped: true });
expect(
expect(results).toHaveLength(1);
expect(results[0]).toMatchObject({
payoutId: "po_1",
success: false,
skipped: true,
});
expect(
🤖 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__/enterprise/process-payouts-false-failed.test.ts` around lines 153 -
155, The test currently asserts results[0] has payoutId "po_1" and skipped: true
but omits success: false; update the assertion for the CAS-lost/skip branch to
also assert success: false (either by expanding the toMatchObject to include
success: false or adding a separate expect(results[0].success).toBe(false)),
targeting the existing results array and the entry with payoutId "po_1".

Comment thread app/api/participants/class/[classId]/route.ts Outdated
Comment on lines +27 to +34
// S1 (B1-personal-retrofit): the OrgContextFilter dropdown lets a
// consultant who works for multiple orgs toggle between "Personal" /
// "<org>" / "All" (drives the ?orgScope= URL param via useOrgScope).
// Self-hides for consultants with zero org memberships. Note: the tab's
// /api/events/* fetches don't consume orgScope yet — wiring the scope
// into those endpoints is tracked follow-up work, not a regression of
// this page (the deleted query was the only thing that ever read it,
// and its data went nowhere).

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Org filter is currently non-functional on this tab.

Lines 31-34 confirm orgScope is not consumed by the /api/events/* reads, so changing OrgContextFilter does not affect RequestSlotAllocationTab results. Either wire orgScope through the tab’s fetch path now, or hide/disable this filter until that backend contract is implemented.

Also applies to: 55-58

🤖 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/consultant/`[consultantId]/(features)/requests/page.tsx around
lines 27 - 34, The OrgContextFilter currently has no effect because its orgScope
is not consumed by the tab’s API reads; either hide/disable the filter until
backend support exists or wire orgScope through the tab’s fetch path: Option A
(quick): remove or conditionally render OrgContextFilter (the component using
useOrgScope) on the page so consultants with multiple orgs don't see a
non-functional control; Option B (preferred long-term): add an orgScope
parameter to RequestSlotAllocationTab (and any child fetch helpers) and append
it as a query param to the /api/events/* requests so the tab’s data requests
honor useOrgScope — implement one of these two fixes and leave a TODO
referencing RequestSlotAllocationTab, OrgContextFilter, useOrgScope and the
/api/events/* endpoints.

transitionConsultationRequest,
transitionSubscriptionRequest,
} from "@/lib/booking/transitions";
import { cancelPaymentIntent } from "@/scripts/payments/cleanup-abandoned-payments";

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial | ⚖️ Poor tradeoff

Verify the import direction: lib/ should not depend on scripts/.

Importing cancelPaymentIntent from @/scripts/payments/cleanup-abandoned-payments creates a backward dependency. Generally, scripts should import reusable logic from lib/, not the other way around. Consider moving the gateway cancellation logic to a shared module under lib/payments/gateway/ so both the operation and the cleanup script can import it.

🤖 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 `@lib/payments/operations/cancel-pending.ts` at line 10, The file imports
cancelPaymentIntent from a scripts/ path causing a backward dependency; move the
gateway cancellation logic into a shared lib module (e.g., create
lib/payments/gateway/cancel-payment.ts or similar) exporting
cancelPaymentIntent, update the import in
lib/payments/operations/cancel-pending.ts to import from that new lib module,
and change the cleanup script (scripts/payments/cleanup-abandoned-payments) to
import cancelPaymentIntent from the same new shared lib module so both consumers
use lib/payments/gateway::cancelPaymentIntent.

Comment on lines +796 to +806
const pending = await prisma.organizationPayout.findMany({
where: { status: "PENDING" },
select: { id: true },
});
result.scanned = pending.length;

for (const p of pending) {
try {
const out = await processOrgPayout(p.id);
if (out.status === "PROCESSING") {
result.advanced++;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

processPendingOrgPayouts cannot retry transient gateway submission failures.

At Line 796, the driver only loads PENDING rows. But processOrgPayout intentionally leaves transient gateway failures in PROCESSING (to be retried). Those rows are never revisited by this loop, so they can remain stuck until manual intervention.

Consider adding a dedicated retry path for eligible PROCESSING org payouts (for example, rows without terminal gateway outcome) or adjusting the processing contract so retries are reachable from the cron driver.

🤖 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 `@lib/payments/payouts/org-payout-service.ts` around lines 796 - 806,
processPendingOrgPayouts currently only selects organizationPayout rows with
status "PENDING", so payouts left in "PROCESSING" after transient gateway
submission failures are never retried; update the driver in
processPendingOrgPayouts to also fetch eligible "PROCESSING" rows (e.g., query
prisma.organizationPayout.findMany for status IN ["PENDING","PROCESSING"] or run
a second query for PROCESSING where gatewayOutcome is null/non-terminal or
lastAttempt is older than a retry threshold), ensure the selection uses the same
select shape (id) and increments result.scanned accordingly, and keep using
processOrgPayout(p.id) so transient failures can be retried without manual
intervention.

Comment on lines +61 to +66
/**
* #776 — true when another run's CAS claim won this payout
* (APPROVED → PROCESSING matched zero rows). The claiming run owns the
* outcome; callers must not count a skipped payout as processed or failed.
*/
skipped?: boolean;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# 1) Find all call sites of payout processing.
rg -nP --type=ts -C4 '\bprocessApprovedPayouts\s*\('

# 2) Find result aggregation patterns that may classify by !success.
rg -nP --type=ts -C4 'filter\(\s*\(?\w+\)?\s*=>\s*!\s*\w+\.success'
rg -nP --type=ts -C4 '(failed|successful|processed)\s*[:=].*filter\('

# 3) Confirm skipped-aware logic exists where summaries/exits are computed.
rg -nP --type=ts -C4 '\.skipped\b|skipped\s*[:=]'

Repository: Practitionist/familiarise_web

Length of output: 33549


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Show the aggregation/success logic in the job + routes that consume processApprovedPayouts()
for f in \
  jobs/payouts/process-payouts.ts \
  app/api/cleanup/process-payouts/route.ts \
  app/api/admin/payouts/process/route.ts \
  app/api/admin/system-jobs/run/route.ts \
  lib/payments/payouts/payout-service.ts
do
  echo "===== $f ====="
  # print the most relevant surrounding regions (small windows)
  case "$f" in
    jobs/payouts/process-payouts.ts)
      sed -n '1,120p' "$f" | nl -ba | sed -n '1,120p'
      ;;
    app/api/cleanup/process-payouts/route.ts)
      sed -n '1,120p' "$f" | nl -ba | sed -n '1,120p'
      ;;
    app/api/admin/payouts/process/route.ts)
      sed -n '1,120p' "$f" | nl -ba | sed -n '1,120p'
      ;;
    app/api/admin/system-jobs/run/route.ts)
      sed -n '140,240p' "$f" | nl -ba | sed -n '140,240p'
      ;;
    lib/payments/payouts/payout-service.ts)
      sed -n '430,620p' "$f" | nl -ba | sed -n '430,620p'
      ;;
  esac
done

Repository: Practitionist/familiarise_web

Length of output: 179


🏁 Script executed:

#!/bin/bash
set -euo pipefail

for f in \
  jobs/payouts/process-payouts.ts \
  app/api/cleanup/process-payouts/route.ts \
  app/api/admin/payouts/process/route.ts \
  app/api/admin/system-jobs/run/route.ts \
  lib/payments/payouts/payout-service.ts
do
  echo "===== $f ====="
  case "$f" in
    jobs/payouts/process-payouts.ts)
      sed -n '1,120p' "$f" | cat -n
      ;;
    app/api/cleanup/process-payouts/route.ts)
      sed -n '1,120p' "$f" | cat -n
      ;;
    app/api/admin/payouts/process/route.ts)
      sed -n '1,120p' "$f" | cat -n
      ;;
    app/api/admin/system-jobs/run/route.ts)
      sed -n '140,240p' "$f" | cat -n
      ;;
    lib/payments/payouts/payout-service.ts)
      sed -n '420,620p' "$f" | cat -n
      ;;
  esac
done

Repository: Practitionist/familiarise_web

Length of output: 21121


🏁 Script executed:

#!/bin/bash
set -euo pipefail
sed -n '1,120p' lib/payments/payouts/payout-service.ts | cat -n

Repository: Practitionist/familiarise_web

Length of output: 4117


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# 1) Inspect the specific test that mentions `#776` skipped:true
sed -n '1,240p' __tests__/enterprise/process-payouts-false-failed.test.ts | cat -n

# 2) Find other aggregations that look at r.success / !r.success together with PayoutResult arrays
rg -n --type=ts 'processApprovedPayouts\('
rg -n --type=ts 'filter\(\s*\(r\)\s*=>\s*r\.success\)|filter\(\s*\(?\w+\)?\s*=>\s*!\s*\w+\.success'
rg -n --type=ts 'skipped\b' jobs/payouts app/api lib/payments __tests__ scripts/utils | head -n 200

Repository: Practitionist/familiarise_web

Length of output: 11622


Fix: payout skipped:true is still counted as failure in API/system-job entrypoints

processApprovedPayouts() returns CAS-lost outcomes as { success:false, skipped:true }, but these consumers compute failed as results.filter(r => !r.success) and derive overall success from that—so skipped payouts incorrectly fail the whole run:

  • app/api/cleanup/process-payouts/route.ts
  • app/api/admin/system-jobs/run/route.ts (process-payouts job)
    (Meanwhile jobs/payouts/process-payouts.ts is already safe because it excludes r.skipped in summarize().)

Update aggregations to treat skipped:true as neutral (e.g. failed = results.filter(r => !r.success && !r.skipped)), and add coverage for #776 at these entrypoints.

🤖 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 `@lib/payments/payouts/payout-service.ts` around lines 61 - 66,
processApprovedPayouts returns CAS-lost outcomes as { success:false,
skipped:true } but callers compute failures with results.filter(r => !r.success)
which counts skipped items as failures; update the aggregations in the API
entrypoints that consume processApprovedPayouts (the route handlers that compute
failed/success summaries) to treat skipped payouts as neutral by changing
failure logic to something like failed = results.filter(r => !r.success &&
!r.skipped) and derive overall success accordingly, and add a unit/integration
test covering ticket `#776` to ensure skipped:true does not mark the run as
failed; look for usages of processApprovedPayouts and the results variable in
the route handlers and adjust their summarize/aggregation logic (mirroring
jobs/payouts/process-payouts.ts behavior).

Comment on lines +624 to +638
// Sync the denormalized User.*ProfileId columns the session layer
// reads (BetterAuth additionalFields). Runtime onboarding sets these;
// the seed previously left them null — every seeded session then
// carried an undefined profile id, which (pre-guard) dropped
// ownership filters in routes that fed it straight into a Prisma
// where clause.
await prisma.user.update({
where: { id: user.id },
data: {
consultantProfileId: user.consultantProfile?.id ?? null,
consulteeProfileId: user.consulteeProfile?.id ?? null,
staffProfileId: user.staffProfile?.id ?? null,
adminProfileId: user.adminProfile?.id ?? null,
},
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Make per-user seed writes atomic to prevent partially initialized users.

Line 630 introduces a second write in the user-creation path, but this flow is still non-atomic. If user.create succeeds and user.update or later account.create fails, the catch at Line 689 logs and continues, leaving inconsistent seeded rows (including null denormalized profile IDs again).

Proposed fix (transactional per-user write unit)
-      const user = await prisma.user.create({
-        data: userData,
-        include: {
-          consultantProfile: true,
-          consulteeProfile: true,
-          staffProfile: true,
-          adminProfile: true,
-        },
-      });
-
-      await prisma.user.update({
-        where: { id: user.id },
-        data: {
-          consultantProfileId: user.consultantProfile?.id ?? null,
-          consulteeProfileId: user.consulteeProfile?.id ?? null,
-          staffProfileId: user.staffProfile?.id ?? null,
-          adminProfileId: user.adminProfile?.id ?? null,
-        },
-      });
-
-      await prisma.account.create({
-        data: {
-          userId: user.id,
-          accountId: user.id,
-          providerId: "credential",
-          password: hashedPassword,
-        },
-      });
+      const user = await prisma.$transaction(async (tx) => {
+        const created = await tx.user.create({
+          data: userData,
+          include: {
+            consultantProfile: true,
+            consulteeProfile: true,
+            staffProfile: true,
+            adminProfile: true,
+          },
+        });
+
+        await tx.user.update({
+          where: { id: created.id },
+          data: {
+            consultantProfileId: created.consultantProfile?.id ?? null,
+            consulteeProfileId: created.consulteeProfile?.id ?? null,
+            staffProfileId: created.staffProfile?.id ?? null,
+            adminProfileId: created.adminProfile?.id ?? null,
+          },
+        });
+
+        await tx.account.create({
+          data: {
+            userId: created.id,
+            accountId: created.id,
+            providerId: "credential",
+            password: hashedPassword,
+          },
+        });
+
+        return created;
+      });
🤖 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 `@prisma/seedFiles/1a-create-users.ts` around lines 624 - 638, The per-user
seeding must be atomic: wrap the user creation, the subsequent
prisma.user.update that syncs profile IDs, and related account.create calls into
a single Prisma transaction so either all writes succeed or none persist.
Replace the separate calls around user.create / prisma.user.update /
account.create with a prisma.$transaction block and use the transaction client
(tx.user.create, tx.user.update, tx.account.create) for those operations;
preserve existing error handling by letting the transaction throw on failure so
partial writes are rolled back.

Comment on lines +247 to +251
check(
"leg1/B: webhook win confirmed the booking (no tentative residue)",
tentativeLeft === 0,
{ tentativeLeft, confirmed, parent },
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Strengthen webhook-win assertions to avoid false positives.

In the cancel-lost branch, asserting only tentativeLeft === 0 is too weak; deleted slots + cancelled parent can also satisfy it. Assert webhook-win invariants explicitly (e.g., confirmed slots present and parent not CANCELLED).

🤖 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/typescript/race-conditions/scenarios/07-real-api-booking/test-cancel-pending-vs-webhook.ts`
around lines 247 - 251, The current assertion only checks tentativeLeft === 0
which can be true in cancel-lost cases; update the webhook-win assertion in the
test (the check call that uses tentativeLeft, confirmed, parent) to explicitly
assert webhook-win invariants: ensure confirmed indicates at least one confirmed
slot (e.g., confirmed > 0 or confirmed contains the expected slot) and ensure
parent is not the CANCELLED state (parent !== "CANCELLED"), in addition to
tentativeLeft === 0, so the test cannot pass from deleted/cancelled residues.

teetangh and others added 2 commits June 11, 2026 18:14
…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>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
scripts/waitlist/process-expired-notifications.ts (1)

101-133: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Both waitlist loops still allow duplicate notifications after partial failure. Each path performs non-idempotent email/notification work before a durable, single-run claim is persisted. If the process fails after the side effect but before the corresponding DB state is recorded, the next rerun can pick up the same row again and resend the user-facing notification.

🤖 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 `@scripts/waitlist/process-expired-notifications.ts` around lines 101 - 133,
The loops call non-idempotent side-effects (sendWaitlistExpiredEmail and
handleSlotOpening) before persisting a durable claim, allowing duplicates on
retry; change the flow to first acquire and persist a single-run claim/lock on
the waitlist entry (e.g., set a processing/notified flag or lease in the row via
an atomic update or transaction on the entry id) and only then perform
sendWaitlistExpiredEmail and handleSlotOpening, and if those side-effects fail
record the error but leave the claim so retries don't resend; ensure
functions/branches referencing sendWaitlistExpiredEmail, handleSlotOpening,
entry, errors, and emailed use that persisted claim to guard against duplicate
sends.
app/api/participants/class/[classId]/route.ts (1)

90-110: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

The truncation flag is derived from the cap hit, not from an actual extra row.

Both handlers mark waitlistTruncated as soon as the query returns 500 records, which mislabels an exact-cap waitlist as truncated. The shared fix is to fetch WAITLIST_CAP + 1, set the flag from the presence of that extra row, and slice the response back to 500.

🤖 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 90 - 110, Change
the waitlist fetch to request one more row than WAITLIST_CAP, then set
waitlistTruncated based on whether the returned array length is greater than
WAITLIST_CAP, and finally slice the waitlist to WAITLIST_CAP before returning;
specifically update the prisma.waitlist.findMany call to use take: WAITLIST_CAP
+ 1, compute waitlistTruncated = waitlist.length > WAITLIST_CAP, and set the
returned waitlist = waitlist.slice(0, WAITLIST_CAP) so an exact-cap list is not
misreported as truncated.
♻️ Duplicate comments (1)
prisma/seedFiles/1a-create-users.ts (1)

614-653: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Make per-user seed writes atomic (partial-write risk still present).

user.create, user.update, and account.create are still separate writes. If a later step fails, the script logs and continues with partially initialized users.

Suggested fix
-      const user = await prisma.user.create({
-        data: userData,
-        include: {
-          consultantProfile: true,
-          consulteeProfile: true,
-          staffProfile: true,
-          adminProfile: true,
-        },
-      });
-
-      await prisma.user.update({
-        where: { id: user.id },
-        data: {
-          consultantProfileId: user.consultantProfile?.id ?? null,
-          consulteeProfileId: user.consulteeProfile?.id ?? null,
-          staffProfileId: user.staffProfile?.id ?? null,
-          adminProfileId: user.adminProfile?.id ?? null,
-        },
-      });
-
-      await prisma.account.create({
-        data: {
-          userId: user.id,
-          accountId: user.id,
-          providerId: "credential",
-          password: hashedPassword,
-        },
-      });
+      const user = await prisma.$transaction(async (tx) => {
+        const created = await tx.user.create({
+          data: userData,
+          include: {
+            consultantProfile: true,
+            consulteeProfile: true,
+            staffProfile: true,
+            adminProfile: true,
+          },
+        });
+
+        await tx.user.update({
+          where: { id: created.id },
+          data: {
+            consultantProfileId: created.consultantProfile?.id ?? null,
+            consulteeProfileId: created.consulteeProfile?.id ?? null,
+            staffProfileId: created.staffProfile?.id ?? null,
+            adminProfileId: created.adminProfile?.id ?? null,
+          },
+        });
+
+        await tx.account.create({
+          data: {
+            userId: created.id,
+            accountId: created.id,
+            providerId: "credential",
+            password: hashedPassword,
+          },
+        });
+
+        return created;
+      });
🤖 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 `@prisma/seedFiles/1a-create-users.ts` around lines 614 - 653, The seeding
creates a user, then updates it, then creates an account in separate calls
(prisma.user.create, prisma.user.update, prisma.account.create), leaving partial
state if a later step fails; wrap the per-user operations in a single
transaction (use prisma.$transaction) that performs the create (with include),
the update of denormalized profile ID columns, and the account.create together
so they atomically commit or roll back; use the transaction result to update the
in-memory user object (copy consultantProfileId, consulteeProfileId,
staffProfileId, adminProfileId from the transactional user result) and propagate
errors instead of continuing on partial failures.
🤖 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 `@lib/payments/payouts/org-payout-service.ts`:
- Around line 619-623: The returned object incorrectly hardcodes
submittedToGateway: true on the success path; modify the logic in
org-payout-service.ts so submitOrgPayoutToGateway() returns a boolean (or
otherwise communicates whether it actually submitted) and use that boolean for
submittedToGateway in the returned value instead of true, ensuring the
unsupported-gateway early-return path (the branch in submitOrgPayoutToGateway())
results in submittedToGateway: false.

In `@scripts/waitlist/process-expired-notifications.ts`:
- Around line 38-45: The wrappers using withCronLock() request failMode: "open"
but the shared withCronLock implementation still converts a failed/null lock
acquisition into a CronLockHeldError; update withCronLock so that when lock
acquisition returns null/false and the passed options.failMode === "open" it
does NOT throw CronLockHeldError but instead returns/short-circuits (or logs and
returns) so callers like the process-expired-notifications entry that call
processExpirationsCore() truly execute in "fail-open" mode; ensure the change
references the withCronLock acquisition path and preserves the existing throw
for failMode !== "open".

---

Outside diff comments:
In `@app/api/participants/class/`[classId]/route.ts:
- Around line 90-110: Change the waitlist fetch to request one more row than
WAITLIST_CAP, then set waitlistTruncated based on whether the returned array
length is greater than WAITLIST_CAP, and finally slice the waitlist to
WAITLIST_CAP before returning; specifically update the prisma.waitlist.findMany
call to use take: WAITLIST_CAP + 1, compute waitlistTruncated = waitlist.length
> WAITLIST_CAP, and set the returned waitlist = waitlist.slice(0, WAITLIST_CAP)
so an exact-cap list is not misreported as truncated.

In `@scripts/waitlist/process-expired-notifications.ts`:
- Around line 101-133: The loops call non-idempotent side-effects
(sendWaitlistExpiredEmail and handleSlotOpening) before persisting a durable
claim, allowing duplicates on retry; change the flow to first acquire and
persist a single-run claim/lock on the waitlist entry (e.g., set a
processing/notified flag or lease in the row via an atomic update or transaction
on the entry id) and only then perform sendWaitlistExpiredEmail and
handleSlotOpening, and if those side-effects fail record the error but leave the
claim so retries don't resend; ensure functions/branches referencing
sendWaitlistExpiredEmail, handleSlotOpening, entry, errors, and emailed use that
persisted claim to guard against duplicate sends.

---

Duplicate comments:
In `@prisma/seedFiles/1a-create-users.ts`:
- Around line 614-653: The seeding creates a user, then updates it, then creates
an account in separate calls (prisma.user.create, prisma.user.update,
prisma.account.create), leaving partial state if a later step fails; wrap the
per-user operations in a single transaction (use prisma.$transaction) that
performs the create (with include), the update of denormalized profile ID
columns, and the account.create together so they atomically commit or roll back;
use the transaction result to update the in-memory user object (copy
consultantProfileId, consulteeProfileId, staffProfileId, adminProfileId from the
transactional user result) and propagate errors instead of continuing on partial
failures.
🪄 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: 00495290-6ca5-4381-9822-4ae3850e4a84

📥 Commits

Reviewing files that changed from the base of the PR and between 93a0941 and b51fb19.

📒 Files selected for processing (19)
  • .github/workflows/handle-stuck-payouts.yml
  • .github/workflows/reconcile-payout-status.yml
  • TESTING_PLAN.md
  • __tests__/enterprise/live-payout-submission.test.ts
  • __tests__/payments/cancel-pending-checkout.test.ts
  • app/api/participants/class/[classId]/route.ts
  • app/api/participants/webinar/[webinarId]/route.ts
  • jobs/payouts/process-payouts.ts
  • jobs/waitlist/process-expired-notifications.ts
  • jobs/waitlist/send-expiration-reminders.ts
  • lib/data/explore-programs.ts
  • lib/db/connect-retry.ts
  • lib/payments/operations/cancel-pending.ts
  • lib/payments/operations/refund.ts
  • lib/payments/payouts/org-payout-service.ts
  • prisma/seedFiles/1a-create-users.ts
  • scripts/waitlist/process-expired-notifications.ts
  • scripts/waitlist/send-expiration-reminders.ts
  • tests/typescript/race-conditions/scenarios/07-real-api-booking/test-cancel-pending-vs-webhook.ts

Comment on lines +619 to +623
return {
status: "PROCESSING" as const,
submittedToGateway: true,
claimed: true,
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

submittedToGateway is incorrectly hardcoded to true on the success path.

At Line 619, this branch reports submittedToGateway: true even when submitOrgPayoutToGateway() returns early without submitting (e.g., unsupported gateway branch at Line 669-677). This misreports execution state and can hide unsubmitted PROCESSING rows.

Proposed fix
-async function submitOrgPayoutToGateway(payoutId: string): Promise<void> {
+async function submitOrgPayoutToGateway(payoutId: string): Promise<boolean> {
@@
   if (payout.paymentGateway !== "RAZORPAY") {
@@
-    return;
+    return false;
   }
@@
   await prisma.organizationPayout.update({
@@
   });
+  return true;
 }
@@
-        await submitOrgPayoutToGateway(payoutId);
+        const submitted = await submitOrgPayoutToGateway(payoutId);
         return {
           status: "PROCESSING" as const,
-          submittedToGateway: true,
+          submittedToGateway: submitted,
           claimed: 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 `@lib/payments/payouts/org-payout-service.ts` around lines 619 - 623, The
returned object incorrectly hardcodes submittedToGateway: true on the success
path; modify the logic in org-payout-service.ts so submitOrgPayoutToGateway()
returns a boolean (or otherwise communicates whether it actually submitted) and
use that boolean for submittedToGateway in the returned value instead of true,
ensuring the unsupported-gateway early-return path (the branch in
submitOrgPayoutToGateway()) results in submittedToGateway: false.

Comment on lines +38 to +45
return withCronLock(
"process-expired-notifications",
{ failMode: "open" },
async () => {
await withDbConnectRetry(() => prisma.$queryRaw`SELECT 1`);
return processExpirationsCore();
},
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

The new waitlist wrappers are not truly fail-open. Both scripts opt into failMode: "open", but the shared withCronLock() path still converts a null/failed acquisition into CronLockHeldError. Because the job wrappers treat that exception as a clean skip, a real Redis/circuit-open outage now silently suppresses both waitlist crons instead of preserving pre-lock execution.

🤖 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 `@scripts/waitlist/process-expired-notifications.ts` around lines 38 - 45, The
wrappers using withCronLock() request failMode: "open" but the shared
withCronLock implementation still converts a failed/null lock acquisition into a
CronLockHeldError; update withCronLock so that when lock acquisition returns
null/false and the passed options.failMode === "open" it does NOT throw
CronLockHeldError but instead returns/short-circuits (or logs and returns) so
callers like the process-expired-notifications entry that call
processExpirationsCore() truly execute in "fail-open" mode; ensure the change
references the withCronLock acquisition path and preserves the existing throw
for failMode !== "open".

@teetangh teetangh merged commit f898bff into dev Jun 11, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant