Stop auto-failing manual payments (TPago/bank/cash) after the pending TTL.

Exclude manual providers from booking cleanup, hold them via the 72h sweep instead, and let admins reopen failed payments to pending.

Co-authored-by: Cursor <cursoragent@cursor.com>

backend/src/lib/bookingCleanup.ts

@@ -5,11 +5,20 @@
 // abandoned checkout would otherwise hold those seats forever. This job cancels
 // pending tickets whose payment is still 'pending' (i.e. never paid and not
 // awaiting admin approval) after a configurable TTL, freeing the seats.
+//
+// Two exclusions:
+//   - Manual-verification providers (bank transfer / TPago / cash) are never
+//     auto-failed; they are settled by an admin and instead follow the 72h on-hold
+//     sweep. See holdSweep.ts and MANUAL_PAYMENT_PROVIDERS.
+//   - Staleness is measured from `updatedAt`, not `createdAt`, so an admin action
+//     (e.g. reopening a payment to 'pending' via /reopen) restarts the TTL rather
+//     than being immediately re-failed on the next run.
 
-import { and, eq, lt, inArray } from 'drizzle-orm';
+import { and, eq, lt, inArray, notInArray } from 'drizzle-orm';
 import { db, dbAll, tickets, payments } from '../db/index.js';
 import { getNow, toDbDate } from './utils.js';
 import { getLock } from './stores/lock.js';
+import { MANUAL_PAYMENT_PROVIDERS } from './manualProviders.js';
 
 function getTtlMs(): number {
   const minutes = parseInt(process.env.PENDING_BOOKING_TTL_MINUTES || '30', 10);
@@ -20,8 +29,9 @@ function getTtlMs(): number {
  * Cancel stale pending bookings. Returns the number of tickets cancelled.
  *
  * A booking is considered stale when its payment is still 'pending' (not
- * 'pending_approval', which means an admin is reviewing a manual transfer) and
- * older than PENDING_BOOKING_TTL_MINUTES.
+ * 'pending_approval', which means an admin is reviewing a manual transfer),
+ * uses a non-manual provider, and has not been touched (updatedAt) for
+ * PENDING_BOOKING_TTL_MINUTES.
  */
 export async function cleanupStalePendingBookings(): Promise<number> {
   const cutoff = toDbDate(new Date(Date.now() - getTtlMs()));
@@ -35,7 +45,8 @@ export async function cleanupStalePendingBookings(): Promise<number> {
       .from(payments)
       .where(and(
         eq((payments as any).status, 'pending'),
-        lt((payments as any).createdAt, cutoff)
+        notInArray((payments as any).provider, [...MANUAL_PAYMENT_PROVIDERS]),
+        lt((payments as any).updatedAt, cutoff)
       ))
   );
 

backend/src/lib/holdRecovery.ts

@@ -1,10 +1,15 @@
-// Shared capacity-checked recovery for on-hold bookings.
+// Shared capacity-checked recovery for released bookings.
 //
-// When a booking is put on hold, its ticket(s) drop out of the capacity-counting
-// statuses ('pending', 'confirmed', 'checked_in'), releasing the seat. Recovering
-// an on-hold booking (user "I've paid" again, or an admin reactivating / marking it
-// paid) must atomically re-check that the event still has room before re-reserving
-// the seat, exactly like the original booking-creation flow in routes/tickets.ts.
+// When a booking is put on hold (or failed/cancelled), its ticket(s) drop out of the
+// capacity-counting statuses ('pending', 'confirmed', 'checked_in'), releasing the seat.
+// Recovering such a booking (user "I've paid" again, or an admin reactivating / marking
+// it paid / reopening a failed payment) must atomically re-check that the event still has
+// room before re-reserving the seat, exactly like the original booking-creation flow in
+// routes/tickets.ts.
+//
+// Callers choose which ticket statuses are eligible to be re-reserved via
+// `options.fromTicketStatuses` (default ['on_hold']); tickets not in that list — and
+// tickets that already hold a seat — are left untouched and don't consume capacity.
 
 import { eq, and, inArray, sql } from 'drizzle-orm';
 import { db, dbGet, tickets, payments, events, isSqlite } from '../db/index.js';
@@ -19,22 +24,29 @@ export class HoldCapacityError extends Error {
 interface ReserveOptions {
   paidByAdminId?: string;
   extraPaymentFields?: Record<string, any>;
+  /** Ticket statuses eligible to be flipped to targetTicketStatus. Default: ['on_hold']. */
+  fromTicketStatuses?: Array<'on_hold' | 'cancelled' | 'pending'>;
 }
 
 /**
- * Re-reserve seats for a group of on-hold tickets (e.g. all tickets sharing a
+ * Re-reserve seats for a group of released tickets (e.g. all tickets sharing a
  * bookingId), atomically re-checking capacity before flipping their status.
- * Throws HoldCapacityError if the event no longer has room for ticketIds.length seats.
+ * Only tickets whose current status is in `fromTicketStatuses` are flipped.
+ * Capacity is asserted against the number of those tickets that don't already
+ * hold a seat, so re-reserving tickets that are already seated is a no-op.
+ * Throws HoldCapacityError if the event no longer has room.
  */
 export async function reserveOnHoldBooking(
   eventId: string,
   ticketIds: string[],
   targetTicketStatus: 'pending' | 'confirmed',
-  targetPaymentStatus: 'pending_approval' | 'paid',
+  targetPaymentStatus: 'pending_approval' | 'paid' | 'pending',
   options: ReserveOptions = {}
 ): Promise<void> {
   if (ticketIds.length === 0) return;
 
+  const fromTicketStatuses = options.fromTicketStatuses ?? ['on_hold'];
+
   const event = await dbGet<any>(
     (db as any).select().from(events).where(eq((events as any).id, eventId))
   );
@@ -53,12 +65,15 @@ export async function reserveOnHoldBooking(
     if (options.paidByAdminId) paymentUpdate.paidByAdminId = options.paidByAdminId;
   }
 
-  const assertCapacity = (reserved: number) => {
+  // `needed` is how many of these tickets don't currently hold a seat and so must
+  // be found new capacity; tickets already in a seat-holding status cost nothing.
+  const assertCapacity = (reserved: number, needed: number) => {
+    if (needed <= 0) return;
     if (isEventSoldOut(event.capacity, reserved)) {
       throw new HoldCapacityError(0);
     }
     const seatsLeft = calculateAvailableSeats(event.capacity, reserved);
-    if (ticketIds.length > seatsLeft) {
+    if (needed > seatsLeft) {
       throw new HoldCapacityError(seatsLeft);
     }
   };
@@ -73,13 +88,21 @@ export async function reserveOnHoldBooking(
           sql`${(tickets as any).status} IN ('pending', 'confirmed', 'checked_in')`
         ))
         .get();
-      assertCapacity(Number(countRow?.count || 0));
+      const neededRow = tx
+        .select({ count: sql<number>`count(*)` })
+        .from(tickets)
+        .where(and(
+          inArray((tickets as any).id, ticketIds),
+          sql`${(tickets as any).status} NOT IN ('pending', 'confirmed', 'checked_in')`
+        ))
+        .get();
+      assertCapacity(Number(countRow?.count || 0), Number(neededRow?.count || 0));
 
       tx.update(tickets)
         .set({ status: targetTicketStatus })
         .where(and(
           inArray((tickets as any).id, ticketIds),
-          eq((tickets as any).status, 'on_hold')
+          inArray((tickets as any).status, fromTicketStatuses)
         ))
         .run();
 
@@ -99,13 +122,22 @@ export async function reserveOnHoldBooking(
             sql`${(tickets as any).status} IN ('pending', 'confirmed', 'checked_in')`
           ))
       );
-      assertCapacity(Number(countRow?.count || 0));
+      const neededRow = await dbGet<any>(
+        tx
+          .select({ count: sql<number>`count(*)` })
+          .from(tickets)
+          .where(and(
+            inArray((tickets as any).id, ticketIds),
+            sql`${(tickets as any).status} NOT IN ('pending', 'confirmed', 'checked_in')`
+          ))
+      );
+      assertCapacity(Number(countRow?.count || 0), Number(neededRow?.count || 0));
 
       await tx.update(tickets)
         .set({ status: targetTicketStatus })
         .where(and(
           inArray((tickets as any).id, ticketIds),
-          eq((tickets as any).status, 'on_hold')
+          inArray((tickets as any).status, fromTicketStatuses)
         ));
 
       await tx.update(payments)

backend/src/lib/holdSweep.ts

@@ -1,17 +1,23 @@
-// Auto-hold stale pending-approval bookings.
+// Auto-hold stale manual-payment bookings.
 //
-// A payment enters 'pending_approval' when a user clicks "I've paid" on a manual
-// payment method (bank transfer / TPago) and is waiting for an admin to review it.
-// If no admin acts within HOLD_THRESHOLD_HOURS, this job silently moves the payment
-// (and its ticket) to 'on_hold', which drops it out of the capacity-counting statuses
-// ('pending', 'confirmed', 'checked_in') and so releases the seat back to the event.
-// The user receives no notification — they can recover via "I've paid" again, and an
-// admin can reactivate or mark it paid directly, both re-checking capacity.
+// This job releases the seat held by an abandoned manual-payment booking (bank
+// transfer / TPago / cash) after HOLD_THRESHOLD_HOURS. It covers two states, both of
+// which keep a seat reserved while awaiting a human:
+//   - 'pending_approval': the user clicked "I've paid" and is waiting for an admin.
+//   - 'pending' on a manual provider (see MANUAL_PAYMENT_PROVIDERS): the booking was
+//     never settled (these are exempt from the 30-min auto-fail in bookingCleanup.ts,
+//     so this is their only seat-release path).
+// In either case the payment (and its ticket) is silently moved to 'on_hold', which
+// drops it out of the capacity-counting statuses ('pending', 'confirmed', 'checked_in')
+// and so releases the seat back to the event. The user receives no notification — they
+// can recover via "I've paid" again, and an admin can reactivate or mark it paid
+// directly, both re-checking capacity.
 
-import { and, eq, lt, inArray } from 'drizzle-orm';
+import { and, or, eq, lt, inArray } from 'drizzle-orm';
 import { db, dbAll, tickets, payments } from '../db/index.js';
 import { getNow, toDbDate } from './utils.js';
 import { getLock } from './stores/lock.js';
+import { MANUAL_PAYMENT_PROVIDERS } from './manualProviders.js';
 
 function getThresholdMs(): number {
   const hours = parseInt(process.env.HOLD_THRESHOLD_HOURS || '72', 10);
@@ -19,7 +25,8 @@ function getThresholdMs(): number {
 }
 
 /**
- * Move stale pending-approval payments (and their tickets) to 'on_hold'.
+ * Move stale awaiting-verification payments (and their tickets) to 'on_hold'.
+ * Covers 'pending_approval' payments and 'pending' payments on manual providers.
  * Returns the number of payments put on hold.
  */
 export async function sweepStaleApprovals(): Promise<number> {
@@ -33,7 +40,13 @@ export async function sweepStaleApprovals(): Promise<number> {
       })
       .from(payments)
       .where(and(
-        eq((payments as any).status, 'pending_approval'),
+        or(
+          eq((payments as any).status, 'pending_approval'),
+          and(
+            eq((payments as any).status, 'pending'),
+            inArray((payments as any).provider, [...MANUAL_PAYMENT_PROVIDERS])
+          )
+        ),
         lt((payments as any).createdAt, cutoff)
       ))
   );
@@ -59,7 +72,7 @@ export async function sweepStaleApprovals(): Promise<number> {
       ));
   }
 
-  console.log(`[HoldSweep] Put ${stale.length} stale pending-approval payment(s) on hold.`);
+  console.log(`[HoldSweep] Put ${stale.length} stale awaiting-verification payment(s) on hold.`);
   return stale.length;
 }
 

backend/src/lib/manualProviders.ts

@@ -0,0 +1,7 @@
+// Payment providers that require a human to verify the money arrived.
+//
+// These methods (bank transfer / TPago / cash) are never auto-confirmed and —
+// crucially — are never auto-failed by the stale-booking cleanup: an admin settles
+// them by hand. Note this is broader than the set of methods that expose an online
+// "I've paid" step (bank transfer / TPago only); cash is settled at the door.
+export const MANUAL_PAYMENT_PROVIDERS = ['bank_transfer', 'tpago', 'cash'] as const;