Merge pull request #1103 from Tria-plc/fixes

Fix: add the payment drain tail

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=

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<string, unknown> | 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<string, unknown> }) => {
+      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<string, unknown> | null) {
     const mg = {

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<Invoice | null> {
     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,

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",

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;
 

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<string, Booking> = { 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<string, unknown>),
         status: 'SELECTED_FOR_BATCH',
         trainScheduleId: exportScheduleId,
-        paymentDeadline: new Date(Date.now() - 60_000),
+        paymentDeadline: fullyLapsedDeadline(),
         originYardId: 'yard-a',
         destinationYardId: 'yard-b',
         priorityScore: 0,

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<void> {
+    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(() => {

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<void> {
+    await this.dataSource
+      .getRepository(BookingBatchOffer)
+      .update({ invoiceId, status: 'EXPIRED' }, { status: 'OFFERED' });
+  }
+
   async findOpenOffer(bookingId: string): Promise<BookingBatchOffer | null> {
     return this.dataSource.getRepository(BookingBatchOffer).findOne({
       where: { bookingId, status: 'OFFERED' },

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);
+    }
+  });
+});

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"

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

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);
+});