6.8 KiB
Refinement Phase 6 — Money-path correctness completion — Report (2026-07-13)
Track: backend (money path) · Depends on: nothing hard (do before real BNPL/manual refunds — phase 8)
· Gate: dotnet build 0 new warnings · dotnet test 396 pass (383 prior + 13 new).
The headline fix (6.1) — the unreachable BNPL/manual refund settlement is now wired
Before this phase, a card refund cleared its refund_payable ↔ escrow_held leg immediately, but a
BNPL-revert / manual-bank refund was left in processing with the clearing "deferred to reconciliation" — and
no reconciliation path existed: Refund.MarkSucceededAsync had zero callers, and nothing performed
processing → succeeded. Every BNPL/manual refund permanently overstated escrow_held and stranded
refund_payable; the ledger could never reconcile with the bank.
Now:
ConfirmRefundSettlementCommand(Features/Refunds/Commands/ConfirmRefundSettlement/) transitionsprocessing → succeeded, stamps the settled instant, and postsLedgerPosting.RefundPayableClearingin the same commit. It runs under the samebooking:{id}:refundlock asCreateRefundCommandand re-reads the tracked refund inside the lock, so a racing/replayed confirm sees committed truth and no-ops (never double-clears). Idempotent: an already-succeededrefund is a no-op success.MarkRefundSettlementFailedCommand(.../MarkRefundSettlementFailed/) is the counterpart —processing → failed, no ledger moves.- Admin surface:
POST admin_refunds/{id}/confirm_settlement+.../mark_failedonAdminRefundsController. - BNPL callback branch:
HandleBnplCallbackgained aRefundConfirmedaction. A provider event whose type says the revert/refund cash-back completed/confirmed/settled (or "cashback") resolves theprocessingrefund created against that order'spayment_transaction(GetProcessingRefundIdForTransactionAsync) and dispatchesConfirmRefundSettlementCommand.ResolveActionchecks this before the order-levelsettlebranch sorevert_settleddoesn't fall through. - Domain: the misnamed
Refund.MarkSucceededAsync(not async, uncalled) was renamedMarkSucceededReconciled.
Proof: RefundSettlementTests — a BNPL refund lands processing (3 reversal legs, no clearing) → confirm →
succeeded, clearing posts, the ledger reconciles (Σdebit = Σcredit, refund_payable fully drained,
escrow_held credited back); a replayed confirm stays at 5 legs (no double-clear); mark_failed leaves 3 legs
and blocks a later confirm (409).
6.4 — crash-window closed
CreateRefundCommand now persists the refund row (approved) and commits before calling the external
channel; then executes the channel against the persisted row and commits the outcome (succeeded/processing/failed
- ledger). Same claim-first / execute-second shape the webhook handler uses — a crash between provider success
and our commit now leaves a reconcilable
approvedrow instead of a silently-executed refund with no record.
6.2 — forward-dep FKs added (additive migration RefinementPhase6MoneyFks)
Real FKs (all nullable, ON DELETE NO ACTION) on the columns b11 shipped FK-less "until the target ships"
(the targets all shipped in b13/b15): refunds.ticket_id → messaging.Tickets,
nurse_clawbacks.original_payout_id/recovered_in_payout_id → payouts.NursePayouts,
invoices.partner_center_id → partner.PartnerCenters (+ index). The three now-false config-doc comments were
corrected. Referential integrity no longer rests on application discipline alone.
6.3 — IAuditable extended to the admin-decided money & trust entities
Refund, NurseClawback, NursePayout, NursePayoutBatch, NurseVerification are now IAuditable, so the
AuditFieldInterceptor writes an append-only audit_logs diff row on create + every admin decision (approve /
reject / process / settle, and the verification is_verified decision). NursePayout.IbanSnapshot (encrypted)
carries [AuditRedacted] so the diff records a marker, never the plaintext IBAN.
6.6 — orphaned config key retired
refund_ticket_required (seed row id 19) had no consumer left (b15 unconditionally auto-opens a refund ticket).
The seed row is deleted by the migration and the false description removed; the test-host stub was dropped.
6.5 — the previously-untested admin money paths now have tests (+13 tests)
ClawbackWriteOffTests— write-off posts a balancedDEBIT bad_debt / CREDIT nurse_clawback_receivablegroup and resolves the clawback; 404 unknown; 409 second write-off. (Was zero-coverage.)Racing_same_key_insert_is_caught_as_an_idempotent_no_op(added toPaymentWebhookTests) — a provider that omitsexternal_event_idskips the read-dedup, so the(provider_code, external_event_id)UNIQUE is the sole backstop (the exact state a true concurrent insert reaches); a colliding insert hitsDbUpdateExceptionand is treated as an idempotent duplicate no-op (no confirm, no ledger).MessagingInternalBoundaryTests(Foundation, handler-level) — theis_internalboundary: user thread view strips internal notes; admin view returns them; non-staff admin-view request is forbidden; non-staff can't post an internal note; a staff internal note never surfaces in the user view.RefundSettlementTests— the 6.1 settlement (both channels) + idempotency + mark_failed.
Test-infra note (why a refund test host changed)
Adding the refunds.ticket_id FK means SQLite (which EF enables FK enforcement on) rejects a refund whose
ticket_id points at a non-existent ticket. RefundsTestHost now seeds a real Ticket and exposes
Senders() whose OpenTicket hook returns that real id (replacing the TestSenders.WithTicketHooks() fake id 1
in the refund tests). PaymentsTestHost/PayoutsTestHost were unaffected (they leave the new FK columns null).
Contracts / docs updated (same change)
dev/contracts/domains/refunds-invoices.md— the two new endpoints +RefundSettlementshape + changelog + corrected therefund_ticket_requirednote.dev/contracts/openapi/swagger.v1.json— refreshed (additive: the two routes +RefundSettlementResult).server/CLAUDE.md— refunds/payments section (settlement wiring + crash-window + FKs + retired config), the audit-interceptor note (expanded IAuditable set +[AuditRedacted]), and the feature map.
Follow-ups (out of scope, for later phases)
- A
mark_failedafter a successful provider revert leaves the reversal ledger posted with no clearing (the money is genuinely in limbo — an ops reconciliation case). Deliberate: the reversal is not un-posted. - The BNPL/PSP mocks stay until phase 8 (external rails); the settlement path is now complete behind them.
audit_logsgrowth (2.3 grows it faster) — retention/archival is phase 9 (§7.4).