From 77317e4a45596b4b0b5b1ac37961ac51c934af17 Mon Sep 17 00:00:00 2001 From: Stephen Adamson Date: Sun, 16 Aug 2026 18:20:29 +0100 Subject: [PATCH] =?UTF-8?q?fix:=20payments=20money-safety=20round-2=20?= =?UTF-8?q?=E2=80=94=20webhook=20booking=20gate=20+=20M2=20stranded-charge?= =?UTF-8?q?=20refund,=20sync-path=20status=20guards,=20gift-card=20cancel?= =?UTF-8?q?=20re-issue=20reconcile,=20till-sweep=20clawback=20lock,=20swee?= =?UTF-8?q?p=20VAT=20rescue,=20refund=20credit=20routing?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - webhook payment.updated now re-reads the booking FOR UPDATE: non-payable booking -> payment failed + stranded-charge refund row + flood-capped alert; payable booking -> full completion side-effects; gift-card rows stay pending (C6 same-key retry); unknown events -> 503 so Square retries - sync-path completion flips (saved-card, tip, online) guarded AND status='pending' + post-flip re-read; postChargeRecheck failed-mark guarded — no webhook-first double-processing, no phantom split rows - CancelGiftCard resume reconciles ALL Square refunds (pending blocks re-issue; COMPLETED sum >= entitlement resolves+neutralizes; else re-issues only the difference under a fresh key) — closes double-refund - sweep till_sales fail/clawback takes crussell:till: lock + post-lock status re-read; recordUntrackedTillSalePayment applies VAT; rescue align clears VAT fields before re-apply (split-accurate VAT, tip rows stay VAT-free) - refunds: chargeAggKey widened, redeemed-card refunds route to user_giftcard_balances, guest cash refunds recorded failed + notification - handlers: tip split-records excluded from VAT loop, splitIdempotencyKey hashed, cross-booking key reuse 409, refunded payments excluded from existingCount, SCA-mint exemption for save_card, non-COMPLETED results routed Co-authored-by: Sisyphus Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) --- backend/handlers/payments/charge_helpers.go | 15 +- backend/handlers/payments/giftcards.go | 226 +++++- backend/handlers/payments/handlers.go | 344 ++++++++- backend/handlers/payments/refund_policy.go | 2 +- backend/handlers/payments/refunds.go | 138 +++- backend/handlers/payments/sweep.go | 268 ++++++- backend/handlers/payments/till.go | 292 +++++++- backend/handlers/webhooks/square.go | 756 ++++++++++++++++++-- init-scripts/init-script.sql | 55 +- 9 files changed, 1947 insertions(+), 149 deletions(-) diff --git a/backend/handlers/payments/charge_helpers.go b/backend/handlers/payments/charge_helpers.go index 0b12284..a24fe52 100644 --- a/backend/handlers/payments/charge_helpers.go +++ b/backend/handlers/payments/charge_helpers.go @@ -294,9 +294,22 @@ func postChargeRecheck(ctx context.Context, w http.ResponseWriter, tx pgx.Tx, bo if !payable { log.Printf("CRITICAL: Square payment %s (ID=%s) for booking %s was processed but booking is now %q — marking %s failed; money taken at Square MUST be refunded manually", sqStatus, sqPayID, bookingID, recheckStatus, chargeNoun) - if _, upErr := tx.Exec(ctx, `UPDATE payments SET status = 'failed' WHERE id = $1`, paymentID); upErr != nil { + // R10: guard the failed mark on status='pending' so it can never + // clobber a row the Square payment.completed webhook already resolved + // to 'completed'. In the race where the webhook wins the booking + // FOR UPDATE lock between the Square call and this recheck, the + // payment is genuinely completed — the cancellation refund path + // computes refunds from completed payments and will reverse the money + // there. Marking such a row 'failed' would strand a completed charge + // off the refund ledger entirely. A 0-row update (row already + // completed/failed) is left as-is; the CRITICAL log below still + // alerts ops that money landed on a no-longer-payable booking. + if res, upErr := tx.Exec(ctx, `UPDATE payments SET status = 'failed' WHERE id = $1 AND status = 'pending'`, paymentID); upErr != nil { log.Printf("CRITICAL: Square payment %s (ID=%s) landed on %q booking %s but marking %s failed errored: %v — manual reconciliation required", sqStatus, sqPayID, recheckStatus, bookingID, chargeNoun, upErr) + } else if res.RowsAffected() == 0 { + log.Printf("Square payment %s (ID=%s) landed on %q booking %s but payment row %s is no longer 'pending' (webhook/sweep already resolved it) — leaving the row as-is; not marking it failed", + sqStatus, sqPayID, bookingID, chargeNoun, paymentID) } if cErr := tx.Commit(ctx); cErr != nil { log.Printf("CRITICAL: Square payment %s (ID=%s) landed on %q booking %s and committing the failed mark errored: %v — manual reconciliation required", diff --git a/backend/handlers/payments/giftcards.go b/backend/handlers/payments/giftcards.go index b766a0c..74cfacd 100644 --- a/backend/handlers/payments/giftcards.go +++ b/backend/handlers/payments/giftcards.go @@ -109,8 +109,17 @@ type BuyGiftCardRequest struct { RecipientType string `json:"recipient_type"` RecipientEmail string `json:"recipient_email,omitempty"` CardID *string `json:"card_id,omitempty"` - NewCardToken *string `json:"new_card_token,omitempty"` - SaveCard bool `json:"save_card"` + // SavedCardID (saved_card_id) is the SCA path's reference to the stored + // saved card (user_saved_cards.id), mirroring CreateBookingPayment / + // CreateTerminalPayment / TillSaleRequest. It coexists with NewCardToken + // when the frontend sends the SCA tokenize-result — + // card.tokenize(verificationDetails, cardId) — as new_card_token: the + // tokenize-result token is a fresh one-time source_id and the saved-card + // row supplies the Square customer (resolveChargeSource). Without a token + // it behaves exactly like card_id (legacy saved-card charge). + SavedCardID *string `json:"saved_card_id,omitempty"` + NewCardToken *string `json:"new_card_token,omitempty"` + SaveCard bool `json:"save_card"` // IdempotencyKey is optional (M1): if not provided, a deterministic key is // generated server-side based on user_id + amount + recipient_type + card_id. // This ensures retries of the same logical purchase use the same key while @@ -1492,12 +1501,25 @@ func BuyGiftCard(w http.ResponseWriter, r *http.Request) { // distinct no-key purchases never collapse onto one dedup key. Clients who // need full control still supply their own idempotency_key; that path is // unchanged. + + // savedCardRef is the effective saved-card reference for this request: + // the legacy card_id field OR the SCA path's saved_card_id (they are the + // same user_saved_cards.id; card_id wins when both are sent). Mirrors + // CreateBookingPayment. scaTokenizedSavedCard is the SCA tokenize-result + // wire contract: the token is the one-time charge source and the saved-card + // row supplies the Square customer. + savedCardRef := req.CardID + if savedCardRef == nil || *savedCardRef == "" { + savedCardRef = req.SavedCardID + } + scaTokenizedSavedCard := req.NewCardToken != nil && *req.NewCardToken != "" && savedCardRef != nil && *savedCardRef != "" + clientSuppliedKey := req.IdempotencyKey != "" var noKeyCardPart string if !clientSuppliedKey { noKeyCardPart = "new" - if req.CardID != nil && *req.CardID != "" { - noKeyCardPart = *req.CardID + if savedCardRef != nil && *savedCardRef != "" { + noKeyCardPart = *savedCardRef } } if err := validators.Validate.Struct(&req); err != nil { @@ -1524,12 +1546,24 @@ func BuyGiftCard(w http.ResponseWriter, r *http.Request) { return } - if err := ValidateCardInfo(req.CardID, nil, req.NewCardToken); err != nil { + if err := ValidateCardInfo(req.CardID, req.SavedCardID, req.NewCardToken); err != nil { log.Printf("Failed to process request: %v", err) http.Error(w, "Invalid request", http.StatusBadRequest) return } + // The SCA tokenize-result wire contract requires the saved-card reference + // in saved_card_id — the frontend sends it there, never card_id (the + // verification_token-less legacy buy keeps card_id alone). A token riding + // alongside the legacy card_id field is the ambiguous shape that used to + // silently drop saved_card_id and charge the SCA tokenize-result as a + // new-card one-off without a customer binding — reject it outright. + if req.CardID != nil && *req.CardID != "" && req.NewCardToken != nil && *req.NewCardToken != "" { + log.Printf("Gift-card buy rejected: card_id + new_card_token coexist — the SCA tokenize-result path requires saved_card_id") + http.Error(w, "Invalid card configuration: use saved_card_id for an SCA tokenized saved-card purchase", http.StatusBadRequest) + return + } + if err := ValidateVerificationToken(req.VerificationToken); err != nil { log.Printf("Failed to process request: %v", err) http.Error(w, "Invalid request", http.StatusBadRequest) @@ -1709,7 +1743,7 @@ func BuyGiftCard(w http.ResponseWriter, r *http.Request) { if req.VerificationToken != nil { giftCardVerificationToken = *req.VerificationToken } - if (req.CardID != nil && *req.CardID != "") || req.SaveCard { + if (savedCardRef != nil && *savedCardRef != "" && !scaTokenizedSavedCard) || req.SaveCard { if gateOK, _ := requireTwoFactorForCardAccess(w, r, paymentService, userID, giftCardVerificationToken, reusePendingID == ""); !gateOK { return } @@ -1720,8 +1754,11 @@ func BuyGiftCard(w http.ResponseWriter, r *http.Request) { var savedCardCustomerID string // Resolve the new-card-vs-saved-card Square source — shared with // CreateBookingPayment/CreateTipPayment (see resolveChargeSource for the - // R6 rationale). - sourceID, savedCardID, savedCardCustomerID, sourceOK := resolveChargeSource(ctx, w, paymentService, userID, req.NewCardToken, req.CardID, req.SaveCard, "Card not found") + // R6 rationale). savedCardRef (card_id OR saved_card_id) is passed as the + // card reference; when an SCA tokenize-result token rides along in + // NewCardToken, resolveChargeSource uses the token as the source and the + // card row for the customer. + sourceID, savedCardID, savedCardCustomerID, sourceOK := resolveChargeSource(ctx, w, paymentService, userID, req.NewCardToken, savedCardRef, req.SaveCard, "Card not found") if !sourceOK { return } @@ -1885,7 +1922,7 @@ func BuyGiftCard(w http.ResponseWriter, r *http.Request) { // saving a new card during this purchase — is customer-initiated: Square // requires customer_details on stored-credential payments. A one-off cnon: // nonce charge is not a stored credential and needs none. - if req.CardID != nil || req.SaveCard { + if (savedCardRef != nil && *savedCardRef != "") || req.SaveCard { paymentReq.CustomerDetails = &square.CreateCustomerDetails{CustomerInitiated: true} } @@ -1949,7 +1986,7 @@ func BuyGiftCard(w http.ResponseWriter, r *http.Request) { // records the completed charge (the gate verified without consuming, so a // failed/ambiguous Square charge did not burn the code and a same-key // retry could re-verify the SAME code). - if (req.CardID != nil && *req.CardID != "") || req.SaveCard { + if (savedCardRef != nil && *savedCardRef != "" && !scaTokenizedSavedCard) || req.SaveCard { if consErr := twofa.ConsumePendingCode(ctx, issueTx, userID); consErr != nil { log.Printf("CRITICAL: Square payment succeeded (ID=%s) but consuming the 2FA code for user %s failed: %v — manual reconciliation required", paymentResult.SquarePayID, userID, consErr) http.Error(w, "internal server error", http.StatusInternalServerError) @@ -2865,36 +2902,137 @@ func cancelGiftCardForUser(ctx context.Context, w http.ResponseWriter, r *http.R refundAmount -= priorAmount refundKey = paymentID + "-gccancel-diff-" + strconv.FormatInt(int64(math.Round(refundAmount*100)), 10) log.Printf("Gift-card purchase %s has a prior partial refund of £%.2f — issuing the £%.2f remainder", paymentID, priorAmount, refundAmount) - case "pending": - // In-flight refund for this payment. Only resume if the pending - // row is OURS; a foreign pending row must not be re-issued. - if !strings.HasPrefix(priorKey, paymentID+"-gccancel-") { + case "pending", "failed": + // Resume an in-flight (pending) or previously-failed refund for + // this payment. Only resume a pending row that is OURS; a foreign + // pending row must not be re-issued. + if priorStatus == "pending" && !strings.HasPrefix(priorKey, paymentID+"-gccancel-") { log.Printf("Gift-card cancel rejected: payment %s has a pending refund row %s from another flow", paymentID, priorRefundID) http.Error(w, "A refund for this gift card purchase is already being processed — please try again later", http.StatusConflict) return } refundID = priorRefundID - refundKey = priorKey - refundAmount = priorAmount resumingRefund = true - case "failed": - // Money provably never moved — safe to re-issue with the stored - // key (or persist the deterministic key when a legacy row has - // none, mirroring ensureRefundKey). - refundID = priorRefundID - refundAmount = priorAmount - resumingRefund = true - if priorKey != "" { - refundKey = priorKey - } else { - if _, uErr := tx.Exec(ctx, ` - UPDATE refunds SET idempotency_key = $1 - WHERE id = $2 AND idempotency_key IS NULL`, refundKey, refundID); uErr != nil { - log.Printf("Failed to persist refund idempotency key for %s: %v", refundID, uErr) - http.Error(w, "internal server error", http.StatusInternalServerError) + // currentEntitlement is what THIS request's eligibility re-verified + // (the full purchase value, or the spend-verified unspent remainder). + // The prior attempt's amount may predate a till spend, so never + // re-issue more than min(priorAmount, currentEntitlement). + currentEntitlement := refundAmount + + // Reconcile EVERY refund Square holds for this payment (pending and + // completed, any amount), not just an exact-amount COMPLETED one. + // The prior attempt may still be PENDING (in-flight) at Square — + // invisible to an exact-amount reconcile — and re-issuing under a + // fresh amount-derived key while it is in flight would mint a + // SECOND Square refund (double refund once the first completes). + // Only the full picture of the payment's Square refunds can decide + // whether a re-issue is safe, how much is still owed, and whether + // the money has already moved. + sqRefunds, recErr := reconcileAllRefundsAtSquare(ctx, squarePaymentID) + if recErr != nil { + // Square state unknown — do not re-issue (double-refund risk) + // and do not resolve: leave the row pending for the sweep. + log.Printf("Gift-card cancel resume aborted for %s: could not reconcile prior refund %s against Square: %v", code, refundID, recErr) + http.Error(w, "Refund state could not be verified — please try again", http.StatusInternalServerError) + return + } + + // Any still-in-flight (PENDING) Square refund for this payment + // means money MAY still land. Re-issuing a second refund now would + // double-refund once the first completes. Leave the row 'pending' + // (this request makes no writes) for the sweep's refund + // reconciliation to settle, and do not touch the card. + for i := range sqRefunds { + if sqRefunds[i].Status == "PENDING" { + log.Printf("Gift-card cancel resume for %s: Square refund %s for payment %s is still PENDING (in flight) — leaving refund row %s pending for sweep reconciliation", code, sqRefunds[i].ID, squarePaymentID, refundID) + http.Error(w, "This refund is still being processed by the payment provider — please try again later", http.StatusConflict) return } } + + // Every Square refund for this payment is terminal. FAILED / + // REJECTED / CANCELED refunds never moved money and count as zero; + // sum the COMPLETED (and APPROVED — Square's terminal-completed) + // amounts to learn what has actually landed. + entitlementPence := int64(math.Round(currentEntitlement * 100)) + alreadyRefundedPence := int64(0) + completedRefundID := "" + for i := range sqRefunds { + switch sqRefunds[i].Status { + case "COMPLETED", "APPROVED": + alreadyRefundedPence += sqRefunds[i].Amount + if completedRefundID == "" { + completedRefundID = sqRefunds[i].ID + } + } + } + + // Never end up refunded more than the prior attempt claimed OR the + // re-verified entitlement, whichever is lower: a till spend between + // attempts shrinks the entitlement, and the prior attempt never had + // the right to more than its own amount. + maxRefund := math.Min(priorAmount, currentEntitlement) + + if alreadyRefundedPence >= int64(math.Round(maxRefund*100)) { + // The money has already moved — at least the full cancellation + // entitlement has landed at Square. Mark the row completed with + // the Square refund id and neutralise the card so the returned + // value can never be spent on top of it. + if remaining > 0 { + if cerr := cancelGiftCardFunding(ctx, tx, code, userID, refundID, refundAmount); cerr != nil { + log.Printf("CRITICAL: gift card %s was already refunded at Square but funding reversal failed: %v — MANUAL RECONCILIATION REQUIRED", code, cerr) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + } + if _, upErr := tx.Exec(ctx, `UPDATE refunds SET status = 'completed', square_refund_id = $1 WHERE id = $2`, completedRefundID, refundID); upErr != nil { + log.Printf("CRITICAL: gift card %s was already refunded at Square but refund row %s could not be resolved: %v — MANUAL RECONCILIATION REQUIRED", code, refundID, upErr) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + if cerr := tx.Commit(ctx); cerr != nil { + log.Printf("Failed to commit gift-card cancel transaction: %v", cerr) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + w.WriteHeader(http.StatusOK) + if err := json.NewEncoder(w).Encode(map[string]any{ + "status": "success", + "message": "This gift card had already been refunded and has now been cancelled.", + "refund_id": refundID, + "amount_refunded": refundAmount, + }); err != nil { + log.Printf("Failed to encode JSON response: %v", err) + } + return + } + + // Less than the entitlement has landed — re-issue ONLY the + // difference (entitlement minus what already completed at Square), + // still capped at min(priorAmount, currentEntitlement), under a + // FRESH deterministic key for THAT difference. The prior key + // encodes the prior amount and must NOT be reused when the amount + // changed: replaying the stale amount after a till spend would + // over-refund. When nothing has landed the key is derived exactly + // as the original attempt's (same amount → same key → Square dedups + // a prior landed refund). The row is updated to the new key+amount + // so a crash-retry re-reads the same state and the sweep reconciles + // consistently. + refundAmount = float64(entitlementPence-alreadyRefundedPence) / 100 + if refundAmount > maxRefund { + refundAmount = maxRefund + } + if alreadyRefundedPence > 0 { + refundKey = paymentID + "-gccancel-diff-" + strconv.FormatInt(int64(math.Round(refundAmount*100)), 10) + } else { + refundKey = paymentID + "-gccancel-" + strconv.FormatInt(int64(math.Round(refundAmount*100)), 10) + } + if _, uErr := tx.Exec(ctx, `UPDATE refunds SET amount = $1, idempotency_key = $2 WHERE id = $3`, refundAmount, refundKey, refundID); uErr != nil { + log.Printf("Failed to update resumed gift-card cancel refund %s to £%.2f / key %s: %v", refundID, refundAmount, refundKey, uErr) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + log.Printf("Gift-card purchase %s has already refunded £%.2f at Square of the £%.2f entitlement — issuing the £%.2f remainder with a fresh key", paymentID, float64(alreadyRefundedPence)/100, currentEntitlement, refundAmount) } } @@ -3154,6 +3292,32 @@ func cancelGiftCardForUser(ctx context.Context, w http.ResponseWriter, r *http.R } } +// reconcileAllRefundsAtSquare lists EVERY refund Square has recorded for the +// payment (pending AND completed, unfiltered by amount) so the gift-card cancel +// resume can see an in-flight (PENDING) prior attempt that +// reconcileRefundAtSquareExact — which only matches an exact-amount COMPLETED +// refund — would miss. Missing it is the double-refund bug: the resume would +// re-issue under a fresh amount-derived key while the prior refund is still in +// flight, minting a SECOND Square refund that lands on top of the first. The +// returned slice lets the caller classify refunds by Status (PENDING blocks a +// re-issue; COMPLETED/APPROVED sum to the already-refunded money; +// FAILED/REJECTED/CANCELED never moved money and count as zero). A non-nil +// error means Square state is unknown and no re-issue decision may be made. +func reconcileAllRefundsAtSquare(ctx context.Context, chargeID string) ([]square.RefundResult, error) { + refunds, err := SquareClient.ListPaymentRefunds(ctx, chargeID, time.Time{}) + if err != nil { + log.Printf("Failed to reconcile refunds for charge %s against Square: %v", chargeID, err) + return nil, err + } + ours := make([]square.RefundResult, 0, len(refunds)) + for _, r := range refunds { + if r.PaymentID == chargeID { + ours = append(ours, r) + } + } + return ours, nil +} + // giftCardSpendAtTill returns the total value of completed payments made // against the gift card at the till. Till spend (handlers.go) decrements // amount_remaining and records a payments row with payment_method='giftcard', diff --git a/backend/handlers/payments/handlers.go b/backend/handlers/payments/handlers.go index 1cb5576..487463b 100644 --- a/backend/handlers/payments/handlers.go +++ b/backend/handlers/payments/handlers.go @@ -1297,14 +1297,68 @@ func CreateTerminalPayment(w http.ResponseWriter, r *http.Request) { } } - if _, upErr := recheckTx.Exec(r.Context(), - `UPDATE payments SET status = 'completed', square_payment_id = $1 WHERE id = $2`, + // R10: guard the completion flip on status='pending'. The Square + // payment.completed webhook (webhooks/square.go) can win the booking + // FOR UPDATE lock between Square returning and this flip, completing + // the payment row and running the booking-completion side-effects + // itself. Without the guard this UPDATE would blindly re-flip the + // already-completed row (RowsAffected=1), re-run VAT and the + // completion side-effects below, and double-record money. With the + // guard, a row the webhook/sweep already resolved is a no-op + // (RowsAffected=0): the payment IS completed — skip the side-effects + // and report success. + flipRes, upErr := recheckTx.Exec(r.Context(), + `UPDATE payments SET status = 'completed', square_payment_id = $1 WHERE id = $2 AND status = 'pending'`, paymentResult.SquarePayID, paymentID, - ); upErr != nil { + ) + if upErr != nil { log.Printf("CRITICAL: Square payment %s succeeded but saved-card payment %s update failed: %v — manual reconciliation required", paymentResult.SquarePayID, paymentID, upErr) http.Error(w, "internal server error", http.StatusInternalServerError) return } + if flipRes.RowsAffected() == 0 { + // Defense-in-depth re-read (R10): the guarded flip no-opped, so a + // concurrent resolver already moved the row off 'pending'. Re-read + // the status inside THIS transaction to distinguish a webhook- + // completed row (skip everything — the webhook already ran the + // completion side-effects) from a vanished/failed row (CRITICAL). + var curStatus string + rErr := recheckTx.QueryRow(r.Context(), `SELECT status FROM payments WHERE id = $1`, paymentID).Scan(&curStatus) + if rErr != nil || curStatus != "completed" { + log.Printf("CRITICAL: Square payment %s (ID=%s) succeeded but payment row %s could not be verified as completed (re-read status=%q err=%v) — manual reconciliation required", + paymentResult.Status, paymentResult.SquarePayID, paymentID, curStatus, rErr) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + log.Printf("Saved-card payment %s for booking %s was already resolved to %q by a concurrent resolver (Square webhook/sweep) before the sync completion flip — skipping completion side-effects", paymentID, bookingID, curStatus) + // MEDIUM-2: burn the re-issued 2FA code anyway (idempotent) — the + // charge reached terminal success, so a single-use code re-issued + // for this retry must not authorize another charge. + if bookingUserID.Valid && reusePendingRecord { + if consErr := twofa.ConsumePendingCode(r.Context(), recheckTx, bookingUserID.String); consErr != nil { + log.Printf("CRITICAL: Square payment %s succeeded but consuming the 2FA code for user %s failed: %v — manual reconciliation required", paymentResult.SquarePayID, bookingUserID.String, consErr) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + } + if cErr := recheckTx.Commit(r.Context()); cErr != nil { + log.Printf("CRITICAL: Square payment %s succeeded but committing the post-charge transaction for already-completed payment %s failed: %v — manual reconciliation required", + paymentResult.SquarePayID, paymentID, cErr) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + // The payment IS completed (by the webhook/sweep) — report success + // exactly like the normal completion path; no side-effects re-run. + if err := json.NewEncoder(w).Encode(map[string]any{ + "payment_id": paymentID, + "status": "COMPLETED", + "card_brand": paymentResult.CardBrand, + "card_last4": paymentResult.CardLast4, + }); err != nil { + log.Printf("Failed to encode JSON response: %v", err) + } + return + } // MEDIUM-2 / finding 1: the charge reached its terminal SUCCESS state. // For a FRESH charge the gate already consumed the code (consume=true // at verify time — single-use), so nothing is left to do here. A @@ -2226,6 +2280,39 @@ func CreateBookingPayment(w http.ResponseWriter, r *http.Request) { return case err != nil && !errors.Is(err, pgx.ErrNoRows): log.Printf("Failed to check idempotency: %v", err) + case errors.Is(err, pgx.ErrNoRows): + // R12: no payment on THIS booking carries the key, but the key may + // still exist globally — payments.idempotency_key is UNIQUE — on a + // DIFFERENT booking. Without a guard the pending insert below would die + // on the constraint → 500 → the frontend's same-key retry 500s forever. + // Mirror the A6 cross-user collision pattern (giftcards.go:1592-1607): + // a foreign match is not this booking's operation, so derive a fresh + // deterministic key for THIS booking and proceed as a fresh charge. + // A deterministic no-client-key fallback already embeds this booking's + // id, so this branch can only fire for a client-supplied key. + var foreignID string + fErr := tx.QueryRow(r.Context(), ` + SELECT id FROM payments + WHERE idempotency_key = $1 AND booking_id != $2 + `, req.IdempotencyKey, bookingID).Scan(&foreignID) + if fErr == nil { + log.Printf("Payment idempotency key %q matched payment row %s on a different booking — deriving a fresh deterministic key for booking %s", req.IdempotencyKey, foreignID, bookingID) + cardPart := "new" + if savedCardRef != nil && *savedCardRef != "" { + cardPart = *savedCardRef + } + derivedKey, dErr := deriveBookingPaymentIdempotencyKey(r.Context(), tx, bookingID, req.PaymentType, req.Amount, cardPart) + if dErr != nil { + log.Printf("Failed to derive fresh booking idempotency key after cross-booking collision: %v", dErr) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + req.IdempotencyKey = derivedKey + } else if !errors.Is(fErr, pgx.ErrNoRows) { + log.Printf("Failed to check cross-booking idempotency collision for key %q: %v", req.IdempotencyKey, fErr) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } } // 2FA gating (C5): persisting a card requires 2FA when the feature is @@ -2254,7 +2341,23 @@ func CreateBookingPayment(w http.ResponseWriter, r *http.Request) { // needed. The SAVE gate is skipped because the combined path never persists // a card (resolveChargeSource uses the token as a one-time source, no // card-on-file is created). - if req.SaveCard && !scaTokenizedSavedCard { + // + // R13: the charge-time SAVE gate's tokenForwardedToSquare=false variant + // refuses 402 verification_required in an enforced deployment — but a + // genuine SCA tokenize-result (cnon:sca-... / verify_mock_..., the shapes + // the frontend's tokenizeWithVerification mints) only exists after the + // buyer completed the STORE-intent SCA, so it is SCA-proven and skips the + // gate. This unblocks the NEW-card SCA tokenize-result save (a cnon:sca-... + // token with save_card=true and no saved-card reference), which + // resolveChargeSource persists via CreateCardOnFile. A plain card-entry + // nonce (cnon:... without the sca- marker — card.tokenize() with no + // verification) is NOT an SCA proof and keeps failing closed through the + // gate — 402 in an enforced deployment (auth-F1). + saveGateToken := "" + if req.NewCardToken != nil { + saveGateToken = *req.NewCardToken + } + if req.SaveCard && !scaTokenizedSavedCard && !isSCATokenizeResultShape(saveGateToken) { if gateOK, _ := requireTwoFactorForCardAccessWithTokenValidation(w, r, service, userID, bookingVerificationToken, true, false); !gateOK { return } @@ -2270,13 +2373,17 @@ func CreateBookingPayment(w http.ResponseWriter, r *http.Request) { if req.PaymentType != "partial" { var existingCount int if err := tx.QueryRow(r.Context(), ` - SELECT COUNT(*) FROM payments - WHERE booking_id = $1 - AND status = 'completed' - AND payment_method NOT IN ('discount', 'on_the_house') + SELECT COUNT(*) FROM payments p + WHERE p.booking_id = $1 + AND p.status = 'completed' + AND p.payment_method NOT IN ('discount', 'on_the_house') AND ( - payment_type = $2 - OR ($2 IN ('full', 'deposit') AND payment_type = 'deposit') + p.payment_type = $2 + OR ($2 IN ('full', 'deposit') AND p.payment_type = 'deposit') + ) + AND NOT EXISTS ( + SELECT 1 FROM refunds r + WHERE r.payment_id = p.id AND r.status IN ('completed', 'pending') ) `, bookingID, req.PaymentType).Scan(&existingCount); err == nil && existingCount > 0 { log.Printf("Payment rejected: booking %s already has a completed %q payment", bookingID, req.PaymentType) @@ -2688,6 +2795,20 @@ func CreateBookingPayment(w http.ResponseWriter, r *http.Request) { return } + // R11: a nil error does not mean the payment is COMPLETED. Square's + // terminal payment states are COMPLETED, CANCELED, FAILED; APPROVED + // (authorization-only) and PENDING are NON-terminal — both can still + // transition to COMPLETED. Mirror the stale-pending sweep's classification + // (sweep.go classifyStalePendingByKey): APPROVED/PENDING stay pending for a + // later sweep run to re-poll (staleReconcileLeavePending), CANCELED/FAILED + // are marked failed (the charge never landed). A status-blind handler that + // records 'completed' on nil error alone would book money that was never + // collected. + if paymentResult.Status != "COMPLETED" { + routeNonCompletedPayment(r.Context(), w, bookingID, paymentID, paymentResult, "booking") + return + } + paymentAmount := float64(chargeAmount) / 100.0 // Step 3: Square succeeded — record the completed payment state in a NEW @@ -2793,7 +2914,17 @@ func CreateBookingPayment(w http.ResponseWriter, r *http.Request) { // fields are cleared so apply_vat_to_payment recomputes on the final amount // — the pending record had VAT applied at the pre-split amount. primary := records[0] - if _, upErr := tx2.Exec(r.Context(), ` + // R10: guard the split-primary flip on status='pending'. The Square + // payment.completed webhook can win the booking FOR UPDATE lock between + // the Square call and this UPDATE and complete the payment row + run the + // completion side-effects itself. Without the guard this UPDATE would + // blindly re-flip the already-completed row (RowsAffected=1), OVERWRITE + // the primary row's amount/payment_type/fees/VAT with the split values, + // and re-insert duplicate split tip/balance rows — the ledger would no + // longer reconcile to the actual Square charge. With the guard a row the + // webhook/sweep already resolved is a no-op (RowsAffected=0): the payment + // IS completed — skip the split/booking side-effects and report success. + flipRes, upErr := tx2.Exec(r.Context(), ` UPDATE payments SET status = 'completed', square_payment_id = $1, @@ -2805,13 +2936,62 @@ func CreateBookingPayment(w http.ResponseWriter, r *http.Request) { vat_amount = NULL, net_amount = NULL, updated_at = NOW() - WHERE id = $5 - `, paymentResult.SquarePayID, primary.Amount, primary.PaymentType, primary.Fees, paymentID); upErr != nil { + WHERE id = $5 AND status = 'pending' + `, paymentResult.SquarePayID, primary.Amount, primary.PaymentType, primary.Fees, paymentID) + if upErr != nil { log.Printf("CRITICAL: Square payment %s (ID=%s) was processed but updating payment %s to completed failed: %v — manual reconciliation required", paymentResult.Status, paymentResult.SquarePayID, paymentID, upErr) http.Error(w, "internal server error", http.StatusInternalServerError) return } + if flipRes.RowsAffected() == 0 { + // Defense-in-depth re-read (R10): distinguish a webhook-completed row + // (skip everything — the webhook already ran the completion side- + // effects; the row keeps the webhook's values, not the split overwrite) + // from a vanished/failed row (CRITICAL — money was taken at Square). + var curStatus string + rErr := tx2.QueryRow(r.Context(), `SELECT status FROM payments WHERE id = $1`, paymentID).Scan(&curStatus) + if rErr != nil || curStatus != "completed" { + log.Printf("CRITICAL: Square payment %s (ID=%s) was processed but payment row %s could not be verified as completed (re-read status=%q err=%v) — manual reconciliation required", + paymentResult.Status, paymentResult.SquarePayID, paymentID, curStatus, rErr) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + log.Printf("Payment %s for booking %s was already resolved to %q by a concurrent resolver (Square webhook/sweep) before the sync completion flip — skipping split records, VAT and booking-completion side-effects", paymentID, bookingID, curStatus) + // MEDIUM-2: burn the re-issued 2FA code anyway (idempotent) — the + // charge reached terminal success, so a single-use code re-issued for + // this retry must not authorize another charge. + if savedCardRef != nil && *savedCardRef != "" && reusePendingRecord { + if consErr := twofa.ConsumePendingCode(r.Context(), tx2, userID); consErr != nil { + log.Printf("CRITICAL: Square payment %s (ID=%s) was processed but consuming the 2FA code for user %s failed: %v — manual reconciliation required", + paymentResult.Status, paymentResult.SquarePayID, userID, consErr) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + } + if cErr := tx2.Commit(r.Context()); cErr != nil { + log.Printf("CRITICAL: Square payment %s (ID=%s) was processed but committing the transaction for already-completed payment %s failed: %v — manual reconciliation required", + paymentResult.Status, paymentResult.SquarePayID, paymentID, cErr) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + // The payment IS completed (by the webhook/sweep) — report success + // exactly like the normal completion path; no side-effects re-run. + if err := json.NewEncoder(w).Encode(PaymentResponse{ + ID: paymentID, + BookingID: bookingID, + PaymentType: req.PaymentType, + Status: "completed", + Amount: chargeAmount, + CardBrand: paymentResult.CardBrand, + CardLast4: paymentResult.CardLast4, + ReceiptURL: paymentResult.ReceiptURL, + CreatedAt: clock.Now().Format(time.RFC3339), + }); err != nil { + log.Printf("Failed to encode JSON response: %v", err) + } + return + } // MEDIUM-2 / finding 1: a saved-card charge reached its terminal SUCCESS // state. For a FRESH charge the gate already consumed the code @@ -2846,10 +3026,24 @@ func CreateBookingPayment(w http.ResponseWriter, r *http.Request) { // Apply VAT to all split records if the business is VAT-registered. // Must be inside the transaction so VAT updates are atomic with inserts. + // Tip records are EXCLUDED: vat.go's single-source policy (see + // ApplyVATToBookingPayment, which skips discount / on_the_house / tip rows) + // and every other tip path (CreateTipPayment, the terminal sweep rescue) + // apply VAT to the booking portion only — an overflow payment that carves a + // payment_type='tip' record must never have VAT applied to the tip. The + // records slice holds the primary (records[0], the committed pending row + // paymentID) followed by the additional split records (paymentIDs, in + // order), so the loop keys on the record's own payment_type. vatCfg, vatErr := GetVATConfig(r.Context(), tx2) if vatErr == nil && vatCfg.IsVATRegistered { - vatIDs := append([]string{paymentID}, paymentIDs...) - for _, pid := range vatIDs { + for i, rec := range records { + if rec.PaymentType == "tip" { + continue + } + pid := paymentID + if i > 0 { + pid = paymentIDs[i-1] + } if _, execErr := tx2.Exec(r.Context(), "SELECT apply_vat_to_payment($1, $2)", pid, vatCfg.DefaultVATRate); execErr != nil { log.Printf("Failed to apply VAT to payment %s: %v", pid, execErr) } @@ -2940,6 +3134,30 @@ func CreateBookingPayment(w http.ResponseWriter, r *http.Request) { } } +// routeNonCompletedPayment handles a CreatePayment result whose status is not +// COMPLETED, mirroring the stale-pending sweep's classification (sweep.go +// classifyStalePendingByKey): APPROVED and PENDING are NON-terminal — Square +// may still settle the charge — so the pending row is left pending for a later +// sweep run to re-poll (staleReconcileLeavePending); CANCELED, FAILED and any +// unknown status are terminal non-success — the charge never landed, so the row +// is marked failed so a same-key retry can never issue a second Square charge. +// The HTTP response is a 5xx in both cases: the payment is NOT complete, the +// frontend must not show success, and the sweep reconciles the row in the +// background regardless. +func routeNonCompletedPayment(ctx context.Context, w http.ResponseWriter, bookingID, paymentID string, paymentResult *square.PaymentResult, label string) { + switch paymentResult.Status { + case "APPROVED", "PENDING": + log.Printf("%s payment %s for booking %s is %q at Square (non-terminal) — leaving the row pending; the stale-pending sweep will reconcile it", label, paymentID, bookingID, paymentResult.Status) + http.Error(w, "Payment is still processing at the payment provider and has not yet been confirmed", http.StatusInternalServerError) + default: + log.Printf("%s payment %s for booking %s is %q at Square (terminal non-success) — marking the row failed", label, paymentID, bookingID, paymentResult.Status) + if _, upErr := db.Conn.Exec(ctx, `UPDATE payments SET status = 'failed' WHERE id = $1 AND status = 'pending'`, paymentID); upErr != nil { + log.Printf("CRITICAL: Square %s payment %s (ID=%s) is %q but marking payment %s failed errored: %v — manual reconciliation required", label, paymentID, paymentResult.SquarePayID, paymentResult.Status, paymentID, upErr) + } + http.Error(w, "Payment failed", http.StatusPaymentRequired) + } +} + // campaignExhaustedAtApplyError reports that a discount campaign the customer // was shown as eligible at preview time was exhausted (times_redeemed reached // max_redemptions) by the time the payment applied it (B13). lostPence is the @@ -3325,13 +3543,18 @@ func buildTerminalSplitRecords(primary PaymentRecord, info *BookingPaymentInfo, // record. The terminal base key (booking + amount + Square payment ID) can be // long enough that appending a -split-tip suffix would exceed the // payments.idempotency_key VARCHAR(64) limit (e.g. the dev mock's 28-char -// "pay_mock_" payment IDs); the base is truncated so the suffix -// always fits. Uniqueness is preserved: the truncated base still embeds the -// booking ID and Square payment ID, and the suffix differs per split record. +// "pay_mock_" payment IDs); an over-length base is routed through +// the deterministic truncateIdempotencyKey helper (idempotency_helpers.go) so +// the key always fits and the suffix stays verbatim (the split role remains +// readable and the -split-1 / -split-tip distinction is exact). Deterministic +// hashing — NOT a raw prefix cut — preserves uniqueness: two distinct base keys +// (e.g. Square payment IDs differing only in their tail) must never collapse +// onto the same truncated prefix, which would 500 the second insert on the +// UNIQUE(idempotency_key) index. func splitIdempotencyKey(base, suffix string) string { maxBase := 64 - len(suffix) if len(base) > maxBase { - base = base[:maxBase] + base = truncateIdempotencyKey("split", base) } return base + suffix } @@ -3510,6 +3733,20 @@ func isTokenLikeSaveSource(cardToken string) bool { return strings.HasPrefix(cardToken, "cnon:") || strings.HasPrefix(cardToken, "ccof:") || strings.HasPrefix(cardToken, "verify_mock_") } +// isSCATokenizeResultShape reports whether a card token is a genuine SCA +// tokenize-result — the shapes Square mints only AFTER the buyer completed +// issuer verification (the frontend's tokenizeWithVerification returns +// cnon:sca-__ok|_deny; verify_mock_ is the dev/mock transition +// shape). Mirrors the dev mock's isSCATokenizeResultSource. Unlike +// isTokenLikeSaveSource (the DEDICATED add-card surface, where every token +// passed through a STORE-intent tokenize flow), a plain card-entry nonce +// (cnon:... without the sca- marker) is NOT an SCA proof — it is a raw +// card.tokenize() result — so the charge-time SAVE gate must keep refusing it +// 402 in an enforced deployment (auth-F1). +func isSCATokenizeResultShape(cardToken string) bool { + return strings.HasPrefix(cardToken, "cnon:sca-") || strings.HasPrefix(cardToken, "verify_mock_") +} + // isDefinitiveCardSaveFailure reports whether a CreatePaymentMethod error is a // definitive client rejection — an expired/invalid/already-used card source or // a declined card that can never be saved — as opposed to an ambiguous @@ -5033,6 +5270,15 @@ func CreateTipPayment(w http.ResponseWriter, r *http.Request) { return } + // R11: a nil error does not mean the payment is COMPLETED — see + // CreateBookingPayment. APPROVED/PENDING stay pending for the sweep to + // re-poll (staleReconcileLeavePending); CANCELED/FAILED are marked failed + // (the tip charge never landed). + if paymentResult.Status != "COMPLETED" { + routeNonCompletedPayment(r.Context(), w, bookingID, paymentID, paymentResult, "tip") + return + } + // Step 3a: post-charge recheck (R9). A concurrent cancellation/eviction // can move the booking out of a payable state between the pre-charge // status check and the Square charge completing. A tip landing on a @@ -5065,16 +5311,70 @@ func CreateTipPayment(w http.ResponseWriter, r *http.Request) { } // Step 3: Square succeeded — update the payment record. - if _, upErr := recheckTx.Exec(r.Context(), - `UPDATE payments SET status = 'completed', square_payment_id = $1 WHERE id = $2`, + // R10: guard the flip on status='pending'. The Square payment.completed + // webhook can win the booking FOR UPDATE lock between the Square call and + // this UPDATE and complete the tip row itself. Without the guard this + // would blindly re-flip the already-completed row and re-run the 2FA + // consume below; with it, a row the webhook/sweep already resolved is a + // no-op (RowsAffected=0): the tip IS completed — report success. + flipRes, upErr := recheckTx.Exec(r.Context(), + `UPDATE payments SET status = 'completed', square_payment_id = $1 WHERE id = $2 AND status = 'pending'`, paymentResult.SquarePayID, paymentID, - ); upErr != nil { + ) + if upErr != nil { log.Printf("Failed to update payment %s after Square success: %v (square_payment_id=%s)", paymentID, upErr, paymentResult.SquarePayID) // Square charge succeeded but status update failed. // Record stays 'pending' for manual reconciliation. http.Error(w, "internal server error", http.StatusInternalServerError) return } + if flipRes.RowsAffected() == 0 { + // Defense-in-depth re-read (R10): distinguish a webhook-completed tip + // row (skip the side-effects — the webhook already recorded it) from + // a vanished/failed row (CRITICAL — money was taken at Square). + var curStatus string + rErr := recheckTx.QueryRow(r.Context(), `SELECT status FROM payments WHERE id = $1`, paymentID).Scan(&curStatus) + if rErr != nil || curStatus != "completed" { + log.Printf("CRITICAL: Square tip payment %s (ID=%s) was processed but tip row %s could not be verified as completed (re-read status=%q err=%v) — manual reconciliation required", + paymentResult.Status, paymentResult.SquarePayID, paymentID, curStatus, rErr) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + log.Printf("Tip payment %s for booking %s was already resolved to %q by a concurrent resolver (Square webhook/sweep) before the sync completion flip — skipping completion side-effects", paymentID, bookingID, curStatus) + // MEDIUM-2: burn the re-issued 2FA code anyway (idempotent) — the + // charge reached terminal success, so a single-use code re-issued for + // this retry must not authorize another charge. + if req.CardID != nil && *req.CardID != "" && reusePendingRecord { + if consErr := twofa.ConsumePendingCode(r.Context(), recheckTx, userID); consErr != nil { + log.Printf("CRITICAL: Square tip payment %s (ID=%s) was processed but consuming the 2FA code for user %s failed: %v — manual reconciliation required", + paymentResult.Status, paymentResult.SquarePayID, userID, consErr) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + } + if cErr := recheckTx.Commit(r.Context()); cErr != nil { + log.Printf("CRITICAL: Square tip payment %s (ID=%s) succeeded but committing the post-charge transaction for already-completed tip %s failed: %v — manual reconciliation required", + paymentResult.Status, paymentResult.SquarePayID, paymentID, cErr) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + // The tip IS completed (by the webhook/sweep) — report success exactly + // like the normal completion path; no side-effects re-run. + if err := json.NewEncoder(w).Encode(PaymentResponse{ + ID: paymentID, + BookingID: bookingID, + PaymentType: "tip", + Status: "completed", + Amount: req.Amount, + CardBrand: paymentResult.CardBrand, + CardLast4: paymentResult.CardLast4, + ReceiptURL: paymentResult.ReceiptURL, + CreatedAt: clock.Now().Format(time.RFC3339), + }); err != nil { + log.Printf("Failed to encode JSON response: %v", err) + } + return + } // MEDIUM-2 / finding 1: a saved-card tip charge reached its terminal // SUCCESS state. For a FRESH charge the gate already consumed the code // (single-use at verify time); for a PENDING-REUSE retry (gate passed diff --git a/backend/handlers/payments/refund_policy.go b/backend/handlers/payments/refund_policy.go index b55e77b..3531015 100644 --- a/backend/handlers/payments/refund_policy.go +++ b/backend/handlers/payments/refund_policy.go @@ -23,7 +23,7 @@ const ( // deposit REQUIRED at booking time, used by bookings.go): the promotion // threshold is about already-paid money, not the amount to demand up front, // even though both are 20% today. - depositPromotionMinPct = 0.2 + depositPromotionMinPct = RequiredDepositPct LoyaltyStampCost = 10 LoyaltyDiscountPercent = 10.0 diff --git a/backend/handlers/payments/refunds.go b/backend/handlers/payments/refunds.go index 2bca6ce..df2bd3c 100644 --- a/backend/handlers/payments/refunds.go +++ b/backend/handlers/payments/refunds.go @@ -578,6 +578,44 @@ func ProcessCancellationRefundTx( } break } + // A REDEEMED card can no longer be used — the terminal path rejects + // redeemed cards and RedeemGiftCard refuses a second redemption — so + // crediting amount_remaining back onto the card would strand the + // refunded money forever. Route the credit to the booking user's + // gift-card account balance instead: the same destination the + // redemption moved the card's value to (RedeemGiftCard zeroes + // amount_remaining and credits the balance). Guests/ownerless + // bookings have no balance to credit — record 'failed' for admin + // reconciliation, mirroring the cash branch's guest handling. + var redeemedBy *string + if err := tx.QueryRow(ctx, `SELECT redeemed_by FROM gift_cards WHERE id = $1`, *giftCardID).Scan(&redeemedBy); err != nil { + // Fail-open like the expiry check below: a failed read cannot + // prove the card is redeemed, so the card-credit path proceeds + // (a deleted card is still caught by the M2 RowsAffected check). + log.Printf("Failed to check gift card %s redemption status: %v — proceeding with card credit", *giftCardID, err) + } else if redeemedBy != nil { + if isGuest || bookingUserID == "" { + if isGuest { + log.Printf("Guest giftcard refund on redeemed card %s: booking %s, payment %s, amount £%.2f — no balance credit, recorded 'failed' (admin must hand out cash at till)", *giftCardID, bookingID, paymentID, refundThisPayment) + } else { + log.Printf("Gift card %s is redeemed and payment %s has no booking user — no balance credit, recorded 'failed' (admin reconciliation)", *giftCardID, paymentID) + } + creditFailed = true + break + } + log.Printf("Gift card %s is redeemed — crediting £%.2f refund to user %s gift-card balance instead", *giftCardID, refundThisPayment, bookingUserID) + if _, balErr := tx.Exec(ctx, ` + INSERT INTO user_giftcard_balances (user_id, balance, updated_at) + VALUES ($1, $2, NOW()) + ON CONFLICT (user_id) DO UPDATE SET + balance = user_giftcard_balances.balance + EXCLUDED.balance, + updated_at = NOW() + `, bookingUserID, refundThisPayment); balErr != nil { + log.Printf("Failed to credit user %s gift-card balance for refund of booking %s: %v", bookingUserID, bookingID, balErr) + creditFailed = true + } + break + } // expiry_date is maintained by EVERY gift-card write that counts as // a "use" (balance check, top-up, transfer, redeem, payment, refund // credit), so a non-NULL expiry_date means the rolling timer is @@ -633,16 +671,20 @@ func ProcessCancellationRefundTx( case "cash": if isGuest || bookingUserID == "" { if isGuest { - log.Printf("Guest cash refund: booking %s, payment %s, amount £%.2f — admin must process cash refund at till", bookingID, paymentID, refundThisPayment) + log.Printf("Guest cash refund: booking %s, payment %s, amount £%.2f — no balance credit, recorded 'failed' (admin must hand out cash at till)", bookingID, paymentID, refundThisPayment) } else { - log.Printf("Cash refund: payment %s has no booking user — skipping balance credit", paymentID) - } - if bookingUserLookupFailed { - // The booking-user lookup errored, so this may be a real - // user whose balance credit was skipped — a 'completed' - // refund would claim money was returned when it wasn't. - creditFailed = true + log.Printf("Cash refund: payment %s has no booking user — no balance credit, recorded 'failed' (admin reconciliation)", paymentID) } + // No balance can be credited for a guest/ownerless booking, so + // the money never moves here. Recording the row 'completed' + // (the pre-fix behaviour) would let the over-refund guard + // (GetAlreadyRefundedAmount counts completed) permanently block + // re-issuance if the admin never hands out the cash. Record + // 'failed' instead: the post-commit pre-pass in + // ProcessPendingSquareRefunds surfaces a 'refund_failed' admin + // notification so the pending payout stays visible, and the + // guard stays open for a re-issued attempt. + creditFailed = true } else { log.Printf("Crediting £%.2f to user %s balance for cash payment %s", refundThisPayment, bookingUserID, paymentID) if _, balErr := tx.Exec(ctx, ` @@ -902,6 +944,40 @@ func ProcessPendingSquareRefunds(ctx context.Context, bookingID string, reason s insertRefundFailedNotifications(ctx, failedIDs) } + // (a2) Guest/ownerless cash and redeemed-gift-card cancellation refunds are + // recorded 'failed' in the cancellation loop (no balance could be + // credited — the money never moved) and must surface in the admin + // notification centre so the cash is actually handed out / reconciled. + // Runs POST-COMMIT, mirroring the Square-less pre-pass above (the rows + // committed with the enclosing transaction). The (reason, booking_id) + // NOT EXISTS dedup in insertRefundFailedNotifications prevents + // double-notifying across re-runs. + cashFailedRows, err := db.Conn.Query(ctx, ` + SELECT r.id FROM refunds r + JOIN payments p ON p.id = r.payment_id + WHERE r.booking_id = $1 AND r.status = 'failed' AND r.origin = 'cancellation' + AND p.payment_method IN ('cash', 'giftcard') + `, bookingID) + if err != nil { + log.Printf("Failed to query failed cash/giftcard refunds for booking %s: %v", bookingID, err) + } else { + var cashFailedIDs []string + for cashFailedRows.Next() { + var id string + if err := cashFailedRows.Scan(&id); err == nil { + cashFailedIDs = append(cashFailedIDs, id) + } + } + if err := cashFailedRows.Err(); err != nil { + log.Printf("Failed to iterate failed cash/giftcard refunds for booking %s: %v", bookingID, err) + } + cashFailedRows.Close() + if len(cashFailedIDs) > 0 { + log.Printf("Surfacing %d failed cash/giftcard refund(s) for booking %s (guest/ownerless — admin must arrange payout)", len(cashFailedIDs), bookingID) + } + insertRefundFailedNotifications(ctx, cashFailedIDs) + } + // (b) This booking's card charges with pending refunds — one aggregated // Square refund per charge. for _, chargeID := range queryChargesWithPendingRefunds(ctx, "r.booking_id = $1", bookingID) { @@ -950,6 +1026,37 @@ func SweepPendingSquareRefunds(ctx context.Context) (int, error) { insertRefundFailedNotifications(ctx, failedIDs) } + // (a2) Guest/ownerless cash and redeemed-gift-card cancellation refunds + // recorded 'failed' (money never moved) must surface in the admin + // notification centre so the cash is actually handed out / reconciled. + // Mirrors the booking-scoped pre-pass in ProcessPendingSquareRefunds; + // the (reason, booking_id) NOT EXISTS dedup prevents double-notifying. + cashFailedRows, err := db.Conn.Query(ctx, ` + SELECT r.id FROM refunds r + JOIN payments p ON p.id = r.payment_id + WHERE r.status = 'failed' AND r.origin = 'cancellation' + AND p.payment_method IN ('cash', 'giftcard') + `) + if err != nil { + log.Printf("Failed to query failed cash/giftcard refunds during sweep: %v", err) + } else { + var cashFailedIDs []string + for cashFailedRows.Next() { + var id string + if err := cashFailedRows.Scan(&id); err == nil { + cashFailedIDs = append(cashFailedIDs, id) + } + } + if err := cashFailedRows.Err(); err != nil { + log.Printf("Failed to iterate failed cash/giftcard refunds during sweep: %v", err) + } + cashFailedRows.Close() + if len(cashFailedIDs) > 0 { + log.Printf("Surfacing %d failed cash/giftcard refund(s) (guest/ownerless — admin must arrange payout)", len(cashFailedIDs)) + } + insertRefundFailedNotifications(ctx, cashFailedIDs) + } + // (b) Charges with pending refunds — one aggregated Square refund each. processed := 0 for _, chargeID := range queryChargesWithPendingRefunds(ctx, "") { @@ -1539,16 +1646,27 @@ func idsOf(rows []pendingChargeRow) []string { return ids } -// aggRefundKeySuffix returns a deterministic 12-hex-char suffix for a set of +// aggRefundKeySuffix returns a deterministic hex suffix for a set of // identifiers. The identifiers (CHAR(12) refund ids, or an over-length // square_payment_id) are sorted, sha256'd and truncated, so the SAME input // always yields the SAME suffix. Used to compress an over-length chargeID into // a fixed-width prefix for the charge-level idempotency key (see chargeAggKey). +// +// The suffix is the full SHA-256 hex trimmed to the budget left by the +// "-square-agg" structure: chargeAggKey appends 11 chars, so the suffix is +// capped at maxIdempotencyKeyLength-11 = 34 chars (136 bits). The historical +// 12-char (48-bit) truncation made a collision between two DISTINCT chargeIDs +// plausible enough that Square's global idempotency-key dedup could silently +// swallow the second charge's refund (lost money, no refund row); 136 bits +// makes a cross-charge collision cryptographically negligible. func aggRefundKeySuffix(ids []string) string { sorted := append([]string(nil), ids...) sort.Strings(sorted) h := sha256.Sum256([]byte(strings.Join(sorted, ""))) - return fmt.Sprintf("%x", h)[:12] + // The suffix is joined with "-square-agg" by chargeAggKey; keep the whole + // key inside Square's 45-char idempotency-key limit. + const aggSuffixLen = maxIdempotencyKeyLength - len("-square-agg") + return fmt.Sprintf("%x", h)[:aggSuffixLen] } // chargeAggKey builds the charge-level idempotency key for an aggregated diff --git a/backend/handlers/payments/sweep.go b/backend/handlers/payments/sweep.go index 06294d5..fcef404 100644 --- a/backend/handlers/payments/sweep.go +++ b/backend/handlers/payments/sweep.go @@ -237,6 +237,94 @@ type staleRow struct { B1Attempts int } +// acquireTillSaleSweepLock serializes the sweep's fail/clawback of a stale +// till_sale on the SAME advisory lock a same-key retry pins while its Square +// charge is mid-flight: "crussell:till:" (till.go — the retry +// path holds it from before the Square round-trip until the row is completed). +// +// The race this closes (FIX 1, MAJOR): without the lock, the sweep can probe +// Square for the charge under the key while the retry's charge has not landed +// yet, read "no payment under the key", mark the sale failed and claw back the +// funded gift card — then the retry's charge lands. The retry's completion +// UPDATE (guarded on status='pending') matches 0 rows and logs CRITICAL: the +// customer is charged at Square AND the funding is clawed back. By holding the +// same lock before failing/clawing-back, the sweep either serializes after the +// retry (the row is already 'completed' — the fail/clawback is a 0-row no-op) +// or the lock is contended and the row is DEFERRED to a later pass (the retry +// may still land its charge). +// +// Returns (release, true) when the lock is held — the caller MUST call +// release() before resolving the row; (nil, false) when the lock could not be +// acquired within the bounded try-lock (~3s) — a same-key retry is likely +// mid-flight at Square, so the caller must SKIP the row this pass (it is +// reconciled again next sweep) — or a pinned pool connection could not be +// acquired. A row with NO idempotency key returns a no-op release with true: +// no retry path can hold a lock on it, so there is nothing to serialize on. +func acquireTillSaleSweepLock(ctx context.Context, r staleRow) (release func(), ok bool) { + if r.IdempotencyKey == "" { + return func() {}, true + } + pinConn, err := db.Conn.Acquire(ctx) + if err != nil { + log.Printf("Failed to acquire connection for till-sale sweep lock on stale row %s: %v", r.ID, err) + return nil, false + } + lockKey := "crussell:till:" + r.IdempotencyKey + acquired, lErr := acquireAdvisoryLock(ctx, pinConn, lockKey) + if lErr != nil { + log.Printf("Failed to acquire till-sale sweep lock %s for stale row %s: %v", lockKey, r.ID, lErr) + pinConn.Release() + return nil, false + } + if !acquired { + // A same-key retry holds the lock and its Square charge may be + // mid-flight — deferring avoids failing/clawing-back a sale whose + // charge can still land. The row is reconciled again next pass. + log.Printf("Stale pending till_sale %s: till-sale lock %s is contended within the bound — a same-key retry may be mid-flight at Square — deferring the row to a later sweep pass", r.ID, lockKey) + pinConn.Release() + return nil, false + } + return func() { + releasePaymentLock(pinConn, lockKey) + pinConn.Release() + }, true +} + +// tillSalePendingAfterLock re-reads a till_sales row's CURRENT stored +// idempotency_key and status AFTER the sweep acquired its advisory lock and +// reports whether the fail/clawback may still proceed. +// +// This closes the residual window of the FIX-1 lock (BUG 2, MAJOR): a same-key +// retry of a KEYLESS sale locks the derived BASE key (crussell:till:) +// while its slot scan may resolve a SUFFIXED final key (base-2) that becomes +// the STORED key — the key the sweep locks (till.go). Until the retry side +// pins the final key too, the sweep and the retry do NOT serialize, so the +// retry can complete the sale (status → 'completed', its Square charge +// landed) while the sweep waited on — or just acquired — its lock on the stale +// stored key. A blind fail/clawback in that window would reverse the funding +// of a sale whose charge is real. Re-reading the row under the lock detects +// the retry's completion; a row that is no longer 'pending' — or whose stored +// key changed while the sweep waited — is left alone and reconciled again +// under its current key next sweep. +func tillSalePendingAfterLock(ctx context.Context, r staleRow) bool { + var key, status string + if err := db.Conn.QueryRow(ctx, ` + SELECT COALESCE(idempotency_key, ''), status FROM till_sales WHERE id = $1 + `, r.ID).Scan(&key, &status); err != nil { + log.Printf("Failed to re-read till sale %s after acquiring its sweep lock (%v) — skipping the fail/clawback this pass; the row is reconciled next sweep", r.ID, err) + return false + } + if status != "pending" { + log.Printf("Stale pending till sale %s is now %q (a same-key retry completed or failed it while the sweep waited) — skipping the fail/clawback; the row is reconciled next sweep", r.ID, status) + return false + } + if key != r.IdempotencyKey { + log.Printf("Stale pending till sale %s's stored idempotency key changed from %q to %q while the sweep waited (a same-key retry re-scanned its slot) — skipping the fail/clawback on the stale key; the row is reconciled next sweep under its current key", r.ID, r.IdempotencyKey, key) + return false + } + return true +} + // sweepStaleRows resolves the stale pending rows of one table. Rows with a // square_payment_id are reconciled at Square first (COMPLETED → 'completed', // anything else → 'failed' exactly as the legacy bulk UPDATE did); rows @@ -312,11 +400,39 @@ func sweepStaleRows(ctx context.Context, table string, cutoff time.Time) (resolv // completed (NOT_FOUND / non-COMPLETED) claws back the funded gift // card atomically with the failed mark (HIGH-1). The blind-fail path // (no square_payment_id — the charge outcome is unknown) NEVER claws - // back: the money may have landed at Square. - if table == "till_sales" && r.SquarePaymentID != "" && r.HasGiftCard { - if clawbackTillSaleFunding(ctx, r) { - resolved++ + // back: the money may have landed at Square. FIX 1: a till_sale is + // resolved under its own till-sale advisory lock so the sweep cannot + // race a late same-key retry whose Square charge is mid-flight. + if table == "till_sales" { + release, lockOK := acquireTillSaleSweepLock(ctx, r) + if !lockOK { + continue } + // BUG 2: the lock may not serialize against a keyless-sale retry + // (its lock is the derived base key, ours is the stored suffix key) + // — re-read the row and refuse to fail/clawback a sale a retry + // already completed or re-keyed while the sweep waited. + if !tillSalePendingAfterLock(ctx, r) { + release() + continue + } + if r.SquarePaymentID != "" && r.HasGiftCard { + if clawbackTillSaleFunding(ctx, r) { + resolved++ + } + } else if failStaleRow(ctx, table, r.ID) { + resolved++ + // A5a: blind-failing a row with NO square_payment_id leaves the + // charge outcome unknown (the money may have landed at Square + // with a lost response), so surface it in the admin notification + // centre. Rows that reach this line WITH a square_payment_id + // were PROVED never-charged by the by-id reconcile and need no + // alert. + if r.SquarePaymentID == "" { + notifyStaleRowCritical(ctx, r) + } + } + release() continue } if failStaleRow(ctx, table, r.ID) { @@ -388,6 +504,26 @@ func sweepKeyedStaleRows(ctx context.Context, table string, cutoff time.Time) (r // A row here is failed (NOT clawed back — the duplicate money may stand // at Square) with a CRITICAL notification for manual reconciliation. if hasFailedSweepDuplicateRefund(ctx, table, r.ID) || r.B1Attempts >= b1DuplicateRefundAttemptCap { + // FIX 1: a till_sale is failed under its own till-sale advisory + // lock so a same-key retry mid-flight at Square cannot land its + // charge onto a row the sweep just marked failed. + if table == "till_sales" { + release, lockOK := acquireTillSaleSweepLock(ctx, r) + if !lockOK { + continue + } + if !tillSalePendingAfterLock(ctx, r) { + release() + continue + } + if failStaleRow(ctx, table, r.ID) { + resolved++ + } + notifyStaleRowCritical(ctx, r) + log.Printf("Stale pending %s row %s has a FAILED or attempt-capped B1 auto-refund of a replay-induced duplicate charge (b1_attempts=%d) — never re-replaying the expired key; marked failed — MANUAL RECONCILIATION REQUIRED: check Square for the duplicate charge and refund it", table, r.ID, r.B1Attempts) + release() + continue + } if failStaleRow(ctx, table, r.ID) { resolved++ } @@ -403,6 +539,27 @@ func sweepKeyedStaleRows(ctx context.Context, table string, cutoff time.Time) (r // A5a: this blind-fail can be hiding a real charge (the lost // response the stored key was meant to reconcile), so the admin // notification centre must surface it too. + // FIX 1: a till_sale is blind-failed under its own till-sale + // advisory lock so a same-key retry mid-flight at Square cannot + // land its charge onto a row the sweep just marked failed. + if table == "till_sales" { + release, lockOK := acquireTillSaleSweepLock(ctx, r) + if !lockOK { + continue + } + if !tillSalePendingAfterLock(ctx, r) { + release() + continue + } + if failStaleRow(ctx, table, r.ID) { + resolved++ + unverifiable++ + notifyStaleRowCritical(ctx, r) + } + release() + log.Printf("Stale pending %s row %s has a stored idempotency key but is already past Square's key retention window — marked failed without a replay reconcile (may have been charged with a lost response)", table, r.ID) + continue + } if failStaleRow(ctx, table, r.ID) { resolved++ unverifiable++ @@ -444,10 +601,27 @@ func sweepKeyedStaleRows(ctx context.Context, table string, cutoff time.Time) (r // Square proved the charge never happened (key unknown / FAILED) — // a till sale's funded gift card is clawed back atomically with the // failed mark, unlike the blind-fail path where the outcome is unknown. - if table == "till_sales" && r.HasGiftCard { - if clawbackTillSaleFunding(ctx, r) { + // FIX 1: the fail/clawback runs under the row's till-sale advisory + // lock so the sweep cannot race a late same-key retry whose Square + // charge is mid-flight (its "no payment under the key" probe would + // be premature). + if table == "till_sales" { + release, lockOK := acquireTillSaleSweepLock(ctx, r) + if !lockOK { + continue + } + if !tillSalePendingAfterLock(ctx, r) { + release() + continue + } + if r.HasGiftCard { + if clawbackTillSaleFunding(ctx, r) { + resolved++ + } + } else if failStaleRow(ctx, table, r.ID) { resolved++ } + release() continue } if failStaleRow(ctx, table, r.ID) { @@ -460,7 +634,36 @@ func sweepKeyedStaleRows(ctx context.Context, table string, cutoff time.Time) (r // till sale's funded gift card is NOT clawed back. Mark the parent // row failed, raise the CRITICAL notification, and set b1_attempts // to the cap so the expired key is never replayed (each replay - // would mint ANOTHER charge). + // would mint ANOTHER charge). FIX 1: a till_sale is failed under + // its own till-sale advisory lock so a same-key retry mid-flight at + // Square cannot land its charge onto a row the sweep just marked + // failed. + if table == "till_sales" { + release, lockOK := acquireTillSaleSweepLock(ctx, r) + if !lockOK { + continue + } + if !tillSalePendingAfterLock(ctx, r) { + release() + continue + } + if failStaleRow(ctx, table, r.ID) { + resolved++ + } + notifyStaleRowCritical(ctx, r) + setB1AttemptsToCap(ctx, table, r.ID) + // Loop-B finding (MED): a capped-fail on a till_sale leaves its + // funded gift card outstanding — the duplicate charge at Square is + // REAL and a manual Square refund is the only reversal. Surface the + // outstanding funding with a gift_card_transactions trace so the + // operator sees it (see insertOutstandingGiftCardFundingTrace for + // the manual-resolution path). + if r.HasGiftCard { + insertOutstandingGiftCardFundingTrace(ctx, r) + } + release() + continue + } if failStaleRow(ctx, table, r.ID) { resolved++ } @@ -759,22 +962,37 @@ func applyStaleRescueRecords(ctx context.Context, tx pgx.Tx, r staleRow, records if len(records) == 0 { return } + // The align UPDATE clears the VAT fields exactly like the live post-charge + // path (handlers.go align UPDATE sets is_vat_applicable = FALSE, vat_rate = + // NULL, vat_amount = NULL, net_amount = NULL before re-applying VAT). + // WITHOUT the clearing (pre-fix bug) the pending row's VAT — computed at + // step-1 insert time on the FULL pre-split charge (handlers.go step-1 + // ApplyVATToBookingPayment) — survived the align, and the re-apply below was + // a silent no-op because apply_vat_to_payment is guarded on vat_amount IS + // NULL (init-script.sql): the rescued primary row kept VAT on the wrong + // (larger) base. Clearing first makes the recompute effective on the split + // amount, and the SQL's payment_type != 'tip' guard leaves an all-tip + // rescue (the primary aligned to the carved tip record) VAT-free. if _, upErr := tx.Exec(ctx, ` UPDATE payments SET amount = $1, payment_type = $2, fees = $3, + is_vat_applicable = FALSE, + vat_rate = NULL, + vat_amount = NULL, + net_amount = NULL, updated_at = NOW() WHERE id = $4 `, records[0].Amount, records[0].PaymentType, records[0].Fees, r.ID); upErr != nil { log.Printf("MEDIUM-3: failed to align rescued payment %s to its split primary (%v) — manual reconciliation recommended", r.ID, upErr) return } - // M5: the align UPDATE used to NULL the VAT fields, dropping a rescued - // charge out of VAT reporting. The synchronous post-charge path applies VAT - // per record (ApplyVATToBookingPayment); the sweep re-applies it after the - // align. apply_vat_to_payment is idempotent (guarded on vat_amount IS NULL) - // and skips discount/on_the_house/tip rows internally. + // M5: the synchronous post-charge path applies VAT per record + // (ApplyVATToBookingPayment); the sweep re-applies it after the align — + // now effective because the align cleared the stale VAT fields above. + // apply_vat_to_payment is idempotent (guarded on vat_amount IS NULL) and + // skips discount/on_the_house/tip rows internally. ApplyVATToBookingPayment(ctx, tx, r.ID) svc := NewPaymentService() for _, rec := range records[1:] { @@ -1092,6 +1310,26 @@ func parseReplayedCreatedAt(pr *square.PaymentResult) (time.Time, bool) { // seeding the Square payment at test time, and a retained-key dedup returns // that seeded payment). The gate mirrors the snapshot-decryption gate, which // also runs only in a non-dev/mock env. +// +// MONEY-DECISION PARITY (FIX 1, MINOR — pinned by +// TestReplayRevealsNewCharge_DevMockGate_MoneyDecisionParity): the gate's +// dev/mock outcome — "not a new charge", rescue the row — is IDENTICAL to what +// the ungated prod classifier decides for every payment the dev mock can +// actually return from ReplayPaymentByKey. The mock has NO key-retention / +// key-expiry concept: a replay of a key already in its ledger dedups to the +// ORIGINAL payment (created ~at row-creation time — inside the legitimate +// window, so prod would also rescue it), and the only other COMPLETED outcome +// is a fresh charge the mock creates AT the replay instant for an unknown key — +// which lands within replayRescueUpperBoundSkew (5s) of replayAt and is +// therefore still legitimate to prod (the same-key-retry tolerance). The gate +// short-circuits ONLY the prod-branch scenarios the mock cannot produce — an +// UNPARSEABLE created_at (prod: leave the row PENDING with a CRITICAL +// notification) and a payment created beyond the upper-bound skew (prod: B1 +// auto-refund the provable duplicate) — so the dev sweep path deliberately +// does NOT exercise those prod branches. If the mock ever gains a key-expiry +// simulation that returns a payment created beyond the skew, the pin test +// fails and this gate MUST be revisited (prod and dev would diverge on the +// money decision for a provably-new expired-key charge). func replayRevealsNewCharge(r staleRow, replayAt time.Time, pr *square.PaymentResult) (newCharge bool, created time.Time, createdOK bool) { if IsExplicitDevOrMockEnv() { return false, time.Time{}, false @@ -2681,6 +2919,12 @@ func recordUntrackedTillSalePayment(ctx context.Context, saleID string, pr *squa log.Printf("Terminal till sale %s was already resolved — skipping untracked terminal completion record", saleID) return false } + // FIX 2: the synchronous till-completion path (GetTillCheckoutStatus) and + // the stale-pending rescue apply VAT via ApplyVATToTillSale; an untracked + // terminal till sale recorded here must do the same or the sale silently + // drops out of VAT reporting. The SQL function is idempotent (guarded on + // vat_amount IS NULL), so a run racing the poll's own apply is a no-op. + ApplyVATToTillSale(ctx, db.Conn, saleID) log.Printf("CRITICAL: recorded untracked terminal till-sale charge %s (sale %s) from a stale checkout — sale marked completed (never polled by the frontend)", pr.SquarePayID, saleID) return true } diff --git a/backend/handlers/payments/till.go b/backend/handlers/payments/till.go index 2bbca1f..db4b288 100644 --- a/backend/handlers/payments/till.go +++ b/backend/handlers/payments/till.go @@ -342,6 +342,235 @@ func revertGiftCardFunding(ctx context.Context, action, giftCardID string, amoun return RevertGiftCardFunding(ctx, action, giftCardID, amount, redeemToUserID, tillSaleID) } +// recreditTillSaleFundingAfterMidFlightResolve restores a till sale's gift-card +// funding after a concurrent clawback reverted it while a same-key retry's +// Square charge was mid-flight (FIX 1). The retry's charge HAS landed, so the +// customer paid for a card that was clawed back — this re-credits the funding +// so the money is not lost, turning "charged AND clawed back" into "charged +// AND funded". It runs BEFORE the handler fails the response; the sale row +// itself stays 'failed' (completing it would race the writer that resolved it) +// and is left for manual reconciliation. +// +// Re-credit is strictly conditional so it can never DOUBLE-fund a card: +// - the charge must be CONFIRMED at Square (re-query GetPayment) — a charge +// that did not land has nothing to fund; +// - the funding must actually have been CLAWED BACK — a create's card was +// deleted (row gone) and a topup's amount was subtracted with its +// transaction removed. If the funding is still intact (the sale was +// resolved without a clawback, e.g. a blind-fail) there is nothing to +// restore and re-crediting would double the value. +// +// Returns true when the funding was re-credited; false when the charge could +// not be confirmed, the funding was never clawed back, or the re-credit +// failed (CRITICAL logged for manual reconciliation). +func recreditTillSaleFundingAfterMidFlightResolve(ctx context.Context, req TillSaleRequest, tillSaleID, giftCardID string, pr *square.PaymentResult) bool { + if pr == nil || pr.SquarePayID == "" { + return false + } + // Confirm the charge landed at Square (the retry just created it, but the + // re-query makes the decision authoritative rather than trusting the + // response alone). + confirm, cErr := SquareClient.GetPayment(ctx, pr.SquarePayID) + if cErr != nil || confirm == nil || confirm.Status != "COMPLETED" { + log.Printf("CRITICAL: till sale %s charge %s could not be confirmed at Square (re-query: %v) — gift-card funding NOT re-credited — MANUAL RECONCILIATION REQUIRED", tillSaleID, pr.SquarePayID, cErr) + return false + } + if req.Action == "create" { + // A create's clawback DELETED the card. If it still exists the funding + // was never reverted — nothing to restore (re-creating would double it). + var cardExists bool + if err := db.Conn.QueryRow(ctx, `SELECT EXISTS(SELECT 1 FROM gift_cards WHERE id = $1)`, giftCardID).Scan(&cardExists); err != nil { + log.Printf("Failed to check gift card %s for mid-flight funding re-credit: %v", giftCardID, err) + return false + } + if cardExists { + return false + } + return recreditClawedBackCreateFunding(ctx, req, tillSaleID, giftCardID) + } + // Topup: the clawback removed THIS sale's top-up transaction. If it still + // exists the funding was never reverted — nothing to restore. + var txExists bool + if err := db.Conn.QueryRow(ctx, ` + SELECT EXISTS(SELECT 1 FROM gift_card_transactions WHERE gift_card_id = $1 AND reference_type = 'till_sale' AND reference_id = $2) + `, giftCardID, tillSaleID).Scan(&txExists); err != nil { + log.Printf("Failed to check top-up transaction for mid-flight funding re-credit: %v", err) + return false + } + if txExists { + return false + } + return recreditClawedBackTopupFunding(ctx, req, tillSaleID, giftCardID) +} + +// recreditClawedBackCreateFunding re-creates the gift card a create-sale +// clawback deleted (the charge landed after the clawback), mirroring the +// create funding block of CreateTillSale: a fresh card for the amount, the +// purchase transaction, the redeem-to-account credit, and the till_sales +// item_id re-pointed at the new card so the reference stays valid. The new +// card gets a fresh auto-generated code — the original code is gone with the +// deleted row. All writes commit in ONE transaction so a crash cannot leave a +// funded card with no sale linkage. Returns true on success. +func recreditClawedBackCreateFunding(ctx context.Context, req TillSaleRequest, tillSaleID, giftCardID string) bool { + expiryMonths, expiryErr := GetGiftCardExpiryMonths(ctx, db.Conn) + if expiryErr != nil { + log.Printf("Failed to query gift card expiry months (using default %d): %v", defaultGiftCardExpiryMonths, expiryErr) + expiryMonths = defaultGiftCardExpiryMonths + } + purchaseVoucherType := "SPV" + if err := db.Conn.QueryRow(ctx, `SELECT COALESCE(voucher_type, 'SPV') FROM business_settings LIMIT 1`).Scan(&purchaseVoucherType); err != nil { + log.Printf("Failed to query voucher type: %v", err) + } + if purchaseVoucherType == "" { + purchaseVoucherType = "SPV" + } + purchaseVoucherType = effectiveVoucherTypeForPurchase(purchaseVoucherType) + + var createdBy string + if err := db.Conn.QueryRow(ctx, `SELECT created_by FROM till_sales WHERE id = $1`, tillSaleID).Scan(&createdBy); err != nil { + log.Printf("Failed to read till sale %s created_by for funding re-credit: %v", tillSaleID, err) + return false + } + + tx, err := db.Conn.Begin(ctx) + if err != nil { + log.Printf("Failed to begin funding re-credit transaction for till sale %s: %v", tillSaleID, err) + return false + } + defer func() { + if rErr := tx.Rollback(ctx); rErr != nil && !errors.Is(rErr, pgx.ErrTxClosed) { + log.Printf("Failed to rollback funding re-credit transaction for till sale %s: %v", tillSaleID, rErr) + } + }() + + var newCardID string + if err := tx.QueryRow(ctx, ` + INSERT INTO gift_cards (total_funds_added, amount_remaining, created_by, is_inventory, last_used_at, expiry_date, voucher_type_at_purchase) + VALUES ($1, $1, $2, FALSE, NOW(), NOW() + ($4 * INTERVAL '1 month'), $3) + RETURNING id + `, req.Amount, createdBy, purchaseVoucherType, expiryMonths).Scan(&newCardID); err != nil { + log.Printf("CRITICAL: failed to re-create gift card for mid-flight create clawback of till sale %s: %v — MANUAL RECONCILIATION REQUIRED", tillSaleID, err) + return false + } + if _, err := tx.Exec(ctx, ` + INSERT INTO gift_card_transactions (gift_card_id, transaction_type, amount, reference_type, reference_id, user_id, notes) + VALUES ($1, 'purchase', $2, 'till_sale', $3, $4, NULL) + `, newCardID, req.Amount, tillSaleID, req.UserID); err != nil { + log.Printf("CRITICAL: failed to record the re-credited gift-card purchase for till sale %s: %v — MANUAL RECONCILIATION REQUIRED", tillSaleID, err) + return false + } + if req.RedeemToUserID != nil && *req.RedeemToUserID != "" { + if _, err := tx.Exec(ctx, ` + UPDATE gift_cards + SET amount_remaining = 0, + redeemed_at = NOW(), + redeemed_by = $1, + last_used_at = NOW(), + expiry_date = NOW() + ($3 * INTERVAL '1 month') + WHERE id = $2 + `, *req.RedeemToUserID, newCardID, expiryMonths); err != nil { + log.Printf("CRITICAL: failed to re-apply the redeem for re-credited gift card %s (till sale %s): %v — MANUAL RECONCILIATION REQUIRED", newCardID, tillSaleID, err) + return false + } + if _, err := tx.Exec(ctx, ` + INSERT INTO user_giftcard_balances (user_id, balance, updated_at) + VALUES ($1, $2, NOW()) + ON CONFLICT (user_id) DO UPDATE SET + balance = user_giftcard_balances.balance + EXCLUDED.balance, + updated_at = NOW() + `, *req.RedeemToUserID, req.Amount); err != nil { + log.Printf("CRITICAL: failed to re-credit the redeemed balance for re-credited gift card %s (till sale %s): %v — MANUAL RECONCILIATION REQUIRED", newCardID, tillSaleID, err) + return false + } + } + if _, err := tx.Exec(ctx, `UPDATE till_sales SET item_id = $1 WHERE id = $2`, newCardID, tillSaleID); err != nil { + log.Printf("CRITICAL: failed to re-point till sale %s at the re-credited gift card %s: %v — MANUAL RECONCILIATION REQUIRED", tillSaleID, newCardID, err) + return false + } + if err := tx.Commit(ctx); err != nil { + log.Printf("CRITICAL: failed to commit the gift-card funding re-credit for till sale %s: %v — MANUAL RECONCILIATION REQUIRED", tillSaleID, err) + return false + } + log.Printf("Re-credited gift-card funding for till sale %s: re-created card %s (create %s after a mid-flight clawback — original card %s was deleted)", tillSaleID, newCardID, req.Action, giftCardID) + return true +} + +// recreditClawedBackTopupFunding re-adds the amount a topup-sale clawback +// subtracted, mirroring the top-up funding block of CreateTillSale: the amount +// is added back to the card's funds and this sale's top-up transaction is +// re-inserted. All writes commit in ONE transaction. Returns true on success. +func recreditClawedBackTopupFunding(ctx context.Context, req TillSaleRequest, tillSaleID, giftCardID string) bool { + // A concurrent gift-card cancellation can have removed the card entirely; + // then the funding cannot be re-credited. + var cardExists bool + if err := db.Conn.QueryRow(ctx, `SELECT EXISTS(SELECT 1 FROM gift_cards WHERE id = $1)`, giftCardID).Scan(&cardExists); err != nil { + log.Printf("Failed to check gift card %s for mid-flight funding re-credit: %v", giftCardID, err) + return false + } + if !cardExists { + log.Printf("CRITICAL: till sale %s top-up funding cannot be re-credited — gift card %s no longer exists — MANUAL RECONCILIATION REQUIRED", tillSaleID, giftCardID) + return false + } + + expiryMonths, expiryErr := GetGiftCardExpiryMonths(ctx, db.Conn) + if expiryErr != nil { + log.Printf("Failed to query gift card expiry months (using default %d): %v", defaultGiftCardExpiryMonths, expiryErr) + expiryMonths = defaultGiftCardExpiryMonths + } + + tx, err := db.Conn.Begin(ctx) + if err != nil { + log.Printf("Failed to begin funding re-credit transaction for till sale %s: %v", tillSaleID, err) + return false + } + defer func() { + if rErr := tx.Rollback(ctx); rErr != nil && !errors.Is(rErr, pgx.ErrTxClosed) { + log.Printf("Failed to rollback funding re-credit transaction for till sale %s: %v", tillSaleID, rErr) + } + }() + + var isInventory bool + var previousTotal float64 + if err := tx.QueryRow(ctx, `SELECT is_inventory, total_funds_added FROM gift_cards WHERE id = $1`, giftCardID).Scan(&isInventory, &previousTotal); err != nil { + log.Printf("CRITICAL: failed to read gift card %s for the top-up funding re-credit (till sale %s): %v — MANUAL RECONCILIATION REQUIRED", giftCardID, tillSaleID, err) + return false + } + + if _, err := tx.Exec(ctx, ` + UPDATE gift_cards + SET total_funds_added = total_funds_added + $1, + amount_remaining = amount_remaining + $1, + last_used_at = NOW(), + expiry_date = NOW() + ($3 * INTERVAL '1 month') + WHERE id = $2 + `, req.Amount, giftCardID, expiryMonths); err != nil { + log.Printf("CRITICAL: failed to re-credit the top-up amount on gift card %s (till sale %s): %v — MANUAL RECONCILIATION REQUIRED", giftCardID, tillSaleID, err) + return false + } + + transactionType := "topup" + var notes *string + if isInventory && previousTotal == 0 { + transactionType = "purchase" + n := "first top-up on inventory card" + notes = &n + } + if _, err := tx.Exec(ctx, ` + INSERT INTO gift_card_transactions (gift_card_id, transaction_type, amount, reference_type, reference_id, user_id, notes) + VALUES ($1, $2, $3, 'till_sale', $4, $5, $6) + `, giftCardID, transactionType, req.Amount, tillSaleID, req.UserID, notes); err != nil { + log.Printf("CRITICAL: failed to re-insert the top-up transaction for till sale %s (gift card %s): %v — MANUAL RECONCILIATION REQUIRED", tillSaleID, giftCardID, err) + return false + } + + if err := tx.Commit(ctx); err != nil { + log.Printf("CRITICAL: failed to commit the gift-card funding re-credit for till sale %s: %v — MANUAL RECONCILIATION REQUIRED", tillSaleID, err) + return false + } + log.Printf("Re-credited gift-card funding for till sale %s: added back £%.2f on card %s (topup after a mid-flight clawback)", tillSaleID, req.Amount, giftCardID) + return true +} + func CreateTillSale(w http.ResponseWriter, r *http.Request) { ctx := r.Context() // Defense-in-depth admin check (S-1) — a till sale moves money (charges a @@ -623,6 +852,51 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) { } } + // FIX 2 (F6): the sweep serializes its fail/claw-back of a stale pending + // till_sale on "crussell:till:" + // (acquireTillSaleSweepLock, sweep.go) — but the advisory lock acquired + // above is keyed on the REQUEST's base key. A keyless sale's slot scan can + // advance the final stored key past the base (e.g. "till--1" when a + // COMPLETED/FAILED sale occupies the base slot), and the pending-reuse + // resolution above can adopt a row's STORED key that differs from the + // request key. When they diverge, a retry would hold the base lock while + // the sweep holds the stored-key lock — two DIFFERENT locks that do not + // serialize — so the sweep could fail the sale + claw back the funded gift + // card while this retry's Square charge is mid-flight: customer charged AND + // funding clawed back. req.IdempotencyKey at this point IS the key the + // till_sales INSERT below stores (the FINAL key), so it must ALSO be locked + // before the money-critical section. + // + // The base lock is deliberately kept held: it makes the slot scan-and-insert + // atomic against a concurrent identical keyless create (releasing it right + // after the scan would let the second sale derive the same free slot, insert + // a duplicate and die on the idempotency_key UNIQUE constraint). The final + // lock is acquired in the SAME "crussell:till:" family on the SAME pinned + // conn, in the consistent order base → final. The sweep holds only the ONE + // stored-key lock and never acquires a base key, and no other code path + // acquires "crussell:till:" keys in reverse order, so no lock-ordering cycle + // is possible. When the final key equals the base key (no suffix, no + // adoption), the base lock already IS the final lock and nothing more is + // needed. A bounded try-lock matches the base lock's pool-exhaustion + // defence: if the sweep holds the stored key across its fail/claw-back we + // surface 409 "try again" instead of pinning a pool conn for its duration. + finalLockKey := req.IdempotencyKey + if finalLockKey != lockKey { + finalLockOK, fErr := acquireAdvisoryLock(ctx, pinConn, "crussell:till:"+finalLockKey) + if fErr != nil { + log.Printf("Failed to acquire till-sale final-key serialization lock for %s: %v", finalLockKey, fErr) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + if !finalLockOK { + log.Printf("Till-sale final-key serialization lock for %s not acquired within bound — a sweep or same-key retry is in progress", finalLockKey) + http.Error(w, "Sale in progress, try again", http.StatusConflict) + return + } + defer releasePaymentLock(pinConn, "crussell:till:"+finalLockKey) + log.Printf("Till-sale final key %s differs from base key %s — holding both till locks across the Square round-trip so the sweep's stored-key lock serializes", finalLockKey, lockKey) + } + service := NewPaymentService() // MEDIUM-5: the shared £5,000/day admin gift-card cap (giftcard_limits.go @@ -1377,11 +1651,19 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) { } if tillTag.RowsAffected() == 0 { // The stale-pending sweep (or a clawback) resolved the sale while - // the Square charge was in flight: the customer WAS charged and the - // gift card WAS funded, but the row no longer says 'pending'. - // Mirroring the non-Square pending-reuse path below, never report - // success for a row the DB doesn't agree on. - log.Printf("CRITICAL: Square payment %s succeeded but till_sale %s was already resolved (0 rows updated) — charge taken and card funded; MANUAL RECONCILIATION REQUIRED", paymentResult.SquarePayID, tillSaleID) + // the Square charge was in flight: the customer WAS charged, but + // the row no longer says 'pending' and a concurrent clawback may + // have reverted the funded gift card. Mirroring the non-Square + // pending-reuse path below, never report success for a row the DB + // doesn't agree on. FIX 1: this retry's charge just landed, so + // re-credit any clawed-back gift-card funding BEFORE failing + // loudly — the customer ends up "charged AND funded" instead of + // "charged AND clawed back". + if recreditTillSaleFundingAfterMidFlightResolve(ctx, req, tillSaleID, giftCardID, paymentResult) { + log.Printf("CRITICAL: Square payment %s succeeded but till_sale %s was already resolved (0 rows updated) — gift-card funding re-credited after the concurrent clawback; sale left failed — MANUAL RECONCILIATION REQUIRED to complete the sale record", paymentResult.SquarePayID, tillSaleID) + } else { + log.Printf("CRITICAL: Square payment %s succeeded but till_sale %s was already resolved (0 rows updated) — charge taken and gift-card funding NOT re-credited; MANUAL RECONCILIATION REQUIRED", paymentResult.SquarePayID, tillSaleID) + } http.Error(w, "internal server error", http.StatusInternalServerError) return } diff --git a/backend/handlers/webhooks/square.go b/backend/handlers/webhooks/square.go index 0b48f60..179d21c 100644 --- a/backend/handlers/webhooks/square.go +++ b/backend/handlers/webhooks/square.go @@ -12,12 +12,15 @@ import ( "fmt" "io" "log" + "math" "net/http" "os" + "strconv" "strings" "sync" "time" + "crussell/clock" "crussell/db" "crussell/handlers/payments" "crussell/internal/adminnotify" @@ -795,37 +798,58 @@ func markPaymentFailed(ctx context.Context, paymentID string) error { return nil } -// squarePaymentKnown reports whether ANY local payments row carries the Square -// payment id, whatever its status. The pending-only UPDATE in -// handlePaymentUpdated matches zero rows both when no row exists at all and -// when the row already settled (e.g. a completed webhook replay of an -// already-'completed' charge). This existence check distinguishes those two: -// a row existing means the event is a plain no-op replay of a known charge, -// while no row at all is the signature of an ORPHANED sweep-minted duplicate. -// Unlike findPaymentBySquareID (which swallows errors), a DB failure here -// propagates so the caller rejects the event and Square retries. -func squarePaymentKnown(ctx context.Context, squarePaymentID string) (bool, error) { - var one int - err := db.Conn.QueryRow(ctx, - `SELECT 1 FROM payments WHERE square_payment_id = $1 LIMIT 1`, squarePaymentID).Scan(&one) - if errors.Is(err, pgx.ErrNoRows) { - return false, nil - } +// squareChargeKnown reports whether ANY local row — a payments row OR a +// till_sales row — carries the Square payment id, whatever its status. The +// pending-only reconcile in reconcileCompletedPayments matches zero payments +// rows both when no row exists at all and when the row already settled (e.g. a +// completed webhook replay of an already-'completed' charge); a till_sales row +// counts as known too because a gift-card/retail till charge has no payments +// row at all and its completion event must still be acked (the till_sales +// reconcile runs after). This existence check distinguishes "known — a plain +// no-op replay" from "no row at all — the signature of an unresolved unknown +// charge". A DB failure here propagates so the caller rejects the event and +// Square retries. +func squareChargeKnown(ctx context.Context, squarePaymentID string) (bool, error) { + var known bool + err := db.Conn.QueryRow(ctx, ` + SELECT EXISTS(SELECT 1 FROM payments WHERE square_payment_id = $1) + OR EXISTS(SELECT 1 FROM till_sales WHERE square_payment_id = $1) + `, squarePaymentID).Scan(&known) if err != nil { return false, err } - return true, nil + return known, nil +} + +// squareRefundKnown reports whether ANY local refunds row carries the Square +// refund id, whatever its status. Used by handleRefundUpdated to distinguish a +// plain no-op replay (row exists, already in the target status) from a refund +// event arriving before the local row was created — the latter must NOT be +// acked (the refund row can be created in the same transaction as the charge, +// and acking would drop the terminal settlement events forever). A DB failure +// propagates so the caller rejects the event and Square retries. +func squareRefundKnown(ctx context.Context, squareRefundID string) (bool, error) { + var known bool + err := db.Conn.QueryRow(ctx, `SELECT EXISTS(SELECT 1 FROM refunds WHERE square_refund_id = $1)`, squareRefundID).Scan(&known) + if err != nil { + return false, err + } + return known, nil } // findPendingByOrphanKeys locates the pending ORIGIN row of a likely -// sweep-minted duplicate charge: the pending row that shares the replayed -// charge's idempotency key (primary — exact, because Square returns the key on -// the Payment object and payments.idempotency_key is UNIQUE) or, failing that, +// sweep-minted duplicate charge: the row that shares the replayed charge's +// idempotency key (primary — exact, because Square returns the key on the +// Payment object and payments.idempotency_key is UNIQUE) or, failing that, // the pending row whose booking/gift-card reference AND amount match the // payload's reference_id/amount_money (the sweep replays the stored snapshot -// verbatim, preserving both). Only 'pending' rows WITHOUT a square_payment_id -// are candidates — that is exactly the population the keyed stale-pending sweep -// replays (sweep.go), so a match is the sweep-minted duplicate's origin. +// verbatim, preserving both). 'pending' rows WITHOUT a square_payment_id are +// the population the keyed stale-pending sweep replays (sweep.go), so a match +// is the sweep-minted duplicate's origin. 'failed' rows are ALSO matched on +// the idempotency-key path: a prior webhook delivery (or the sweep's B1 +// blind-fail) already marked the origin failed, and a re-delivery of the same +// orphan event must find it again to ack idempotently instead of treating the +// resolved orphan as a fresh unknown charge (round-8 fix 3). // // AGE GATE (webhook-vs-response race): the candidates are additionally limited // to rows old enough to have actually been replayed by the sweep @@ -837,13 +861,13 @@ func squarePaymentKnown(ctx context.Context, squarePaymentID string) (bool, erro // payment. When the gate blocks a match the caller leaves the row pending // (critical-log), never fails it — the sweep will reconcile it later. func findPendingByOrphanKeys(ctx context.Context, payment squarePaymentPayload) (paymentID, bookingID string, found bool, err error) { - replayEligibleSince := time.Now().Add(-payments.SweepKeyedReplayAge()) + replayEligibleSince := clock.Now().Add(-payments.SweepKeyedReplayAge()) if payment.IdempotencyKey != "" { var pid string var bid *string err := db.Conn.QueryRow(ctx, ` SELECT id, booking_id FROM payments - WHERE status = 'pending' AND idempotency_key = $1 AND square_payment_id IS NULL + WHERE status IN ('pending', 'failed') AND idempotency_key = $1 AND square_payment_id IS NULL AND created_at <= $2 ORDER BY created_at DESC, id DESC LIMIT 1 @@ -967,8 +991,16 @@ func detectOrphanedReplayCharge(ctx context.Context, payment squarePaymentPayloa return err } if !found { - log.Printf("[SQUARE-WEBHOOK] payment.updated: COMPLETED square payment %s matches no local row and no pending origin row by idempotency key/reference_id — acknowledging (not a sweep-minted duplicate)", payment.ID) - return nil + // Round-8 fix 3: a COMPLETED payment that matches NO local row and NO + // pending origin row by idempotency key/reference_id is a genuinely + // unknown charge this server never created — possibly a duplicate minted + // outside the sweep's replay, or a charge whose local row was erased. + // Acking it (200 + dedup row) would drop the event forever, hiding money + // the merchant holds. Return a retryable error so the caller responds + // 503: Square re-delivers with backoff, and its retry budget bounds the + // retries (the stale-pending sweeps remain the eventual backstop). + log.Printf("[SQUARE-WEBHOOK] payment.updated: COMPLETED square payment %s matches no local row and no pending origin row by idempotency key/reference_id — returning 503 so Square retries (unresolved unknown money event)", payment.ID) + return fmt.Errorf("COMPLETED square payment %s matches no local row and no pending origin row — unresolved unknown money event, rejecting so Square retries", payment.ID) } // B1-EVIDENCE GATE: only treat the origin as a sweep-minted duplicate when // the sweep actually minted+attempted to refund it (b1_attempts > 0 or a @@ -1046,50 +1078,52 @@ func handlePaymentUpdated(data json.RawMessage) error { log.Printf("[SQUARE-WEBHOOK] payment.updated: square payment %s status %q is non-terminal — no local state change", payment.ID, payment.Status) return nil } - // Only 'pending' rows are candidates for a terminal transition — the same - // conservative rule the stale-pending sweeps use. A webhook for an already - // settled row (Square fires payment.updated for ANY field change, e.g. fee - // recalculation on a fully-refunded charge) must never revert a terminal - // status like 'refunded' back to 'completed'. - tag, err := db.Conn.Exec(ctx, - `UPDATE payments SET status = $1, updated_at = NOW() WHERE square_payment_id = $2 AND status = 'pending'`, - localStatus, payment.ID) - if err != nil { - log.Printf("[SQUARE-WEBHOOK] Failed to update payment %s to status %s: %v", payment.ID, localStatus, err) - return err - } - if tag.RowsAffected() > 0 { - log.Printf("[SQUARE-WEBHOOK] payment.updated: square payment %s → local status %s", payment.ID, localStatus) - } else if localStatus == "completed" { - // B1-support: a COMPLETED payment.updated that matched NO pending - // 'payments' row may be an ORPHANED SWEEP-MINTED duplicate — the - // stale-pending sweep replayed a stored idempotency key against an - // expired key, Square landed a NEW charge whose id matches nothing - // locally, and this is that new charge's completion event. If NO local - // row carries this square_payment_id at all (not even a settled one), - // hunt for the pending origin row and settle it so the sweep never - // blind-fails or double-rescues it. A settled row existing means this - // is a plain no-op replay of a known charge and is left untouched. - known, kErr := squarePaymentKnown(ctx, payment.ID) - if kErr != nil { - log.Printf("[SQUARE-WEBHOOK] Failed to check whether square payment %s is known: %v", payment.ID, kErr) - return kErr + if localStatus == "completed" { + // CRITICAL (round-8): a COMPLETED charge must be gated against the + // booking BEFORE any pending row is promoted. reconcileCompletedPayments + // re-reads every pending payments row carrying this square payment id + // inside a transaction and, per row, re-checks the booking status FOR + // UPDATE: a cancelled/lapsed/no-show booking refuses the completion (row + // marked failed + automatic cancellation refund row + critical + // notification), a payable booking is completed with the split/VAT/ + // deposit-promotion/fully-paid-completion side-effects the sweep rescue + // applies, and a booking-less row (gift-card purchase) is left pending + // for the same-key retry that delivers the card (C6). A COMPLETED event + // matching no local row at all is the unresolved-unknown case: the + // orphaned-replay detection runs (B1) and a genuinely unknown charge is + // NOT acked — the returned error makes the caller respond 503 so Square + // retries instead of the event being dropped forever. + if err := reconcileCompletedPayments(ctx, payment); err != nil { + return err } - if !known { - if dErr := detectOrphanedReplayCharge(ctx, payment); dErr != nil { - return dErr - } + } else { + // A Square FAILED/CANCELED event on a still-pending row. Only 'pending' + // rows are candidates for a terminal transition — the same conservative + // rule the stale-pending sweeps use. A webhook for an already settled + // row (Square fires payment.updated for ANY field change, e.g. fee + // recalculation on a fully-refunded charge) must never revert a terminal + // status like 'refunded' back to 'completed'. + tag, err := db.Conn.Exec(ctx, + `UPDATE payments SET status = $1, updated_at = NOW() WHERE square_payment_id = $2 AND status = 'pending'`, + localStatus, payment.ID) + if err != nil { + log.Printf("[SQUARE-WEBHOOK] Failed to update payment %s to status %s: %v", payment.ID, localStatus, err) + return err + } + if tag.RowsAffected() > 0 { + log.Printf("[SQUARE-WEBHOOK] payment.updated: square payment %s → local status %s", payment.ID, localStatus) + } + // A definitively failed charge (Square FAILED/CANCELED) claws back the + // gift-card funding those pending sales added, exactly like the + // stale-pending sweep (handlers/payments/sweep.go); an ambiguous status + // never reaches here. + if localStatus == "failed" { + return clawbackFailedTillSales(ctx, payment.ID) } } // A Square charge can also map to a till_sales row (online gift-card // purchase, retail at the till) — reconcile those too. Same pending-only - // guard: never revert a terminal till-sale status. A definitively failed - // charge (Square FAILED/CANCELED) claws back the gift-card funding those - // pending sales added, exactly like the stale-pending sweep - // (handlers/payments/sweep.go); an ambiguous status never reaches here. - if localStatus == "failed" { - return clawbackFailedTillSales(ctx, payment.ID) - } + // guard: never revert a terminal till-sale status. tsTag, err := db.Conn.Exec(ctx, `UPDATE till_sales SET status = $1, updated_at = NOW() WHERE square_payment_id = $2 AND status = 'pending'`, localStatus, payment.ID) @@ -1103,6 +1137,565 @@ func handlePaymentUpdated(data json.RawMessage) error { return nil } +// webhookPendingCharge is a local pending payments row awaiting a COMPLETED +// Square reconcile. +type webhookPendingCharge struct { + id string + bookingID *string + amountPence int64 + paymentType string + paymentMethod string + idemKey sql.NullString + createdBy *string +} + +// reconcileCompletedPayments applies a COMPLETED Square payment to every local +// pending payments row carrying that square_payment_id. It mirrors the +// stale-pending sweep's gateStalePaymentRescueOnBooking + +// rescueStaleRowCompletedTx (handlers/payments/sweep.go): each pending row's +// booking is re-read FOR UPDATE inside one transaction, and +// +// - a cancelled/lapsed/no-show booking refuses the completion: the row is +// marked FAILED (never completed — the cancellation refund path computes +// refunds from completed payments and would miss it, charging a customer +// with NO automatic refund, F3), an M2 auto-refund row for the full +// stranded charge is inserted (same shape and origin 'cancellation' as +// ProcessCancellationRefundTx / the sweep's gate) so the pending-refund +// sweep issues the Square refund, and a critical admin notification is +// raised; +// - a payable booking is completed exactly like the sweep rescue / live path: +// the row is promoted to 'completed', re-split (any overflow carved as a +// tip), VAT applied, a deposit-paid pending_release booking promoted to +// 'confirmed', and a fully-paid booking completed with the +// loyalty/campaign side-effects (ApplyBookingCompletionSideEffects); +// - a booking-less row (gift-card purchase) is LEFT pending — the same-key +// retry delivers the card, never the webhook (C6). +// +// A COMPLETED event matching no local pending row at all is the unknown-event +// case: a settled row existing (payments or till_sales) means a plain no-op +// replay and is acked, otherwise the orphaned-replay detection runs (B1) and a +// genuinely unknown charge returns a retryable error (the caller responds 503, +// no dedup row, so Square re-delivers instead of the event being dropped +// forever). A non-nil error always means the caller must NOT commit the dedup +// row. Idempotent: every UPDATE is status='pending'-guarded and the refund row +// uses the deterministic paymentID+"-square-"+pence key. +func reconcileCompletedPayments(ctx context.Context, payment squarePaymentPayload) error { + tx, err := db.Conn.Begin(ctx) + if err != nil { + return err + } + defer func() { + if err := tx.Rollback(ctx); err != nil && !errors.Is(err, pgx.ErrTxClosed) { + log.Printf("[SQUARE-WEBHOOK] Failed to roll back completed-payment reconcile tx for square payment %s: %v", payment.ID, err) + } + }() + + rows, err := tx.Query(ctx, ` + SELECT id, booking_id, amount, payment_type, payment_method, idempotency_key, created_by + FROM payments + WHERE square_payment_id = $1 AND status = 'pending' + ORDER BY created_at DESC, id DESC + `, payment.ID) + if err != nil { + return err + } + // refusedBookings collects the bookings whose stranded charges were refused + // (row failed + auto-refund inserted) so their critical admin notifications + // can be raised AFTER the tx commits — an in-tx notification's + // admin_notifications FK check on the still-locked booking row would wait on + // our own uncommitted xmax (the sweep's gate notifies after commit for the + // same reason). + var refusedBookings []string + var pending []webhookPendingCharge + for rows.Next() { + var pr webhookPendingCharge + var amount float64 + if err := rows.Scan(&pr.id, &pr.bookingID, &amount, &pr.paymentType, &pr.paymentMethod, &pr.idemKey, &pr.createdBy); err != nil { + rows.Close() + return err + } + pr.amountPence = int64(math.Round(amount * 100)) + pending = append(pending, pr) + } + rows.Close() + if err := rows.Err(); err != nil { + return err + } + + if len(pending) == 0 { + // No local pending payments row carries this square payment id. + // Commit the read-only tx before the out-of-tx existence checks. + if err := tx.Commit(ctx); err != nil { + return err + } + known, kErr := squareChargeKnown(ctx, payment.ID) + if kErr != nil { + log.Printf("[SQUARE-WEBHOOK] Failed to check whether square payment %s is known: %v", payment.ID, kErr) + return kErr + } + if known { + // A settled row (payments or till_sales) exists — a plain no-op + // replay of a known charge, left untouched. + return nil + } + // B1-support: hunt for an ORPHANED SWEEP-MINTED duplicate (the + // stale-pending sweep replayed a stored idempotency key against an + // expired key and Square landed a NEW charge with no local row). A + // resolved replay (origin found + B1 evidence) returns nil; a + // genuinely unknown payment returns a retryable error. + return detectOrphanedReplayCharge(ctx, payment) + } + + for i := range pending { + pr := pending[i] + if pr.bookingID == nil { + // C6: a booking-less row is a gift-card purchase. The same-key + // retry delivers the card through the synchronous purchase path — + // never auto-complete it here (the charge would be recorded while + // the card was never minted). Leave pending; the dedup row still + // commits and the sync retry flips the row when it delivers. + log.Printf("[SQUARE-WEBHOOK] payment.updated: square payment %s COMPLETED for booking-less row %s (gift-card purchase) — leaving pending for the same-key retry to deliver the card (C6)", payment.ID, pr.id) + continue + } + // Re-read the booking status FOR UPDATE so the decision serializes + // against a concurrent cancellation (mirrors gateStalePaymentRescueOnBooking). + var status string + if err := tx.QueryRow(ctx, `SELECT status FROM bookings WHERE id = $1 FOR UPDATE`, *pr.bookingID).Scan(&status); err != nil { + // Booking gone or unreadable: the money state is unknown. Never + // complete on an unknown state — a completed payment on a vanished + // booking would strand the charge outside the refund system. + log.Printf("[SQUARE-WEBHOOK] CRITICAL: Square reports payment %s COMPLETED but re-reading booking %s status failed (%v) — leaving row %s pending — MANUAL RECONCILIATION REQUIRED", payment.ID, *pr.bookingID, err, pr.id) + continue + } + if !webhookBookingStatusAllowsCompleted(status) { + // Cancelled / lapsed / no-show booking: completing the payment + // would charge a customer for a booking the cancellation flow + // already closed, with NO automatic refund (F3). Mark the row + // FAILED so the same-key retry can never reuse it, and auto-create + // the M2 refund row for the full stranded charge. The critical + // notification is raised AFTER the tx commits (it cannot run inside + // the tx: its admin_notifications FK check on the locked booking row + // would wait on our own uncommitted xmax — the sweep's gate notifies + // after commit for the same reason, sweep.go). + if webhookFailStrandedCharge(ctx, tx, pr, status, payment.ID) { + log.Printf("[SQUARE-WEBHOOK] CRITICAL: square payment %s is COMPLETED but booking %s is %q — payment %s marked FAILED instead of completed; an automatic refund row was created for the stranded charge — verify the Square refund settles", payment.ID, *pr.bookingID, status, pr.id) + refusedBookings = append(refusedBookings, *pr.bookingID) + } + continue + } + // Payable booking — promote the pending row to completed and run the + // split/VAT/completion side-effects exactly like the sweep rescue. + // + // R9 ordering guarantee (webhook vs. the synchronous saved-card path, + // handlers.go CreateBookingPayment): both paths contend on the SAME + // booking row — this reconcile re-reads the booking FOR UPDATE above, + // the sync path's postChargeRecheck takes the identical lock — and the + // payment row's status='pending' is the single mutual-exclusion point. + // Exactly one of them can win the guarded flip: + // + // - sync path commits first: this SELECT (status='pending' filter + // above) finds no row at all and the event is acked as a plain + // known-charge replay (squareChargeKnown); even if the row were + // somehow still selected, this guarded UPDATE returns + // RowsAffected()==0 and the side-effects are skipped; + // - webhook wins the flip: the guarded UPDATE succeeds and the + // side-effects run HERE, inside this tx; the sync path's own + // status='pending'-guarded completion (added in the same fix) then + // no-ops and never re-runs its side-effects. + // + // The side-effects therefore run at most once, by whichever path won the + // flip — never twice — so the split rows (UNIQUE idempotency_key, e.g. + // "-split-1") can never be minted by both paths. + tag, err := tx.Exec(ctx, ` + UPDATE payments SET status = 'completed', updated_at = NOW() + WHERE id = $1 AND status = 'pending' + `, pr.id) + if err != nil { + return err + } + if tag.RowsAffected() == 0 { + // Already resolved concurrently (the sync path won the flip) — + // nothing to do. + continue + } + log.Printf("[SQUARE-WEBHOOK] payment.updated: square payment %s → local status completed (row %s)", payment.ID, pr.id) + webhookApplyCompletedPaymentRecords(ctx, tx, pr, payment.ID) + // The webhook won the flip, so it owns the side-effects: verify they + // actually landed inside this tx instead of silently claiming a + // completion whose bookkeeping was skipped. + webhookVerifyCompletionSideEffects(ctx, tx, pr) + } + + if err := tx.Commit(ctx); err != nil { + log.Printf("[SQUARE-WEBHOOK] Failed to commit completed-payment reconcile for square payment %s: %v", payment.ID, err) + return err + } + // Raise the refused-booking critical notifications AFTER the commit — the + // booking row lock is released now, so the notification's FK check on it + // cannot wait on our own transaction. + for _, bookingID := range refusedBookings { + insertCriticalPaymentNotification(ctx, bookingID, "") + } + return nil +} + +// webhookBookingStatusAllowsCompleted reports whether a charge that already +// went through Square can still be recorded as a completed payment. It mirrors +// the payments package's bookingStatusAllowsCompletedPayment (handlers.go) — +// the exact predicate the sweep's gate uses — replicated here because the +// payments package is mid-edit by another agent and must not be modified. A +// booking that legitimately completed ('completed') must still accept the +// recorded payment; a cancelled, lapsed, or no-show booking must NOT — the +// money would bypass the cancellation refund system. +func webhookBookingStatusAllowsCompleted(status string) bool { + switch status { + case "confirmed", "pending", "pending_release", "in_progress", "completed": + return true + default: + return false + } +} + +// webhookFailStrandedCharge marks a pending payment row FAILED on a +// cancelled/lapsed/no-show booking whose charge already completed at Square and +// auto-creates the M2 cancellation refund row for the full stranded charge. +// Mirrors gateStalePaymentRescueOnBooking's refused branch (sweep.go). Returns +// true when the row was actually refused (still pending); false when it was +// already resolved concurrently. The critical admin notification is NOT raised +// here — the caller raises it after the tx commits (an in-tx notification's +// admin_notifications FK check on the locked booking row would wait on our own +// uncommitted xmax). +func webhookFailStrandedCharge(ctx context.Context, tx pgx.Tx, pr webhookPendingCharge, bookingStatus, squarePaymentID string) bool { + tag, err := tx.Exec(ctx, ` + UPDATE payments SET status = 'failed', updated_at = NOW() + WHERE id = $1 AND status = 'pending' + `, pr.id) + if err != nil { + log.Printf("[SQUARE-WEBHOOK] CRITICAL: Square payment %s is COMPLETED but booking %s is %q — marking the row failed errored (%v) — MANUAL RECONCILIATION REQUIRED", squarePaymentID, *pr.bookingID, bookingStatus, err) + return false + } + if int(tag.RowsAffected()) == 0 { + // Already resolved concurrently — nothing left to refuse. + return false + } + if pr.amountPence > 0 { + // M2: a refund row for the full stranded charge. Same shape and origin + // ('cancellation') ProcessCancellationRefundTx records, so the pending + // Square refund sweep aggregates and issues it at Square. The + // deterministic key mirrors the cancellation path (paymentID + "-square-" + // + amount pence) and the UNIQUE conflict guard makes a re-run + // idempotent. + if _, rfErr := tx.Exec(ctx, ` + INSERT INTO refunds (payment_id, booking_id, amount, status, reason, idempotency_key, created_by, created_at, origin) + VALUES ($1, $2, $3, 'pending', $4, $5, $6, NOW(), 'cancellation') + ON CONFLICT (idempotency_key) DO NOTHING + `, pr.id, *pr.bookingID, float64(pr.amountPence)/100.0, + "Stranded charge on cancelled booking "+bookingStatus+" — auto-refunded by webhook", + pr.id+"-square-"+strconv.FormatInt(pr.amountPence, 10), pr.createdBy); rfErr != nil { + log.Printf("[SQUARE-WEBHOOK] CRITICAL: square payment %s (row %s) is COMPLETED but creating its auto-refund row errored (%v) — MANUAL RECONCILIATION REQUIRED: refund the charge manually", squarePaymentID, pr.id, rfErr) + } + } + return true +} + +// webhookApplyCompletedPaymentRecords runs the post-completion bookkeeping for +// a payable-booking payment the webhook just promoted to 'completed': re-split +// the charge (any overflow carved as its own tip record), apply VAT, promote a +// deposit-paid pending_release booking to 'confirmed', and complete a fully-paid +// booking with the loyalty/campaign side-effects. It mirrors the sweep rescue's +// buildStaleRescueRecords + applyStaleRescueRecords (sweep.go) and the live +// post-charge path (handlers.go). All work runs inside the caller's reconcile +// transaction; a bookkeeping failure is logged and never aborts the money +// mutation (the charge already completed at Square — never worse than the +// pre-fix minimal flip). +func webhookApplyCompletedPaymentRecords(ctx context.Context, tx pgx.Tx, pr webhookPendingCharge, squarePaymentID string) { + // A tip-type row is never re-split (a tip can never fully pay a booking). + if pr.paymentType == "tip" { + return + } + info, err := payments.NewPaymentService().GetBookingPaymentInfo(ctx, *pr.bookingID) + if err != nil || info == nil { + log.Printf("[SQUARE-WEBHOOK] CRITICAL: failed to load booking info for completion of payment %s (booking %s): %v — row completed un-split; manual reconciliation recommended", pr.id, *pr.bookingID, err) + return + } + amount := float64(pr.amountPence) / 100.0 + spID := squarePaymentID + primary := payments.PaymentRecord{ + BookingID: *pr.bookingID, + PaymentType: pr.paymentType, + PaymentMethod: pr.paymentMethod, + Status: "completed", + Amount: amount, + SquarePaymentID: &spID, + CreatedBy: pr.createdBy, + CreatedAt: clock.Now(), + UpdatedAt: clock.Now(), + } + if pr.idemKey.Valid { + k := pr.idemKey.String + primary.IdempotencyKey = &k + } + records, splitErr := webhookBuildSplitRecords(primary, pr.paymentType, info, amount) + if splitErr != nil { + log.Printf("[SQUARE-WEBHOOK] CRITICAL: buildSplitRecords rejected the completion split for payment %s (booking %s): %v — row completed un-split; manual reconciliation recommended", pr.id, *pr.bookingID, splitErr) + return + } + if len(records) == 0 { + return + } + // Align the primary row to records[0] (deposit/balance/full portion). + if _, upErr := tx.Exec(ctx, ` + UPDATE payments SET + amount = $1, + payment_type = $2, + fees = $3, + updated_at = NOW() + WHERE id = $4 + `, records[0].Amount, records[0].PaymentType, records[0].Fees, pr.id); upErr != nil { + log.Printf("[SQUARE-WEBHOOK] CRITICAL: failed to align completed payment %s to its split primary (%v) — manual reconciliation recommended", pr.id, upErr) + return + } + // apply_vat_to_payment is idempotent (guarded on vat_amount IS NULL) and + // skips discount/on_the_house/tip rows internally. + payments.ApplyVATToBookingPayment(ctx, tx, pr.id) + svc := payments.NewPaymentService() + for _, rec := range records[1:] { + pid, cErr := svc.CreatePaymentRecordTx(ctx, tx, rec, nil) + if cErr != nil { + log.Printf("[SQUARE-WEBHOOK] CRITICAL: failed to insert completion split record for payment %s (%v) — manual reconciliation recommended", pr.id, cErr) + return + } + payments.ApplyVATToBookingPayment(ctx, tx, pid) + } + webhookPromoteAndCompleteBooking(ctx, tx, *pr.bookingID) +} + +// webhookVerifyCompletionSideEffects is the R9 belt-and-braces backstop: once +// the webhook wins the guarded flip it OWNS the post-completion bookkeeping, so +// before the reconcile commits it re-checks, inside the same tx, that a +// fully-paid booking actually ended up 'completed'. Every failure inside +// webhookApplyCompletedPaymentRecords already raises its own CRITICAL log, but +// this cross-check means a skipped side-effect chain can never be silently +// claimed as a completion. Log-only and never aborting: the charge already +// completed at Square, so the money mutation stands either way and the operator +// is told to reconcile. +func webhookVerifyCompletionSideEffects(ctx context.Context, tx pgx.Tx, pr webhookPendingCharge) { + var status string + if err := tx.QueryRow(ctx, `SELECT status FROM bookings WHERE id = $1`, *pr.bookingID).Scan(&status); err != nil { + log.Printf("[SQUARE-WEBHOOK] CRITICAL: payment %s completed on booking %s but verifying the booking state failed (%v) — MANUAL RECONCILIATION REQUIRED", pr.id, *pr.bookingID, err) + return + } + if status != "completed" && webhookBookingIsFullyPaid(ctx, tx, *pr.bookingID) { + log.Printf("[SQUARE-WEBHOOK] CRITICAL: payment %s made booking %s fully paid but the booking is %q, not 'completed' — the completion side-effects were skipped — MANUAL RECONCILIATION REQUIRED", pr.id, *pr.bookingID, status) + } +} + +// webhookBuildSplitRecords partitions a completed webhook-reconciled charge +// into its deposit/balance/tip payment records, mirroring the payments +// package's buildSplitRecords (handlers.go) — the exact split the sweep rescue +// and the live post-charge path apply. The records always partition +// paymentAmount exactly (booking portion + any tip). Kept in sync by hand: the +// payments package is mid-edit by another agent and must not be modified. +func webhookBuildSplitRecords(primary payments.PaymentRecord, reqPaymentType string, info *payments.BookingPaymentInfo, paymentAmount float64) ([]payments.PaymentRecord, error) { + // After the booking starts there is no deposit protection window, but an + // overpayment beyond the remaining booking value is still gratuity and must + // be carved out as its own payment_type='tip' record (F3) — mirroring + // buildTerminalSplitRecords' post-start carve. + if clock.Now().After(info.StartTime) { + remaining := math.Max(0, info.TotalAmount-info.TotalPaid) + bookingPortion := math.Min(paymentAmount, remaining) + bookingPortion = math.Round(bookingPortion*100) / 100 + tipPortion := math.Round((paymentAmount-bookingPortion)*100) / 100 + if tipPortion > 0.004 { // payments.roundingEpsilon (errors.go) + // Belt-and-braces: the online tip bound (payments.maxOnlineTipPence, + // £250) applies to every tip row. Returning an error (never + // clamping) preserves the partition invariant. + if tipPence := int64(math.Round(tipPortion * 100)); tipPence > 25000 { + return nil, fmt.Errorf("webhookBuildSplitRecords: post-start carve for booking %s would mint a tip of %d pence, exceeding the £250 online tip cap (charge %.2f)", primary.BookingID, tipPence, paymentAmount) + } + records := []payments.PaymentRecord{primary} + records[0].Amount = bookingPortion + tip := primary + tip.PaymentType = "tip" + tip.Amount = tipPortion + tip.Fees = 0 + if primary.IdempotencyKey != nil { + k := *primary.IdempotencyKey + "-split-tip" + tip.IdempotencyKey = &k + } + return append(records, tip), nil + } + return []payments.PaymentRecord{primary}, nil + } + + // Deposit portion: up to 50% of total, minus what's already been paid. + maxDeposit := info.TotalAmount * payments.ProtectedDepositMaxPct + remainingDepositRoom := math.Max(0, maxDeposit-info.TotalPaid) + depositAmount := math.Min(paymentAmount, remainingDepositRoom) + depositAmount = math.Round(depositAmount*100) / 100 + + // Balance portion: covers whatever is still owed on the booking. + remainingAfterDeposit := math.Round((paymentAmount-depositAmount)*100) / 100 + bookingRemaining := math.Max(0, info.TotalAmount-info.TotalPaid-depositAmount) + balancePortion := math.Min(remainingAfterDeposit, bookingRemaining) + balancePortion = math.Round(balancePortion*100) / 100 + + // Tip: anything beyond the booking total. + tipPortion := math.Round((remainingAfterDeposit-balancePortion)*100) / 100 + if tipPortion > 0.004 { + if tipPence := int64(math.Round(tipPortion * 100)); tipPence > 25000 { + return nil, fmt.Errorf("webhookBuildSplitRecords: pre-start carve for booking %s would mint a tip of %d pence, exceeding the £250 online tip cap (charge %.2f)", primary.BookingID, tipPence, paymentAmount) + } + } + + var records []payments.PaymentRecord + splitIdx := 0 + + if depositAmount > 0.004 { + dep := primary + dep.PaymentType = "deposit" + dep.Amount = depositAmount + records = append(records, dep) + splitIdx++ + } + + if balancePortion > 0.004 { + bal := primary + bal.Amount = balancePortion + bal.Fees = 0 + if primary.IdempotencyKey != nil { + k := *primary.IdempotencyKey + fmt.Sprintf("-split-%d", splitIdx) + bal.IdempotencyKey = &k + } + totalPaidAfterBalance := info.TotalPaid + depositAmount + balancePortion + switch { + case totalPaidAfterBalance >= info.TotalAmount && totalPaidAfterBalance-balancePortion > 0: + bal.PaymentType = "balance" + case totalPaidAfterBalance >= info.TotalAmount: + bal.PaymentType = "full" + default: + bal.PaymentType = "partial" + } + records = append(records, bal) + splitIdx++ + } + + if tipPortion > 0.004 { + tip := primary + tip.PaymentType = "tip" + tip.Amount = tipPortion + tip.Fees = 0 + splitIdx++ + if primary.IdempotencyKey != nil { + k := *primary.IdempotencyKey + fmt.Sprintf("-split-%d", splitIdx) + tip.IdempotencyKey = &k + } + records = append(records, tip) + } + + // Defensive fallback: nothing was appended (deposit, balance, AND tip all + // zero — impossible given paymentAmount is validated > 0 upstream, so this + // is a pure safety net). + if len(records) == 0 { + primary.Fees = 0 + records = append(records, primary) + } + return records, nil +} + +// webhookPromoteAndCompleteBooking runs the booking-state transitions the live +// post-charge path applies after a payment is recorded (handlers.go): a +// deposit-paid pending_release booking is promoted to 'confirmed' (payment +// covers >= 20% of the total), and a fully-paid booking is completed with the +// loyalty/campaign/name_history side-effects (completeActiveBookingFromPayment + +// ApplyBookingCompletionSideEffects). Runs inside the caller's reconcile tx so +// the state changes and the payment flip commit atomically. +func webhookPromoteAndCompleteBooking(ctx context.Context, tx pgx.Tx, bookingID string) { + // Deposit promotion: paid >= 20% of the booking total promotes a + // pending_release booking to confirmed (handlers.go depositPromotionMinPct). + var depositMet bool + if err := tx.QueryRow(ctx, fmt.Sprintf(` + WITH booking_total AS ( + SELECT total_amount * 100 AS total_pence FROM bookings WHERE id = $1 + ), + paid_total AS ( + SELECT COALESCE(SUM(amount), 0) * 100 AS paid_pence + FROM payments + WHERE booking_id = $1 AND status = 'completed' + AND payment_type != 'tip' + AND payment_method NOT IN ('discount', 'on_the_house') + ) + SELECT pt.paid_pence >= ROUND(bt.total_pence * %f) + FROM booking_total bt, paid_total pt + `, 0.2), bookingID).Scan(&depositMet); err != nil { + log.Printf("[SQUARE-WEBHOOK] Failed to check deposit threshold for booking %s: %v", bookingID, err) + } + if depositMet { + if _, err := tx.Exec(ctx, ` + UPDATE bookings SET status = 'confirmed', updated_at = NOW() + WHERE id = $1 AND status = 'pending_release' + `, bookingID); err != nil { + log.Printf("[SQUARE-WEBHOOK] ALERT: payment completed but failed to promote booking %s from pending_release: %v", bookingID, err) + } + } + + if webhookBookingIsFullyPaid(ctx, tx, bookingID) { + webhookCompleteActiveBooking(ctx, tx, bookingID) + } +} + +// webhookBookingIsFullyPaid reports whether completed payments toward the +// booking (excluding tips and on-the-house rows, but INCLUDING discount rows) +// cover 100% of the booking total. Mirrors the payments package's +// bookingIsFullyPaid (completion.go). +func webhookBookingIsFullyPaid(ctx context.Context, q db.Querier, bookingID string) bool { + var fullyPaid bool + if err := q.QueryRow(ctx, ` + WITH booking_total AS ( + SELECT total_amount * 100 AS total_pence FROM bookings WHERE id = $1 + ), + paid_total AS ( + SELECT COALESCE(SUM(amount), 0) * 100 AS paid_pence + FROM payments + WHERE booking_id = $1 AND status = 'completed' + AND payment_type != 'tip' + AND payment_method NOT IN ('on_the_house') + ) + SELECT pt.paid_pence >= bt.total_pence AND bt.total_pence > 0 + FROM booking_total bt, paid_total pt + `, bookingID).Scan(&fullyPaid); err != nil { + log.Printf("[SQUARE-WEBHOOK] Failed to check full-payment threshold for booking %s: %v", bookingID, err) + } + return fullyPaid +} + +// webhookCompleteActiveBooking transitions an active booking to 'completed' and +// runs the completion side-effects, all within tx. It is a no-op if the booking +// is not in an active (completable) status, so cancelled, no-show and +// deposit-lapsed bookings are never auto-completed — and once completed it can +// never re-fire, because the status filter no longer matches. Mirrors +// completeActiveBookingFromPayment (completion.go). +func webhookCompleteActiveBooking(ctx context.Context, tx pgx.Tx, bookingID string) { + var completedID string + err := tx.QueryRow(ctx, ` + UPDATE bookings SET status = 'completed', updated_at = NOW() + WHERE id = $1 AND status IN ('pending', 'confirmed', 'in_progress', 'pending_release') + RETURNING id + `, bookingID).Scan(&completedID) + if err != nil { + if !errors.Is(err, pgx.ErrNoRows) { + log.Printf("[SQUARE-WEBHOOK] ALERT: failed to complete fully-paid booking %s: %v", bookingID, err) + } + return + } + var userID string + if uErr := tx.QueryRow(ctx, `SELECT user_id FROM bookings WHERE id = $1`, bookingID).Scan(&userID); uErr != nil { + log.Printf("[SQUARE-WEBHOOK] ALERT: booking %s completed by payment but failed to load user for side-effects: %v", bookingID, uErr) + return + } + payments.ApplyBookingCompletionSideEffects(ctx, tx, bookingID, userID) +} + // clawbackFailedTillSales reverts the gift-card funding of every still-pending // till sale funded by a Square charge that is DEFINITIVELY failed (Square // FAILED/CANCELED — never ambiguous). It mirrors the stale-pending sweep's @@ -1233,6 +1826,21 @@ func handleRefundUpdated(data json.RawMessage) error { // money that already moved). Leave the row 'pending'; the Square status is // logged for the audit trail. if refund.Status == "APPROVED" { + // Round-8 fix 4: an APPROVED refund arriving BEFORE the local refund row + // exists must not be acked-and-dropped — the refund row can be created + // in the same transaction as the charge in some paths, and acking here + // would lose the terminal COMPLETED/FAILED settlement events forever (no + // sweep fallback re-discovers an APPROVED-only trail). Return 503 so + // Square re-delivers once the row exists. + known, kErr := squareRefundKnown(ctx, refund.ID) + if kErr != nil { + log.Printf("[SQUARE-WEBHOOK] Failed to check whether square refund %s is known: %v", refund.ID, kErr) + return kErr + } + if !known { + log.Printf("[SQUARE-WEBHOOK] refund.updated: square refund %s status APPROVED arrived with NO local refund row yet — returning 503 so Square retries (the refund row may be inserted in the same transaction as the charge)", refund.ID) + return fmt.Errorf("refund.updated APPROVED for square refund %s with no local refund row yet — rejecting so Square retries", refund.ID) + } log.Printf("[SQUARE-WEBHOOK] refund.updated: square refund %s status APPROVED is non-terminal for the webhook (Square may still settle COMPLETED or FAILED) — leaving the local row pending", refund.ID) return nil } @@ -1255,6 +1863,22 @@ func handleRefundUpdated(data json.RawMessage) error { } if tag.RowsAffected() > 0 { log.Printf("[SQUARE-WEBHOOK] refund.updated: square refund %s → local status %s", refund.ID, localStatus) + } else { + // Zero rows: the refund is already 'completed' (a plain no-op + // replay) OR the local row does not exist yet. Round-8 fix 4: a + // genuinely absent row must not be acked — the settlement would be + // dropped forever. Return 503 so Square re-delivers once the row + // appears (the refund row can be created in the same transaction as + // the charge in some paths). + known, kErr := squareRefundKnown(ctx, refund.ID) + if kErr != nil { + log.Printf("[SQUARE-WEBHOOK] Failed to check whether square refund %s is known: %v", refund.ID, kErr) + return kErr + } + if !known { + log.Printf("[SQUARE-WEBHOOK] refund.updated: square refund %s status COMPLETED arrived with NO local refund row yet — returning 503 so Square retries (the refund row may be inserted in the same transaction as the charge)", refund.ID) + return fmt.Errorf("refund.updated COMPLETED for square refund %s with no local refund row yet — rejecting so Square retries", refund.ID) + } } // B1 sweep-dup refunds: a COMPLETED promotion must ALSO resolve the // parent payment/till_sale row. The B1 re-poll pass (sweepPendingB1Refunds, diff --git a/init-scripts/init-script.sql b/init-scripts/init-script.sql index e9917c7..9e113ce 100644 --- a/init-scripts/init-script.sql +++ b/init-scripts/init-script.sql @@ -927,6 +927,18 @@ CREATE TABLE admin_notifications ( reason admin_notification_reason NOT NULL, booking_id CHAR(12) REFERENCES bookings(id), user_id CHAR(12) REFERENCES users(id), + -- Money-critical event detail (FIX 3b): populated by the + -- 'critical_payment_log' / 'refund_failed' insert sites (Square webhooks, + -- sweep, account-erasure, refunds) so the admin notifications page can + -- render WHAT happened without opening the CRITICAL logs. All three are + -- nullable so the routine insert sites (new_booking, pending_booking, ...) + -- keep working unchanged. Column contract: amount = the money amount + -- involved, square_id = the Square payment/dispute/checkout identifier, + -- description = a short human-readable detail line. See the adminnotify + -- package doc for the exact per-site mapping. + amount NUMERIC(10,2), + square_id TEXT, + description TEXT, acknowledged_at TIMESTAMPTZ, created_at TIMESTAMPTZ DEFAULT NOW() ); @@ -934,6 +946,21 @@ CREATE TABLE admin_notifications ( CREATE INDEX idx_admin_notifications_reason ON admin_notifications(reason); CREATE INDEX idx_admin_notifications_acknowledged_at ON admin_notifications(acknowledged_at); +-- Flood-cap suppression counter (adminnotify.MaxUnacknowledgedCriticalLogs): +-- one row per capped reason, counting how many alerts were dropped while that +-- reason's unacknowledged admin_notifications queue sat at the cap. Written by +-- CriticalLogsCapExceeded at every insert site (the single choke point, so no +-- insert site needs to change) and read by GET /api/admin/notifications as the +-- operator-facing "suppressed this cycle" count — only reasons whose queue is +-- STILL at the cap are reported, so the count naturally resets once the +-- operator acknowledges the queue down. +CREATE TABLE admin_notification_suppressions ( + reason admin_notification_reason PRIMARY KEY, + suppressed_count INT NOT NULL DEFAULT 0, + first_suppressed_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + last_suppressed_at TIMESTAMPTZ NOT NULL DEFAULT NOW() +); + -- User notification preferences for future user notification system CREATE TABLE user_notification_preferences ( id CHAR(12) PRIMARY KEY DEFAULT generate_user_notification_preferences_id(), @@ -1965,7 +1992,8 @@ BEGIN is_vat_applicable = TRUE, updated_at = NOW() WHERE id = payment_id - AND vat_amount IS NULL; + AND vat_amount IS NULL + AND payment_type != 'tip'; END; $$ LANGUAGE plpgsql; @@ -2757,3 +2785,28 @@ CREATE INDEX IF NOT EXISTS idx_refresh_tokens_user_id ON refresh_tokens (user_id CREATE INDEX IF NOT EXISTS idx_refresh_tokens_expires_at ON refresh_tokens (expires_at); CREATE INDEX IF NOT EXISTS idx_refresh_tokens_family_id ON refresh_tokens (family_id); +-- ============================================================================ +-- Durable S3/R2 profile-picture deletion outbox (GDPR erasure completeness) +-- ============================================================================ +-- +-- DeleteAccountHandler persists one row here INSIDE its anonymization +-- transaction (before it commits), mirroring the Square erasure outbox (Fault +-- A1 → A2): the object-store deletion of the user's profile photo (PII) must +-- survive a process crash, or the photo would be retained indefinitely. The +-- async cleanup goroutine is the primary drain (deleting the row once the +-- object is confirmed gone); the retry-s3-deletions job +-- (internal/jobs/cleanup.go) is the crash safety net that completes any +-- deletion the goroutine could not finish. Rows are deleted on success; a +-- failed deletion stays in place and is retried on every job run. +CREATE TABLE IF NOT EXISTS pending_s3_deletions ( + id CHAR(12) PRIMARY KEY DEFAULT generate_short_id('pending_s3_deletions'), + user_id CHAR(12), -- erased/anonymized user (no FK: guest users are deleted) + bucket TEXT NOT NULL, -- S3/R2 bucket holding the object + object_key TEXT NOT NULL, -- object key (e.g. profiles/.jpg) + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + attempts INT NOT NULL DEFAULT 0, -- retry attempts so far (operator visibility) + last_error TEXT -- most recent deletion error (operator visibility) +); + +CREATE INDEX IF NOT EXISTS idx_pending_s3_deletions_created_at ON pending_s3_deletions (created_at); +