From acb6f286d4a501a300010124a713ebb609b1928a Mon Sep 17 00:00:00 2001 From: Marshal Date: Thu, 30 Jul 2026 12:01:59 +0000 Subject: [PATCH] fix(payments): reconcile with gateway before expiring unpaid holds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the fixed 5-minute grace with a settlement check at expiry: expire() calls the payment API's reconcile endpoint — paid intents are kept and allocated via payment.succeeded, unverifiable results defer expiry to the next tick, only verifiably unpaid holds expire. --- .../src/modules/billing/billing.service.ts | 17 +++++-- .../modules/payment/payment-client.service.ts | 18 +++++++ .../src/modules/payment/payment.service.ts | 24 +++++++++ .../booking-batch.constants.ts | 8 --- .../booking-batch.service.spec.ts | 38 ++++++++++---- .../train-scheduling/booking-batch.service.ts | 51 ++++++++++++++----- .../booking-split.service.spec.ts | 7 ++- .../booking-window.service.ts | 9 ++-- 8 files changed, 130 insertions(+), 42 deletions(-) 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 da5886b81..ca43295d1 100644 --- a/apps/edr-freight-api/src/modules/billing/billing.service.ts +++ b/apps/edr-freight-api/src/modules/billing/billing.service.ts @@ -983,6 +983,17 @@ export class BillingService { * never fire before the link exists. Throws when the invoice is not found or * not in an open/payable status. */ + /** + * Settlement check before expiring a payable order (reconcile-before-expire): + * live-queries the gateway for any settled intent on the source order. Kept + * on billing so the domain never talks to the payment service directly. + */ + reconcilePayable( + sourceId: string, + ): Promise<{ paid: boolean; unverifiable: boolean }> { + return this.payment.reconcileShipment(sourceId); + } + async payInvoice( invoiceId: string, opts: { @@ -1004,9 +1015,9 @@ export class BillingService { // A booking's PREPAID invoice is only payable inside its pay window — // `dueAt` mirrors booking.paymentDeadline (issuePayable at reserve time). - // Blocking INITIATION here is what makes the deadline real: the settle - // sweep's grace only shelters payments that started before this gate. - // Other invoice types keep dueAt as display-only. + // Blocking INITIATION here is what makes the deadline real: a payment + // STARTED before this gate but settling late is still honored by the + // expire-time gateway reconcile. Other invoice types keep dueAt display-only. if ( invoice.source === Freight.InvoiceSource.Booking && invoice.type === "PREPAID" && diff --git a/apps/edr-freight-api/src/modules/payment/payment-client.service.ts b/apps/edr-freight-api/src/modules/payment/payment-client.service.ts index 4c2ebe971..81d1a2a0d 100644 --- a/apps/edr-freight-api/src/modules/payment/payment-client.service.ts +++ b/apps/edr-freight-api/src/modules/payment/payment-client.service.ts @@ -31,6 +31,24 @@ export class PaymentClientService { return this.call("POST", "/payments/initiate", request); } + /** + * POST /payments/reconcile — settlement check for a domain order + * (reconcile-before-cancel). Live-queries every non-failed intent at the + * provider and registers any late capture found (flips it to SUCCEEDED and + * emits payment.succeeded). `unverifiable: true` = could not confirm + * "not paid" — the caller must NOT cancel/expire the order. + */ + async reconcileReference( + referenceType: PaymentReferenceType, + referenceId: string, + ): Promise<{ paid: boolean; unverifiable: boolean }> { + return this.call("POST", "/payments/reconcile", { + service: PaymentService.FREIGHT, + referenceType, + referenceId, + }); + } + /** GET /payments/intents?… — active intent by domain reference; null when none exists. */ async getIntentByReference( referenceType: PaymentReferenceType, diff --git a/apps/edr-freight-api/src/modules/payment/payment.service.ts b/apps/edr-freight-api/src/modules/payment/payment.service.ts index d96bcf492..6cb359dba 100644 --- a/apps/edr-freight-api/src/modules/payment/payment.service.ts +++ b/apps/edr-freight-api/src/modules/payment/payment.service.ts @@ -188,6 +188,30 @@ export class PaymentService { * marked paid WITHOUT emitting — the caller (billing) settles inline after it * has stored the intent id, avoiding a settle-before-correlation race. */ + /** + * Reconcile-before-cancel: ask the payment service whether ANY intent for + * this shipment actually settled at the provider (bank/gateway). A late + * capture found there is registered as SUCCEEDED and emits payment.succeeded, + * which drives the normal paid flow. A network/provider error reports + * `unverifiable` — the caller must not expire the order on unknown. + */ + async reconcileShipment( + referenceId: string, + ): Promise<{ paid: boolean; unverifiable: boolean }> { + try { + const result = await this.paymentClient.reconcileReference( + PaymentReferenceType.SHIPMENT, + referenceId, + ); + return { paid: result.paid, unverifiable: result.unverifiable }; + } catch (err) { + this.logger.warn( + `Reconcile for shipment ${referenceId} failed: ${(err as Error).message}`, + ); + return { paid: false, unverifiable: true }; + } + } + async initiate(input: InitiateIntentInput): Promise { try { 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 6414ba768..d43cff60b 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 @@ -9,14 +9,6 @@ export const BATCH_TIMEZONE = 'Africa/Addis_Ababa'; -/** - * Slack after a booking's paymentDeadline before the settle sweep expires it. - * Covers gateway/webhook lag for a payment STARTED inside the window — - * initiation itself is hard-blocked at the deadline (BillingService.payInvoice), - * so this never extends the time a customer has to begin paying. - */ -export const PAYMENT_GRACE_MS = 5 * 60_000; - /** How long before the pay deadline the one reminder notification goes out. */ export const PAYMENT_REMINDER_LEAD_MS = 10 * 60_000; 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 cedcfcb19..004c28e6b 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 @@ -151,6 +151,8 @@ describe('BookingBatchService — PAID reconcile', () => { { issuePayable: jest.fn().mockResolvedValue(null), expirePayable: jest.fn().mockResolvedValue(undefined), + // Gateway reconcile-before-expire: default = verifiably unpaid. + reconcilePayable: jest.fn().mockResolvedValue({ paid: false, unverifiable: false }), } as never, { emitPhase: jest.fn() } as never, { computeSubmitPriorityScore: jest.fn().mockResolvedValue(0) } as never, @@ -709,7 +711,13 @@ describe('BookingBatchService — PAID reconcile', () => { notifier as never, { addTimeout: jest.fn(), deleteTimeout: jest.fn(), doesExist: jest.fn() } as never, trainSchedulingService as never, - { issuePayable: jest.fn(), expirePayable: jest.fn() } as never, + { + issuePayable: jest.fn(), + expirePayable: jest.fn(), + reconcilePayable: jest + .fn() + .mockResolvedValue({ paid: false, unverifiable: false }), + } as never, { emitPhase: jest.fn() } as never, { computeSubmitPriorityScore: jest.fn().mockResolvedValue(0) } as never, undefined, @@ -732,7 +740,13 @@ describe('BookingBatchService — PAID reconcile', () => { notifier as never, { addTimeout: jest.fn(), deleteTimeout: jest.fn(), doesExist: jest.fn() } as never, trainSchedulingService as never, - { issuePayable: jest.fn(), expirePayable: jest.fn() } as never, + { + issuePayable: jest.fn(), + expirePayable: jest.fn(), + reconcilePayable: jest + .fn() + .mockResolvedValue({ paid: false, unverifiable: false }), + } as never, { emitPhase: jest.fn() } as never, { computeSubmitPriorityScore: jest.fn().mockResolvedValue(0) } as never, undefined, @@ -763,7 +777,13 @@ describe('BookingBatchService — PAID reconcile', () => { notifier as never, { addTimeout: jest.fn(), deleteTimeout: jest.fn(), doesExist: jest.fn() } as never, trainSchedulingService as never, - { issuePayable: jest.fn(), expirePayable: jest.fn() } as never, + { + issuePayable: jest.fn(), + expirePayable: jest.fn(), + reconcilePayable: jest + .fn() + .mockResolvedValue({ paid: false, unverifiable: false }), + } as never, { emitPhase: jest.fn() } as never, { computeSubmitPriorityScore: jest.fn().mockResolvedValue(0) } as never, undefined, @@ -836,8 +856,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', - // Past deadline + the 5-minute webhook grace, so the settle expires it. - paymentDeadline: new Date(Date.now() - 6 * 60_000), + paymentDeadline: new Date(Date.now() - 60_000), }); const waiting = booking('waiting', 10, { trainScheduleId: null }); @@ -872,8 +891,7 @@ describe('BookingBatchService — PAID reconcile', () => { it('serialises concurrent settles so the same reservation is not settled twice', async () => { const lapsed = booking('lapsed', 50, { status: 'SELECTED_FOR_BATCH', - // Past deadline + the 5-minute webhook grace, so the settle expires it. - paymentDeadline: new Date(Date.now() - 6 * 60_000), + paymentDeadline: new Date(Date.now() - 60_000), }); // Both callers read the reservation; the lock must stop the second from // acting on rows the first already expired. (The PAYMENT transition and the @@ -904,8 +922,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', - // Past deadline + the 5-minute webhook grace, so the settle expires it. - paymentDeadline: new Date(Date.now() - 6 * 60_000), + paymentDeadline: new Date(Date.now() - 60_000), }); bookingsRepository.findReservedForSchedule .mockResolvedValueOnce([latePaid]) @@ -1019,8 +1036,7 @@ describe('BookingBatchService — PAID reconcile', () => { ...(waiting as unknown as Record), status: 'SELECTED_FOR_BATCH', trainScheduleId: exportScheduleId, - // Past deadline + the 5-minute webhook grace, so the settle expires it. - paymentDeadline: new Date(Date.now() - 6 * 60_000), + paymentDeadline: new Date(Date.now() - 60_000), 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 88cd51b62..37e2e99e8 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 @@ -61,7 +61,6 @@ import { DEFAULT_CONTAINER_WAGON_LENGTH_METERS, DEFAULT_CONTAINER_WAGON_TARE_TONS, DEFAULT_WAGONS_PER_BOOKING, - PAYMENT_GRACE_MS, PAYMENT_REMINDER_LEAD_MS, } from "./booking-batch.constants"; import { @@ -2368,11 +2367,11 @@ export class BookingBatchService implements OnModuleInit { const isPaid = (b: Booking) => b.paymentStatus === "PAID" || b.status === "PAID"; - // Grace: a payment started inside the window may land minutes late via the - // gateway webhook — don't expire until the slack has passed too. + // 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. const isExpired = (b: Booking) => b.paymentDeadline - ? b.paymentDeadline.getTime() + PAYMENT_GRACE_MS <= now + ? b.paymentDeadline.getTime() <= now : expireUnpaidUnknownDeadline; for (const booking of reserved) { @@ -3062,6 +3061,31 @@ export class BookingBatchService implements OnModuleInit { } return; } + // Reconcile-before-expire (only when a pay window was actually open): + // no webhook arrived, so ask the gateway DIRECTLY whether the money + // landed. A late capture found there is registered as SUCCEEDED and + // emits payment.succeeded — that event marks the booking PAID and + // allocates it, so we just leave the hold alone here. `unverifiable` + // (provider query errored / payment still in flight) means we could not + // confirm "not paid" — never expire on unknown; the next settle tick + // asks again. + if (reason === "payment" && (fresh?.paymentDeadline ?? booking.paymentDeadline)) { + const reconcile = await this.billing.reconcilePayable(booking.id); + if (reconcile.paid) { + this.logger.log( + `[BATCH] expire skipped for ${booking.reference} — gateway ` + + `reconcile found a settled payment; payment.succeeded will allocate it`, + ); + return; + } + if (reconcile.unverifiable) { + this.logger.warn( + `[BATCH] expire deferred for ${booking.reference} — settlement ` + + `unverifiable at the gateway; retrying next settle tick`, + ); + return; + } + } } const freedScheduleId = booking.trainScheduleId; await this.bookingsRepository.update(booking.id, { @@ -4037,10 +4061,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 + grace has lapsed no longer - // blocks capacity, even before the 10s sweep flips it to EXPIRED — so - // availability shown to the next customer is honest between ticks. - const graceCutoff = Date.now() - PAYMENT_GRACE_MS; + // 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. + const deadlineCutoff = Date.now(); const reserved = ( await this.bookingsRepository.findReservedForSchedule(schedule.id) ).filter( @@ -4048,7 +4074,7 @@ export class BookingBatchService implements OnModuleInit { b.paymentStatus === "PAID" || b.status === "PAID" || b.paymentDeadline == null || - b.paymentDeadline.getTime() > graceCutoff, + b.paymentDeadline.getTime() > deadlineCutoff, ); for (const b of [...allocated, ...reserved]) { budget.subtract( @@ -4144,7 +4170,7 @@ export class BookingBatchService implements OnModuleInit { b.paymentStatus !== "PAID" && b.status !== "PAID" && b.paymentDeadline != null && - b.paymentDeadline.getTime() + PAYMENT_GRACE_MS > now, + b.paymentDeadline.getTime() > now, ); } @@ -4368,10 +4394,7 @@ export class BookingBatchService implements OnModuleInit { private armSettle(scheduleId: string): void { void this.scheduleById(scheduleId) .then((schedule) => this.paymentWindowMsFor(schedule)) - // The timer covers the grace too — firing at the bare deadline would - // settle before the sweep's grace cutoff and find nothing to expire. - .then((windowMs: number) => { - const delayMs = windowMs + PAYMENT_GRACE_MS; + .then((delayMs: number) => { this.removeTimeout(scheduleId); const handle = setTimeout(() => { void this.settleBatch(scheduleId).catch((err) => diff --git a/apps/edr-freight-api/src/modules/train-scheduling/booking-split.service.spec.ts b/apps/edr-freight-api/src/modules/train-scheduling/booking-split.service.spec.ts index fc47d8a70..50cba543f 100644 --- a/apps/edr-freight-api/src/modules/train-scheduling/booking-split.service.spec.ts +++ b/apps/edr-freight-api/src/modules/train-scheduling/booking-split.service.spec.ts @@ -72,7 +72,12 @@ describe('BookingSplitService — applySplit split marking', () => { dataSource as never, {} as never, {} as never, - { expirePayable: jest.fn() } as never, + { + expirePayable: jest.fn(), + reconcilePayable: jest + .fn() + .mockResolvedValue({ paid: false, unverifiable: false }), + } as never, { payNowPartial: jest.fn() } as never, ); return { service, bookingRepo, contractRepo }; diff --git a/apps/edr-freight-api/src/modules/train-scheduling/booking-window.service.ts b/apps/edr-freight-api/src/modules/train-scheduling/booking-window.service.ts index 77c4cfd0d..6cdef8c05 100644 --- a/apps/edr-freight-api/src/modules/train-scheduling/booking-window.service.ts +++ b/apps/edr-freight-api/src/modules/train-scheduling/booking-window.service.ts @@ -20,7 +20,7 @@ import { import { BookingBatchService } from './booking-batch.service'; import { BookingWindowGateway } from './booking-window.gateway'; import { TrainSchedulingService, effectiveWindowConfig } from './train-scheduling.service'; -import { BATCH_TIMEZONE, PAYMENT_GRACE_MS } from './booking-batch.constants'; +import { BATCH_TIMEZONE } from './booking-batch.constants'; import { bookingCloseCutoff, clampCloseToOfficeHours, @@ -602,10 +602,9 @@ export class BookingWindowService implements OnModuleInit { .createQueryBuilder('b') .select('DISTINCT b.train_schedule_id', 'scheduleId') .where(`b.status IN ('SELECTED_FOR_BATCH', 'AWAITING_PAYMENT')`) - // Deadline + grace: a payment started in-window may webhook in late. - .andWhere('b.payment_deadline <= :graceCutoff', { - graceCutoff: new Date(Date.now() - PAYMENT_GRACE_MS), - }) + // Deadline is the line — expire() itself reconciles against the gateway + // before actually expiring, so a late in-window payment is still caught. + .andWhere('b.payment_deadline <= now()') .andWhere('b.train_schedule_id IS NOT NULL') .getRawMany<{ scheduleId: string }>(); for (const { scheduleId } of overdue) {