Files
baya-monorepo/dev/shared-working-context/reports/refinement-phase-6-report.md
T
2026-07-13 17:03:45 +03:30

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/) transitions processing → succeeded, stamps the settled instant, and posts LedgerPosting.RefundPayableClearing in the same commit. It runs under the same booking:{id}:refund lock as CreateRefundCommand and 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-succeeded refund is a no-op success.
  • MarkRefundSettlementFailedCommand (.../MarkRefundSettlementFailed/) is the counterpart — processing → failed, no ledger moves.
  • Admin surface: POST admin_refunds/{id}/confirm_settlement + .../mark_failed on AdminRefundsController.
  • BNPL callback branch: HandleBnplCallback gained a RefundConfirmed action. A provider event whose type says the revert/refund cash-back completed/confirmed/settled (or "cashback") resolves the processing refund created against that order's payment_transaction (GetProcessingRefundIdForTransactionAsync) and dispatches ConfirmRefundSettlementCommand. ResolveAction checks this before the order-level settle branch so revert_settled doesn't fall through.
  • Domain: the misnamed Refund.MarkSucceededAsync (not async, uncalled) was renamed MarkSucceededReconciled.

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 approved row 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 balanced DEBIT bad_debt / CREDIT nurse_clawback_receivable group 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 to PaymentWebhookTests) — a provider that omits external_event_id skips 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 hits DbUpdateException and is treated as an idempotent duplicate no-op (no confirm, no ledger).
  • MessagingInternalBoundaryTests (Foundation, handler-level) — the is_internal boundary: 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 + RefundSettlement shape + changelog + corrected the refund_ticket_required note.
  • 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_failed after 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_logs growth (2.3 grows it faster) — retention/archival is phase 9 (§7.4).