Skip to content
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
ce199cc
Protect committed bookings from refunds
stefan-burke Jul 11, 2026
6848c5c
Keep orphaned payment replays handled
stefan-burke Jul 11, 2026
ee33cd8
Recover tickets after committed booking errors
stefan-burke Jul 11, 2026
2fd68ed
Complete recovered paid bookings
stefan-burke Jul 12, 2026
bf49d48
Strengthen payment recovery tests
stefan-burke Jul 12, 2026
d4640c3
Cover pruned balance replay
stefan-burke Jul 12, 2026
6ccf91c
Keep payment recovery terminal
stefan-burke Jul 12, 2026
bd5c2f5
Finalize with attendee ticket token
stefan-burke Jul 12, 2026
1b4edd4
Require scoped payment ticket token
stefan-burke Jul 12, 2026
de20ef4
Protect committed bookings from refunds
stefan-burke Jul 11, 2026
047c11c
Keep orphaned payment replays handled
stefan-burke Jul 11, 2026
2ec493d
Recover tickets after committed booking errors
stefan-burke Jul 11, 2026
5345be9
Complete recovered paid bookings
stefan-burke Jul 12, 2026
40ca897
Strengthen payment recovery tests
stefan-burke Jul 12, 2026
0d5e41b
Cover pruned balance replay
stefan-burke Jul 12, 2026
5a70440
Keep payment recovery terminal
stefan-burke Jul 12, 2026
95dece8
Finalize with attendee ticket token
stefan-burke Jul 12, 2026
d10db72
Require scoped payment ticket token
stefan-burke Jul 12, 2026
af47a14
Merge remote-tracking branch 'origin/fix/committed-booking-refunds' i…
stefan-burke Jul 12, 2026
3eaebc6
Restore recovered contact history
stefan-burke Jul 12, 2026
93ca61a
Recover after ticket redirect race
stefan-burke Jul 12, 2026
df56d4b
Merge branch 'main' into fix/committed-booking-refunds
stefan-burke Jul 12, 2026
10dc882
Merge remote-tracking branch 'origin/fix/committed-booking-refunds' i…
stefan-burke Jul 12, 2026
1aac4e0
Update recovered ticket copy assertion
stefan-burke Jul 12, 2026
a1d8563
Make payment recovery writes idempotent
stefan-burke Jul 12, 2026
653191b
Merge remote-tracking branch 'origin/main' into fix/committed-booking…
stefan-burke Jul 12, 2026
a3284cc
Use shared listing question links
stefan-burke Jul 12, 2026
b02e9f1
Stage paid bookings before checkout
stefan-burke Jul 12, 2026
4236e09
Harden checkout cleanup and servicing edits
stefan-burke Jul 12, 2026
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
24 changes: 6 additions & 18 deletions TODO.md
Original file line number Diff line number Diff line change
Expand Up @@ -338,26 +338,14 @@ existing ticket?) and likely adding payment-intent uniqueness — out of scope f
a test-only file split. Starting point: `src/features/api/payment-processing.ts`
(the `/payment/success` finalize path) and `#shared/db/processed-payments.ts`.

## Payment-processing review follow-ups (from PR #1692)
## Payment-processing review follow-up (from PR #1692)

Both items describe behaviour that predates the payment-processing split (the
code was moved verbatim from the old `payment-processing.ts` monolith). They are
This item describes behaviour that predates the payment-processing split (the
code was moved verbatim from the old `payment-processing.ts` monolith). It is
recorded here because the split PR was a pure reorganisation — changing this
behaviour there would be out of scope — and CodeRabbit flagged them as worth a
look.

- **Refund after a committed booking** (`src/features/api/payment-processing/index.ts`,
the `try { honoured = await createAttendeeForSession(...) } catch` in
`processReservedSession`). `createAttendeeForSession` commits the attendee +
bookings atomically, then runs `ensureAllBookings` (a post-commit read). If
that post-write step *threw*, the `catch` would route to `storeRefundedBooking`
— refunding a booking that actually persisted. Today `ensureAllBookings`
returns a structured `{ ok: false }` rather than throwing on the capacity path,
so the window is theoretical, but it isn't guarded structurally. Fix direction:
narrow the `try` to the pre-commit call only, or guarantee the post-commit
cleanup path is non-throwing, so a persisted booking can never be refunded.
Add a regression test that makes the post-commit step throw and asserts no
refund is issued.
behaviour there would have been out of scope — and CodeRabbit flagged it as
worth a look.

- **Per-item DB reads not batched** (`src/features/api/payment-processing/items.ts`
`validateAllItems`, and `package-pricing.ts` `loadPackagePricingByGroup`).
`validateAllItems` calls `getListingWithCount` once per item in a loop, and
Expand Down
168 changes: 66 additions & 102 deletions src/features/api/payment-processing/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*
* unreserved → reserved → (finalized success | terminal failure)
*
* via the steps: validate → reserve → process → record-outcome.
* via validate → reserve → process → record-outcome.
*
* 1. validate — `validatePaidSession` (classify.ts) confirms with the provider
* that the session is paid and `classifySession` proves (via a signed price
Expand All @@ -14,23 +14,14 @@
* 2. reserve — `processPaymentSession` claims the idempotency lock
* (`reserveSession`); a conflict replays the already-recorded outcome
* (`handleReservationConflict`) instead of re-processing.
* 3. process — `processReservedSession`, holding the lock, turns the signed
* session into either a real ticket (`createAttendeeForSession`) / a settled
* balance (`settleBalanceSession`), or — for ANY reason it can't be honoured
* (charge mismatch, a price edited mid-checkout, a sold-out extra, a full
* event, a since-deleted listing, or an unexpected error after the charge) —
* a quantity-0 placeholder that is refunded (`storeRefundedBooking`), so a
* paid customer is never dropped.
* 3. process — `processReservedSession` creates a ticket, settles a balance,
* or stores and refunds a quantity-0 placeholder when the paid booking cannot
* be honoured before its atomic write commits.
* 4. record-outcome — `processPaymentSession` records a handled failure as the
* session's terminal outcome (`markSessionFailed`) so a later redirect/webhook
* replays the same result, or releases the reservation when a real refund
* failed so the next provider redelivery re-attempts it.
*
* The HTTP plumbing (redirect + webhook handlers, routing) lives in
* `webhooks.ts` and calls into this module. The steps above are split across
* sibling files (metadata, classify, cancel, refunds, items, package-pricing,
* pricing, create, store-refund); this file is the orchestration that wires
* them into the two-phase locked lifecycle.
*/

import {
Expand All @@ -45,10 +36,10 @@ import {
checkoutIntentForSession,
paidPricingRefund,
} from "#routes/api/payment-processing/pricing.ts";
import { recoverOrRefundUnexpectedCreate } from "#routes/api/payment-processing/recovery.ts";
import {
chargeMismatchSpec,
deletedListingSpec,
refundSpec,
refuseMismatch,
} from "#routes/api/payment-processing/refunds.ts";
import {
Expand All @@ -66,6 +57,7 @@ import type {
} from "#routes/api/webhook-types.ts";
import { eventGroupHasLegs } from "#shared/accounting/queries.ts";
import { type PricedOrder, priceCheckout } from "#shared/checkout-pricing.ts";
import { generateTicketToken } from "#shared/crypto/utils.ts";
import { balanceEventGroup } from "#shared/db/attendees/balance.ts";
import { buyerVisits, specsFromRefs } from "#shared/db/modifier-resolve.ts";
import {
Expand All @@ -77,22 +69,18 @@ import {
reserveSession,
setSessionTicketTokens,
} from "#shared/db/processed-payments.ts";
import { logDebug } from "#shared/logger.ts";
import { withPaymentTicketToken } from "#shared/payment-ticket-token.ts";
import { bookingLedgerDisposition } from "#shared/session-ledger.ts";
import { logAndNotifyRegistration } from "#shared/webhook.ts";

type SessionProcessorOptions = { storeTokens?: boolean };

/** The shared shape of the two-phase session processors: reserve/process a paid
* session by id, given its validated data, and resolve to a {@link
* PaymentResult}. */
type SessionProcessor = (
sessionId: string,
data: ValidatedSession,
options?: SessionProcessorOptions,
) => Promise<PaymentResult>;

/** Handle the "already reserved" branch of reserveSession */
const handleReservationConflict = async (
intent: BookingIntent,
existing: ProcessedPayment,
Expand All @@ -103,47 +91,37 @@ const handleReservationConflict = async (
attendee_id: existing.attendee_id,
});
}
// A recorded terminal failure replays the same handled outcome (refund
// already issued, sold out, price changed) without re-validating or
// re-refunding. failure_data is encrypted, so this read is async.
// Replay an encrypted terminal outcome without revalidating or refunding.
const failure = await parseSessionFailure(existing.failure_data);
if (failure) return { ...failure, success: false };
// Otherwise reserved but not finalized — another request is mid-flight.
return {
error: "Payment is being processed. Please wait a moment and refresh.",
status: 409,
success: false,
};
};

/**
* Replay a payment session the ledger already records as resolved to
* `attendeeId`: heal the fresh reservation at that attendee — token-safely, so a
* racing delivery's finalized tokens survive (see {@link
* finalizeSessionIfUnresolved}) — and return success. NEVER refunds: the money is
* already in the ledger against this attendee. Tokens come back empty, so the
* redirect renders directly from the attendee. Shared by the booking-replay and
* balance-replay preflights.
*/
const replaySuccess = async (
sessionId: string,
attendeeId: number,
listingId: number,
paymentReference = "",
): Promise<PaymentResult> => {
/** Heal a fresh reservation from the ledger's attendee, preserving any tokens
* finalized by a racing delivery. The recorded money is never refunded. */
type ReplaySuccessInput = {
attendeeId: number;
listingId: number;
paymentReference: string;
sessionId: string;
};

const replaySuccess = async ({
attendeeId,
listingId,
paymentReference,
sessionId,
}: ReplaySuccessInput): Promise<PaymentResult> => {
await finalizeSessionIfUnresolved(sessionId, attendeeId, paymentReference);
logDebug("Payment", `Replayed already-ledgered session ${sessionId}`);
return sessionSuccess(attendeeId, listingId);
};

/**
* Acknowledge a session the ledger already accounts for but whose booking is
* gone — an operator deleted the attendee (its sale/payment legs remain) or it
* was a refunded quantity-0 placeholder. The money is already recorded, so we
* neither refund again nor recreate the booking: return a terminal handled
* outcome (200 — the webhook acks it, the redirect shows it as processed) and
* leave the orphaned ledger rows for the operator to reconcile.
*/
/** Acknowledge recorded money whose booking is gone without refunding again or
* recreating it; its orphaned ledger rows remain for operator reconciliation. */
const alreadyHandledSession = (
sessionId: string,
listingId: number,
Expand All @@ -154,64 +132,47 @@ const alreadyHandledSession = (
success: false,
Comment thread
stefan-burke marked this conversation as resolved.
Resolved
});

/**
* The booking-session ledger preflight: the durable ledger — not the prunable
* processed_payments row — is the source of truth for "already honoured", so
* before validating, pricing, or refunding, resolve what it already records.
* Returns the replay outcome for a session it has seen (a live booking replays as
* success; an orphaned one is acknowledged), or null for a session it has never
* recorded (process it fresh). The single guard that stops a late replay — after
* the idempotency row is pruned or lost to a stale-reservation cleanup — from
* refunding a live ticket via the deleted-listing, price-change, inactive-listing,
* or capacity refund paths below.
*/
const replaySessionFromLedger = async (
/** Resolve a booking against the durable ledger before any validation, pricing,
* or refund path. This protects live tickets after their idempotency row is lost. */
/** Returns null when the ledger has not recorded this session yet. */
const replaySessionFromLedgerOrNull = async (
sessionId: string,
listingId: number,
paymentReference: string,
): Promise<PaymentResult | null> => {
const disposition = await bookingLedgerDisposition(sessionId);
switch (disposition.status) {
case "unrecorded":
return null;
case "booked":
return replaySuccess(sessionId, disposition.attendeeId, listingId);
return replaySuccess({
attendeeId: disposition.attendeeId,
listingId,
paymentReference,
sessionId,
});
case "orphaned":
return alreadyHandledSession(sessionId, listingId);
}
};

/**
* The balance-settlement counterpart of {@link replaySessionFromLedger}: replay a
* balance session whose payment leg the ledger already records (its idempotency
* row was pruned or lost), or null to settle it fresh. The attendee is known from
* the proof-bound intent, so — unlike the booking path — there is no orphaned
* case to resolve.
*/
/** Replay a balance payment already recorded by the ledger, or settle it fresh. */
const replayBalanceFromLedger = async (
sessionId: string,
attendeeId: number,
listingId: number,
paymentReference: string,
): Promise<PaymentResult | null> =>
(await eventGroupHasLegs(await balanceEventGroup(sessionId)))
? replaySuccess(sessionId, attendeeId, listingId, paymentReference)
? replaySuccess({ attendeeId, listingId, paymentReference, sessionId })
: null;

/**
* Process a session we have just reserved (holding the lock). A signed session
* either becomes a real ticket or — for ANY reason we can't honour it (charge
* mismatch, a price edited mid-checkout, a sold-out extra, a full event, a
* since-deleted listing, or an unexpected error after the charge) — is kept as a
* quantity-0 placeholder and refunded, so a paid customer is never dropped. Every
* failure returned here is a handled terminal outcome; processPaymentSession
* records it so a later redirect/webhook replays the same result instead of
* re-running refunds or stalling behind the idempotency lock.
*/
const processReservedSession: SessionProcessor = async (
sessionId,
data,
options,
) => {
/** Process a reserved session into a ticket or handled terminal outcome. Errors
* after an atomic booking commit propagate rather than refunding a live ticket. */
const processReservedSession = async (
sessionId: string,
data: ValidatedSession,
): Promise<PaymentResult> => {
const { session, intent, verdict } = data;
const signedListingId = intent.items[0]!.e;

Expand Down Expand Up @@ -241,8 +202,12 @@ const processReservedSession: SessionProcessor = async (
// or refund path runs below — so a late delivery (after the prunable idempotency
// row is gone) never refunds a live ticket via the deleted-listing, price-change,
// inactive-listing, or capacity paths, nor double-books it.
const replay = await replaySessionFromLedger(sessionId, signedListingId);
if (replay) return replay;
const replay = await replaySessionFromLedgerOrNull(
sessionId,
signedListingId,
session.paymentReference,
);
if (replay !== null) return replay;

// Phase 2: Validate listings.
const validated = await validateAllItems(session, intent);
Expand Down Expand Up @@ -298,23 +263,27 @@ const processReservedSession: SessionProcessor = async (
// booking at quantity 0 and refunds rather than dropping a paid customer: a
// structured sold-out/capacity/encryption result, OR an unexpected throw after
// the charge (which would otherwise crash-loop the webhook over paid money).
const preparedTicketToken = generateTicketToken();
await setSessionTicketTokens(sessionId, [preparedTicketToken]);
let honoured: Awaited<ReturnType<typeof createAttendeeForSession>>;
try {
honoured = await createAttendeeForSession(
session,
intent,
validatedItems,
pricingIntent,
pricedOrder,
honoured = await withPaymentTicketToken(preparedTicketToken, () =>
createAttendeeForSession(
session,
intent,
validatedItems,
pricingIntent,
pricedOrder,
),
);
} catch (error) {
return storeRefundedBooking(
// The atomic create may have committed before result handling or the client
// threw. Recheck its reservation on the primary before moving money.
return recoverOrRefundUnexpectedCreate(
session,
intent,
placeholders,
refundSpec("unexpected_error")(
`Unexpected error completing session ${session.id}: ${String(error)}`,
),
error,
);
}
if (!honoured.ok) {
Expand All @@ -332,11 +301,6 @@ const processReservedSession: SessionProcessor = async (
const firstAttendee = createdEntries[0]!;
const ticketToken = firstAttendee.attendee.ticket_token;

// Persist the ticket token for webhook replay when the caller needs it.
if (options?.storeTokens !== false) {
await setSessionTicketTokens(sessionId, [ticketToken]);
}

const codeSpecs = modifierSpecs.filter((s) => s.trigger === "code");
if (codeSpecs.length > 0) {
await logPromoCodeModifiers(
Expand All @@ -357,15 +321,15 @@ const processReservedSession: SessionProcessor = async (
export const processPaymentSession: SessionProcessor = async (
sessionId,
data,
options,
_options,
) => {
// Phase 1: Reserve the session (claim the lock)
const reservation = await reserveSession(sessionId);
if (!reservation.reserved) {
return handleReservationConflict(data.intent, reservation.existing);
}

const result = await processReservedSession(sessionId, data, options);
const result = await processReservedSession(sessionId, data);

// A refund of a real payment that FAILED must stay retryable, and the very
// next provider redelivery should re-attempt it. Releasing the reservation
Expand Down
Loading