Commit Graph
6 Commits
Author SHA1 Message Date
popertots 352f9e50d4 Fix till-sale money safety: sweep gift-card clawback, orphaned-checkout cancel, cash-retry reconciliation
HIGH-1: a till sale whose card-machine checkout is provably dead, or whose Square reconcile proves the charge never landed, now claws back the funded gift card atomically with the failed mark (claim-first gating UPDATE serializes against the admin retry; blind-fail and ambiguous/lost-response rows never claw back, and a bare CANCEL_REQUESTED is not treated as proof of non-completion). HIGH-2: a card-machine checkout created at Square but not committed is cancelled on any pre-commit failure. HIGH-3: a cash/on_the_house retry of a pending card sale is reconciled at Square first (COMPLETED rescues + refuses cash; NOT_FOUND/FAILED/CANCELED allows cash; lost-response forces the card-method retry; ambiguous rejects). GetTillCheckoutStatus no longer resurrects a swept-failed sale, and the cash-completion UPDATE checks RowsAffected so the admin is never told to take cash against an already-resolved sale. 23 new tests covering the clawback matrix, the reconcile-or-reject matrix, cancel-on-error, and a till concurrency test.
2026-08-22 00:34:49 +01:00
popertots 8416f033d6 Fix gift-card transfer deadlock: lock card rows in deterministic ID order
TransferGiftCard locked source-then-destination in caller-chosen order, so concurrent cross-transfers (A→B and B→A) acquired row locks in opposite orders and deadlocked (SQLSTATE 40P01), aborting one transfer with a generic 500. Both rows are now locked in lexicographically sorted ID order (case-insensitive, matching the 12-hex mixed-case card codes), with the scanned values mapped back to source/destination roles afterward. Adds a deterministic concurrency regression test that holds both row locks on dedicated connections to force the AB-BA cycle, proving the old code deadlocks and the new code serializes cleanly.
2026-08-22 00:34:49 +01:00
popertots 738f6b6a51 Expand payment test coverage: lock contention, nonce-direct, provisional rows, GDPR scrub, validators
Close the coverage-gap round: terminal CreateCheckout-failure marks the provisional row failed, GetCheckoutStatus reference_id mismatch 400, deadline wire shape, concurrent loyalty redemption 409, delete_guest_user + stale-guest saved-card scrubbing, ValidateAmount and isTokenLike direct units, bounded try-lock timeout, buildSplitRecords tip-overflow, and concurrent same-key dedup for gift card / booking / tip / checkout.
2026-08-22 00:34:49 +01:00
popertots 7439fa86c1 Fix payment review round 3: saved-card idempotency, stale-pending sweep, webhook fail-closed
R1/R4: saved_card branch in CreateTerminalPayment now mirrors CreateTipPayment
- advisory lock (crussell:payment:<bookingID>) serializes concurrent double-clicks
- deterministic key bookingID-sc-type-amount-cardID (<=45 chars) so a lost-response
  retry derives the same key and dedups instead of double-charging
- idempotency switch inside the lock: completed -> dedup, pending -> reuse with
  pence amount-guard, failed -> clean 409
- success response includes card_brand/card_last4 (frontend already reads them)

R2: add 'failed' case to all four retry switches (tip, booking, gift card, till)
- a swept/definitively-rejected record returns 409 instead of 500-ing on the
  idempotency_key UNIQUE constraint

R3: extend SweepStalePendingPayments to till_sales card rows
- sweeps pending till_sales (online_square/in_person_card) past Square's ~24h
  key retention, closing the double-charge window for till sales
- swept rows logged with the same CRITICAL manual-reconciliation marker as the
  refund sweep

Webhook fail-closed: reject 503 when SQUARE_WEBHOOK_SIGNATURE_KEY unset, 403 on
bad signature (was: skip verification in dev)

Refund status resolution: refunds now resolve by Square status
(COMPLETED/PENDING/FAILED/REJECTED) instead of assuming completed; real error
codes (REFUND_AMOUNT_INVALID, PAYMENT_NOT_REFUNDABLE, REFUND_ALREADY_PENDING)
added to the definitive/processed classification

HTTP client: CreateCard key truncated to <=45 chars, device_options always sent
(env SQUARE_TERMINAL_DEVICE_ID fallback), processing_fee reads amount_money,
ListCards cursor loop, refund keys hashed to <=45 chars

Other fixes: payment/till/gift-card advisory-lock + FOR UPDATE asymmetries,
GetPaymentByID NULL scans, loyalty redemption lock, card upsert on conflict,
mock ccof: prefix parity, IsValidSquareCheckoutID for real Square IDs,
isAdminRequest defense-in-depth on all 6 admin payment handlers, webhook
signature docs, M8/L5 debug markers removed

Docs: README/FC/TM/Overview updated (22 jobs, 20 CRITICAL sites, 23-section
GDPR export, sweep jobs, webhook fail-closed); P11 plan marks remaining items
(sandbox smoke test, M-8 customer_id, saved-card key dedup trade-off) as
deferred with rationale; gap backlog pruned of completed items
2026-08-22 00:34:49 +01:00
popertots 9ff591fa4e Fix payment review round 2: refund idempotency, pending-resume safety, terminal-completion lock
Refund idempotency (P2):
- RefundRequest gains an optional client idempotency_key: two DISTINCT equal
  partial refunds of one payment no longer collide on the amount-derived key
  (the second was silently swallowed as a dedup)
- Extract resumeManualPendingRefund: resumes a pending refund with the row's
  OWN stored key, so Square's key dedup returns the original refund if the
  prior attempt completed — never issues a second
- (payment, amount) pending fallback: when the exact-key lookup misses (admin
  reopened the modal, new UUID), resume the matching pending row instead of
  creating a second pending row the sweep would double-process
- 409 in-flight guard: if a pending refund exists for the payment but no
  same-amount row matches, reject a different-amount refund (money state at
  Square is unknown — no new refund is safe until it resolves)
- Frontend (EditBookingModal): UUID per refund attempt, reused on retry,
  mirroring the tip flow

Terminal completion (P3):
- GetCheckoutStatus serializes on pg_advisory_lock('crussell:terminal:' ||
  SquarePayID) on a pinned connection — concurrent polls of the same checkout
  can no longer both pass the dedup SELECT and race the UNIQUE constraint

Card-on-file / doc-only:
- Document why CreateCardOnFile is NOT rolled back on payment failure
  (deterministic sha256 retry returns the same card; deletion breaks it)
- Document HasCompletedPayment's deliberate 'tip' exclusion

Regression tests:
- TestRefund_TwoEqualPartialRefunds_ClientKeyDisambiguates
- TestRefund_PendingResume_NewKeyAfterModalReopen (proves stored-key resume)
- TestRefund_PendingResume_DifferentAmountRejected (409 + no second row)
- TestRefund_GuardCountsPendingRefunds updated: 400 -> 409 (in-flight guard
  fires first — strictly safer, blocks before any Square attempt)
- TestGetCheckoutStatus_ConcurrentPolls_SingleRecord (real two-goroutine race)
2026-08-22 00:34:49 +01:00
popertots 53ca89603d Fix payment review round: till integrity, HTTP client tests, concurrency tests, card-selection consolidation
Addresses the payment review (all 10 blocking + 2 minor findings):

Till money-integrity (CreateTillSale):
- Add pg_advisory_lock on the idempotency key (concurrent same-key double-funding race)
- Guard amount on pending-reuse retry (mirrors tip/gift-card guards)
- Explicitly complete the row for cash/on_the_house pending-reuse
- Reject method-switch on a live card-machine checkout (double-charge guard)
- 3 regression tests (amount-mismatch, cash-completes-row, method-switch)

BookingFlow:
- Fetch saved cards at the deposit step (was dead code)
- Charge the server-computed deposit_amount, not the client estimate

HTTP client tests (was untested): doJSON error parsing, refund sentinel
classification, payment/refund/card wire shapes, checkout polling states,
list-refunds pagination + 20-page guard, sha256 card idempotency key

Concurrency regression tests: real two-goroutine races for BuyGiftCard,
tip, and booking-payment locks asserting exactly-one record each

Frontend:
- Fix CRIT-1: zero-saved-card users blocked (all flows now handle it)
- Consolidate tip/deposit/Buy-Gift-Card card UI onto CardSelection
- Explicit save-card consent checkbox (was silent/inconsistent)
- Fix stale saved-card field names in BookingFlow (last4 -> last_4)
- Unique instance ids (crypto.randomUUID) in CardSelection/SquareCardInput
- UserPaymentModal: keep card form mounted on error + Try Again button

Health/docs: /api/health reports square state (mock/ok, was not_implemented),
close P1 backlog, correct stale webhook and env-var claims
2026-08-22 00:34:49 +01:00