From 6befa55a35d6cc690811b3c434f842bd46855235 Mon Sep 17 00:00:00 2001 From: Kaustav Ghosh <44238657+teetangh@users.noreply.github.com> Date: Sat, 5 Sep 2026 07:07:43 +0530 Subject: [PATCH 1/3] fix(enterprise): a wallet-funded overage is collected by the wallet debit, never by inflating the payment, and cap and member-overage refusals reach the buyer as business errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On the WALLET rail the debit taken when a booking commits is the whole nominal price, so an over-cap booking is already paid for. The CHARGE_ORG branch nonetheless carved from an INVOICE_ACCRUAL leg that a wallet parent never has, fell into its own "not reachable" additive fallback, wrote an OVERAGE_INVOICE_ACCRUAL leg and incremented Payment.amount — which broke the leg-sum identity and made a later cancellation refund the org more than its wallet was ever debited. The wallet rail is now resolved first and records the OverageEvent as CHARGED and settled against the payment whose WALLET leg collected it, with no leg and no amount change. A surcharge, or an org-sponsored payment carrying none of the three funding legs, fails closed with a business error instead of inflating the amount. CHARGE_MEMBER on a WALLET account is refused at programme create and patch time, and the checkout backstop now carries a stable code and a 409. PROGRAM_CAP_EXHAUSTED and the per-assignment session cap reach the route with their own status and toast, because the checkout catch rethrows any error whose code is registered in BUSINESS_ERROR_CODES. The refund-reconcile sweep skips STRIPE rows while the rail is fenced and counts them, instead of failing the run. Closes #1458 Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01MgadJydWEKkdhhzY58yiL7 --- .../overage-settlement-legsum.test.ts | 48 +++++- __tests__/enterprise/reachable-paths.test.ts | 25 +++ .../gateway-fence-classification.test.ts | 44 +++++ .../reconcile-reservation-match.test.ts | 31 ++++ app/api/cleanup/reconcile-refunds/route.ts | 12 +- .../[orgId]/programs/[programId]/route.ts | 19 ++- .../organizations/[orgId]/programs/route.ts | 18 +++ .../05-booking-to-earnings.md | 8 +- docs/payments/05-b2c-b2b-funding-seam.md | 10 ++ jobs/refunds/reconcile-pending-refunds.ts | 3 + lib/enterprise/reachable-paths.ts | 37 ++++- .../payment-error-classification.ts | 46 ++++++ lib/errors/mapping/payment-error-toast-map.ts | 17 ++ lib/payments/billing/overage-settlement.ts | 153 ++++++++++++++++-- lib/payments/operations/checkout.ts | 27 +++- lib/payments/operations/refund.ts | 16 +- prisma/schema.prisma | 4 +- scripts/reconcile/reconcile-ledgers.ts | 7 +- scripts/refunds/reconcile-pending-refunds.ts | 35 ++++ 19 files changed, 537 insertions(+), 23 deletions(-) diff --git a/__tests__/enterprise/overage-settlement-legsum.test.ts b/__tests__/enterprise/overage-settlement-legsum.test.ts index 978f06b86..596e80b29 100644 --- a/__tests__/enterprise/overage-settlement-legsum.test.ts +++ b/__tests__/enterprise/overage-settlement-legsum.test.ts @@ -37,8 +37,12 @@ function makeTx(opts: { surchargeBps?: number | null; priceCap?: number | null; overageBehavior?: "CHARGE_ORG" | "CHARGE_MEMBER"; + /** #1458 — which funding rail wrote the parent's base leg. */ + baseSource?: "INVOICE_ACCRUAL" | "WALLET"; }) { - const legs: Leg[] = [{ source: "INVOICE_ACCRUAL", amountPaise: opts.price }]; + const legs: Leg[] = [ + { source: opts.baseSource ?? "INVOICE_ACCRUAL", amountPaise: opts.price }, + ]; const payment = { amount: opts.price }; const children: { amount: number }[] = []; let childSeq = 0; @@ -168,6 +172,48 @@ describe("recordOverageAtCheckout — CHARGE_ORG leg-sum invariant (#785)", () = }); }); +describe("recordOverageAtCheckout — CHARGE_ORG on the WALLET rail (#1458)", () => { + it("leaves the payment at the wallet debit, adds no leg, and records the overage as collected", async () => { + const walletDebit = 258_326; + const { state, tx } = makeTx({ + price: walletDebit, + cap: 5, + used: 5, + baseSource: "WALLET", + }); + await recordOverageAtCheckout({ + tx: tx as unknown as Tx, + ...callArgs(walletDebit), + }); + + // The wallet already took the whole price at commit, so the marginal is + // collected: no OVERAGE_INVOICE_ACCRUAL leg, no amount bump. + expect(state.legs).toEqual([ + { source: "WALLET", amountPaise: walletDebit }, + ]); + expect(state.payment.amount).toBe(walletDebit); + expect(sum(state.legs)).toBe(state.payment.amount); + expect(tx.paymentLeg.create).not.toHaveBeenCalled(); + expect(tx.payment.update).not.toHaveBeenCalled(); + + expect(tx.overageEvent.create).toHaveBeenCalledWith( + expect.objectContaining({ + data: expect.objectContaining({ + overageBehavior: "CHARGE_ORG", + chargeStatus: "CHARGED", + paymentId: "pay1", + settledAt: expect.any(Date), + }), + }), + ); + + // The cancellation quote is a percentage of Payment.amount and the refund + // cascade splits it across the legs — with one WALLET leg equal to amount, + // a 100% refund returns exactly the debit and not a paisa more. + expect(state.payment.amount).toBe(walletDebit); + }); +}); + describe("recordOverageAtCheckout — CHARGE_MEMBER parent carve (#785)", () => { it("carves basePaise off the org parent; member child pays the marginal (no double-collect)", async () => { const { state, tx } = makeTx({ diff --git a/__tests__/enterprise/reachable-paths.test.ts b/__tests__/enterprise/reachable-paths.test.ts index 5741da067..bc2a9a1f5 100644 --- a/__tests__/enterprise/reachable-paths.test.ts +++ b/__tests__/enterprise/reachable-paths.test.ts @@ -13,6 +13,7 @@ import { REACHABLE_ORG_FUNDING_PATHS, isReachableOrgFundingPath, + overageBehaviorUnsupportedReason, capabilityOf, } from "@/lib/enterprise/reachable-paths"; @@ -69,6 +70,30 @@ describe("REACHABLE_ORG_FUNDING_PATHS — v0 lockdown matrix", () => { }); }); + // #1458 — the matrix sanctions SPONSOR + WALLET + CREDIT_POOL, but a wallet + // debit takes the whole booking price at commit, so there is nothing left to + // carve back out for a member charge. Checkout could only fail closed after + // the member had picked a slot; the config is what has to be refused. + describe("overageBehaviorUnsupportedReason", () => { + it("refuses CHARGE_MEMBER on a WALLET-funded account, naming #715", () => { + const reason = overageBehaviorUnsupportedReason( + "WALLET", + "CHARGE_MEMBER", + ); + expect(reason).toContain("#715"); + }); + + it("allows CHARGE_ORG and BLOCK on WALLET, and CHARGE_MEMBER on INVOICE", () => { + expect( + overageBehaviorUnsupportedReason("WALLET", "CHARGE_ORG"), + ).toBeNull(); + expect(overageBehaviorUnsupportedReason("WALLET", "BLOCK")).toBeNull(); + expect( + overageBehaviorUnsupportedReason("INVOICE", "CHARGE_MEMBER"), + ).toBeNull(); + }); + }); + describe("capabilityOf", () => { it.each([ [true, false, "SPONSOR"], diff --git a/__tests__/payments/gateway-fence-classification.test.ts b/__tests__/payments/gateway-fence-classification.test.ts index 67ba28f25..6305d15ea 100644 --- a/__tests__/payments/gateway-fence-classification.test.ts +++ b/__tests__/payments/gateway-fence-classification.test.ts @@ -78,4 +78,48 @@ describe("gateway fence classification", () => { expect(toast.title).toBe("Domain Verification Required"); expect(toast.description).toBeTruthy(); }); + + // #1458 — the overage settlement throws PROGRAM_CAP_EXHAUSTED as a 402 with + // copy the buyer can act on, but the checkout catch rewrote it to "Failed to + // record payment information" and the classifier answered 500 UNKNOWN_ERROR. + it("classifies PROGRAM_CAP_EXHAUSTED as a 402 with its own toast", () => { + const classified = classifyError( + Object.assign(new Error("cycle ceiling reached"), { + httpStatus: 402, + code: "PROGRAM_CAP_EXHAUSTED", + }), + ); + + expect(classified.errorType).toBe(ErrorTypes.PROGRAM_CAP_EXHAUSTED); + expect(classified.isBusinessError).toBe(true); + expect(classified.httpStatus).toBe(402); + + const toast = getErrorToast(classified.errorType); + expect(toast.title).not.toBe("Something Went Wrong"); + expect(toast.description).toContain("programme budget"); + }); + + it("classifies the other checkout-transaction refusals off their codes", () => { + expect( + classifyError( + Object.assign(new Error("session cap"), { + code: "PROGRAM_SESSION_CAP_REACHED", + }), + ).httpStatus, + ).toBe(402); + expect( + classifyError( + Object.assign(new Error("member overage"), { + code: "OVERAGE_CHARGE_MEMBER_UNSUPPORTED", + }), + ).httpStatus, + ).toBe(409); + expect( + classifyError( + Object.assign(new Error("funding"), { + code: "OVERAGE_UNSUPPORTED_FUNDING", + }), + ).httpStatus, + ).toBe(409); + }); }); diff --git a/__tests__/payments/reconcile-reservation-match.test.ts b/__tests__/payments/reconcile-reservation-match.test.ts index e2fde6a50..10ca9e032 100644 --- a/__tests__/payments/reconcile-reservation-match.test.ts +++ b/__tests__/payments/reconcile-reservation-match.test.ts @@ -264,4 +264,35 @@ describe("reconcilePendingRefunds real-id PENDING polling", () => { expect(result.failedCount).toBe(0); expect(refundTable.update).not.toHaveBeenCalled(); }); + + // #1458 — with STRIPE_ENABLED unset, the Stripe client is never built, so + // getRefund threw for every Stripe row, the error list filled up and the whole + // run reported success:false — the cleanup route answered 500 for what is + // deliberate configuration. + test("a fenced STRIPE refund is skipped and counted, not failed", async () => { + const previous = process.env.STRIPE_ENABLED; + delete process.env.STRIPE_ENABLED; + try { + refundTable.findMany.mockResolvedValueOnce([]).mockResolvedValueOnce([ + { + id: "row_stripe", + refundId: "re_real", + status: "PENDING", + amountPaise: 10_000, + createdAt: new Date(Date.now() - 3 * HOUR), + payment: { paymentGateway: "STRIPE" }, + }, + ]); + + const result = await reconcilePendingRefunds(); + + expect(mockGet).not.toHaveBeenCalled(); + expect(result.skippedFenced).toBe(1); + expect(result.success).toBe(true); + expect(result.errors).toEqual([]); + } finally { + if (previous === undefined) delete process.env.STRIPE_ENABLED; + else process.env.STRIPE_ENABLED = previous; + } + }); }); diff --git a/app/api/cleanup/reconcile-refunds/route.ts b/app/api/cleanup/reconcile-refunds/route.ts index 25db41c2f..0b5a113c3 100644 --- a/app/api/cleanup/reconcile-refunds/route.ts +++ b/app/api/cleanup/reconcile-refunds/route.ts @@ -7,7 +7,11 @@ * Schedule: Every 15 minutes (via GitHub Actions or external cron) */ -import { cleanupRoute, parseLimitParam } from "@/lib/cron/cleanup-route"; +import { + cleanupRoute, + parseLimitParam, + statusFor, +} from "@/lib/cron/cleanup-route"; import { reconcilePendingRefunds } from "@/scripts/refunds/reconcile-pending-refunds"; export const { GET, POST } = cleanupRoute({ @@ -21,7 +25,13 @@ export const { GET, POST } = cleanupRoute({ reconciledCount: r.reconciledCount, failedCount: r.failedCount, skippedCount: r.skippedCount, + skippedFenced: r.skippedFenced, }), + // #1458 — a fenced-gateway skip is a healthy run with something an operator + // should know about: PENDING refunds exist on a rail this deployment does not + // poll. 207 says exactly that, where the old behaviour was a 500 because every + // fenced row threw and landed in `errors`. + status: (r) => statusFor(r, r.skippedFenced > 0), // #1390 review — the constant 200 masked a caught job error (success:false) // as healthy; the default statusFor already reads result.success. failureMessage: "Failed to reconcile refunds", diff --git a/app/api/organizations/[orgId]/programs/[programId]/route.ts b/app/api/organizations/[orgId]/programs/[programId]/route.ts index 38fabe727..cab5e25c5 100644 --- a/app/api/organizations/[orgId]/programs/[programId]/route.ts +++ b/app/api/organizations/[orgId]/programs/[programId]/route.ts @@ -16,6 +16,7 @@ import { requireOrgAccess } from "@/lib/auth-helpers"; import { AUDIT_ACTIONS } from "@/lib/enterprise/audit-actions"; import { transitionProgram } from "@/lib/enterprise/transitions"; import { getProgramLockState } from "@/lib/enterprise/config-lock"; +import { overageBehaviorUnsupportedReason } from "@/lib/enterprise/reachable-paths"; import { withSerializableRetry } from "@/lib/db/serializable-retry"; const ProgramStatusSchema = z.enum([ @@ -154,7 +155,15 @@ async function applyProgramPatch( const touchesMoney = MONEY_FIELDS.some((f) => body[f] !== undefined); const current = await tx.program.findFirst({ where: { id: programId, contract: { organizationId: orgId } }, - include: { licensedSeatConfig: true, creditPoolConfig: true }, + include: { + licensedSeatConfig: true, + creditPoolConfig: true, + // #1458 — the funding source decides which overage behaviours can + // actually be collected, so the merged-config check below needs it. + contract: { + select: { billingAccount: { select: { fundingSource: true } } }, + }, + }, }); if (!current) { throw Object.assign(new Error("Program not found"), { @@ -235,6 +244,14 @@ async function applyProgramPatch( "overageSurchargeBps has no effect with overageBehavior=BLOCK — remove it or pick CHARGE_MEMBER/CHARGE_ORG.", ); } + // #1458 — same funding-source rule the create route applies, re-checked on + // the merged config so a patch cannot assemble a combination the create + // route would have refused. + const overageReason = overageBehaviorUnsupportedReason( + current.contract.billingAccount?.fundingSource ?? null, + merged.overageBehavior, + ); + if (overageReason) fail(overageReason); } // #777 §B — archiving guard: an archived program is skipped by the cycle diff --git a/app/api/organizations/[orgId]/programs/route.ts b/app/api/organizations/[orgId]/programs/route.ts index 524110c3e..85b277bd4 100644 --- a/app/api/organizations/[orgId]/programs/route.ts +++ b/app/api/organizations/[orgId]/programs/route.ts @@ -16,6 +16,7 @@ import { AUDIT_ACTIONS } from "@/lib/enterprise/audit-actions"; import { capabilityOf, isReachableOrgFundingPath, + overageBehaviorUnsupportedReason, } from "@/lib/enterprise/reachable-paths"; import { sumPaise } from "@/lib/payments/utils/money"; @@ -315,6 +316,23 @@ export async function POST( ); } + // #1458 — the matrix above sanctions the funding shape but says nothing about + // what happens past the cap. CHARGE_MEMBER on a wallet-funded contract only + // failed at checkout, inside the booking transaction, so the refusal landed on + // a member who had already picked a slot. Refuse it here instead. + const overageReason = overageBehaviorUnsupportedReason( + fundingSource, + body.type === "LICENSED_SEAT" + ? body.licensedSeatConfig.overageBehavior + : body.creditPoolConfig.overageBehavior, + ); + if (overageReason) { + return NextResponse.json( + { error: overageReason, code: "INVALID_OVERAGE_CONFIG" }, + { status: 400 }, + ); + } + // #751 — two ACTIVE programs on the same contract with intersecting // coveredPlanTypes make checkout's program resolution ambiguous (the // booking lands on whichever resolves first) and can double-entitle a diff --git a/docs/enterprise/10-money-and-ledger/05-booking-to-earnings.md b/docs/enterprise/10-money-and-ledger/05-booking-to-earnings.md index 6512e7b51..f82577717 100644 --- a/docs/enterprise/10-money-and-ledger/05-booking-to-earnings.md +++ b/docs/enterprise/10-money-and-ledger/05-booking-to-earnings.md @@ -203,10 +203,16 @@ On a real over-cap checkout, `recordOverageAtCheckout` (`lib/payments/billing/ov - **Circuit breaker.** `maxOveragePerCyclePaise` is a per-cycle overage ceiling. If `cycleOverageSoFarPaise + marginal` would breach it, the mapper returns `decision: BLOCK, chargeTo: null` **regardless of `overageBehavior`** — the recorder throws `PROGRAM_CAP_EXHAUSTED` (HTTP 402), the same shape as a `BLOCK`-behavior refusal but with a distinct code so the dashboard can say "cycle ceiling" vs "per-member allocation". An unknown/missing `overageBehavior` also **fails safe to `BLOCK`**. - **`CHARGE_MEMBER`** → a parent-linked **PENDING side-`Payment`** for the marginal (gateway _not_ called inside the tx; the order is minted lazily when the member opens the resume-checkout surface) + an `OverageEvent(PENDING)`. To avoid double-collecting `basePaise`, checkout carves it out of the org-funded parent's `INVOICE_ACCRUAL` leg (fail-closed: a non-invoice-funded parent has no credit-back path yet, #715, so it aborts rather than double-charge). Member is notified (`notifyOrgProgramOverageDue`) with a pay deep link. The webhook later posts the `OVERAGE_MEMBER` org-relief leg ([§4.8 of ledger & postings](03-ledger-and-postings.md)). -- **`CHARGE_ORG`** → carve `basePaise` out of the base `INVOICE_ACCRUAL` leg and write the marginal as a distinct **`OVERAGE_INVOICE_ACCRUAL`** leg (the distinct source dodges the `@@unique([paymentId, source])` clash) + an `OverageEvent(PENDING)`. The cycle-close rollup turns it into an `InvoiceLineItem` and walks the event `PENDING → ACCRUED → CHARGED` ([invoicing](08-invoicing.md)). +- **`CHARGE_ORG` on the INVOICE rail** → carve `basePaise` out of the base `INVOICE_ACCRUAL` leg and write the marginal as a distinct **`OVERAGE_INVOICE_ACCRUAL`** leg (the distinct source dodges the `@@unique([paymentId, source])` clash) + an `OverageEvent(PENDING)`. The cycle-close rollup turns it into an `InvoiceLineItem` and walks the event `PENDING → ACCRUED → CHARGED` ([invoicing](08-invoicing.md)). +- **`CHARGE_ORG` on the WALLET rail (#1458)** → nothing is billed, because the wallet debit taken when the booking committed is the whole nominal price and therefore already contains the over-cap pass-through. The recorder writes **no** leg and does **not** touch `Payment.amount`; it records an `OverageEvent` that is born `CHARGED` with `settledAt` stamped and `paymentId` pointing at the booking payment whose `WALLET` leg collected it. That event carries no `invoiceLineItemId`, so the reconciler's link invariant accepts either link as proof of collection. Anything else on this rail fails closed with a business error rather than inflating the payment: a positive `overageSurchargeBps` is a markup the wallet debit never took, and an org-sponsored payment carrying none of the `WALLET` / `INVOICE_ACCRUAL` / `LICENSE` funding legs means the funding seam itself has drifted. +- **`CHARGE_MEMBER` is not available on a WALLET account (#715, guarded in #1458).** Collecting from the member requires carving the over-cap portion back out of the parent, which on the wallet rail would mean crediting the wallet mid-transaction — a path that has never been built. `overageBehaviorUnsupportedReason` (`lib/enterprise/reachable-paths.ts`) refuses the combination when the programme is created or patched, so an operator cannot save a configuration whose only outcome is a refused booking. Checkout keeps its fail-closed throw for programmes configured before that guard existed, now carrying the code `OVERAGE_CHARGE_MEMBER_UNSUPPORTED` and an HTTP 409. The `chargeStatus` state machine itself is a single guarded transition (`transitionOverage`, `overage-transitions.ts`); the overage-event lifecycle table of states (`PENDING/ACCRUED/CHARGED/BLOCKED/REVERSED/FAILED`) is in [funding & programs](../00-foundations/03-funding-and-programs.md) / [programs](../30-programs-and-lifecycle/02-programs.md). +Because a wallet-funded overage never adds to `Payment.amount`, a cancellation of such a booking refunds exactly what the wallet was debited. The refund cascade splits the refund across the payment's legs, and the single `WALLET` leg equals `Payment.amount`, so a full refund credits the wallet back to the balance it held before the booking. The same cascade reverses the `CHARGED` event, because the money it represented has just been returned and the programme's per-cycle ceiling has to be released with it. + +The refusals checkout can raise from inside its transaction all carry a machine-readable code, and the catch around that transaction rethrows any error whose code is registered in `BUSINESS_ERROR_CODES` instead of rewriting it. `PROGRAM_CAP_EXHAUSTED` therefore reaches the buyer as the HTTP 402 it was thrown as, with a toast that names the admin action, rather than as the 500 "Something Went Wrong" it used to collapse into. + --- ## 7. Design decisions & trade-offs diff --git a/docs/payments/05-b2c-b2b-funding-seam.md b/docs/payments/05-b2c-b2b-funding-seam.md index 0705bb5ff..11038a6c1 100644 --- a/docs/payments/05-b2c-b2b-funding-seam.md +++ b/docs/payments/05-b2c-b2b-funding-seam.md @@ -68,6 +68,16 @@ Legs are **append-only** — a refund never mutates the original leg; it upserts Invariant: `sum(non-reversal, non-REFERRAL_CREDIT legs.amountPaise) == Payment.amount` when legs are present (LICENSE legs contribute 0). The referral credit sits outside the sum because `Payment.amount` is the post-credit gateway charge, so the credit has already been deducted from it and counting the leg as well would demand it twice (#1347). See [payment legs §3](../enterprise/10-money-and-ledger/09-payment-legs.md#3-invariants). +### Programme overage across the rails (#1458) + +A booking that breaches its programme's cap does not settle the same way on every rail, because the rails collect at different moments. + +On the **INVOICE** rail the org has not paid anything yet, so the marginal is carved out of the base `INVOICE_ACCRUAL` leg into an `OVERAGE_INVOICE_ACCRUAL` leg and billed at the month-end rollup. On the **WALLET** rail the debit taken when the booking committed is the whole nominal price, so the overage is collected the moment the booking commits: no `OVERAGE_INVOICE_ACCRUAL` leg is written, `Payment.amount` is left exactly at the wallet debit, and the `OverageEvent` is recorded as `CHARGED` and settled against the payment whose `WALLET` leg collected it. Writing a leg there would have broken the leg-sum invariant above and, worse, incrementing `Payment.amount` on top of it made a later cancellation refund the organisation more than its wallet was ever debited. + +`CHARGE_MEMBER` is **not available on a WALLET-funded billing account**. Charging the member requires carving the over-cap portion back out of the parent payment, which on this rail would mean crediting the wallet mid-transaction — the credit-back that #715 has never built. The combination is refused when a programme is created or patched, and checkout keeps a fail-closed refusal (`OVERAGE_CHARGE_MEMBER_UNSUPPORTED`, HTTP 409) for any programme configured before that guard existed. + +`PROGRAM_CAP_EXHAUSTED` is the contract for the per-cycle overage ceiling. The settlement code throws it as an HTTP 402 with a machine-readable code, the checkout transaction's catch rethrows it unchanged because that code is registered in `BUSINESS_ERROR_CODES`, and the route answers 402 with a toast telling the buyer that the organisation's programme budget for this cycle is used up. It is a modelled outcome, so Sentry records it as expected volume rather than a fault; the two overage funding refusals above are deliberately not modelled, because they mean a programme was configured in a shape no rail can collect on. + --- ## Refund Visibility Across the Seam diff --git a/jobs/refunds/reconcile-pending-refunds.ts b/jobs/refunds/reconcile-pending-refunds.ts index 7b2a88515..6273162f1 100644 --- a/jobs/refunds/reconcile-pending-refunds.ts +++ b/jobs/refunds/reconcile-pending-refunds.ts @@ -31,6 +31,9 @@ function outputToGitHubActions(result: RefundReconciliationResult): void { `reconciled_count=${result.reconciledCount}`, `failed_count=${result.failedCount}`, `skipped_count=${result.skippedCount}`, + // #1458 — surfaced as its own output so a workflow can alert on refunds + // stranded behind a gateway fence without parsing the log. + `skipped_fenced=${result.skippedFenced}`, `success=${result.success}`, ].join("\n"); diff --git a/lib/enterprise/reachable-paths.ts b/lib/enterprise/reachable-paths.ts index 54b45c77d..7ae0075ac 100644 --- a/lib/enterprise/reachable-paths.ts +++ b/lib/enterprise/reachable-paths.ts @@ -12,7 +12,11 @@ * for that capability. */ -import type { FundingSource, ProgramType } from "@prisma/client"; +import type { + FundingSource, + OverageBehavior, + ProgramType, +} from "@prisma/client"; export type ReachableCapability = | "PERSONAL_TAG" // not a real Program — tag-only attribution @@ -60,6 +64,37 @@ export function isReachableOrgFundingPath( ); } +/** + * The reason a programme's overage behaviour cannot be honoured on a given + * funding source, or `null` when the combination is supported. + * + * #1458 — the funding matrix above says which (capability, funding, programme + * type) shapes exist; it says nothing about what happens once a booking goes + * past the cap, and that gap let a wallet-funded organisation save a programme + * that charges its members. Collecting from a member requires carving the + * over-cap portion back out of the parent payment, which on the wallet rail + * would mean crediting the wallet mid-transaction — the credit-back that #715 + * has never built. Checkout therefore refused the booking at commit, after the + * member had already picked a slot. Refusing the CONFIGURATION instead means + * the state is unreachable rather than merely fatal. + * + * The message is returned rather than thrown so both the create route (a Zod + * refinement) and the patch route (an inline `fail()`) can raise it in their own + * shape without either of them owning the rule. + */ +export function overageBehaviorUnsupportedReason( + fundingSource: FundingSource | null, + overageBehavior: OverageBehavior, +): string | null { + if (fundingSource === "WALLET" && overageBehavior === "CHARGE_MEMBER") { + return ( + "A wallet-funded organisation cannot charge members for bookings past the programme cap, because the wallet debit has already collected the whole booking price and the member credit-back is not implemented (#715). " + + "Choose CHARGE_ORG, which is collected by that same wallet debit, or BLOCK to stop over-cap bookings." + ); + } + return null; +} + /** * Resolve a capability label from the canSponsor / canHost booleans. * SPONSOR-only (canSponsor=true, canHost=false) → "SPONSOR". diff --git a/lib/errors/classification/payment-error-classification.ts b/lib/errors/classification/payment-error-classification.ts index 8ce341980..b9996a4fd 100644 --- a/lib/errors/classification/payment-error-classification.ts +++ b/lib/errors/classification/payment-error-classification.ts @@ -39,6 +39,12 @@ export const ErrorTypes = { // intercepts DomainVerificationRequiredError by instanceof and hardcodes this // string in the response JSON. DOMAIN_VERIFICATION_REQUIRED: "DOMAIN_VERIFICATION_REQUIRED", + // #1458 — the three org-programme refusals checkout can raise from inside its + // transaction. Each is a rejection the buyer or their admin can act on, so + // each carries its own toast rather than sharing one "config" bucket. + PROGRAM_CAP_EXHAUSTED: "PROGRAM_CAP_EXHAUSTED_ERROR", + PROGRAM_SESSION_CAP_REACHED: "PROGRAM_SESSION_CAP_REACHED_ERROR", + OVERAGE_CHARGE_MEMBER_UNSUPPORTED: "OVERAGE_CHARGE_MEMBER_UNSUPPORTED_ERROR", // Infrastructure failures (unexpected — ops/dev needs to investigate) PAYMENT_CONFIG: "PAYMENT_CONFIG_ERROR", @@ -224,8 +230,48 @@ export const BUSINESS_ERROR_CODES: ReadonlyArray<{ errorType: ErrorTypes.DOMAIN_VERIFICATION_REQUIRED, httpStatus: 403, }, + // #1458 — all four are thrown from inside the checkout transaction, where the + // catch used to rewrite anything it did not recognise to "Failed to record + // payment information". With a row here the code survives the rethrow and the + // buyer gets the status and the copy that match the actual refusal. + { + code: "PROGRAM_CAP_EXHAUSTED", + errorType: ErrorTypes.PROGRAM_CAP_EXHAUSTED, + httpStatus: 402, + }, + { + code: "PROGRAM_SESSION_CAP_REACHED", + errorType: ErrorTypes.PROGRAM_SESSION_CAP_REACHED, + httpStatus: 402, + }, + { + code: "OVERAGE_CHARGE_MEMBER_UNSUPPORTED", + errorType: ErrorTypes.OVERAGE_CHARGE_MEMBER_UNSUPPORTED, + httpStatus: 409, + }, + { + code: "OVERAGE_UNSUPPORTED_FUNDING", + errorType: ErrorTypes.UNSUPPORTED_CONFIG, + httpStatus: 409, + }, ] as const; +/** + * True when an error's `code` is one this module already resolves to a status + * and a toast. + * + * #1458 — the checkout transaction's catch has to decide whether an error is + * safe to rethrow unchanged. Asking "is this code registered?" is the same + * question the classifier answers a moment later, so the two can never disagree + * about which refusals reach the buyer intact. + */ +export function isBusinessErrorCode(code: unknown): boolean { + return ( + typeof code === "string" && + BUSINESS_ERROR_CODES.some((entry) => entry.code === code) + ); +} + /** * Read a string `code` off an error without importing the class that set it — * the toast map re-exports this module into client bundles, so it must stay diff --git a/lib/errors/mapping/payment-error-toast-map.ts b/lib/errors/mapping/payment-error-toast-map.ts index d26fbda8f..9af01a2dd 100644 --- a/lib/errors/mapping/payment-error-toast-map.ts +++ b/lib/errors/mapping/payment-error-toast-map.ts @@ -103,6 +103,23 @@ const ERROR_TOAST_MAP: Record = { description: "Invoice funding needs a verified domain on your organisation; ask your billing admin to verify it, or pay by card instead. Your card was not charged.", }, + // #1458 — the programme ran out of budget or was configured with a rail we do + // not collect on. Neither is fixed by retrying, so each toast names the person + // who can actually unblock the booking. + [ErrorTypes.PROGRAM_CAP_EXHAUSTED]: { + title: "Programme Budget Used Up", + description: + "Your organisation's programme budget for this cycle is used up; ask your admin or pay yourself if allowed.", + }, + [ErrorTypes.PROGRAM_SESSION_CAP_REACHED]: { + title: "Programme Session Cap Reached", + description: null, // The server message already names the admin action. + }, + [ErrorTypes.OVERAGE_CHARGE_MEMBER_UNSUPPORTED]: { + title: "Programme Not Bookable Past Its Cap", + description: + "This programme is set to charge members for bookings past its cap, which is not available on a wallet-funded organisation. Ask your billing admin to switch the programme to charge the organisation or to block over-cap bookings.", + }, [ErrorTypes.UNKNOWN]: { title: "Something Went Wrong", description: null, // Use the server's specific message diff --git a/lib/payments/billing/overage-settlement.ts b/lib/payments/billing/overage-settlement.ts index 8bcda240f..b18c46352 100644 --- a/lib/payments/billing/overage-settlement.ts +++ b/lib/payments/billing/overage-settlement.ts @@ -15,6 +15,7 @@ import { } from "@prisma/client"; import { computeOverageForBooking } from "@/lib/payments/billing/overage"; import { notifyOrgProgramOverageDue } from "@/lib/novu/org-workflows"; +import { PaymentError } from "@/lib/payments/core/types"; import type { Tx } from "@/lib/prisma"; import { sumPaise } from "@/lib/payments/utils/money"; @@ -217,8 +218,11 @@ export async function recordOverageAtCheckout( // (marginalPaise) covers the over-cap portion. Without this, basePaise is // collected TWICE — once in the parent's base leg, once in the member charge // (coveredPaise + basePaise == price). INVOICE accrual carves cleanly; a - // WALLET-funded parent would also need a balance credit-back (not reachable - // with current configs — no WALLET program charges overage). + // WALLET-funded parent would also need a balance credit-back, which is not + // built (#715) — so #1458 added a config-time guard + // (overageBehaviorUnsupportedReason) that refuses CHARGE_MEMBER on a WALLET + // account, and the throw below is the fail-closed backstop for a programme + // configured before that guard existed. if (basePaise > 0) { const parentBase = await tx.paymentLeg.findUnique({ where: { paymentId_source: { paymentId, source: "INVOICE_ACCRUAL" } }, @@ -228,19 +232,30 @@ export async function recordOverageAtCheckout( // (e.g. a WALLET/LICENSE-funded parent), basePaise was already collected via // that funding source and the member side-charge above bills it AGAIN. The // credit-back path for non-invoice parents isn't built (#715), so abort the - // tx rather than silently double-collect basePaise. Reachable today because - // isReachableOrgFundingPath doesn't constrain overageBehavior by funding. + // tx rather than silently double-collect basePaise. Only reachable for a + // programme saved before #1458's config guard, which now refuses the + // combination at create and patch time. if (!parentBase || parentBase.amountPaise < basePaise) { - const carveErr = new Error( - `CHARGE_MEMBER overage on payment ${paymentId}: cannot carve basePaise=${basePaise} ` + - `from parent INVOICE_ACCRUAL leg (${parentBase ? parentBase.amountPaise : "absent"}); ` + - `non-invoice-funded member-overage credit-back not implemented (#715) — refusing to double-collect`, + // #1458 — a stable code and a 409 so the route answers the buyer with + // the admin action instead of a generic 500; the operator detail stays + // in the Sentry context below, not in the message the page renders. + const carveErr = new PaymentError( + "This programme charges members for bookings past its cap, which is not supported on this organisation's funding source. Ask your billing admin to switch the programme to charge the organisation, or to block over-cap bookings.", + "OVERAGE_CHARGE_MEMBER_UNSUPPORTED", ); // A genuine coverage gap (#715), not a modelled outcome — this aborts // a booking with real money on the line. reportSentryError(carveErr, { subsystem: "payments", - contexts: { overage: { paymentId, basePaise } }, + contexts: { + overage: { + paymentId, + basePaise, + parentInvoiceAccrualPaise: parentBase + ? parentBase.amountPaise + : null, + }, + }, }); throw carveErr; } @@ -275,13 +290,56 @@ export async function recordOverageAtCheckout( // double-bills the org by basePaise (and breaks Σlegs == amount). Carve // basePaise OUT of the base leg into the explicit OVERAGE_INVOICE_ACCRUAL // leg; only the surcharge is genuinely-additional money (marginal == base + - // surcharge). When no base INVOICE_ACCRUAL leg exists (wallet/license-funded) - // nothing is carved and the overage is fully additive — `marginal − carved` - // yields the right `amount` bump either way. + // surcharge). + // + // #1458 — which funding rail paid the parent decides whether the marginal is + // new money at all, so the WALLET rail is resolved FIRST and never reaches + // the additive branch below. + const walletLeg = await tx.paymentLeg.findUnique({ + where: { paymentId_source: { paymentId, source: "WALLET" } }, + select: { amountPaise: true }, + }); + if (walletLeg) { + return recordWalletCollectedOrgOverage(tx, { + paymentId, + programAssignmentId, + bookingUtilizationId: bu.id, + basePaise, + surchargePaise, + marginalPaise, + currency, + }); + } + const baseLeg = await tx.paymentLeg.findUnique({ where: { paymentId_source: { paymentId, source: "INVOICE_ACCRUAL" } }, select: { amountPaise: true }, }); + if (!baseLeg) { + // #1458 — the old fallback treated "no base leg" as licence-funded and + // made the overage fully additive. That is right for LICENSE (the licence + // leg is 0 and the marginal is the only money on the payment) and wrong + // for anything else, where inflating Payment.amount invents a charge no + // rail ever collected. Prove the LICENSE rail before taking that branch. + const licenseLeg = await tx.paymentLeg.findUnique({ + where: { paymentId_source: { paymentId, source: "LICENSE" } }, + select: { amountPaise: true }, + }); + if (!licenseLeg) { + const fundingErr = new PaymentError( + "This booking is past your programme's cap and the programme's funding source cannot be charged for the difference. Ask your billing admin to review the programme's overage settings.", + "OVERAGE_UNSUPPORTED_FUNDING", + ); + // A coverage gap, not a modelled outcome: an org-sponsored payment is + // supposed to carry exactly one of the WALLET/INVOICE_ACCRUAL/LICENSE + // funding legs, so reaching here means the funding seam itself drifted. + reportSentryError(fundingErr, { + subsystem: "payments", + contexts: { overage: { paymentId, marginalPaise } }, + }); + throw fundingErr; + } + } const carved = baseLeg && baseLeg.amountPaise >= basePaise ? basePaise : 0; if (carved > 0) { await tx.paymentLeg.update({ @@ -326,6 +384,77 @@ export async function recordOverageAtCheckout( return null; } +/** + * Record a CHARGE_ORG overage on a WALLET-funded parent (#1458). + * + * On the wallet rail the debit taken when the booking committed is the whole + * nominal price, so the over-cap pass-through (`basePaise`) is already in the + * platform's hands the moment the transaction commits. There is nothing left to + * bill: the event is born CHARGED and settled, pointing at the payment whose + * WALLET leg collected it. Writing an OVERAGE_INVOICE_ACCRUAL leg here instead + * would break the `Σ non-credit legs == Payment.amount` identity the DB trigger + * enforces, and incrementing `Payment.amount` on top of it made a later + * cancellation refund the organisation more than its wallet was ever debited. + * + * The surcharge is the one part the wallet debit did NOT collect, because it is + * a markup on top of the price rather than a slice of it. No rail collects it + * after the fact without inflating the amount again, so the booking is refused + * rather than quietly under-collected; the config-time guard in + * `lib/enterprise/reachable-paths.ts` is what keeps operators out of this state. + */ +async function recordWalletCollectedOrgOverage( + tx: Tx, + args: { + paymentId: string; + programAssignmentId: string; + bookingUtilizationId: string; + basePaise: number; + surchargePaise: number; + marginalPaise: number; + currency: Currency; + }, +): Promise { + if (args.surchargePaise > 0) { + const surchargeErr = new PaymentError( + "This programme adds a surcharge to bookings past its cap, which a wallet-funded organisation cannot be charged for. Ask your billing admin to remove the overage surcharge or to block over-cap bookings.", + "OVERAGE_UNSUPPORTED_FUNDING", + ); + reportSentryError(surchargeErr, { + subsystem: "payments", + contexts: { + overage: { + paymentId: args.paymentId, + surchargePaise: args.surchargePaise, + }, + }, + }); + throw surchargeErr; + } + + await tx.overageEvent.create({ + data: { + programAssignmentId: args.programAssignmentId, + bookingUtilizationId: args.bookingUtilizationId, + overageBehavior: "CHARGE_ORG", + basePaise: args.basePaise, + surchargePaise: args.surchargePaise, + marginalPaise: args.marginalPaise, + currency: args.currency, + // CHARGED is the enum's "money collected" state and the wallet debit is + // that collection, so the event is settled at birth. It carries no + // invoiceLineItemId because it never reaches an invoice — `paymentId` is + // the proof of collection instead, and the reconciler's (G2) link + // invariant accepts either. + chargeStatus: "CHARGED", + settledAt: new Date(), + paymentId: args.paymentId, + }, + }); + + // Nothing is owed by anyone, so there is no bell to ring. + return null; +} + /** * Ring the member-due bell for a committed overage. Fire-and-forget: a booking * that is already paid for must not fail because a notification did not go out. diff --git a/lib/payments/operations/checkout.ts b/lib/payments/operations/checkout.ts index 4006822eb..466e80968 100644 --- a/lib/payments/operations/checkout.ts +++ b/lib/payments/operations/checkout.ts @@ -108,6 +108,7 @@ import { import { sumPaise } from "@/lib/payments/utils/money"; import { MARKETPLACE_VISIBILITY } from "@/lib/api/plans/visibility"; import { resolveCancellationPolicySnapshot } from "@/lib/payments/operations/cancellation-policy"; +import { isBusinessErrorCode } from "@/lib/errors/classification/payment-error-classification"; // Re-export for backward compatibility export const unifiedCheckoutSchema = checkoutSchema; @@ -3329,8 +3330,15 @@ export async function handleCheckout( // never happens. exhaustedBell.programAssignmentId = programAssignmentId; - throw new Error( - "Your program has hit its session cap for this cycle. Ask your organization admin to upgrade the program or wait for the next cycle.", + // #1458 — a stable code, because the message-preservation + // list below never matched this sentence and the buyer got + // "Failed to record payment information" for a cap they can + // ask an admin to raise. + throw Object.assign( + new Error( + "Your program has hit its session cap for this cycle. Ask your organization admin to upgrade the program or wait for the next cycle.", + ), + { httpStatus: 402, code: "PROGRAM_SESSION_CAP_REACHED" }, ); } throw err; @@ -3748,6 +3756,11 @@ export async function handleCheckout( dbError instanceof WalletFrozenError || dbError instanceof ProgramAssignmentLimitError || dbErrorCode === "PROGRAM_CAP_EXHAUSTED" || + // #1458 — the per-assignment session cap is the same class of modelled + // refusal as the per-cycle overage ceiling above. The overage funding + // codes are deliberately NOT here: they mean a programme was configured + // in a shape we cannot collect on, which has to keep paging. + dbErrorCode === "PROGRAM_SESSION_CAP_REACHED" || (dbError instanceof Error && modelledOutcomePatterns.some((msg) => // Word-bounded: bare `includes` let "full" match "successful" and @@ -3775,6 +3788,16 @@ export async function handleCheckout( throw dbError; } + // #1458 — an error carrying a registered business code already resolves + // to its own status and toast in the classifier, so rewriting it to the + // generic message below is pure loss: PROGRAM_CAP_EXHAUSTED was thrown as + // a 402 with actionable copy and reached the buyer as a 500 + // "Something Went Wrong". Codes are checked before messages because a + // code survives a reworded sentence and a substring does not. + if (dbError instanceof Error && isBusinessErrorCode(dbErrorCode)) { + throw dbError; + } + // Preserve specific error messages (duplicate registration, full capacity, etc.) if (dbError instanceof Error) { const preservedMessages = [ diff --git a/lib/payments/operations/refund.ts b/lib/payments/operations/refund.ts index 8967ecd64..1f51fd196 100644 --- a/lib/payments/operations/refund.ts +++ b/lib/payments/operations/refund.ts @@ -1199,11 +1199,23 @@ export async function applyRefundCascade( bookingUtilizationId: payment.bookingUtilization.id, chargeStatus: "CHARGED", }, - select: { id: true, overageBehavior: true, paymentId: true }, + select: { + id: true, + overageBehavior: true, + paymentId: true, + invoiceLineItemId: true, + }, }); + // #1458 — the wallet rail collects a CHARGE_ORG marginal inside this very + // payment's WALLET debit, so the leg reversal above has already credited it + // back. There is no invoice behind it and therefore no credit note to wait + // for; gating on one left the event permanently CHARGED, still eating the + // programme's per-cycle overage ceiling after the booking was refunded. + const walletCollected = + charged?.paymentId === payment.id && charged?.invoiceLineItemId === null; if ( charged?.overageBehavior === "CHARGE_ORG" && - refundCreditNote.creditNoteId + (refundCreditNote.creditNoteId || walletCollected) ) { await transitionOverage( tx, diff --git a/prisma/schema.prisma b/prisma/schema.prisma index 7e65837f1..6d0dc5fb5 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -6855,7 +6855,9 @@ model OverageEvent { currency Currency @default(INR) chargeStatus OverageChargeStatus @default(PENDING) /// CHARGE_MEMBER: Payment row for the instant overage charge. - /// CHARGE_ORG: Payment row of the rolled-up cycle invoice. + /// CHARGE_ORG: Payment row of the rolled-up cycle invoice, or — on the WALLET + /// rail (#1458) — the booking Payment whose WALLET leg already collected the + /// marginal, in which case the event is born CHARGED with no invoiceLineItemId. paymentId String? // #781 §B — SetNull kept deliberately: the abandoned-side-charge sweep // deletes never-captured PENDING payments; the event row (and its diff --git a/scripts/reconcile/reconcile-ledgers.ts b/scripts/reconcile/reconcile-ledgers.ts index 9b207ed81..9acd631b4 100644 --- a/scripts/reconcile/reconcile-ledgers.ts +++ b/scripts/reconcile/reconcile-ledgers.ts @@ -446,6 +446,11 @@ async function runReconcileLedgersUnlocked( overageBehavior: "CHARGE_ORG", chargeStatus: { in: ["ACCRUED", "CHARGED"] }, invoiceLineItemId: null, + // #1458 — a wallet-funded CHARGE_ORG overage is collected by the + // booking's own wallet debit and never reaches an invoice, so it is + // born CHARGED with a paymentId and no line item. Either link is + // proof of collection; neither is the drift this check hunts. + paymentId: null, }, { chargeStatus: "CHARGED", settledAt: null }, ], @@ -485,7 +490,7 @@ async function runReconcileLedgersUnlocked( paymentId: ev.paymentId, invoiceLineItemId: ev.invoiceLineItemId, settledAt: ev.settledAt, - note: "OverageEvent link/state invariant violated: CHARGE_MEMBER pending/failed/charged without a side-Payment, CHARGE_ORG accrued/charged without an InvoiceLineItem, or CHARGED without settledAt. Trace the transitionOverage() path that produced this state.", + note: "OverageEvent link/state invariant violated: CHARGE_MEMBER pending/failed/charged without a side-Payment, CHARGE_ORG accrued/charged with neither an InvoiceLineItem nor the wallet-funded booking Payment that collected it (#1458), or CHARGED without settledAt. Trace the transitionOverage() path that produced this state.", }, }); } diff --git a/scripts/refunds/reconcile-pending-refunds.ts b/scripts/refunds/reconcile-pending-refunds.ts index c4e261453..eb221d8bd 100644 --- a/scripts/refunds/reconcile-pending-refunds.ts +++ b/scripts/refunds/reconcile-pending-refunds.ts @@ -34,6 +34,13 @@ export interface RefundReconciliationResult { reconciledCount: number; failedCount: number; skippedCount: number; + /** + * #1458 — the subset of `skippedCount` that was left alone because its + * gateway is fenced off for this deployment. Reported separately so an + * operator can tell "nothing to do" from "there is settled money we are not + * polling because STRIPE_ENABLED is off". + */ + skippedFenced: number; errors: string[]; timestamp: string; } @@ -90,8 +97,22 @@ async function reconcilePendingRefundsUnlocked( let reconciledCount = 0; let failedCount = 0; let skippedCount = 0; + let skippedFenced = 0; let totalProcessed = 0; + /** + * #1458 — a PENDING refund on a gateway this deployment has fenced off is not + * reconcilable: the gateway client is never constructed, so `listRefunds` / + * `getRefund` throw, every fenced row lands in `errors`, and the whole run + * reports `success: false` — a 500 from the cleanup route for a condition + * that is deliberate configuration. `assertGatewayUsable` cannot be reused + * here because it deliberately leaves refund LOOKUPS open, so that a Payment + * already written against Stripe stays refundable after the fence goes up. + * Skip the row, count it, and let the summary say so. + */ + const isFencedGateway = (gateway: PaymentGateway): boolean => + gateway === PaymentGateway.STRIPE && process.env.STRIPE_ENABLED !== "true"; + // ------------------------------------------------------------------ // Pass 1 — placeholders // ------------------------------------------------------------------ @@ -132,6 +153,14 @@ async function reconcilePendingRefundsUnlocked( skippedCount++; continue; } + if (isFencedGateway(refund.payment.paymentGateway)) { + console.log( + `⏭️ Skipping refund ${refund.id} - ${refund.payment.paymentGateway} is fenced off for this deployment`, + ); + skippedCount++; + skippedFenced++; + continue; + } // Query gateway for actual refunds on this payment const gatewayRefunds = await listRefunds( @@ -270,6 +299,11 @@ async function reconcilePendingRefundsUnlocked( skippedCount++; continue; } + if (isFencedGateway(refund.payment.paymentGateway)) { + skippedCount++; + skippedFenced++; + continue; + } const gatewayRefund = await getRefund( refund.refundId, @@ -317,6 +351,7 @@ async function reconcilePendingRefundsUnlocked( reconciledCount, failedCount, skippedCount, + skippedFenced, errors, timestamp: new Date().toISOString(), }; From f0b2552176a3900f204a4a553928886ee96b5ea9 Mon Sep 17 00:00:00 2001 From: Kaustav Ghosh <44238657+teetangh@users.noreply.github.com> Date: Sat, 5 Sep 2026 07:53:43 +0530 Subject: [PATCH 2/3] fix(payments): a CHARGE_ORG overage surcharge is credited to platform revenue, and the licence rail refuses an overage it can never collect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sentry FAMILIARISE_WEB-28 fired on the #1458 payment: the BOOKING posting is Dr funding legs + a DISCOUNT plug clamped at >= 0 against Cr legs all derived from Payment.originalAmount + taxAmount, so it balances only while the funding legs sum to no more than the nominal gross. The inflated wallet payment overshot by the marginal, threw LedgerImbalanceError and the booking committed with no journal entry — which is what let the inflated refund through. Removing the extra leg fixes the wallet rail by construction. The invoice rail was unbalanced by exactly surchargePaise for the same reason: the carve keeps basePaise inside the price, but marginal = base + surcharge raises the accrual leg and Payment.amount by money that sits outside originalAmount. That surcharge is a markup the platform charges the org for exceeding its own cap, not consultant income, so the posting credits it to PLATFORM_FEE — no new ledger account and no change to what Payment.amount means. The licence rail cannot be balanced at all: a licence leg is deliberately zero while Payment.amount stays at full price, and the leg-sum guard excuses that only while the licence leg is the payment's only funding leg, so an overage leg re-armed the comparison and assert_payment_legs_ok raised at COMMIT. It is now refused at programme-config time and fails closed at checkout with a business error instead of an opaque database violation. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01MgadJydWEKkdhhzY58yiL7 --- .../overage-settlement-legsum.test.ts | 32 ++++++- __tests__/enterprise/reachable-paths.test.ts | 12 +++ .../multi-party-booking-journal.test.ts | 95 +++++++++++++++++++ .../05-booking-to-earnings.md | 3 + lib/enterprise/reachable-paths.ts | 15 +++ lib/payments/billing/overage-settlement.ts | 43 ++++----- lib/payments/payouts/earnings-service.ts | 39 ++++++++ 7 files changed, 215 insertions(+), 24 deletions(-) diff --git a/__tests__/enterprise/overage-settlement-legsum.test.ts b/__tests__/enterprise/overage-settlement-legsum.test.ts index 596e80b29..a371458fb 100644 --- a/__tests__/enterprise/overage-settlement-legsum.test.ts +++ b/__tests__/enterprise/overage-settlement-legsum.test.ts @@ -38,10 +38,14 @@ function makeTx(opts: { priceCap?: number | null; overageBehavior?: "CHARGE_ORG" | "CHARGE_MEMBER"; /** #1458 — which funding rail wrote the parent's base leg. */ - baseSource?: "INVOICE_ACCRUAL" | "WALLET"; + baseSource?: "INVOICE_ACCRUAL" | "WALLET" | "LICENSE"; }) { const legs: Leg[] = [ - { source: opts.baseSource ?? "INVOICE_ACCRUAL", amountPaise: opts.price }, + { + source: opts.baseSource ?? "INVOICE_ACCRUAL", + // A licence leg is deliberately zero-value: the contract already paid. + amountPaise: opts.baseSource === "LICENSE" ? 0 : opts.price, + }, ]; const payment = { amount: opts.price }; const children: { amount: number }[] = []; @@ -212,6 +216,30 @@ describe("recordOverageAtCheckout — CHARGE_ORG on the WALLET rail (#1458)", () // a 100% refund returns exactly the debit and not a paisa more. expect(state.payment.amount).toBe(walletDebit); }); + + // A licence is a flat fee settled at contract time, so its leg is ₹0 while + // Payment.amount stays at the full price. Adding an overage leg re-arms the + // leg-sum comparison the licence carve had suppressed, and the booking used + // to die at COMMIT on assert_payment_legs_ok instead of saying why. + it("LICENSE + CHARGE_ORG is refused rather than made additive", async () => { + const { state, tx } = makeTx({ + price: 100_000, + cap: 5, + used: 5, + baseSource: "LICENSE", + }); + + await expect( + recordOverageAtCheckout({ + tx: tx as unknown as Tx, + ...callArgs(100_000), + }), + ).rejects.toMatchObject({ code: "OVERAGE_UNSUPPORTED_FUNDING" }); + + expect(tx.paymentLeg.create).not.toHaveBeenCalled(); + expect(tx.payment.update).not.toHaveBeenCalled(); + expect(state.legs).toEqual([{ source: "LICENSE", amountPaise: 0 }]); + }); }); describe("recordOverageAtCheckout — CHARGE_MEMBER parent carve (#785)", () => { diff --git a/__tests__/enterprise/reachable-paths.test.ts b/__tests__/enterprise/reachable-paths.test.ts index bc2a9a1f5..c73845daf 100644 --- a/__tests__/enterprise/reachable-paths.test.ts +++ b/__tests__/enterprise/reachable-paths.test.ts @@ -83,6 +83,18 @@ describe("REACHABLE_ORG_FUNDING_PATHS — v0 lockdown matrix", () => { expect(reason).toContain("#715"); }); + it("refuses either charging behaviour on a LICENSE-funded account", () => { + // A flat licence moves no money per booking, so nothing carries the + // marginal and the leg-sum guard rejects the extra leg at COMMIT. + expect( + overageBehaviorUnsupportedReason("LICENSE", "CHARGE_ORG"), + ).toContain("licence"); + expect( + overageBehaviorUnsupportedReason("LICENSE", "CHARGE_MEMBER"), + ).toContain("licence"); + expect(overageBehaviorUnsupportedReason("LICENSE", "BLOCK")).toBeNull(); + }); + it("allows CHARGE_ORG and BLOCK on WALLET, and CHARGE_MEMBER on INVOICE", () => { expect( overageBehaviorUnsupportedReason("WALLET", "CHARGE_ORG"), diff --git a/__tests__/payments/multi-party-booking-journal.test.ts b/__tests__/payments/multi-party-booking-journal.test.ts index 237ec2433..cb9465cf6 100644 --- a/__tests__/payments/multi-party-booking-journal.test.ts +++ b/__tests__/payments/multi-party-booking-journal.test.ts @@ -97,6 +97,8 @@ jest.mock("../../lib/prisma", () => { }, ledgerAccountBalance: { upsert: jest.fn().mockResolvedValue({}) }, paymentLeg: { findMany: jest.fn().mockResolvedValue([]) }, + // #1458 — only read when an OVERAGE_INVOICE_ACCRUAL leg funded the payment. + overageEvent: { findFirst: jest.fn().mockResolvedValue(null) }, consultantEarnings: { findFirst: jest.fn().mockResolvedValue(null), create: jest @@ -158,6 +160,8 @@ const mockedTx = ( membership: { findFirst: jest.Mock }; consultantEarnings: { findFirst: jest.Mock; create: jest.Mock }; ledgerTransaction: { findUnique: jest.Mock; create: jest.Mock }; + paymentLeg: { findMany: jest.Mock }; + overageEvent: { findFirst: jest.Mock }; }; } ).__mockTx; @@ -247,6 +251,8 @@ beforeEach(() => { capturedOrgEarnings = []; mockedTx.consultantEarnings.findFirst.mockResolvedValue(null); mockedTx.ledgerTransaction.findUnique.mockResolvedValue(null); + mockedTx.paymentLeg.findMany.mockResolvedValue([]); + mockedTx.overageEvent.findFirst.mockResolvedValue(null); setStandardRateCard(); }); @@ -459,3 +465,92 @@ describe("#773 multi-party booking journal", () => { expect(mockedTx.ledgerTransaction.create).not.toHaveBeenCalled(); }); }); + +/** + * #1458 / Sentry FAMILIARISE_WEB-28 — the BOOKING posting's credits are all + * derived from `payment.originalAmount` (+ tax), while its debits are the + * funding legs plus a DISCOUNT plug clamped at >= 0. The posting therefore + * balances only while Σ(funding legs) <= originalAmount + tax; anything that + * pushes a funding leg above the nominal price throws LedgerImbalanceError and + * the booking commits with no journal entry at all. + */ +describe("#1458 org-overage rails keep the booking journal balanced", () => { + it("WALLET + CHARGE_ORG: the wallet leg alone funds the price and the posting balances", async () => { + // The exact #1458 payment: a 258,326-paise wallet debit on a 218,920 + + // 39,406 booking. Before the fix an OVERAGE_INVOICE_ACCRUAL leg of 248,326 + // was added and Payment.amount became 506,652, so debits overshot the + // credits by the marginal and the journal was dropped. + setMembershipMap({ [PRIMARY_PROFILE]: null }); + mockedCalculateSplit.mockResolvedValue([]); + mockedTx.paymentLeg.findMany.mockResolvedValue([ + { source: "WALLET", amountPaise: 258_326 }, + ]); + + await createEarningsFromPayment({ + payment: makePayment({ + amount: 258_326, + originalAmount: 218_920, + taxAmount: 39_406, + }), + appointmentType: "CONSULTATION", + }); + + const txn = capturedLedgerTxns[0]; + const legs = legsOf(txn); + const debit = legs + .filter((l) => l.direction === "DEBIT") + .reduce((s, l) => s + l.paise, 0); + const credit = legs + .filter((l) => l.direction === "CREDIT") + .reduce((s, l) => s + l.paise, 0); + expect(debit).toBe(258_326); + expect(credit).toBe(258_326); + // No leg was added and no amount bumped, so the wallet debit is the whole + // funding side and the platform absorbs nothing as DISCOUNT. + expect(legAmount(txn, "DEBIT", "WALLET|_|_|INR")).toBe(258_326); + expect(legAmount(txn, "DEBIT", "DISCOUNT|_|_|INR")).toBe(0); + expect(mockedTx.overageEvent.findFirst).not.toHaveBeenCalled(); + }); + + it("INVOICE + CHARGE_ORG with a surcharge: the surcharge is credited to PLATFORM_FEE", async () => { + // #785's carve leaves INVOICE_ACCRUAL at 0 and OVERAGE_INVOICE_ACCRUAL at + // base + surcharge, and bumps Payment.amount by the surcharge — which is + // real funding that sits OUTSIDE originalAmount. The surcharge is platform + // revenue for exceeding the cap, so it credits PLATFORM_FEE. + setMembershipMap({ [PRIMARY_PROFILE]: null }); + mockedCalculateSplit.mockResolvedValue([]); + mockedTx.paymentLeg.findMany.mockResolvedValue([ + { source: "INVOICE_ACCRUAL", amountPaise: 0 }, + { source: "OVERAGE_INVOICE_ACCRUAL", amountPaise: 125_000 }, + ]); + mockedTx.overageEvent.findFirst.mockResolvedValue({ + surchargePaise: BigInt(25_000), + }); + + await createEarningsFromPayment({ + payment: makePayment({ + amount: 125_000, + originalAmount: 100_000, + taxAmount: 0, + }), + appointmentType: "CONSULTATION", + }); + + const txn = capturedLedgerTxns[0]; + const legs = legsOf(txn); + const debit = legs + .filter((l) => l.direction === "DEBIT") + .reduce((s, l) => s + l.paise, 0); + const credit = legs + .filter((l) => l.direction === "CREDIT") + .reduce((s, l) => s + l.paise, 0); + expect(debit).toBe(125_000); + expect(credit).toBe(125_000); + // 20% of the nominal 100_000 plus the whole 25_000 surcharge; the + // consultant pool stays on the nominal price alone. + expect(legAmount(txn, "CREDIT", "PLATFORM_FEE|_|_|INR")).toBe(45_000); + expect( + legAmount(txn, "CREDIT", `CONSULTANT_PAYABLE|_|${PRIMARY_PROFILE}|INR`), + ).toBe(80_000); + }); +}); diff --git a/docs/enterprise/10-money-and-ledger/05-booking-to-earnings.md b/docs/enterprise/10-money-and-ledger/05-booking-to-earnings.md index f82577717..c04686692 100644 --- a/docs/enterprise/10-money-and-ledger/05-booking-to-earnings.md +++ b/docs/enterprise/10-money-and-ledger/05-booking-to-earnings.md @@ -205,12 +205,15 @@ On a real over-cap checkout, `recordOverageAtCheckout` (`lib/payments/billing/ov - **`CHARGE_MEMBER`** → a parent-linked **PENDING side-`Payment`** for the marginal (gateway _not_ called inside the tx; the order is minted lazily when the member opens the resume-checkout surface) + an `OverageEvent(PENDING)`. To avoid double-collecting `basePaise`, checkout carves it out of the org-funded parent's `INVOICE_ACCRUAL` leg (fail-closed: a non-invoice-funded parent has no credit-back path yet, #715, so it aborts rather than double-charge). Member is notified (`notifyOrgProgramOverageDue`) with a pay deep link. The webhook later posts the `OVERAGE_MEMBER` org-relief leg ([§4.8 of ledger & postings](03-ledger-and-postings.md)). - **`CHARGE_ORG` on the INVOICE rail** → carve `basePaise` out of the base `INVOICE_ACCRUAL` leg and write the marginal as a distinct **`OVERAGE_INVOICE_ACCRUAL`** leg (the distinct source dodges the `@@unique([paymentId, source])` clash) + an `OverageEvent(PENDING)`. The cycle-close rollup turns it into an `InvoiceLineItem` and walks the event `PENDING → ACCRUED → CHARGED` ([invoicing](08-invoicing.md)). - **`CHARGE_ORG` on the WALLET rail (#1458)** → nothing is billed, because the wallet debit taken when the booking committed is the whole nominal price and therefore already contains the over-cap pass-through. The recorder writes **no** leg and does **not** touch `Payment.amount`; it records an `OverageEvent` that is born `CHARGED` with `settledAt` stamped and `paymentId` pointing at the booking payment whose `WALLET` leg collected it. That event carries no `invoiceLineItemId`, so the reconciler's link invariant accepts either link as proof of collection. Anything else on this rail fails closed with a business error rather than inflating the payment: a positive `overageSurchargeBps` is a markup the wallet debit never took, and an org-sponsored payment carrying none of the `WALLET` / `INVOICE_ACCRUAL` / `LICENSE` funding legs means the funding seam itself has drifted. +- **`CHARGE_ORG` on the LICENSE rail is refused (#1458).** A licence is a flat fee settled at contract time, so a licence-funded booking moves no money per booking: its funding leg is deliberately ₹0 while `Payment.amount` stays at the full price, and the leg-sum guard excuses that only while the licence leg is the payment's _only_ funding leg. Adding an overage leg re-arms the comparison, so `assert_payment_legs_ok` raised at COMMIT and the booking died with an opaque database error. There is no per-booking rail to collect the marginal on, so `overageBehaviorUnsupportedReason` refuses any charging behaviour on a licence-funded account and checkout keeps the fail-closed backstop. - **`CHARGE_MEMBER` is not available on a WALLET account (#715, guarded in #1458).** Collecting from the member requires carving the over-cap portion back out of the parent, which on the wallet rail would mean crediting the wallet mid-transaction — a path that has never been built. `overageBehaviorUnsupportedReason` (`lib/enterprise/reachable-paths.ts`) refuses the combination when the programme is created or patched, so an operator cannot save a configuration whose only outcome is a refused booking. Checkout keeps its fail-closed throw for programmes configured before that guard existed, now carrying the code `OVERAGE_CHARGE_MEMBER_UNSUPPORTED` and an HTTP 409. The `chargeStatus` state machine itself is a single guarded transition (`transitionOverage`, `overage-transitions.ts`); the overage-event lifecycle table of states (`PENDING/ACCRUED/CHARGED/BLOCKED/REVERSED/FAILED`) is in [funding & programs](../00-foundations/03-funding-and-programs.md) / [programs](../30-programs-and-lifecycle/02-programs.md). Because a wallet-funded overage never adds to `Payment.amount`, a cancellation of such a booking refunds exactly what the wallet was debited. The refund cascade splits the refund across the payment's legs, and the single `WALLET` leg equals `Payment.amount`, so a full refund credits the wallet back to the balance it held before the booking. The same cascade reverses the `CHARGED` event, because the money it represented has just been returned and the programme's per-cycle ceiling has to be released with it. +An overage also has to keep the booking journal balanced, and that is a tighter constraint than the leg-sum identity. Every credit in the BOOKING posting is derived from `Payment.originalAmount` plus `taxAmount` — the nominal price — while the debits are the funding legs plus a `DISCOUNT` plug clamped at zero or above. The posting therefore balances only while the funding legs sum to no more than the nominal gross. On the wallet rail that now holds by construction. On the invoice rail it does not: the base carve keeps `basePaise` inside the price, but `marginal = base + surcharge` raises both the accrual leg and `Payment.amount` by the surcharge, which is real funding sitting outside the nominal price. The posting therefore credits that surcharge to `PLATFORM_FEE`, because an over-cap surcharge is a markup the platform charges the organisation for exceeding its own cap and not consultant income — the consultant is paid out of `originalAmount`. Without that credit the posting was short by exactly `surchargePaise`, threw `LedgerImbalanceError`, and the booking committed with no journal entry at all (Sentry `FAMILIARISE_WEB-28`). + The refusals checkout can raise from inside its transaction all carry a machine-readable code, and the catch around that transaction rethrows any error whose code is registered in `BUSINESS_ERROR_CODES` instead of rewriting it. `PROGRAM_CAP_EXHAUSTED` therefore reaches the buyer as the HTTP 402 it was thrown as, with a toast that names the admin action, rather than as the 500 "Something Went Wrong" it used to collapse into. --- diff --git a/lib/enterprise/reachable-paths.ts b/lib/enterprise/reachable-paths.ts index 7ae0075ac..86b7d6d9d 100644 --- a/lib/enterprise/reachable-paths.ts +++ b/lib/enterprise/reachable-paths.ts @@ -92,6 +92,21 @@ export function overageBehaviorUnsupportedReason( "Choose CHARGE_ORG, which is collected by that same wallet debit, or BLOCK to stop over-cap bookings." ); } + // A licence is a flat fee settled at contract time, so a licence-funded + // booking collects nothing per booking: its funding leg is deliberately ₹0 + // while `Payment.amount` stays at the full price, and the leg-sum guard + // excuses that only while the licence leg is the payment's ONLY funding leg. + // Charging an overage adds a second leg, which re-arms the comparison and + // makes `assert_payment_legs_ok` raise at COMMIT — so every over-cap booking + // under such a programme died with an opaque database error. There is no + // per-booking rail to collect the marginal on, so the configuration itself is + // refused. + if (fundingSource === "LICENSE" && overageBehavior !== "BLOCK") { + return ( + "A licence-funded programme cannot charge for bookings past its cap, because a licence is a flat fee settled at contract time and no money moves per booking to carry the overage. " + + "Choose BLOCK to stop over-cap bookings, or fund the programme from the organisation's wallet or invoice account." + ); + } return null; } diff --git a/lib/payments/billing/overage-settlement.ts b/lib/payments/billing/overage-settlement.ts index b18c46352..add24f0ce 100644 --- a/lib/payments/billing/overage-settlement.ts +++ b/lib/payments/billing/overage-settlement.ts @@ -317,30 +317,29 @@ export async function recordOverageAtCheckout( }); if (!baseLeg) { // #1458 — the old fallback treated "no base leg" as licence-funded and - // made the overage fully additive. That is right for LICENSE (the licence - // leg is 0 and the marginal is the only money on the payment) and wrong - // for anything else, where inflating Payment.amount invents a charge no - // rail ever collected. Prove the LICENSE rail before taking that branch. - const licenseLeg = await tx.paymentLeg.findUnique({ - where: { paymentId_source: { paymentId, source: "LICENSE" } }, - select: { amountPaise: true }, + // made the overage fully additive, which cannot work on either remaining + // rail. A LICENSE parent keeps `Payment.amount` at the full price behind a + // deliberately ₹0 licence leg, and the leg-sum guard only excuses that + // while the licence leg is the ONLY funding leg: adding an + // OVERAGE_INVOICE_ACCRUAL leg re-arms the comparison and + // `assert_payment_legs_ok` then raises at COMMIT, so the booking already + // died with an opaque Postgres check_violation. A parent with no funding + // leg at all means the funding seam itself drifted. Neither is fixable by + // inflating the amount, so both refuse the booking with an error the buyer + // can take to their admin. + const fundingErr = new PaymentError( + "This booking is past your programme's cap and the programme's funding source cannot be charged for the difference. Ask your billing admin to switch the programme to block over-cap bookings, or to fund it from the organisation's wallet or invoice account.", + "OVERAGE_UNSUPPORTED_FUNDING", + ); + // A coverage gap, not a modelled outcome: it means an operator saved a + // programme whose overage can never be collected. + reportSentryError(fundingErr, { + subsystem: "payments", + contexts: { overage: { paymentId, marginalPaise } }, }); - if (!licenseLeg) { - const fundingErr = new PaymentError( - "This booking is past your programme's cap and the programme's funding source cannot be charged for the difference. Ask your billing admin to review the programme's overage settings.", - "OVERAGE_UNSUPPORTED_FUNDING", - ); - // A coverage gap, not a modelled outcome: an org-sponsored payment is - // supposed to carry exactly one of the WALLET/INVOICE_ACCRUAL/LICENSE - // funding legs, so reaching here means the funding seam itself drifted. - reportSentryError(fundingErr, { - subsystem: "payments", - contexts: { overage: { paymentId, marginalPaise } }, - }); - throw fundingErr; - } + throw fundingErr; } - const carved = baseLeg && baseLeg.amountPaise >= basePaise ? basePaise : 0; + const carved = baseLeg.amountPaise >= basePaise ? basePaise : 0; if (carved > 0) { await tx.paymentLeg.update({ where: { paymentId_source: { paymentId, source: "INVOICE_ACCRUAL" } }, diff --git a/lib/payments/payouts/earnings-service.ts b/lib/payments/payouts/earnings-service.ts index 2d91d277c..f34e09447 100644 --- a/lib/payments/payouts/earnings-service.ts +++ b/lib/payments/payouts/earnings-service.ts @@ -868,6 +868,10 @@ export async function createEarningsFromPayment({ select: { source: true, amountPaise: true }, }); const orgId = payment.organizationId ?? null; + // #1458 — tracked separately from `receivable` so the credit side + // can ask "was a CHARGE_ORG overage funded through this payment?" + // without a second leg query. See the surcharge credit below. + let overageAccrualPaise = 0; const debits: Posting[] = []; const pushDebit = (account: AccountRef, amountPaise: number) => { if (amountPaise > 0) @@ -888,8 +892,11 @@ export async function createEarningsFromPayment({ wallet += leg.amountPaise; break; case "INVOICE_ACCRUAL": + receivable += leg.amountPaise; + break; case "OVERAGE_INVOICE_ACCRUAL": receivable += leg.amountPaise; + overageAccrualPaise += leg.amountPaise; break; case "REFERRAL_CREDIT": promo += leg.amountPaise; @@ -965,6 +972,38 @@ export async function createEarningsFromPayment({ for (const s of Array.from(collabSettlements.values())) { platformFeeCreditPaise += s.orgSplit.platformFeePaise; } + // #1458 (Sentry FAMILIARISE_WEB-28) — every credit above is + // derived from `payment.originalAmount`, the nominal price, while + // the debits are the funding legs. A CHARGE_ORG overage surcharge + // is the one funding amount that is NOT inside the nominal price: + // the base carve keeps `basePaise` in, but `marginal = base + + // surcharge` bumps both the accrual leg and `Payment.amount` by + // the surcharge. Without this credit the posting was short by + // exactly `surchargePaise`, threw LedgerImbalanceError, and the + // booking committed with no journal entry at all. + // + // PLATFORM_FEE is the right account and no new one is needed: an + // over-cap surcharge is a markup the platform charges the org for + // exceeding its own cap, not consultant income — the consultant is + // paid out of `originalAmount`, which the surcharge sits outside + // of. (The surcharge is booked gross of GST; `Payment.taxAmount` + // is computed on the nominal price and is not re-derived for an + // overage, which is the same limitation the invoice rollup has.) + // + // Only read when an OVERAGE_INVOICE_ACCRUAL leg actually funded + // this payment: on the wallet rail the marginal is inside the + // wallet debit and no such leg exists, and a CHARGE_MEMBER + // surcharge rides the member's side-payment, not this journal. + if (overageAccrualPaise > 0) { + const orgOverage = await tx.overageEvent.findFirst({ + where: { + bookingUtilization: { paymentId: payment.id }, + overageBehavior: "CHARGE_ORG", + }, + select: { surchargePaise: true }, + }); + platformFeeCreditPaise += sumPaise(orgOverage?.surchargePaise); + } pushCredit({ kind: "PLATFORM_FEE" }, platformFeeCreditPaise); if (splits.length > 0) { // Multi-party: one payable per party, mirroring the From 7ed768310aa98c618ec671ceb62c26ae2d383b96 Mon Sep 17 00:00:00 2001 From: Kaustav Ghosh <44238657+teetangh@users.noreply.github.com> Date: Sat, 5 Sep 2026 08:52:27 +0530 Subject: [PATCH 3/3] fix(payments): a wallet overage surcharge is refused at configuration time, the ledger reconciler keeps invoices mandatory for accruals, and two org-sponsorship refusals answer with their own status MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review triage on #1460 plus the #1467 fold-in. `overageBehaviorUnsupportedReason` now takes `overageSurchargeBps`, because the surcharge rather than the behaviour is what decides collectability on the wallet rail: the plain over-cap marginal is a slice of the price the wallet debit already took, while a markup on top of that price is money no rail collects afterwards. `recordWalletCollectedOrgOverage` already refuses it, but only at checkout, after the member has picked a slot — so both the create route and the merged-config patch route now refuse the configuration instead, which is the guard the settlement module's own docstring claims exists. The ledger reconciler's (G2) link check had one predicate covering ACCRUED and CHARGED, so the payment-link exception added for wallet-collected overages also suppressed findings for ACCRUED events. ACCRUED means "billed on an issued invoice" and only the rollup produces it, always stamping the line item, so that branch keeps `invoiceLineItemId` mandatory; the CHARGED branch accepts a payment link only when the payment behind it actually carries the WALLET leg that did the collecting. Closes #1467: the no-active-assignment refusal and the dunning-suspend gate both threw bare Errors, so `classifyError` fell through to UNKNOWN_ERROR and answered 500. A member whose organisation's contract had merely lapsed could not tell the refusal from a crash, and every one of them opened a Sentry incident. Both now carry a stable code — `PROGRAM_ASSIGNMENT_INACTIVE` (409) and `BILLING_SUSPENDED_DUNNING` (402), the latter on the in-lock re-check of the same gate too — registered in `BUSINESS_ERROR_CODES` with toasts that name the admin who can unblock the booking. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01MgadJydWEKkdhhzY58yiL7 --- .../overage-settlement-legsum.test.ts | 8 ++-- __tests__/enterprise/reachable-paths.test.ts | 16 ++++++++ .../gateway-fence-classification.test.ts | 19 +++++++++ .../[orgId]/programs/[programId]/route.ts | 3 ++ .../organizations/[orgId]/programs/route.ts | 12 ++++-- lib/enterprise/reachable-paths.ts | 23 +++++++++++ .../payment-error-classification.ts | 19 +++++++++ lib/errors/mapping/payment-error-toast-map.ts | 12 ++++++ lib/payments/operations/checkout.ts | 39 +++++++++++++++---- scripts/reconcile/reconcile-ledgers.ts | 27 +++++++++---- 10 files changed, 155 insertions(+), 23 deletions(-) diff --git a/__tests__/enterprise/overage-settlement-legsum.test.ts b/__tests__/enterprise/overage-settlement-legsum.test.ts index a371458fb..3752beb17 100644 --- a/__tests__/enterprise/overage-settlement-legsum.test.ts +++ b/__tests__/enterprise/overage-settlement-legsum.test.ts @@ -196,6 +196,9 @@ describe("recordOverageAtCheckout — CHARGE_ORG on the WALLET rail (#1458)", () { source: "WALLET", amountPaise: walletDebit }, ]); expect(state.payment.amount).toBe(walletDebit); + // The cancellation quote is a percentage of Payment.amount and the refund + // cascade splits it across the legs, so one WALLET leg equal to amount is + // what makes a 100% refund return exactly the debit and not a paisa more. expect(sum(state.legs)).toBe(state.payment.amount); expect(tx.paymentLeg.create).not.toHaveBeenCalled(); expect(tx.payment.update).not.toHaveBeenCalled(); @@ -210,11 +213,6 @@ describe("recordOverageAtCheckout — CHARGE_ORG on the WALLET rail (#1458)", () }), }), ); - - // The cancellation quote is a percentage of Payment.amount and the refund - // cascade splits it across the legs — with one WALLET leg equal to amount, - // a 100% refund returns exactly the debit and not a paisa more. - expect(state.payment.amount).toBe(walletDebit); }); // A licence is a flat fee settled at contract time, so its leg is ₹0 while diff --git a/__tests__/enterprise/reachable-paths.test.ts b/__tests__/enterprise/reachable-paths.test.ts index c73845daf..bb98ae3a1 100644 --- a/__tests__/enterprise/reachable-paths.test.ts +++ b/__tests__/enterprise/reachable-paths.test.ts @@ -104,6 +104,22 @@ describe("REACHABLE_ORG_FUNDING_PATHS — v0 lockdown matrix", () => { overageBehaviorUnsupportedReason("INVOICE", "CHARGE_MEMBER"), ).toBeNull(); }); + + // A wallet debit collects the booking price, so the plain over-cap amount + // rides along inside it; a surcharge sits on top of that price and nothing + // collects it. Checkout refuses it either way, so the configuration must. + it("refuses a surcharged CHARGE_ORG on WALLET but not the plain one", () => { + expect( + overageBehaviorUnsupportedReason("WALLET", "CHARGE_ORG", 1000), + ).toContain("surcharge"); + expect( + overageBehaviorUnsupportedReason("WALLET", "CHARGE_ORG", 0), + ).toBeNull(); + // The surcharge only matters on the wallet rail — an invoice can carry it. + expect( + overageBehaviorUnsupportedReason("INVOICE", "CHARGE_ORG", 1000), + ).toBeNull(); + }); }); describe("capabilityOf", () => { diff --git a/__tests__/payments/gateway-fence-classification.test.ts b/__tests__/payments/gateway-fence-classification.test.ts index 6305d15ea..435a2ca9a 100644 --- a/__tests__/payments/gateway-fence-classification.test.ts +++ b/__tests__/payments/gateway-fence-classification.test.ts @@ -122,4 +122,23 @@ describe("gateway fence classification", () => { ).httpStatus, ).toBe(409); }); + + // #1467 — a lapsed contract and a dunning suspension are org entitlement + // states the member's admin can clear. Both threw bare Errors, so the + // message-only fallback answered 500 UNKNOWN_ERROR on a routine refusal. + it.each([ + ["PROGRAM_ASSIGNMENT_INACTIVE", 409], + ["BILLING_SUSPENDED_DUNNING", 402], + ])("classifies %s as a business rejection with status %i", (code, status) => { + const classified = classifyError( + Object.assign(new Error("refused"), { code }), + ); + + expect(classified.isBusinessError).toBe(true); + expect(classified.httpStatus).toBe(status); + + const toast = getErrorToast(classified.errorType); + expect(toast.title).not.toBe("Something Went Wrong"); + expect(toast.description).toContain("admin"); + }); }); diff --git a/app/api/organizations/[orgId]/programs/[programId]/route.ts b/app/api/organizations/[orgId]/programs/[programId]/route.ts index cab5e25c5..09ca584b4 100644 --- a/app/api/organizations/[orgId]/programs/[programId]/route.ts +++ b/app/api/organizations/[orgId]/programs/[programId]/route.ts @@ -250,6 +250,9 @@ async function applyProgramPatch( const overageReason = overageBehaviorUnsupportedReason( current.contract.billingAccount?.fundingSource ?? null, merged.overageBehavior, + // #1458 — merged, so a patch that adds a surcharge to an already-saved + // wallet CHARGE_ORG programme is refused as readily as one that sets both. + merged.overageSurchargeBps, ); if (overageReason) fail(overageReason); } diff --git a/app/api/organizations/[orgId]/programs/route.ts b/app/api/organizations/[orgId]/programs/route.ts index 85b277bd4..afac0219c 100644 --- a/app/api/organizations/[orgId]/programs/route.ts +++ b/app/api/organizations/[orgId]/programs/route.ts @@ -320,11 +320,17 @@ export async function POST( // what happens past the cap. CHARGE_MEMBER on a wallet-funded contract only // failed at checkout, inside the booking transaction, so the refusal landed on // a member who had already picked a slot. Refuse it here instead. + const overageConfig = + body.type === "LICENSED_SEAT" + ? body.licensedSeatConfig + : body.creditPoolConfig; const overageReason = overageBehaviorUnsupportedReason( fundingSource, - body.type === "LICENSED_SEAT" - ? body.licensedSeatConfig.overageBehavior - : body.creditPoolConfig.overageBehavior, + overageConfig.overageBehavior, + // #1458 — the surcharge is part of the rule, not a separate knob: CHARGE_ORG + // is collectable on a wallet debit only while the marginal stays inside the + // price that debit took. + overageConfig.overageSurchargeBps, ); if (overageReason) { return NextResponse.json( diff --git a/lib/enterprise/reachable-paths.ts b/lib/enterprise/reachable-paths.ts index 86b7d6d9d..b9fc748de 100644 --- a/lib/enterprise/reachable-paths.ts +++ b/lib/enterprise/reachable-paths.ts @@ -78,6 +78,11 @@ export function isReachableOrgFundingPath( * member had already picked a slot. Refusing the CONFIGURATION instead means * the state is unreachable rather than merely fatal. * + * `overageSurchargeBps` participates because the surcharge, not the behaviour + * alone, decides collectability on the wallet rail: the plain over-cap amount is + * a slice of the price the wallet already debited, while a markup on top of that + * price is money no rail ever collects. + * * The message is returned rather than thrown so both the create route (a Zod * refinement) and the patch route (an inline `fail()`) can raise it in their own * shape without either of them owning the rule. @@ -85,6 +90,7 @@ export function isReachableOrgFundingPath( export function overageBehaviorUnsupportedReason( fundingSource: FundingSource | null, overageBehavior: OverageBehavior, + overageSurchargeBps?: number | null, ): string | null { if (fundingSource === "WALLET" && overageBehavior === "CHARGE_MEMBER") { return ( @@ -92,6 +98,23 @@ export function overageBehaviorUnsupportedReason( "Choose CHARGE_ORG, which is collected by that same wallet debit, or BLOCK to stop over-cap bookings." ); } + // #1458 — CHARGE_ORG on the wallet rail is collectable only while the marginal + // is a slice of the price the wallet already debited. A surcharge is a markup + // ON TOP of that price, so nothing collected it; the only way to would be to + // raise `Payment.amount`, which re-arms the leg-sum trigger against an + // unchanged WALLET leg. recordWalletCollectedOrgOverage() therefore refuses it + // at checkout — after the member has picked a slot — so refuse the + // configuration here for the same reason CHARGE_MEMBER is refused above. + if ( + fundingSource === "WALLET" && + overageBehavior === "CHARGE_ORG" && + (overageSurchargeBps ?? 0) > 0 + ) { + return ( + "A wallet-funded organisation cannot be charged an overage surcharge, because the wallet debit collects the booking price and a surcharge is a markup on top of it that no rail collects afterwards. " + + "Remove the overage surcharge to keep charging the organisation the plain over-cap amount, or choose BLOCK to stop over-cap bookings." + ); + } // A licence is a flat fee settled at contract time, so a licence-funded // booking collects nothing per booking: its funding leg is deliberately ₹0 // while `Payment.amount` stays at the full price, and the leg-sum guard diff --git a/lib/errors/classification/payment-error-classification.ts b/lib/errors/classification/payment-error-classification.ts index b9996a4fd..435a91a96 100644 --- a/lib/errors/classification/payment-error-classification.ts +++ b/lib/errors/classification/payment-error-classification.ts @@ -45,6 +45,11 @@ export const ErrorTypes = { PROGRAM_CAP_EXHAUSTED: "PROGRAM_CAP_EXHAUSTED_ERROR", PROGRAM_SESSION_CAP_REACHED: "PROGRAM_SESSION_CAP_REACHED_ERROR", OVERAGE_CHARGE_MEMBER_UNSUPPORTED: "OVERAGE_CHARGE_MEMBER_UNSUPPORTED_ERROR", + // #1467 — the two org-sponsorship refusals raised BEFORE checkout takes its + // lock. They are entitlement states, not overage states, so they get their own + // types rather than borrowing one of the three above. + PROGRAM_ASSIGNMENT_INACTIVE: "PROGRAM_ASSIGNMENT_INACTIVE_ERROR", + BILLING_SUSPENDED_DUNNING: "BILLING_SUSPENDED_DUNNING_ERROR", // Infrastructure failures (unexpected — ops/dev needs to investigate) PAYMENT_CONFIG: "PAYMENT_CONFIG_ERROR", @@ -254,6 +259,20 @@ export const BUSINESS_ERROR_CODES: ReadonlyArray<{ errorType: ErrorTypes.UNSUPPORTED_CONFIG, httpStatus: 409, }, + // #1467 — both were bare `new Error(...)` and so fell through the + // message-only fallback to 500 UNKNOWN_ERROR. A member whose organisation's + // contract had merely lapsed could not tell the refusal from a crash, and + // every one of them opened a Sentry incident. + { + code: "PROGRAM_ASSIGNMENT_INACTIVE", + errorType: ErrorTypes.PROGRAM_ASSIGNMENT_INACTIVE, + httpStatus: 409, + }, + { + code: "BILLING_SUSPENDED_DUNNING", + errorType: ErrorTypes.BILLING_SUSPENDED_DUNNING, + httpStatus: 402, + }, ] as const; /** diff --git a/lib/errors/mapping/payment-error-toast-map.ts b/lib/errors/mapping/payment-error-toast-map.ts index 9af01a2dd..ed8eed41b 100644 --- a/lib/errors/mapping/payment-error-toast-map.ts +++ b/lib/errors/mapping/payment-error-toast-map.ts @@ -120,6 +120,18 @@ const ERROR_TOAST_MAP: Record = { description: "This programme is set to charge members for bookings past its cap, which is not available on a wallet-funded organisation. Ask your billing admin to switch the programme to charge the organisation or to block over-cap bookings.", }, + // #1467 — the organisation's entitlement, not the booking, is what stops + // these. Retrying changes nothing, so each toast names the admin who can. + [ErrorTypes.PROGRAM_ASSIGNMENT_INACTIVE]: { + title: "No Programme Covers This Booking", + description: + "Your organisation has no active programme assignment for this session type, usually because its contract or programme has ended. Ask your organisation admin to assign you to a programme that covers it, or book it yourself. You were not charged.", + }, + [ErrorTypes.BILLING_SUSPENDED_DUNNING]: { + title: "Organisation Billing Suspended", + description: + "Your organisation has an overdue invoice, so new sponsored bookings are paused until it is paid. Ask your billing admin to settle it, or book this session yourself. You were not charged.", + }, [ErrorTypes.UNKNOWN]: { title: "Something Went Wrong", description: null, // Use the server's specific message diff --git a/lib/payments/operations/checkout.ts b/lib/payments/operations/checkout.ts index 466e80968..4d70c5a30 100644 --- a/lib/payments/operations/checkout.ts +++ b/lib/payments/operations/checkout.ts @@ -1552,8 +1552,15 @@ async function revalidateInsideLock( select: { id: true }, }); if (suspended) { - throw new Error( - "This organization is suspended from new sponsored bookings until its overdue invoice is paid.", + // #1467 — the in-lock re-check of the same dunning gate. It throws + // inside the checkout transaction, so without the code the catch below + // rewrites it to "Failed to record payment information"; with it the + // buyer gets the same 402 the pre-lock gate returns. + throw Object.assign( + new Error( + "This organization is suspended from new sponsored bookings until its overdue invoice is paid.", + ), + { httpStatus: 402, code: "BILLING_SUSPENDED_DUNNING" }, ); } } @@ -2526,8 +2533,15 @@ export async function handleCheckout( orderBy: { dueDate: "asc" }, }); if (suspended) { - throw new Error( - `This organization has an overdue invoice (${suspended.invoiceNumber}) and is suspended from new sponsored bookings until it is paid.`, + // #1467 — same shape as the assignment refusal below: a bare Error here + // would 500 the moment the flag is switched on. 402 because the block is + // lifted by paying money that is already owed, which is exactly what + // Payment Required means to the buyer's client. + throw Object.assign( + new Error( + `This organization has an overdue invoice (${suspended.invoiceNumber}) and is suspended from new sponsored bookings until it is paid.`, + ), + { httpStatus: 402, code: "BILLING_SUSPENDED_DUNNING" }, ); } } @@ -2664,10 +2678,19 @@ export async function handleCheckout( }); if (!assignment) { - throw new Error( - "No active program assignment covers this booking. Ask your organization admin to assign you to a Program that covers " + - appointmentType + - ".", + // #1467 — a lapsed contract or a closed programme is a routine refusal + // the member's own admin can undo, but the bare Error matched nothing in + // BUSINESS_ERROR_PATTERNS and classifyError answered 500 UNKNOWN_ERROR: + // the buyer could not tell it from a crash and Sentry logged a false + // incident. 409 because the request is well-formed and the org's + // entitlement state is what conflicts with it. + throw Object.assign( + new Error( + "No active program assignment covers this booking. Ask your organization admin to assign you to a Program that covers " + + appointmentType + + ".", + ), + { httpStatus: 409, code: "PROGRAM_ASSIGNMENT_INACTIVE" }, ); } programAssignmentId = assignment.id; diff --git a/scripts/reconcile/reconcile-ledgers.ts b/scripts/reconcile/reconcile-ledgers.ts index 9acd631b4..80c4dab7a 100644 --- a/scripts/reconcile/reconcile-ledgers.ts +++ b/scripts/reconcile/reconcile-ledgers.ts @@ -442,15 +442,28 @@ async function runReconcileLedgersUnlocked( chargeStatus: { in: ["PENDING", "FAILED", "CHARGED"] }, paymentId: null, }, + // ACCRUED means "billed on an issued invoice", which only the rollup + // produces and which always stamps the line item. A payment link cannot + // stand in for it, so this branch keeps invoiceLineItemId mandatory. { overageBehavior: "CHARGE_ORG", - chargeStatus: { in: ["ACCRUED", "CHARGED"] }, + chargeStatus: "ACCRUED", invoiceLineItemId: null, - // #1458 — a wallet-funded CHARGE_ORG overage is collected by the - // booking's own wallet debit and never reaches an invoice, so it is - // born CHARGED with a paymentId and no line item. Either link is - // proof of collection; neither is the drift this check hunts. - paymentId: null, + }, + // #1458 — a wallet-funded CHARGE_ORG overage is collected by the + // booking's own wallet debit and never reaches an invoice, so it is born + // CHARGED with a paymentId and no line item. That link is proof of + // collection only when the payment behind it actually carries the WALLET + // leg that did the collecting; any other CHARGED event with no line item + // is still the drift this check hunts. + { + overageBehavior: "CHARGE_ORG", + chargeStatus: "CHARGED", + invoiceLineItemId: null, + OR: [ + { paymentId: null }, + { payment: { legs: { none: { source: "WALLET" } } } }, + ], }, { chargeStatus: "CHARGED", settledAt: null }, ], @@ -490,7 +503,7 @@ async function runReconcileLedgersUnlocked( paymentId: ev.paymentId, invoiceLineItemId: ev.invoiceLineItemId, settledAt: ev.settledAt, - note: "OverageEvent link/state invariant violated: CHARGE_MEMBER pending/failed/charged without a side-Payment, CHARGE_ORG accrued/charged with neither an InvoiceLineItem nor the wallet-funded booking Payment that collected it (#1458), or CHARGED without settledAt. Trace the transitionOverage() path that produced this state.", + note: "OverageEvent link/state invariant violated: CHARGE_MEMBER pending/failed/charged without a side-Payment, CHARGE_ORG accrued without an InvoiceLineItem, CHARGE_ORG charged with neither an InvoiceLineItem nor a booking Payment carrying the WALLET leg that collected it (#1458), or CHARGED without settledAt. Trace the transitionOverage() path that produced this state.", }, }); }