Payments 3/7: Failure Taxonomy, Retries, Partial Outcomes & Payment Reconciliation Engine - #1579
Merged
yusuftomilola merged 1 commit intoAug 22, 2026
Conversation
|
@mftee is attempting to deploy a commit to the naijabuz's projects Team on Vercel. A member of the Team first needs to authorize it. |
…edger Adds a scheduled reconciliation job (@Cron, every 5 minutes) that treats provider truth as authoritative: re-verifies AWAITING_CONFIRMATION payments past a due threshold directly against the provider (reusing DistinctCodes#1571's verify-by-reference call and PaymentConfirmationService's idempotent apply path), with per-payment exponential backoff so a still-processing payment gets polled less often over time rather than every tick. Payments unresolved past a long threshold escalate to a new MANUAL_REVIEW status -- but only on a pass where the provider was actually reachable, so a single provider outage never mass-flags every in-flight payment. Also adds: - A failure taxonomy (PaymentFailureReason: DECLINED, EXPIRED, PROVIDER_ERROR, ABANDONED) recorded alongside FAILED/EXPIRED, plus new MANUAL_REVIEW/DISPUTED/VOIDED statuses in the state machine. - An expiry sweep: INITIATED past TTL -> EXPIRED/ABANDONED, AWAITING_CONFIRMATION past TTL -> EXPIRED/EXPIRED. - Admin recovery endpoints (role-restricted): manual-review queue, metrics, force-reconcile-now, resolve-manually (reason required, audited), void. - A refund sub-ledger (Refund entity) supporting multiple partial refunds per payment, with the check-and-insert locked (pessimistic_write) so two concurrent refund requests that would together exceed the captured amount can never both succeed. - A shared, independently-tested retry-with-backoff utility for outbound provider calls. Closes DistinctCodes#1572
michaelsimeon001
force-pushed
the
feature/1572-payment-reconciliation-engine
branch
from
August 22, 2026 09:22
fa8f7fa to
bbed481
Compare
yusuftomilola
approved these changes
Aug 22, 2026
yusuftomilola
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed the failure taxonomy, retries, partial outcomes & reconciliation engine PR (closes #1572, building on already-merged #1570/#1571).
- Reusing #1571's
verifyByReferenceandPaymentConfirmationService.apply's idempotent path for reconciliation, rather than inventing a parallel resolution mechanism, means a payment resolved by the cron job goes through the exact same terminal-status/duplicate/conflict guarantees as the webhook and verify-on-return paths already do. Genuinely safe to re-run, by construction, not just by testing. - The
MANUAL_REVIEWescalation logic is the standout piece here: separatingproviderErrorStreak(consecutive provider-unreachable attempts) from the age-based due threshold, and only escalating on a pass where the provider was actually reachable, is exactly the right fix for the failure mode most reconciliation systems get wrong — a single provider outage mass-flagging every in-flight payment for human review. Good that this specific scenario is explicitly tested rather than just asserted in the PR description. - Per-payment exponential backoff so a still-processing payment gets polled less often over time (instead of every 5-minute tick hitting the provider) is a reasonable, considerate default against a real payment provider.
- The refund sub-ledger design — "amount refunded" always computed as
SUM(refunds.amount)rather than a boolean flag — is the correct model for supporting multiple partial refunds, and locking the payment row (pessimistic_write) for the duration of the check-and-insert closes the actual race (two concurrent refund requests that would together overshoot the captured amount) at the DB layer rather than relying on an app-level check that a second request could still slip past. The 409-on-loser-request behavior is verified with a real concurrent-equivalent test, not just asserted. - The expiry sweep correctly distinguishes "never even reached the provider" (
INITIATED→ABANDONED) from "did start but nothing confirmed it in time" (AWAITING_CONFIRMATION→EXPIRED) — that distinction is genuinely useful for whoever triages the manual-review queue later. retryWithBackoffas a shared, independently-tested utility (jitter, capped attempts, fail-fast on non-retryable errors) rather than one-off retry logic embedded in the refund flow is good factoring, especially since it's already positioned to be reused by future payment-adjacent work.- Admin recovery surface (manual-review queue, metrics with WARN-on-threshold, force-reconcile, audited resolve-manually requiring a reason, void) gives operators real tools instead of leaving stuck payments as a black box only fixable via direct DB access.
- Test coverage matches the risk surface precisely across all three subsystems — reconciliation (including the outage-vs-escalation distinction and idempotent double-run safety), refunds (the actual concurrency race), and retry/backoff (growth, jitter bounds, capping, fail-fast).
CI is green across Backend, Frontend, and Frontend E2E. Vercel's FAILURE is the usual unauthorized deployment integration link, unrelated to the code.
Approving — thorough, carefully-reasoned closure of the payment reconciliation gap.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ReconciliationService,@Cronevery 5 minutes) that treats provider truth as authoritative: re-verifiesAWAITING_CONFIRMATIONpayments past a due threshold directly against the provider, reusing Payments 2/7: Real-Time Confirmation Pipeline & the App/Provider State Boundary #1571'sverifyByReferencecall andPaymentConfirmationService.apply's idempotent path (a payment resolved here can never be double-applied, and re-running the job is always safe).PAYMENT_RECONCILE_BACKOFF_*) so a still-processing payment (e.g. a bank-side hold) gets polled less and less often over time instead of on every cron tick.MANUAL_REVIEWstatus: payments unresolved pastPAYMENT_MANUAL_REVIEW_AFTER_HOURSescalate to it — but only on a reconciliation pass where the provider was actually reachable.Payment#providerErrorStreaktracks consecutive provider-unreachable attempts separately from the age-based threshold, specifically so a single provider outage across a batch never mass-flags every in-flight payment (tested).PaymentFailureReason:DECLINED,EXPIRED,PROVIDER_ERROR,ABANDONED) recorded onPayment#failureReasonalongside a terminal status, plus newMANUAL_REVIEW/DISPUTED/VOIDEDvalues wired into Payments 1/7: Payment Domain Model, Initiation Flow & Idempotent Transaction Lifecycle #1570's state-machine transition guard.INITIATEDpast its TTL →EXPIRED/ABANDONED(never even reached the provider),AWAITING_CONFIRMATIONpast its TTL →EXPIRED/EXPIRED(did start, but nothing confirmed it in time).PaymentsAdminController, role-restricted toADMIN): manual-review queue list, metrics (queue depth + alert status, logged as aWARNwhen the threshold is exceeded), force-reconcile-now, resolve-manually (reason required, audited viaPayment#manualReviewReason), void.Refundentity /payment_refundstable) supporting multiple partial refunds per payment — "amount refunded" is alwaysSUM(refunds.amount), never a single boolean.RefundsService.requestRefundlocks the payment row (pessimistic_write) for the duration of the check-and-insert, so two refund requests that would together exceed the captured amount can never both succeed — the loser gets a409, not a corrupted ledger (tested).retryWithBackoffutility (exponential backoff + jitter, capped attempts, optionalisRetryablepredicate to fail fast on terminal errors) used by the refund flow's best-effort provider-side execution call.backend/src/payments/README.md, per the issue's definition of done.Test plan
npm run buildnpm run lint(scoped to changed files — 0 errors; the full-repo baseline has pre-existing CRLF noise on untouched files, confirmed unrelated to this change)npm run test— all 9 suites / 189 tests pass, including new specs covering all 4 acceptance criteria:reconciliation.service.spec.ts— resolves a simulated "webhook never arrives" scenario within its due window; excludes payments still in their initial grace window or backoff window; does not escalate toMANUAL_REVIEWon a single simulated provider outage but does escalate once the provider is reachable again and the payment is still stuck; resets the provider-error streak on successful contact; safe to run twice with no duplicateapply()calls (idempotency); expiry sweep with correct failure reasons; all admin recovery actions.refunds.service.spec.ts— partial/full refund booking, multi-partial accumulation intoREFUNDED, and the double-refund race test (second concurrent-equivalent request atomically rejected via the row-lock re-read).retry-with-backoff.spec.ts— retry/success, exhaustion, fail-fast on non-retryable errors, exponential growth + jitter bounds, delay capping.payment-state-machine.spec.tsupdated for the new transitions.Depends on and builds directly on #1570 and #1571 (already merged).
Closes #1572