diff --git a/TODO.md b/TODO.md index 2568cbdc36..b4fc6b59bb 100644 --- a/TODO.md +++ b/TODO.md @@ -989,6 +989,39 @@ doing next time this file is touched. --- +## Placeholder refund — replay marker gap when the atomic ledger batch fails + +*Origin: Codex review on PR #1822 (atomic placeholder payment + refund ledger).* + +`recordPlaceholderRefund` (`src/shared/refund-ledger.ts`) posts the payment +and completed-refund legs as one atomic `postTransferGroups` batch, so a +refund-leg conflict rolls the payment back too (the PR's core requirement). +When that batch fails outright, NO ledger legs land for the booking event +group. The payment flow's durable replay guard is the ledger preflight +(`replaySessionFromLedger` → `bookingLedgerDisposition`: `unrecorded` when +no legs exist), and the primary guard (`markSessionFailed`'s `failure_data` +row) is pruned by `prunePayments` once it ages past retention. So after +pruning, a late webhook/redirect for the same already-refunded session +re-enters `processReservedSession`, sees `unrecorded`, and re-creates a +placeholder attendee + re-calls `tryRefund` (idempotent, so no double payout) +instead of acknowledging the session as already handled. + +This is NOT fully new: on main before PR #1822 the same gap existed for a +payment-post failure (the first `postTransfers` threw → no legs). PR #1822 +widens the failure surface from "payment-post failure only" to "payment-post +OR refund-post failure" (because both are now one atomic batch). Closing it +properly needs a durable handled marker that survives idempotency-row pruning +without breaking the atomic rollback — e.g. a ledger leg that survives even +when the refund leg conflicts (which would violate #1822's acceptance +criterion: "a refund-reference collision proves neither transfer group is +committed"), or a separate replay-state row outside the prunable +`processed_payments` table. The staged-checkout runtime (deferred +foundations item 6 in `PR_SPLIT_PLAN.md`) carries the proper replay/activation +machinery to resolve this. Starting point: the preflight in +`src/features/api/payment-processing/index.ts` (`replaySessionFromLedger`), +the pruner in `src/shared/db/prune.ts` (`prunePayments`), and the +classification in `src/shared/session-ledger.ts`. + ## Localise the confirmation-email template-variable reference table *Origin: CodeRabbit review on PR #1800.* diff --git a/src/shared/refund-ledger.ts b/src/shared/refund-ledger.ts index 3e5116c923..1047f3c1c4 100644 --- a/src/shared/refund-ledger.ts +++ b/src/shared/refund-ledger.ts @@ -23,12 +23,13 @@ import { attendeeAccount, WORLD } from "#shared/accounting/accounts.ts"; import { KIND } from "#shared/accounting/kinds.ts"; /* jscpd:ignore-end */ import { + asOrderLegs, bookingEventGroup, mapBooking, mapRefund, } from "#shared/accounting/mappers.ts"; import { transfersByAccount } from "#shared/accounting/queries.ts"; -import { postTransferGroups, postTransfers } from "#shared/accounting/store.ts"; +import { postTransferGroups } from "#shared/accounting/store.ts"; import { balanceEventGroup } from "#shared/db/attendees/balance.ts"; import type { RefundPaymentReference } from "#shared/db/payment-references.ts"; import { legMatches } from "#shared/ledger/legs.ts"; @@ -270,7 +271,9 @@ export type PlaceholderRefundFacts = { * Record the cash round-trip of a stored-but-refunded placeholder booking — the * quantity-0 line we keep so a signed payment we can't honour is never lost from * the diary. Posts the `payment` we received and, when the provider refund - * succeeded, the `refund_cash` returning it. Deliberately posts NO `sale` leg: + * succeeded, the `refund_cash` returning it. Both event groups are posted in one + * atomic batch, so the payment can never commit without its completed refund. + * Deliberately posts NO `sale` leg: * the booking was never honoured, so no revenue is recognised and the quantity-0 * line's projected `price_paid` stays 0 (a sale leg would re-break that invariant * and read as still-paid). A failed refund posts only the payment, so the ledger @@ -280,7 +283,9 @@ export type PlaceholderRefundFacts = { * {@link recordAttendeeRefund} can't be reused here: this placeholder records a * cash-only booking that was never honoured, so there is no sale leg or * fully-paid account to reverse. Never throws — the provider refund has already - * settled, so a ledger write must not turn it into a 500; a failed post is + * settled, so a ledger write must not turn it into a 500. `posted` reports + * whether every required leg was stored: just the payment when no refund + * completed, or the payment and refund together when one did. A failed post is * logged and reported as `posted: false`. */ export const recordPlaceholderRefund = ( @@ -292,33 +297,29 @@ export const recordPlaceholderRefund = ( "placeholder refund ledger post", facts.attendeeId, async () => { - // A booking whose only money fact is the cash received: gross 0 drops the - // sale leg, leaving just the `payment` leg (mapBooking omits zero-amount legs). - await postTransfers( - await mapBooking({ - amountPaid: facts.amount, - attendeeId: facts.attendeeId, - bookingFee: 0, - eventId: facts.eventId, - lines: [{ gross: 0, listingId: facts.listingId }], - modifiers: [], - occurredAt: facts.occurredAt, - }), - ); - if (!refunded) return false; - // Reverse the payment we just posted as refund_cash (read back so mapRefund - // gets the stored legs). This runs once per session — a redelivery replays the - // terminal outcome before reaching here — so there is never a prior reversal. - const payments = ( - await transfersByAccount(attendeeAccount(facts.attendeeId)) - ).filter((leg) => leg.kind === KIND.payment); - await postTransfers( - await mapRefund({ - memo, - occurredAt: facts.occurredAt, - orderLegs: payments, - }), - ); + // Gross 0 drops the sale leg, leaving just the payment. The refund mapper + // only needs the leg's money identity, so it can map the reversal before + // either group is stored and the batch can commit both or neither. + const payment = await mapBooking({ + amountPaid: facts.amount, + attendeeId: facts.attendeeId, + bookingFee: 0, + eventId: facts.eventId, + lines: [{ gross: 0, listingId: facts.listingId }], + modifiers: [], + occurredAt: facts.occurredAt, + }); + const groups = refunded + ? [ + payment, + await mapRefund({ + memo, + occurredAt: facts.occurredAt, + orderLegs: asOrderLegs(payment, facts.occurredAt), + }), + ] + : [payment]; + await postTransferGroups(groups); return true; }, ); diff --git a/test/shared/refund-ledger-placeholder.test.ts b/test/shared/refund-ledger-placeholder.test.ts new file mode 100644 index 0000000000..c0dfd028a3 --- /dev/null +++ b/test/shared/refund-ledger-placeholder.test.ts @@ -0,0 +1,110 @@ +import { expect } from "@std/expect"; +import { it as test } from "@std/testing/bdd"; +import { attendeeAccount, WORLD } from "#shared/accounting/accounts.ts"; +import { bookingEventGroup } from "#shared/accounting/mappers.ts"; +import { transfersByAccount } from "#shared/accounting/queries.ts"; +import { legReference } from "#shared/accounting/refs.ts"; +import { postTransfers } from "#shared/accounting/store.ts"; +import { balanceOf } from "#shared/ledger/project.ts"; +import { recordPlaceholderRefund } from "#shared/refund-ledger.ts"; +import { describeWithEnv } from "#test-utils/db.ts"; +import { setupErrorSpy } from "#test-utils/error-spy.ts"; +import { BOOKING_AT } from "./refund-ledger/helpers.ts"; + +const PLACEHOLDER = { + amount: 5000, + attendeeId: 7, + eventId: "ph-sess-1", + listingId: 1, + occurredAt: BOOKING_AT, +}; + +const paymentReference = (): Promise => + legReference(["booking", PLACEHOLDER.eventId, "payment"]); + +const refundReference = async (): Promise => + legReference([ + "refund", + await bookingEventGroup(PLACEHOLDER.eventId), + await paymentReference(), + ]); + +describeWithEnv("refund-ledger > recordPlaceholderRefund", { db: true }, () => { + const errors = setupErrorSpy(); + + const blockLeg = async ( + eventGroup: string, + reference: string, + ): Promise => { + await postTransfers([ + { + amount: 100, + destination: attendeeAccount(99), + eventGroup, + kind: "payment", + occurredAt: BOOKING_AT, + reference, + source: WORLD, + }, + ]); + }; + + test("records the cash round-trip with no sale leg, netting to zero", async () => { + expect( + await recordPlaceholderRefund(PLACEHOLDER, "price_changed", true), + ).toEqual({ posted: true }); + const legs = await transfersByAccount( + attendeeAccount(PLACEHOLDER.attendeeId), + ); + const cash = legs.filter((leg) => leg.kind === "refund_cash"); + expect(legs.map((leg) => leg.kind).sort()).toEqual([ + "payment", + "refund_cash", + ]); + expect(cash.length).toBe(1); + expect(cash[0]!.amount).toBe(PLACEHOLDER.amount); + expect(cash[0]!.memo).toBe("price_changed"); + expect(balanceOf(attendeeAccount(PLACEHOLDER.attendeeId))(legs)).toBe(0); + }); + + test("posts only the payment when no refund completed", async () => { + expect( + await recordPlaceholderRefund(PLACEHOLDER, "charge_mismatch", false), + ).toEqual({ posted: true }); + const legs = await transfersByAccount( + attendeeAccount(PLACEHOLDER.attendeeId), + ); + expect(legs.map((leg) => leg.kind)).toEqual(["payment"]); + expect(balanceOf(attendeeAccount(PLACEHOLDER.attendeeId))(legs)).toBe( + PLACEHOLDER.amount, + ); + }); + + test("rolls back the payment when the refund reference conflicts", async () => { + await blockLeg("refund-blocker", await refundReference()); + + expect( + await recordPlaceholderRefund(PLACEHOLDER, "sold_out", true), + ).toEqual({ posted: false }); + expect( + await transfersByAccount(attendeeAccount(PLACEHOLDER.attendeeId)), + ).toEqual([]); + expect(errors.lastMessage()).toContain("E_LEDGER_POST"); + }); + + test("logs and does not throw when the payment reference conflicts", async () => { + await blockLeg("payment-blocker", await paymentReference()); + expect( + await recordPlaceholderRefund(PLACEHOLDER, "sold_out", true), + ).toEqual({ posted: false }); + expect(errors.lastMessage()).toContain("E_LEDGER_POST"); + }); + + test("reports a payment-only conflict as not posted", async () => { + await blockLeg("payment-only-blocker", await paymentReference()); + expect( + await recordPlaceholderRefund(PLACEHOLDER, "charge_mismatch", false), + ).toEqual({ posted: false }); + expect(errors.lastMessage()).toContain("E_LEDGER_POST"); + }); +}); diff --git a/test/shared/refund-ledger.test.ts b/test/shared/refund-ledger.test.ts index 45c0fb8401..c0586320e4 100644 --- a/test/shared/refund-ledger.test.ts +++ b/test/shared/refund-ledger.test.ts @@ -21,12 +21,8 @@ import { getQueryLog, runWithQueryLogContext, } from "#shared/db/query-log.ts"; -import { balanceOf } from "#shared/ledger/project.ts"; import type { AccountRef } from "#shared/ledger/types.ts"; -import { - recordAttendeeRefund, - recordPlaceholderRefund, -} from "#shared/refund-ledger.ts"; +import { recordAttendeeRefund } from "#shared/refund-ledger.ts"; import { describeWithEnv } from "#test-utils/db.ts"; import { setupErrorSpy } from "#test-utils/error-spy.ts"; import { @@ -295,67 +291,3 @@ describeWithEnv("refund-ledger > recordAttendeeRefund", { db: true }, () => { expect(errors.lastMessage()).toContain("E_LEDGER_POST"); }); }); - -// -- recordPlaceholderRefund (cash round-trip, no sale leg) -------------- // - -describeWithEnv("refund-ledger > recordPlaceholderRefund", { db: true }, () => { - const errors = setupErrorSpy(); - const PH = { - amount: 5000, - attendeeId: 7, - eventId: "ph-sess-1", - listingId: 1, - occurredAt: BOOKING_AT, - }; - - test("records the cash round-trip with no sale leg, netting to zero", async () => { - expect(await recordPlaceholderRefund(PH, "price_changed", true)).toEqual({ - posted: true, - }); - const legs = await transfersByAccount(attendeeAccount(7)); - // No revenue recognised — just the payment we received and the refund_cash - // returning it (stamped with the reason), so the line's price_paid stays 0. - expect(legs.some((l) => l.kind === "sale")).toBe(false); - expect(legs.some((l) => l.kind === "payment")).toBe(true); - const cash = legs.filter((l) => l.kind === "refund_cash"); - expect(cash.length).toBe(1); - expect(cash[0]!.amount).toBe(5000); - expect(cash[0]!.memo).toBe("price_changed"); - expect(balanceOf(attendeeAccount(7))(legs)).toBe(0); - }); - - test("posts only the payment when the refund failed (we still hold the money)", async () => { - expect(await recordPlaceholderRefund(PH, "charge_mismatch", false)).toEqual( - { - posted: false, - }, - ); - const legs = await transfersByAccount(attendeeAccount(7)); - expect(legs.some((l) => l.kind === "payment")).toBe(true); - expect(legs.some((l) => l.kind === "refund_cash")).toBe(false); - // The ledger shows we hold their cash until a manual refund reverses it. - expect(balanceOf(attendeeAccount(7))(legs)).toBe(5000); - }); - - test("logs and does not throw when the ledger post conflicts", async () => { - // Pre-claim the payment leg's reference under a different event so the cash-in - // post hits a reference collision and the catch path runs. - const collidingRef = await legReference(["booking", PH.eventId, "payment"]); - await postTransfers([ - { - amount: 100, - destination: attendeeAccount(99), - eventGroup: "blocker", - kind: "payment", - occurredAt: BOOKING_AT, - reference: collidingRef, - source: WORLD, - }, - ]); - expect(await recordPlaceholderRefund(PH, "sold_out", true)).toEqual({ - posted: false, - }); - // The classified error is the operator's only breadcrumb for the miss. - expect(errors.lastMessage()).toContain("E_LEDGER_POST"); - }); -});