diff --git a/docker-compose.it.yaml b/docker-compose.it.yaml index 3fb1b16a2..8bb341f0b 100644 --- a/docker-compose.it.yaml +++ b/docker-compose.it.yaml @@ -163,3 +163,8 @@ services: # RABBITMQ_ENABLED stays "false" (base stack) — it only gates the SMS/email # clients, which must remain off. The payment consumer is wired by # PAYMENT_RABBITMQ_URL alone. + # Drain tail on every pay window. Production defaults to 5 minutes; a + # reservation here lives ~60s, so 5 would push every natural expiry past + # the suite's 180s timeouts. One minute keeps the tail real and observable + # (src/expired-invoice-late-settle.it.ts asserts both sides of it). + FREIGHT_PAYMENT_DRAIN_MINUTES: "1" diff --git a/integration/README.md b/integration/README.md index b35d682ce..9798fd7f8 100644 --- a/integration/README.md +++ b/integration/README.md @@ -57,6 +57,7 @@ gateway-mock-it`) — the code is a read-only mount, not baked into an image. | `src/concurrency.it.ts` | duplicate callbacks, two intents on one invoice, wagon budget, pay-window gate | | `src/authz.it.ts` | cross-tenant isolation, login audience, service-token gates | | `src/cbe-bill.it.ts` | inbound CBE Unified Bill: token → query (hops into freight) → payment | +| `src/expired-invoice-late-settle.it.ts` | pay-window drain tail; a settlement landing after the hold expired still pays the invoice and revives the booking | | `src/bulk-import-full-train.it.ts` | six wheat bookings fill 54 wagons, then gate pass → T1 → dispatch → corridor → arrival → customs tail | | `src/bulk-import-waiting-expiry.it.ts` | exact-fill trio selected, waiting three expire with the day | | `src/bulk-import-split-promote.it.ts` | partial offer, split on settlement, expiry promotion, exact-remainder rebooking | @@ -131,6 +132,16 @@ what it actually does and says so in a comment, so a fix fails loudly: the phased path (`customs_clearing_enabled` on the contract) avoids it — which is why S8's customs tenant is Path B. +- **A live intent makes a hold unexpirable here** (`expired-invoice-late-settle`). + Reconcile-before-expire live-queries every non-FAILED intent; the mock answers + `PROCESSING` for an unpaid order, which the payment API reads as "money in + flight" → `unverifiable` → never expire on unknown. So while a CBE Birr intent + is open, NOTHING retires the reservation — not the settle tick, not the staff + `bookings/:id/expire` override (it runs the same guard). Producing the + expired-invoice-with-a-payment case therefore needs the intent retired first + (`status = 'FAILED'`, which reconcile skips), after which a webhook still + late-captures it (`applyProviderResult`). + ## Gotchas - **One unpaid hold per company.** `assertNoUnpaidHold` blocks a company with a diff --git a/integration/src/expired-invoice-late-settle.it.ts b/integration/src/expired-invoice-late-settle.it.ts new file mode 100644 index 000000000..6fb57f8c0 --- /dev/null +++ b/integration/src/expired-invoice-late-settle.it.ts @@ -0,0 +1,175 @@ +/** + * The swallowed late payment + * (docs/dev-testing/finding-expired-invoice-swallows-payment.md). + * + * Settlement is asynchronous — customer taps pay → provider confirms → payment + * API enqueues → relay delivers — so the success can land after the booking + * window cron has already expired the invoice. Before the fix the money simply + * vanished: the intent flipped to SUCCEEDED, `settleByPaymentId` matched no OPEN + * invoice and returned null, the relay was told `processed: true`, and the + * customer was left debited with an EXPIRED invoice and an EXPIRED booking. + * + * Two layers are asserted here: + * 1. the drain tail — a deadline that just passed does NOT expire anything + * (FREIGHT_PAYMENT_DRAIN_MINUTES, 1 in this stack); + * 2. the backstop — a settlement that lands after the drain is treated exactly + * like an in-window payment: invoice PAID, booking PAID. + */ +import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { apiOk, chief, closeDb, db, gateway, poll } from "./client"; +import { + createImportSchedule, + currentInvoice, + departureAt, + ensureCorridorRoute, + forceWindowOpen, + freightPayment, + gatewayIntent, + invoiceForBooking, + payInvoice, + prepareBooking, + releaseUnpaidHolds, + resetCorridorDay, + runBatch, + type ReadyBooking, +} from "./flows"; + +const DEPARTURE = departureAt(4); +const STAMP = String(Date.now()); + +/** The stack runs a 1-minute tail (FREIGHT_PAYMENT_DRAIN_MINUTES, docker-compose.it.yaml). */ +const DRAIN_MS = 60_000; +/** Long enough for several 10s settle ticks, comfortably inside the tail. */ +const INSIDE_TAIL_MS = DRAIN_MS * 0.4; + +const bookingRow = async (id: string) => + ( + await db<{ status: string; payment_status: string; train_schedule_id: string | null }>( + `SELECT status, payment_status, train_schedule_id FROM freight.bookings WHERE id = $1`, + [id], + ) + )[0]; + +describe("a payment that lands after the pay window is not swallowed", () => { + let booking: ReadyBooking; + let invoiceId: string; + + beforeAll(async () => { + await gateway.reset(); + await releaseUnpaidHolds(); + await ensureCorridorRoute(); + await resetCorridorDay(DEPARTURE); + const schedule = await createImportSchedule({ departure: DEPARTURE }); + await forceWindowOpen(schedule.id, 45); + + booking = await prepareBooking({ + suffix: "LATE1", + departure: DEPARTURE, + runStamp: STAMP, + isoSeed: 0, + twenty: 2, + }); + await runBatch(schedule.id); + invoiceId = (await invoiceForBooking(booking.bookingId)).id; + }); + + afterAll(closeDb); + + it("holds the reservation while the drain tail runs", async () => { + // Deadline just behind now(): the settle tick sees it every 10s and must + // leave it alone — this is the customer who tapped pay in the last seconds. + await db( + `UPDATE freight.bookings SET payment_deadline = now() - interval '5 seconds' + WHERE id = $1`, + [booking.bookingId], + ); + + await new Promise((r) => setTimeout(r, INSIDE_TAIL_MS)); + + const held = await bookingRow(booking.bookingId); + expect(held.status).toBe("SELECTED_FOR_BATCH"); + // The wagons are still HIS — releasing them at the raw deadline would sell + // them to the next customer while his settlement is still in flight. + expect(held.train_schedule_id).toBeTruthy(); + expect((await currentInvoice(invoiceId)).status).not.toBe("EXPIRED"); + }, 60_000); + + it("expires the hold while the customer's payment is still in flight", async () => { + // Open the intent first — payInvoice refuses once dueAt is behind us, which + // is the point: no NEW payment may start, only an in-flight one may land. + const res = await payInvoice(invoiceId, { method: "CBE_BIRR" }); + expect(res.status, JSON.stringify(res.body)).toBeLessThanOrEqual(201); + + const intent = await gatewayIntent(booking.bookingId); + expect(intent.status).toBe("REQUIRES_ACTION"); + expect((await currentInvoice(invoiceId)).payment_id).toBeTruthy(); + + // The provider reported a failure and we retired the intent — the same shape + // the payment API's own sweep writes. This is what makes the hold expirable: + // reconcile-before-expire skips FAILED candidates (intents.service.ts:412) + // and answers a clean "not paid". While the intent is live the mock answers + // PROCESSING → `unverifiable` → NOTHING expires the hold, not the settle tick + // and not staff. The money can still land afterwards: `applyProviderResult` + // registers a late capture on a retired intent (intents.service.ts:576). + await db( + `UPDATE edr_payment.payment_intent SET status = 'FAILED' WHERE id = $1`, + [intent.id], + ); + await apiOk(chief, "post", `/api/train-scheduling/bookings/${booking.bookingId}/expire`); + + const expired = await poll<{ status: string }>( + "invoice EXPIRED with the hold", + `SELECT status FROM freight.invoices WHERE id = $1`, + [invoiceId], + (row) => row?.status === "EXPIRED", + { attempts: 20, intervalMs: 2000 }, + ); + expect(expired.status).toBe("EXPIRED"); + expect((await bookingRow(booking.bookingId)).status).toBe("EXPIRED"); + // The settlement correlation key survives the expiry — `paymentId` is what + // settleByPaymentId looks the invoice up by when the money finally lands. + const linked = (await currentInvoice(invoiceId)).payment_id!; + expect((await freightPayment(linked)).merchant_order_id).toBe( + intent.merchant_order_id, + ); + }, 180_000); + + it("settles the EXPIRED invoice and revives the booking when the money lands", async () => { + const intent = await gatewayIntent(booking.bookingId); + const res = await gateway.webhook({ merchantOrderId: intent.merchant_order_id }); + expect(res.body.delivered).toBe(200); + + const settled = await poll<{ status: string }>( + "payment intent SUCCEEDED", + `SELECT status FROM edr_payment.payment_intent WHERE id = $1`, + [intent.id], + (row) => row?.status === "SUCCEEDED", + { attempts: 20, intervalMs: 1000 }, + ); + expect(settled.status).toBe("SUCCEEDED"); + + // The bug: this row used to sit at EXPIRED with paid_amount null forever, + // because settleByPaymentId only matched OPEN_STATUSES. + const invoice = await poll<{ status: string; paid_amount: string; balance_amount: string }>( + "expired invoice settled by the late payment", + `SELECT status, paid_amount, balance_amount FROM freight.invoices WHERE id = $1`, + [invoiceId], + (row) => row?.status === "PAID", + { attempts: 30, intervalMs: 2000 }, + ); + expect(Number(invoice.paid_amount)).toBeGreaterThan(0); + expect(Number(invoice.balance_amount)).toBe(0); + + // …and the second swallow point: advanceBookingOnPayment used to refuse an + // EXPIRED booking outright, so the money landed on a booking that stayed dead. + const revived = await poll<{ status: string }>( + "expired booking revived by the late payment", + `SELECT status FROM freight.bookings WHERE id = $1`, + [booking.bookingId], + (row) => row?.status === "PAID", + { attempts: 30, intervalMs: 2000 }, + ); + expect(revived.status).toBe("PAID"); + expect((await bookingRow(booking.bookingId)).payment_status).toBe("PAID"); + }, 180_000); +});