fix(payments): the checkout consent gate reads through the transaction, not the pool it is blocking - #1435
Merged
Merged
Conversation
…n, not the pool it is blocking Netlify runs this app with a pg pool of PG_POOL_MAX=1 and a 3 s connect timeout. An interactive Prisma transaction checks out that single connection and holds it until it commits, so any query sent to the global client while a transaction is open queues for a connection that only the blocked transaction can release. The request cannot make progress and pg eventually gives up with "timeout exceeded when trying to connect". That is what killed POST /api/checkout on the deploy preview: the DPDP SESSION_BOOKING gate in validateSlotAvailability called checkConsent, which read on the global client, and validateSlotAvailability is called from inside three separate transactions on the plain Razorpay consultation path (calculateAmountAndValidate, revalidateInsideLock and the Serializable booking transaction). The first of them deadlocked against itself and the route answered 500 with the pg message, before any row was written. The org-sponsored branch of revalidateInsideLock had the same defect on its own consent check. Neither is new to the finance train; both have been on dev since the LCY-2 consent cascade landed, and production runs the same single-connection pool, so consultation checkout could not take a payment there either. checkConsent now takes an optional client that defaults to the global one, following the getUserCredits convention, and both in-transaction call sites pass tx. The gate itself is unchanged and still fails closed. The redundant dynamic imports in validateSlotAvailability are gone; both symbols were already static imports at the top of the file. The pin models the pool rather than the query: a global-client call raised while a transaction is open throws the same pg error the preview logged, so the test fails against the unfixed code with "timeout exceeded when trying to connect" and passes once the gate reads through tx. A second case asserts the withdrawn-consent block still rejects, since a fix that silently disabled the gate would also make the first case pass. lib/compliance/dpdp.ts was already failing prettier --check on dev; formatting the file to commit this change also settles that one unrelated line. Part of #1421 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MgadJydWEKkdhhzY58yiL7
teetangh
added
the
claude-review
✅ Deploy Preview for familiarise ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Essentials Run ID: 📒 Files selected for processing (5)
Included review availability: 0 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour. 📜 Recent review details🧰 Additional context used📓 Path-based instructions (2)Money-critical code.⚙️ CodeRabbit configuration file Files:
Edge cases that must be covered for money tests: zero/negative amounts, currency mismatch, concurrent invocations, expired signatures/orders, partial refunds, idempotent replays.⚙️ CodeRabbit configuration file Files:
🔇 Additional comments (5)
📝 SummarySummary by CodeRabbit
WalkthroughCheckout consent reads now use the active transaction client. Overage, cap-near, and exhausted-cap notifications are captured during checkout and dispatched after commit or rollback. Regression tests cover single-connection transaction behavior. ChangesCheckout hardening
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to Checkout consent validation now uses the active transaction client, avoiding single-connection-pool checkout failures while preserving withdrawn-consent blocking. Notification delivery is deferred until transaction outcomes are known, with no concrete unresolved merge risk identified. Sequence Diagram(s)sequenceDiagram
participant Checkout
participant PrismaTransaction
participant NotificationDispatcher
Checkout->>PrismaTransaction: execute checkout and capture notification payloads
PrismaTransaction-->>Checkout: return commit or rollback result
Checkout->>NotificationDispatcher: deliver notifications after transaction outcome
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
…fter the checkout transaction commits Three Novu bells on the org-programme path issued their ProgramAssignment lookup on the global Prisma client from inside the Serializable checkout transaction. They were fire-and-forget, so they did not block the booking, but under PG_POOL_MAX=1 that query queues behind the transaction's own connection and dies at the 3 s pg connect timeout — and the .catch swallowed it, so on Netlify the bell was simply lost. The comments claimed the outer client was chosen to read committed state; the pool makes that impossible from where the call sat. All three now capture what they need inside the transaction and ring after it has settled. The 80% cap-near warning and the member-due overage charge travel out on the transaction's return value, so they ring only for the attempt that actually committed, and a P2034 retry can no longer ring them twice — which is what the capNearNotified flag existed to prevent, so it is gone. The cap-exhausted bell is the one whose news IS the refusal, so it rides out on a holder and rings from the retry wrapper's catch, preserving today's behaviour of telling the org even though the booking rolled back. recordOverageAtCheckout returns the pending member-due notification instead of ringing it, and the ringing moves to notifyOverageDueAfterCommit in the same module, so the Novu graph stays where its docblock says it lives. The two bells share one lookup and one roster, so the duplicated query and error handling collapse into dispatchProgramBell. approval-path-correctness asserts on checkout source text within a fixed character window of the STEP 5 marker, which a comment above the call can push the call out of. It now strips comments and looks in a wider window, so it pins the code rather than the prose. Part of #1435 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MgadJydWEKkdhhzY58yiL7
|
teetangh
added a commit
that referenced
this pull request
Sep 4, 2026
…fter the checkout transaction commits Three Novu bells on the org-programme path issued their ProgramAssignment lookup on the global Prisma client from inside the Serializable checkout transaction. They were fire-and-forget, so they did not block the booking, but under PG_POOL_MAX=1 that query queues behind the transaction's own connection and dies at the 3 s pg connect timeout — and the .catch swallowed it, so on Netlify the bell was simply lost. The comments claimed the outer client was chosen to read committed state; the pool makes that impossible from where the call sat. All three now capture what they need inside the transaction and ring after it has settled. The 80% cap-near warning and the member-due overage charge travel out on the transaction's return value, so they ring only for the attempt that actually committed, and a P2034 retry can no longer ring them twice — which is what the capNearNotified flag existed to prevent, so it is gone. The cap-exhausted bell is the one whose news IS the refusal, so it rides out on a holder and rings from the retry wrapper's catch, preserving today's behaviour of telling the org even though the booking rolled back. recordOverageAtCheckout returns the pending member-due notification instead of ringing it, and the ringing moves to notifyOverageDueAfterCommit in the same module, so the Novu graph stays where its docblock says it lives. The two bells share one lookup and one roster, so the duplicated query and error handling collapse into dispatchProgramBell. approval-path-correctness asserts on checkout source text within a fixed character window of the STEP 5 marker, which a comment above the call can push the call out of. It now strips comments and looks in a wider window, so it pins the code rather than the prose. Part of #1435 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MgadJydWEKkdhhzY58yiL7
Contributor
Author
|
@coderabbitai review |
|
Contributor
Author
|
@coderabbitai review |
|
Contributor
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
teetangh
added a commit
that referenced
this pull request
Sep 4, 2026
… refund idempotency key stops pretending to be a race Round-2 review triage on this PR plus two findings from the Razorpay productionization audit (#1451). The consultant payout status map had no `failed` entry, so a `payout.failed` delivery fell through to the `|| "PENDING"` default: the payout stayed in flight and its earnings stayed BATCHED, because the un-batch back to READY only runs on the FAILED branch of handlePayoutWebhook. The Stripe twin ten lines below already mapped it. One compact pin covers the consultant path. postRefund retried every 409, but Razorpay answers 409 for two conditions and only one of them is worth waiting on. A key replayed with a different payload answers 409 for as long as the key lives, so the retry bought a wasted second and reported the wrong cause; it now throws immediately as REFUND_IDEMPOTENCY_KEY_REUSED. The key material is untouched. reportTerminalCaptureRace re-reads the payment status instead of echoing the caller's pre-read, which is the doctrine confirmApprovalStatus has followed since #844 — the state named in that report is what an operator reconciles against. Callers pass their own client so no global-client read happens inside a transaction (#1435). The dev mock-webhook gate loses its `VERCEL_ENV === "preview"` disjunct: it was a runtime toggle contradicting the build-time posture the same comment claims, and it was dead anyway because this app deploys on Netlify. Part of #1353 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MgadJydWEKkdhhzY58yiL7
teetangh
added a commit
that referenced
this pull request
Sep 4, 2026
… payment id, deferred webhooks page, and the post-payment channel leg is re-driven off the appointment row (#1391) * fix(payments): refunds and disputes find their Payment by the gateway payment id, deferred webhooks page, and the post-payment channel leg is re-driven off the appointment row Closes #1352 Closes #1353 Closes #1356 Part of #1358 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MgadJydWEKkdhhzY58yiL7 * fix(webhooks): the either-id lookup keeps the old reachability — soft-deleted payments still take their refund and dispute events The `findFirst` that replaced `findUnique({ where: { paymentIntent } })` in handleRefundCreated and handleDisputeCreated carried a `deletedAt: null` filter the original lookup never had. That is a semantic narrowing on a money path: a Payment soft-deleted after capture would stop matching, so its refund event would DEFER and be given up on after 168h, and its dispute would page as CRITICAL_DISPUTE_UNLINKED with earnings left payable. Nothing about adding a second key justifies excluding those rows. The OR-match now reaches exactly what the old lookup reached, plus the gateway payment id. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MgadJydWEKkdhhzY58yiL7 * test(security): the DM-precedence source pin follows the channel block to ensure-channels.ts Two suites assert against `lib/payments/webhooks/handlers.ts` as source TEXT, so extracting the Stream channel block into `ensure-channels.ts` left them reading a file that no longer contains what they pin. Both entries now name the new module; every assertion is kept, because each one is still true there — the code moved, the contract did not. The trial rung's expected substring changed with it: the extracted function early-returns on a missing appointment, so the chain reads `appointment.trialSession?.consultantProfile` rather than optional-chaining off the lookup variable. The negative assertion — that it is NOT the plan author's `trialSession?.subscriptionPlan?.consultantProfile` — is untouched, which is the part that was ever load-bearing. Two other suites also read handlers.ts as text and were verified unaffected: reschedule-respond pins the Novu booked-notification block and participant-shadow-write pins the participant edge, neither of which moved. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MgadJydWEKkdhhzY58yiL7 * fix(payments): a Razorpay error envelope with no body maps to a RefundError instead of a TypeError `handleRazorpayRefundError` and `handleRazorpayError` both guarded with `"error" in error`, which also passes for `{ error: undefined }` — the key is an own property even when its value is not there. Reading `.code` off it then threw `TypeError: Cannot read properties of undefined`, which escaped the handler that exists precisely to classify the failure. Live E2E on 2026-09-04 lost 4 of 8 reconcile-refunds attempts to that TypeError instead of recording a classified RefundError. Both handlers now read the envelope through a shared helper that requires the body to be a non-null object and its `code`/`description` to be strings, so an empty or malformed envelope falls through to the generic error. The 409 to REFUND_IN_FLIGHT mapping in `postRefund` is unchanged. Part of #1353 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MgadJydWEKkdhhzY58yiL7 * fix(payments): the client-confirmation audit row is written inside after(), and the chat-leg queue gets an index Review round 1 on #1391. The `recordSystemEvent` call that records a CLIENT-side confirmation was floated next to the response rather than scheduled with `after()`. On Netlify the invocation freezes once the response is sent, so a detached insert races that freeze — and the confirmations worth auditing are the slow ones, exactly the ones whose audit row could be dropped. It now runs inside the existing `after()` callback, awaited ahead of the pipeline, keeping its `.catch` so it stays best-effort and can never fail a confirmation. `Appointment` had no index able to serve the #1356 chat-leg work queue, which filters `chatChannelEnsuredAt IS NULL AND deletedAt IS NULL` and orders by `createdAt`; every existing index leads with a different column. Prisma cannot express the partial index this really wants, but leading with the two equality columns gets the same seek. The high-`deferCount` refund runbook entry claimed such an event almost always means the payment was never captured. A pre-`gatewayPaymentId` row whose `payments.fetch` translation fails on credentials or availability defers identically, so the entry now sends operators to the local capture state first and to the gateway lookup second. The `ensure-channels` header now states which buyers the sweep actually re-drives: the stamp is per appointment, so for a shared `WEBINAR` or `CLASS` a later buyer whose ensure fails after the row is stamped is caught by `syncUserEventChannels` on their next dashboard load rather than by the sweep. Part of #1353 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MgadJydWEKkdhhzY58yiL7 * fix(payments): a refund idempotency key is rejected rather than rewritten, and the channel re-drive pass has a buyer-operation budget postRefund used to strip the characters Razorpay rejects out of the idempotency key before sending it. That is lossy: two distinct keys can collapse onto one header value, and Razorpay answers the second refund with the first one's result. The key is now validated whole against the documented rule and refused with REFUND_IDEMPOTENCY_KEY_INVALID when it does not match. The only production caller passes Refund.id, so nothing changes in practice and the failure mode is closed. The chat-channel pass of the reconcile sweep took the operator's limit, which reaches 500 appointments, and spent one outbound Stream call per paid buyer of each — unbounded work against the ticker's function ceiling. It now caps at 100 appointments and 500 buyer operations and stops cleanly when the budget is spent; rows it did not reach keep a NULL chatChannelEnsuredAt and the next run resumes oldest-first. Both counts are reported in the run summary. Part of #1353 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MgadJydWEKkdhhzY58yiL7 * fix(payments): the manual-recovery stamp is a compare-and-set, so a late capture cannot resurrect an expired payment Both REQUIRES_MANUAL_RECOVERY branches of handlePaymentSuccess — the capture-amount mismatch and the metadata-validation failure — stamped SUCCEEDED with a bare `tx.payment.update({ where: { id } })`. When the abandoned-payments sweep had already expired the row and released its tentative hold, a late `payment.captured` flipped the EXPIRED payment back to SUCCEEDED, leaving money recorded against an appointment that stayed tentative with nothing left to release it. Every status stamp in this pipeline now rides a compare-and-set: the `paymentStatus: PENDING` predicate sits in the WHERE of an `updateMany`, as ADR 21 requires. A count of zero means the row is already terminal, so the handler writes nothing, records a PAYMENT system error and a Sentry warning naming the order, the current status and the reason, and returns the same result it returned before — the webhook is still acknowledged and Razorpay does not retry it. Two further stamps of the same shape take the same guard: the confirmation write itself, which a replay now reaches, and the post-transaction stamp on the legacy capture that loses the GiST overlap race. The dev replay route built its metadata under the key `consulteeId` while the schema requires `userId`, so every replayed first capture failed validation and took the recovery branch instead of confirming the booking. Renaming the key sends it down the confirmation path, which is why that path needed the guard in the same change. Closes #1439 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MgadJydWEKkdhhzY58yiL7 * fix(payments): post-commit notifications and the chat-channel step are bounded, so after() work cannot starve the single-connection pool Phase 2 of handlePaymentSuccess runs inside after(), on the same warm instance that is already serving the next inbound request, and PG_POOL_MAX=1 means the two share one Prisma connection. Two unawaited Novu triggers ran 39 s each while the chat-channel step waited for that connection and died at the 3 s connect timeout. The triggers are now collected and awaited together, each under a 5 s deadline, before the channel step begins; the channel step has the same deadline and, on timeout, leaves chatChannelEnsuredAt NULL so reconcile-orphaned-confirmations re-drives it. The Novu client gets an explicit request timeout and a bounded retry budget, so an abandoned call cannot keep burning the instance. The ensure-channels read is a narrow select of the ids, org ids and consultant userId the step uses instead of a six-relation include. Money is untouched: the Serializable transaction commits before any of this runs. Closes #1446 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MgadJydWEKkdhhzY58yiL7 * refactor(payments): one compare-and-set stamp helper for the capture handler, and a smaller channel pass Extracts the identical payment->appointment->consultantProfile resolution that the webhook success path and the checkout mock/zero/sponsored path both ran before creating earnings into resolvePaymentForEarnings, and pulls the channel-pass loop body in the orphaned-confirmation reconciler into ensureChannelForOrphan so the sweep function's cognitive complexity clears the gate. No behaviour change. Part of #1439 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MgadJydWEKkdhhzY58yiL7 * chore(sonar): exclude test suites from copy-paste detection in Automatic Analysis too .sonarcloud.properties is what SonarQube Cloud Automatic Analysis reads, not sonar-project.properties (that one is staged for the future CI-based scan). #1330 added the CPD exclusion for __tests__/**,tests/** only to the latter, so the new-code duplication gate kept counting per-file jest.mock scaffolding as duplication against every PR that added test coverage — this PR's own gate failure is that boilerplate, not application code. Part of #1439 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MgadJydWEKkdhhzY58yiL7 * fix(payments): a failed RazorpayX payout reaches FAILED, and a reused refund idempotency key stops pretending to be a race Round-2 review triage on this PR plus two findings from the Razorpay productionization audit (#1451). The consultant payout status map had no `failed` entry, so a `payout.failed` delivery fell through to the `|| "PENDING"` default: the payout stayed in flight and its earnings stayed BATCHED, because the un-batch back to READY only runs on the FAILED branch of handlePayoutWebhook. The Stripe twin ten lines below already mapped it. One compact pin covers the consultant path. postRefund retried every 409, but Razorpay answers 409 for two conditions and only one of them is worth waiting on. A key replayed with a different payload answers 409 for as long as the key lives, so the retry bought a wasted second and reported the wrong cause; it now throws immediately as REFUND_IDEMPOTENCY_KEY_REUSED. The key material is untouched. reportTerminalCaptureRace re-reads the payment status instead of echoing the caller's pre-read, which is the doctrine confirmApprovalStatus has followed since #844 — the state named in that report is what an operator reconciles against. Callers pass their own client so no global-client read happens inside a transaction (#1435). The dev mock-webhook gate loses its `VERCEL_ENV === "preview"` disjunct: it was a runtime toggle contradicting the build-time posture the same comment claims, and it was dead anyway because this app deploys on Netlify. Part of #1353 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MgadJydWEKkdhhzY58yiL7 --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
teetangh
added a commit
that referenced
this pull request
Sep 5, 2026
…nd consent gates read the legacy purpose codes (#1465-triage, #1472) Review triage on #1465 found one real hole in the #1463 self-hold exclusion. `findSelfHoldAppointmentIds` excluded a buyer's own live hold on buyer, plan, status, deletion and window alone, but `findReusablePendingOrderPayment` will only resume or supersede a candidate whose `paymentGateway` and `organizationId` also match this request. A hold minted on another gateway, or under another org scope, was therefore taken off the calendar by a request that could neither adopt nor expire it: the same buyer minted a SECOND tentative appointment and a second payable gateway order over the same window, and both orders could capture. The exclusion now carries the resume gate's own two terms, so an unresumable hold keeps blocking and the buyer waits out its `expiresAt` instead of double-paying. The server-resolved org scope is threaded from `handleCheckout` through `calculateAmountAndValidate`, `revalidateInsideLock` and `createConsultationBooking`, defaulting to null (personal) so a caller that cannot resolve it fails closed. Step 2's duplicate-attempt guard deliberately keeps the unscoped liveness filter: it asks whether the buyer holds this window at all, and scoping it would let a second attempt on another gateway slip past the guard entirely. The plan-scope ternary chain became a switch, which is also the sonar S3358 finding on this PR's new code. Folding in #1472: `checkConsent`, `checkConsentBatch` and `withdrawConsent` matched `purposeCodes` against the canonical code exactly, so an artifact written under the pre-taxonomy kebab-case code (`session-booking`) was invisible to the fail-closed booking gate and every booking against that consultant answered 403 although `withdrawnAt` was null. A consent record is a legal artifact, so the gate has to recognise every code the platform ever wrote: `purposeCodeAliases` resolves a canonical code to itself plus each legacy alias that normalises to it, and the three lookups query `hasSome` over that set. Writes still normalise to the canonical form and the DB is not backfilled (pre-MVP reset). `checkConsent` keeps its `db` parameter — under PG_POOL_MAX=1 it must read through the caller's transaction (#1435). The other two review comments are answered without a code change. Coupling the credit-note refusal event to the refund transaction is refused because `recordSystemEvent` is a deliberately non-cascading global-client sink and the operator signal already survives on the Sentry warning. Cancelling a superseded gateway order is refused because `cancelRazorpayOrder` cannot make a Razorpay order unpayable — it only fetches the order's payments and logs — so the call would add in-lock gateway round-trips and remove nothing; a late capture is already the modelled CAPTURE_AFTER_TERMINAL_PAYMENT path. Closes #1472 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MgadJydWEKkdhhzY58yiL7
teetangh
added a commit
that referenced
this pull request
Sep 5, 2026
…d no longer blocks their own resume (#1462, #1463) (#1465) * fix(payments): empty gateway notes are omitted, and a buyer's own hold no longer blocks their own resume (#1462, #1463) Two B2C checkout defects found by the wave-1A edge-case run, both of which end with the buyer unable to complete a purchase they already started. #1462 — buildPaymentMetadata always emitted `startsAt`/`endsAt` (and four other optional fields) into the gateway order notes, falling back to the empty string. A subscription bought for a scheduling period carries no direct slots, so those keys reached Razorpay as `""`, and `z.string().datetime().optional()` accepts an ABSENT key but rejects an empty string: every capture webhook for such a sale failed validation and stamped the payment SUCCEEDED / REQUIRES_MANUAL_RECOVERY with the buyer already charged. The builder now omits an optional field that has no value, and validateWebhookMetadata strips empty-string entries before normalizing and parsing, because a Razorpay order never expires and the orders already minted with `""` keep replaying. #1463 — validateSlotAvailability rejected any overlapping live hold, including the requesting buyer's own PENDING hold on the very same slot and plan, and it runs before the open-order resume (Rec C, findReusablePendingOrderPayment) that exists to finish exactly that order. A buyer who dismissed the gateway modal was walled out until the hold expired. Both blocking steps, and the consultee-side conflict check inside the checkout lock, now subtract a SELF-HOLD: same buyer, same plan, still-PENDING and still-live payment, and exactly the requested window. Everything else keeps blocking. Three supporting corrections come with it: the helper's buyer parameter was being handed a ConsulteeProfile id and compared to Payment.userId, so the duplicate-hold step could never fire; the resume window gate read only the first 30-minute atom of a booked run, which rejected every consultation longer than half an hour; and superseding an open order expired its Payment while leaving the appointment and slots on the calendar, which put the next attempt back into the same wall. The release now runs in one transaction with the payment CAS and goes through the guarded transitions in lib/booking/transitions.ts. Also downgrades the "invoice already fully credited" credit-note refusal from an error-level Sentry issue to a modelled warning. The cumulative cap from #1393 refusing a second reversal is the cap working, no money moves, and the durable SystemEvent row is unchanged. Closes #1462 Closes #1463 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MgadJydWEKkdhhzY58yiL7 * fix(payments): a hold this request could not resume keeps blocking, and consent gates read the legacy purpose codes (#1465-triage, #1472) Review triage on #1465 found one real hole in the #1463 self-hold exclusion. `findSelfHoldAppointmentIds` excluded a buyer's own live hold on buyer, plan, status, deletion and window alone, but `findReusablePendingOrderPayment` will only resume or supersede a candidate whose `paymentGateway` and `organizationId` also match this request. A hold minted on another gateway, or under another org scope, was therefore taken off the calendar by a request that could neither adopt nor expire it: the same buyer minted a SECOND tentative appointment and a second payable gateway order over the same window, and both orders could capture. The exclusion now carries the resume gate's own two terms, so an unresumable hold keeps blocking and the buyer waits out its `expiresAt` instead of double-paying. The server-resolved org scope is threaded from `handleCheckout` through `calculateAmountAndValidate`, `revalidateInsideLock` and `createConsultationBooking`, defaulting to null (personal) so a caller that cannot resolve it fails closed. Step 2's duplicate-attempt guard deliberately keeps the unscoped liveness filter: it asks whether the buyer holds this window at all, and scoping it would let a second attempt on another gateway slip past the guard entirely. The plan-scope ternary chain became a switch, which is also the sonar S3358 finding on this PR's new code. Folding in #1472: `checkConsent`, `checkConsentBatch` and `withdrawConsent` matched `purposeCodes` against the canonical code exactly, so an artifact written under the pre-taxonomy kebab-case code (`session-booking`) was invisible to the fail-closed booking gate and every booking against that consultant answered 403 although `withdrawnAt` was null. A consent record is a legal artifact, so the gate has to recognise every code the platform ever wrote: `purposeCodeAliases` resolves a canonical code to itself plus each legacy alias that normalises to it, and the three lookups query `hasSome` over that set. Writes still normalise to the canonical form and the DB is not backfilled (pre-MVP reset). `checkConsent` keeps its `db` parameter — under PG_POOL_MAX=1 it must read through the caller's transaction (#1435). The other two review comments are answered without a code change. Coupling the credit-note refusal event to the refund transaction is refused because `recordSystemEvent` is a deliberately non-cascading global-client sink and the operator signal already survives on the Sentry warning. Cancelling a superseded gateway order is refused because `cancelRazorpayOrder` cannot make a Razorpay order unpayable — it only fetches the order's payments and logs — so the call would add in-lock gateway round-trips and remove nothing; a late capture is already the modelled CAPTURE_AFTER_TERMINAL_PAYMENT path. Closes #1472 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MgadJydWEKkdhhzY58yiL7 --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
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.
The blocker
POST /api/checkoutfor a plain Razorpay consultation failed 8/8 times over ten minutes on the PR #1422 deploy preview with HTTP 500{"error":"timeout exceeded when trying to connect"}, while sibling database routes on the same deploy answered in 1-5 seconds. No partial rows were left behind.Netlify runs this app with a pg pool of
PG_POOL_MAX=1and a 3 second connect timeout ([Prisma:INIT] connect=3000ms query=6000ms poolMax=1). An interactive Prisma transaction checks out that single connection and holds it until it commits, so any query sent to the global client while a transaction is open queues for a connection that only the blocked transaction can release. The request cannot make progress and pg gives up with exactly that message.Root cause
lib/payments/operations/checkout.ts:857— the DPDPSESSION_BOOKINGgate insidevalidateSlotAvailabilitycalledcheckConsent, which reads on the global client (lib/compliance/dpdp.ts:194).validateSlotAvailabilityis called from inside three separate transactions on the plain Razorpay consultation path:calculateAmountAndValidatecheckout.ts:500revalidateInsideLockcheckout.ts:1414handleConsultationCheckoutcheckout.ts:2861(Serializable)The first of them deadlocks against itself, which is why nothing was ever written.
revalidateInsideLockhad a second instance of the same defect atcheckout.ts:1528on its own org-sponsored consent check.Neither instance comes from the finance train. Both have been on
devsince commit 20fef46 (Wave 7 — LCY-2 consent cascade, #1255) and production runs the same single-connection pool, so consultation checkout could not take a payment on production either. The earlier end-to-end run passed only because a local dev server runs withpoolMax=10, where a second connection is available and the nested read simply succeeds.The fix
checkConsentnow takes an optional client that defaults to the global one, following thegetUserCredits(userId, db = prisma)convention already used across this codebase, and both in-transaction call sites passtx. The gate itself is unchanged and still fails closed. The redundant dynamic imports invalidateSlotAvailabilityare gone, since both symbols were already static imports at the top of the file. The pool is not widened.Verification
The pin at
__tests__/payments/checkout-pool-1-nesting.test.tsmodels the pool rather than the query: the mocked global client throws the same pg error the preview logged whenever it is touched while a transaction is open. It fails against the unfixed code withtimeout exceeded when trying to connectand passes once the gate reads throughtx. A second case asserts that a consultant who withdrew session-delivery consent is still blocked, because a fix that silently disabled the gate would also make the first case pass.npx tsc --noEmitafternpx prisma generateon a cleared build info: clean.npx eslinton the changed files: no errors, and one pre-existingeqeqeqwarning atcheckout.ts:3271that this change does not touch.npx prettier --checkon the changed files: clean.lib/compliance/dpdp.tswas already failing that check ondev, so formatting the file to commit this change also settles one unrelated line.npx jest __tests__/payments: 53 suites, 508 tests, all passing.Part of #1421
🤖 Generated with Claude Code
https://claude.ai/code/session_01MgadJydWEKkdhhzY58yiL7
Closes #1436