diff --git a/apps/edr-freight-api/.env.example b/apps/edr-freight-api/.env.example index d6a39bfed..9338a958f 100644 --- a/apps/edr-freight-api/.env.example +++ b/apps/edr-freight-api/.env.example @@ -30,6 +30,12 @@ FREIGHT_PORTAL_URL=http://localhost:5173 # Point these at the freight portal's public payment result routes. PAYMENT_RETURN_URL=http://localhost:5173/payment/success PAYMENT_FAILURE_URL=http://localhost:5173/payment/failure + +# Drain tail (minutes) added to every booking pay window before anything expires: +# settlement is asynchronous, so a payment made in the window's last seconds lands +# after the deadline. Nothing is expired, no wagons are resold and no window cycle +# concludes until the tail passes. Defaults to 5 when unset. +FREIGHT_PAYMENT_DRAIN_MINUTES=5 # JWT (used by @tria-plc/api-common SharedAuthModule) JWT_SECRET= JWT_ACCESS_TOKEN_SECRET= diff --git a/apps/edr-freight-api/src/modules/billing/billing.service.spec.ts b/apps/edr-freight-api/src/modules/billing/billing.service.spec.ts index dbf2bd4fd..1356f8cfa 100644 --- a/apps/edr-freight-api/src/modules/billing/billing.service.spec.ts +++ b/apps/edr-freight-api/src/modules/billing/billing.service.spec.ts @@ -203,6 +203,129 @@ describe("BillingService.markInvoiceAsPaid", () => { }); }); +/** + * A gateway success can land after the pay window AND its drain tail (relay + * backlog, payment-api restart, a CBE bill paid at a counter). The money is + * captured either way, so the settle lookup must accept an EXPIRED invoice — + * matching only OPEN_STATUSES used to drop it silently, leaving a debited + * customer with an EXPIRED invoice, an EXPIRED booking and no alert. + */ +describe("BillingService.settleByPaymentId", () => { + function serviceFor(invoice: Record | null) { + const mg = { + findOne: jest.fn().mockResolvedValue(invoice), + update: jest.fn().mockResolvedValue(undefined), + }; + const events = makeEvents(); + // The lookup is by paymentId ALONE — status is judged on the resolved row, + // so a stale capture can never skip past a newer invoice to an older one. + const findOne = jest.fn(({ where }: { where: Record }) => { + expect(where).toEqual({ paymentId: "pay-1" }); + return Promise.resolve(invoice); + }); + const dataSource = { + getRepository: () => ({ findOne }), + transaction: (cb: (mg: unknown) => unknown) => cb(mg), + manager: mg, + }; + const service = new BillingService( + dataSource as never, + {} as never, + {} as never, + events as never, + {} as never, // payment + {} as never, // companies + {} as never, // invoiceDocuments + ); + return { service, mg, events }; + } + + it("settles an EXPIRED invoice — the money was already captured", async () => { + const { service, mg, events } = serviceFor({ + id: "inv-1", + status: Freight.InvoiceStatus.Expired, + source: "booking", + sourceId: "booking-1", + totalAmount: 1500, + paidAt: null, + }); + + const settled = await service.settleByPaymentId("pay-1", "txn-1"); + + expect(settled?.status).toBe(Freight.InvoiceStatus.Paid); + expect(mg.update).toHaveBeenCalledWith( + expect.anything(), + { id: "inv-1" }, + expect.objectContaining({ + status: Freight.InvoiceStatus.Paid, + paidAmount: 1500, + balanceAmount: 0, + }), + ); + // The domain reacts to this — it is what revives the expired booking. + expect(events.emitAsync).toHaveBeenCalledWith( + "booking.invoice.paid", + expect.objectContaining({ invoiceId: "inv-1" }), + ); + }); + + it("still settles an open (PENDING) invoice", async () => { + const { service, mg } = serviceFor({ + id: "inv-1", + status: Freight.InvoiceStatus.Pending, + source: "booking", + sourceId: "booking-1", + totalAmount: 1500, + paidAt: null, + }); + + await service.settleByPaymentId("pay-1"); + + expect(mg.update).toHaveBeenCalled(); + }); + + /** + * `upsertIntent` keeps ONE local payments row per booking reference, so every + * invoice the booking was ever charged on carries the same `paymentId`. A + * capture from a lapsed first attempt must not reach back past the invoice the + * customer actually paid and settle the older EXPIRED one — that would mark two + * invoices paid off a single payment. + */ + it("no-ops when the booking's newest invoice is already PAID", async () => { + const { service, mg, events } = serviceFor({ + id: "inv-2", + status: Freight.InvoiceStatus.Paid, + source: "booking", + sourceId: "booking-1", + totalAmount: 1500, + }); + + expect(await service.settleByPaymentId("pay-1")).toBeNull(); + expect(mg.update).not.toHaveBeenCalled(); + expect(events.emitAsync).not.toHaveBeenCalled(); + }); + + it.each([ + ["CANCELLED", Freight.InvoiceStatus.Cancelled], + ["REFUNDED", Freight.InvoiceStatus.Refunded], + ])("does not settle a %s invoice — that is a refund case", async ( + _label, + status, + ) => { + const { service, mg, events } = serviceFor({ + id: "inv-1", + status, + source: "booking", + sourceId: "booking-1", + totalAmount: 1500, + }); + + expect(await service.settleByPaymentId("pay-1")).toBeNull(); + expect(mg.update).not.toHaveBeenCalled(); + expect(events.emitAsync).not.toHaveBeenCalled(); + }); +}); + describe("BillingService.recordPayment", () => { function serviceFor(invoice: Record | null) { const mg = { diff --git a/apps/edr-freight-api/src/modules/billing/billing.service.ts b/apps/edr-freight-api/src/modules/billing/billing.service.ts index 501bc8ee8..bd2355df5 100644 --- a/apps/edr-freight-api/src/modules/billing/billing.service.ts +++ b/apps/edr-freight-api/src/modules/billing/billing.service.ts @@ -1173,6 +1173,25 @@ export class BillingService { * `paymentId`, marks it paid, and emits `${source}.invoice.paid` for the domain * to advance on. Idempotent — no-op when no open invoice is linked (already * settled, or settled inline by {@link payInvoice}). + * + * EXPIRED is settleable HERE and only here: this is the gateway path, so the + * money is already captured and we are recording a fait accompli. A success can + * land after the pay window plus its drain tail (relay backlog, payment-api + * restart, a CBE bill paid at a counter) — matching only `OPEN_STATUSES` used to + * drop it silently, leaving a debited customer with an EXPIRED invoice and no + * alert. The manual/offline path ({@link recordPayment}) keeps its EXPIRED guard: + * a teller must not accept cash against a lapsed invoice. + * + * The status is checked on the RESOLVED invoice, never inside the lookup. + * `paymentId` is freight's local intent projection, and `upsertIntent` keeps ONE + * row per booking reference across every pay attempt — so a booking that was + * re-invoiced after a lapsed attempt has SEVERAL invoices carrying the same + * `paymentId`. Filtering by status inside the query would let a late capture from + * attempt 1 skip past the already-PAID attempt-2 invoice and settle the older + * EXPIRED one, marking two invoices paid off a single capture. Resolving the + * newest invoice first and then asking whether IT is settleable makes the answer + * "this booking's money is already recorded" instead. CANCELLED/REFUNDED are + * refund cases, not settlements, and are logged rather than settled. */ async settleByPaymentId( paymentId: string, @@ -1180,11 +1199,31 @@ export class BillingService { paidAt?: Date, ): Promise { const invoice = await this.dataSource.getRepository(Invoice).findOne({ - where: { paymentId, status: In(OPEN_STATUSES) }, - order: { issuedAt: "DESC" }, + where: { paymentId }, + // NULLS LAST: a DRAFT invoice has no issuedAt and Postgres sorts NULLs + // first on DESC, which would hand back an unissued invoice. + order: { issuedAt: { direction: "DESC", nulls: "LAST" } }, }); if (!invoice) return null; + const settleable: Freight.InvoiceStatus[] = [ + ...OPEN_STATUSES, + Freight.InvoiceStatus.Expired, + ]; + if (!settleable.includes(invoice.status)) { + // Already PAID is the ordinary idempotent no-op (redelivery, or settled + // inline by payInvoice). Anything else means money was captured with + // nowhere to land — that needs a person, so say so loudly. + if (invoice.status !== Freight.InvoiceStatus.Paid) { + this.logger.error( + `Payment ${paymentId} succeeded but invoice ${invoice.invoiceNumber} ` + + `(${invoice.id}) is ${invoice.status} — nothing settled. The capture ` + + `needs a refund or a manual settlement.`, + ); + } + return null; + } + return this.markInvoiceAsPaid(invoice.id, paymentId, undefined, { providerTxnId, paidAt, diff --git a/apps/edr-freight-api/src/modules/bookings/booking-invoice.service.ts b/apps/edr-freight-api/src/modules/bookings/booking-invoice.service.ts index 82bb1144e..9212bcdef 100644 --- a/apps/edr-freight-api/src/modules/bookings/booking-invoice.service.ts +++ b/apps/edr-freight-api/src/modules/bookings/booking-invoice.service.ts @@ -102,6 +102,10 @@ export class BookingInvoiceService { ); switch (payload.type) { case "PREPAID": + // Before advancing: if this invoice belonged to a partial offer that + // lapsed before the settlement landed, revive it, or the booking boards + // whole having paid only the offered part. + await this.bookingBatch.reviveOfferForInvoice(payload.invoiceId); await this.advanceBookingOnPayment(payload.sourceId); break; default: @@ -164,15 +168,24 @@ export class BookingInvoiceService { // may have moved on or been terminated between invoicing and settlement. // Only advance one that is still awaiting payment: no-op when already PAID, // and refuse to advance a booking in a terminal/advanced status - // (CANCELLED/REJECTED/EXPIRED or already past the payment gate) so we never - // rewrite its status or re-run allocation. + // (CANCELLED/REJECTED or already past the payment gate) so we never rewrite + // its status or re-run allocation. + // + // EXPIRED is NOT in that list: settlement is async, so a payment can land + // after the pay window and its drain tail (relay backlog, payment-api + // restart, a CBE bill paid at a counter). The money was captured, so it gets + // exactly the same treatment as an in-window payment — the booking becomes + // PAID and ensurePaidBookingAllocated re-places it via + // replaceStrandedPaidBooking (a same-day train with room, or a manual-assign + // log). Leaving EXPIRED here debited the customer for nothing. CANCELLED and + // REJECTED stay: a person terminated those, so a payment against them is a + // refund case, not a boarding. if (booking.paymentStatus === "PAID" || booking.status === "PAID") { return; } const TERMINAL_OR_ADVANCED_STATUSES: string[] = [ "CANCELLED", "REJECTED", - "EXPIRED", "IN_TRANSIT", "ARRIVED", "COMPLETED", diff --git a/apps/edr-freight-api/src/modules/train-scheduling/booking-batch.constants.ts b/apps/edr-freight-api/src/modules/train-scheduling/booking-batch.constants.ts index d43cff60b..45370f546 100644 --- a/apps/edr-freight-api/src/modules/train-scheduling/booking-batch.constants.ts +++ b/apps/edr-freight-api/src/modules/train-scheduling/booking-batch.constants.ts @@ -7,11 +7,51 @@ * (BookingWindowService) drives all timing off that config. */ -export const BATCH_TIMEZONE = 'Africa/Addis_Ababa'; +export const BATCH_TIMEZONE = "Africa/Addis_Ababa"; /** How long before the pay deadline the one reminder notification goes out. */ export const PAYMENT_REMINDER_LEAD_MS = 10 * 60_000; +/** Drain tail applied to every pay window when FREIGHT_PAYMENT_DRAIN_MINUTES is unset. */ +export const DEFAULT_PAYMENT_DRAIN_MINUTES = 7; + +/** + * Drain tail on every pay window, in ms. Read per call so the env var can be + * changed without a rebuild (and so tests can set it). + */ +export function paymentDrainMs(): number { + // An empty/blank value is UNSET, not zero — a bare `FREIGHT_PAYMENT_DRAIN_MINUTES=` + // left in a .env must not silently disable the drain (Number('') is 0). + const raw = process.env.FREIGHT_PAYMENT_DRAIN_MINUTES?.trim(); + const minutes = raw ? Number(raw) : NaN; + return ( + (Number.isFinite(minutes) && minutes >= 0 + ? minutes + : DEFAULT_PAYMENT_DRAIN_MINUTES) * 60_000 + ); +} + +/** + * A pay window AND its drain tail have closed. + * + * Settlement is asynchronous (provider confirm → payment-api → outbox relay), so + * a payment made in the last seconds of the window lands after `paymentDeadline`. + * The drain defers the WHOLE expiry pipeline — wagons stay held, the waiting list + * is not promoted, the window cycle does not conclude — so that settlement still + * has a live booking to land on. A payment that arrives even later is not lost + * either: it settles the expired invoice and revives the booking (see + * BillingService.settleByPaymentId + BookingInvoiceService.advanceBookingOnPayment). + * + * No deadline ⇒ never lapsed; callers decide what an unknown deadline means. + */ +export function payWindowLapsed( + deadline: Date | null | undefined, + now: number, + drainMs: number = paymentDrainMs(), +): boolean { + return deadline != null && deadline.getTime() + drainMs <= now; +} + /** Fallback wagons-per-booking when a booking has no computed `wagonsRequired`. */ export const DEFAULT_WAGONS_PER_BOOKING = 1; diff --git a/apps/edr-freight-api/src/modules/train-scheduling/booking-batch.service.spec.ts b/apps/edr-freight-api/src/modules/train-scheduling/booking-batch.service.spec.ts index 1a1a75275..ce0e77a3e 100644 --- a/apps/edr-freight-api/src/modules/train-scheduling/booking-batch.service.spec.ts +++ b/apps/edr-freight-api/src/modules/train-scheduling/booking-batch.service.spec.ts @@ -1,7 +1,14 @@ import { BookingBatchService } from './booking-batch.service'; +import { paymentDrainMs } from './booking-batch.constants'; import { Booking } from '../bookings/entities/booking.entity'; import { WagonStockLedger } from './wagon-stock-ledger.util'; +/** + * A pay window closed long enough ago to be past its drain tail as well — i.e. + * genuinely expirable. Inside the tail nothing expires (see payWindowLapsed). + */ +const fullyLapsedDeadline = () => new Date(Date.now() - 60_000 - paymentDrainMs()); + describe('BookingBatchService — PAID reconcile', () => { const scheduleId = 'schedule-1'; const bookingId = 'booking-1'; @@ -856,7 +863,7 @@ describe('BookingBatchService — PAID reconcile', () => { // One reservation whose pay window lapsed, and one booking on the waiting list. const lapsed = booking('lapsed', 50, { status: 'SELECTED_FOR_BATCH', - paymentDeadline: new Date(Date.now() - 60_000), + paymentDeadline: fullyLapsedDeadline(), }); const waiting = booking('waiting', 10, { trainScheduleId: null }); @@ -888,10 +895,41 @@ describe('BookingBatchService — PAID reconcile', () => { expect((notifier.payNow.mock.calls[0][0] as Booking).id).toBe('waiting'); }); + it('holds a reservation whose deadline passed but whose drain tail has not', async () => { + // Settlement is asynchronous, so a payment made in the window's last + // seconds lands after the deadline. Expiring here would free the wagons + // out from under it — the swallowed-payment finding. + const draining = booking('draining', 50, { + status: 'SELECTED_FOR_BATCH', + paymentDeadline: new Date(Date.now() - 60_000), + }); + const waiting = booking('waiting', 10, { trainScheduleId: null }); + + bookingsRepository.findReservedForSchedule + .mockResolvedValueOnce([draining]) + .mockResolvedValue([]); + bookingsRepository.findBatchPoolByCorridorDay + .mockResolvedValueOnce([waiting]) + .mockResolvedValue([]); + const byId: Record = { draining, waiting }; + dataSource + .getRepository() + .findOne.mockImplementation( + async (opts: { where?: { id?: string } }) => + byId[opts?.where?.id ?? ''] ?? null, + ); + + await service.settleDueReservations(trainId); + + expect(notifier.expired).not.toHaveBeenCalled(); + // ...and its wagons were NOT handed to the waiting list either. + expect(notifier.payNow).not.toHaveBeenCalled(); + }); + it('serialises concurrent settles so the same reservation is not settled twice', async () => { const lapsed = booking('lapsed', 50, { status: 'SELECTED_FOR_BATCH', - paymentDeadline: new Date(Date.now() - 60_000), + paymentDeadline: fullyLapsedDeadline(), }); // Both callers read the reservation; the lock must stop the second from // acting on rows the first already expired. (The PAYMENT transition and the @@ -922,7 +960,7 @@ describe('BookingBatchService — PAID reconcile', () => { it('never expires a reservation whose payment landed — allocates it instead', async () => { const latePaid = booking('late-paid', 50, { status: 'SELECTED_FOR_BATCH', - paymentDeadline: new Date(Date.now() - 60_000), + paymentDeadline: fullyLapsedDeadline(), }); bookingsRepository.findReservedForSchedule .mockResolvedValueOnce([latePaid]) @@ -1036,7 +1074,7 @@ describe('BookingBatchService — PAID reconcile', () => { ...(waiting as unknown as Record), status: 'SELECTED_FOR_BATCH', trainScheduleId: exportScheduleId, - paymentDeadline: new Date(Date.now() - 60_000), + paymentDeadline: fullyLapsedDeadline(), originYardId: 'yard-a', destinationYardId: 'yard-b', priorityScore: 0, diff --git a/apps/edr-freight-api/src/modules/train-scheduling/booking-batch.service.ts b/apps/edr-freight-api/src/modules/train-scheduling/booking-batch.service.ts index 3e1af07b5..966e051d3 100644 --- a/apps/edr-freight-api/src/modules/train-scheduling/booking-batch.service.ts +++ b/apps/edr-freight-api/src/modules/train-scheduling/booking-batch.service.ts @@ -64,6 +64,8 @@ import { DEFAULT_CONTAINER_WAGON_TARE_TONS, DEFAULT_WAGONS_PER_BOOKING, PAYMENT_REMINDER_LEAD_MS, + payWindowLapsed, + paymentDrainMs, } from "./booking-batch.constants"; import { LocomotiveLimits, @@ -701,6 +703,15 @@ export class BookingBatchService implements OnModuleInit { await this.ensurePaidBookingAllocated(bookingId); } + /** + * A late settlement paid a partial offer whose window had lapsed — bring the + * offer back so `ensurePaidBookingAllocated`'s applySplit still reduces the + * booking to what was actually bought. No-op without the split feature. + */ + async reviveOfferForInvoice(invoiceId: string): Promise { + await this.splitService?.reviveOfferForInvoice(invoiceId); + } + /** * Day-level backstop for stranded PAID bookings: reconcilePaidUnlinked is * keyed on train_schedule_id, so a booking whose hold was expired (schedule @@ -2745,11 +2756,13 @@ export class BookingBatchService implements OnModuleInit { const isPaid = (b: Booking) => b.paymentStatus === "PAID" || b.status === "PAID"; - // Deadline is the line — no fixed slack. A payment that beat the deadline - // but whose webhook is late is caught by expire()'s gateway reconcile. + // The deadline carries a drain tail (payWindowLapsed): settlement is async, + // so a payment made in the window's last seconds lands after it. Nothing is + // expired until the tail passes. expire()'s gateway reconcile is the second + // line of defence, not the first. const isExpired = (b: Booking) => b.paymentDeadline - ? b.paymentDeadline.getTime() <= now + ? payWindowLapsed(b.paymentDeadline, now) : expireUnpaidUnknownDeadline; for (const booking of reserved) { @@ -4748,11 +4761,12 @@ export class BookingBatchService implements OnModuleInit { const allocated = (schedule.scheduleBookings ?? []) .map((sb) => sb.booking) .filter((b): b is Booking => Boolean(b)); - // Lazy-expiry guard: a hold whose deadline lapsed no longer blocks - // capacity, even before the 10s sweep flips it to EXPIRED — availability - // shown to the next customer is honest between ticks. A late capture the - // gateway reconcile later confirms lands as PAID and, if the wagons went - // meanwhile, degrades to WAITING_FOR_WAGON for manual placement. + // Lazy-expiry guard: a hold whose deadline AND drain tail lapsed no longer + // blocks capacity, even before the 10s sweep flips it to EXPIRED — + // availability shown to the next customer is honest between ticks. The drain + // has to be honoured here too: releasing the wagons at the raw deadline + // would resell them to someone else while the paying customer's settlement + // is still in flight, stranding it into WAITING_FOR_WAGON. const deadlineCutoff = Date.now(); const reserved = ( await this.bookingsRepository.findReservedForSchedule(schedule.id) @@ -4760,8 +4774,7 @@ export class BookingBatchService implements OnModuleInit { (b) => b.paymentStatus === "PAID" || b.status === "PAID" || - b.paymentDeadline == null || - b.paymentDeadline.getTime() > deadlineCutoff, + !payWindowLapsed(b.paymentDeadline, deadlineCutoff), ); for (const b of [...allocated, ...reserved]) { budget.subtract( @@ -4836,7 +4849,9 @@ export class BookingBatchService implements OnModuleInit { } /** - * A reservation on this schedule still has time left to pay. + * A reservation on this schedule still has time left to pay — including its + * drain tail, so the cycle cannot conclude out from under a settlement that is + * still in flight. * * The PAYMENT phase ends a hair BEFORE its own reservations do: `paymentPhaseEndsAt` * is stamped when the phase starts, then `reserve()` gives each booking @@ -4857,7 +4872,7 @@ export class BookingBatchService implements OnModuleInit { b.paymentStatus !== "PAID" && b.status !== "PAID" && b.paymentDeadline != null && - b.paymentDeadline.getTime() > now, + !payWindowLapsed(b.paymentDeadline, now), ); } @@ -5066,7 +5081,10 @@ export class BookingBatchService implements OnModuleInit { */ private armSettle(scheduleId: string): void { void this.scheduleById(scheduleId) + // + drain tail: firing at the raw deadline is a guaranteed no-op pass now + // that nothing expires until the tail passes. .then((schedule) => this.paymentWindowMsFor(schedule)) + .then((windowMs: number) => windowMs + paymentDrainMs()) .then((delayMs: number) => { this.removeTimeout(scheduleId); const handle = setTimeout(() => { diff --git a/apps/edr-freight-api/src/modules/train-scheduling/booking-split.service.ts b/apps/edr-freight-api/src/modules/train-scheduling/booking-split.service.ts index 9121e4841..ba4f08552 100644 --- a/apps/edr-freight-api/src/modules/train-scheduling/booking-split.service.ts +++ b/apps/edr-freight-api/src/modules/train-scheduling/booking-split.service.ts @@ -302,6 +302,19 @@ export class BookingSplitService { .update({ bookingId, status: 'OFFERED' }, { status: 'EXPIRED' }); } + /** + * A late gateway settlement paid the invoice of an offer whose window had + * already lapsed. The customer bought the offered part, so the offer is live + * again and {@link applySplit} must reduce the booking to it — otherwise the + * booking boards WHOLE having paid only the offered portion. Keyed on the paid + * invoice, never on the booking: an offer that simply timed out stays dead. + */ + async reviveOfferForInvoice(invoiceId: string): Promise { + await this.dataSource + .getRepository(BookingBatchOffer) + .update({ invoiceId, status: 'EXPIRED' }, { status: 'OFFERED' }); + } + async findOpenOffer(bookingId: string): Promise { return this.dataSource.getRepository(BookingBatchOffer).findOne({ where: { bookingId, status: 'OFFERED' }, diff --git a/apps/edr-freight-api/src/modules/train-scheduling/pay-window-drain.spec.ts b/apps/edr-freight-api/src/modules/train-scheduling/pay-window-drain.spec.ts new file mode 100644 index 000000000..6054f28b2 --- /dev/null +++ b/apps/edr-freight-api/src/modules/train-scheduling/pay-window-drain.spec.ts @@ -0,0 +1,67 @@ +import { + DEFAULT_PAYMENT_DRAIN_MINUTES, + paymentDrainMs, + payWindowLapsed, +} from "./booking-batch.constants"; + +/** + * The drain tail is what keeps a pay window's LAST payment from being thrown + * away: settlement is asynchronous (provider confirm → payment-api → outbox + * relay), so a payment made in the window's final seconds lands after + * `paymentDeadline`. Nothing may expire, no wagons may be resold and no window + * cycle may conclude until the tail has passed. + */ +describe("payWindowLapsed — pay-window drain tail", () => { + const deadline = new Date("2026-08-02T22:04:41Z"); + const at = (offsetMs: number) => deadline.getTime() + offsetMs; + const MIN = 60_000; + + afterEach(() => { + delete process.env.FREIGHT_PAYMENT_DRAIN_MINUTES; + }); + + it("is not lapsed before the deadline", () => { + expect(payWindowLapsed(deadline, at(-1 * MIN))).toBe(false); + }); + + it("is not lapsed inside the drain tail", () => { + // The reproduced finding: settled ~7 minutes late. With the default 5-minute + // tail the booking is still live at 4 minutes. + expect(payWindowLapsed(deadline, at(4 * MIN))).toBe(false); + expect(payWindowLapsed(deadline, at(DEFAULT_PAYMENT_DRAIN_MINUTES * MIN - 1))).toBe( + false, + ); + }); + + it("is lapsed once the tail passes", () => { + expect(payWindowLapsed(deadline, at(DEFAULT_PAYMENT_DRAIN_MINUTES * MIN))).toBe( + true, + ); + expect(payWindowLapsed(deadline, at(7 * MIN))).toBe(true); + }); + + it("never lapses without a deadline — callers decide what unknown means", () => { + expect(payWindowLapsed(null, at(60 * MIN))).toBe(false); + expect(payWindowLapsed(undefined, at(60 * MIN))).toBe(false); + }); + + it("honours FREIGHT_PAYMENT_DRAIN_MINUTES", () => { + process.env.FREIGHT_PAYMENT_DRAIN_MINUTES = "20"; + expect(paymentDrainMs()).toBe(20 * MIN); + expect(payWindowLapsed(deadline, at(10 * MIN))).toBe(false); + expect(payWindowLapsed(deadline, at(20 * MIN))).toBe(true); + }); + + it("allows an explicit zero drain (old deadline-is-the-line behaviour)", () => { + process.env.FREIGHT_PAYMENT_DRAIN_MINUTES = "0"; + expect(paymentDrainMs()).toBe(0); + expect(payWindowLapsed(deadline, at(0))).toBe(true); + }); + + it("falls back to the default on garbage or negative values", () => { + for (const bad of ["", "abc", "-3"]) { + process.env.FREIGHT_PAYMENT_DRAIN_MINUTES = bad; + expect(paymentDrainMs()).toBe(DEFAULT_PAYMENT_DRAIN_MINUTES * MIN); + } + }); +}); 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); +});