Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions TODO.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.*
Expand Down
61 changes: 31 additions & 30 deletions src/shared/refund-ledger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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
Expand All @@ -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 = (
Expand All @@ -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);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve a replay marker when the atomic batch fails

When refunded is true and this atomic post rejects the refund group, postWithoutThrowing returns posted: false but the batch leaves no booking event leg at all. The payment flow uses the ledger as the durable replay guard (replaySessionFromLedger treats bookingLedgerDisposition(...).status === "unrecorded" as fresh), and prunePayments later deletes terminal failure_data rows, so a later webhook/redirect for the same already-refunded session can re-enter processing and create another placeholder/refund attempt instead of being acknowledged as handled. Keep some durable handled marker or surface the failure before acknowledging the terminal payment outcome.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Valid concern — recorded in TODO.md (commit 1a12dc9). The atomic rollback is this PR's core requirement: the split plan says "a conflict in the refund leg must roll back the payment leg as well", and the acceptance criterion is "a refund-reference collision proves neither transfer group is committed." Fixing the replay-marker gap without breaking that requires a durable handled marker outside the prunable processed_payments table — the staged-checkout runtime (deferred foundations item 6 in PR_SPLIT_PLAN.md) carries that machinery.

The gap also partially pre-exists on main: before this PR, if the payment post itself failed (reference conflict), no legs landed either, and the same pruning → re-entry path applied. This PR widens the failure surface from "payment-post failure only" to "payment-post or refund-post failure" (because both are now one atomic batch), which is the designed trade-off.

TODO.md has the full context: the preflight (replaySessionFromLedger in src/features/api/payment-processing/index.ts), the pruner (prunePayments in src/shared/db/prune.ts), and the classification (src/shared/session-ledger.ts) are the starting points.

return true;
},
);
110 changes: 110 additions & 0 deletions test/shared/refund-ledger-placeholder.test.ts
Original file line number Diff line number Diff line change
@@ -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<string> =>
legReference(["booking", PLACEHOLDER.eventId, "payment"]);

const refundReference = async (): Promise<string> =>
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<void> => {
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);
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.

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");
});
});
70 changes: 1 addition & 69 deletions test/shared/refund-ledger.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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");
});
});