diff --git a/backend/handlers/payments/sweep.go b/backend/handlers/payments/sweep.go index 971ac7c..595c348 100644 --- a/backend/handlers/payments/sweep.go +++ b/backend/handlers/payments/sweep.go @@ -259,11 +259,28 @@ func reconcileStalePaymentAtSquare(ctx context.Context, table, squarePaymentID s log.Printf("Stale pending %s reconcile for Square payment %s hit an ambiguous error (%v) — leaving pending for a later sweep run", table, squarePaymentID, err) return staleReconcileLeavePending } - if pr.Status != "COMPLETED" { + // Square's terminal payment states are COMPLETED, CANCELED, FAILED; + // APPROVED (authorization-only, delayed capture) and PENDING are + // NON-terminal — both can still transition to COMPLETED (via + // CompletePayment or delay_action=COMPLETE), so clawing back the funded + // gift card on either would risk reversing a charge that later lands. + // This app always creates payments with autocomplete (default true), so + // it never produces APPROVED/PENDING rows today, but the classification + // must match Square's documented state machine (librarian-verified + // 2026-05-20). + switch pr.Status { + case "COMPLETED": + return staleReconcileCompleted + case "CANCELED", "FAILED": + log.Printf("Stale pending %s is %q at Square — marking failed", table, pr.Status) + return staleReconcileDefinitivelyFailed + case "APPROVED", "PENDING": + log.Printf("Stale pending %s is %q at Square (non-terminal) — leaving pending for a later sweep run", table, pr.Status) + return staleReconcileLeavePending + default: log.Printf("Stale pending %s is %q at Square — marking failed", table, pr.Status) return staleReconcileDefinitivelyFailed } - return staleReconcileCompleted } // squarePaymentErrorIsNotFound reports whether a GetPayment error proves the diff --git a/backend/handlers/payments/till.go b/backend/handlers/payments/till.go index c5e80fe..da5b752 100644 --- a/backend/handlers/payments/till.go +++ b/backend/handlers/payments/till.go @@ -127,9 +127,12 @@ func revertGiftCardFunding(ctx context.Context, action, giftCardID string, amoun } if action == "create" { - // A newly created card has exactly one funding transaction (this - // request's purchase) — remove it, then the card itself. - if _, err := tx.Exec(ctx, `DELETE FROM gift_card_transactions WHERE gift_card_id = $1`, giftCardID); err != nil { + // A newly created card's transactions are scoped to THIS sale's + // funding (reference_type='till_sale' AND reference_id=sale id) — never + // a wholesale delete, which would destroy the value of a different + // idempotency-keyed top-up sale that funded the same card before this + // create resolved. Then remove the card itself. + if _, err := tx.Exec(ctx, `DELETE FROM gift_card_transactions WHERE gift_card_id = $1 AND reference_type = 'till_sale' AND reference_id = $2`, giftCardID, tillSaleID); err != nil { return fmt.Errorf("failed to delete gift card transaction: %w", err) } if _, err := tx.Exec(ctx, `DELETE FROM gift_cards WHERE id = $1`, giftCardID); err != nil { @@ -138,12 +141,21 @@ func revertGiftCardFunding(ctx context.Context, action, giftCardID string, amoun // If the card was immediately redeemed to a user balance in this // request, reverse that credit (guarded so it can never go negative). if redeemToUserID != nil && *redeemToUserID != "" { - if _, err := tx.Exec(ctx, ` + bTag, bErr := tx.Exec(ctx, ` UPDATE user_giftcard_balances SET balance = user_giftcard_balances.balance - $1, updated_at = NOW() WHERE user_id = $2 AND balance >= $1 - `, amount, *redeemToUserID); err != nil { - return fmt.Errorf("failed to reverse redeemed gift card balance: %w", err) + `, amount, *redeemToUserID) + if bErr != nil { + return fmt.Errorf("failed to reverse redeemed gift card balance: %w", bErr) + } + if bTag.RowsAffected() == 0 { + // The guard blocked the reversal because some of the credited + // balance was already spent. The sale is still marked failed + // below — do not fail the whole clawback tx — but the + // un-reversed credit must be flagged for manual reconciliation + // (mirrors the top-up branch). + log.Printf("CRITICAL: ... MANUAL RECONCILIATION REQUIRED: create-with-redeem clawback for gift card %s could not fully reverse the £%.2f balance credited to user %s (balance < amount)", giftCardID, amount, *redeemToUserID) } } } else { @@ -945,6 +957,18 @@ func GetTillCheckoutStatus(w http.ResponseWriter, r *http.Request) { return } + // A sale already swept to 'failed' (e.g. its checkout was cancelled as + // stale, or a definitive decline clawed back the card) must never report a + // live payment state: the poll would otherwise tell the admin the terminal + // is still waiting when the sale can no longer be completed. Fail loudly so + // a stale checkout that later resolves at Square is surfaced for manual + // reconciliation instead of silently confusing the operator. + if currentStatus == "failed" { + log.Printf("Till sale %s is already failed but checkout %s is still being polled — refusing to report a live state", tillSaleID, checkoutID) + http.Error(w, "Till sale already failed — manual reconciliation required", http.StatusNotFound) + return + } + paymentResult, err := SquareClient.GetCheckout(r.Context(), checkoutID) if err != nil { if errors.Is(err, square.ErrCheckoutPending) { diff --git a/backend/internal/jobs/cleanup.go b/backend/internal/jobs/cleanup.go index a227e19..4e71533 100644 --- a/backend/internal/jobs/cleanup.go +++ b/backend/internal/jobs/cleanup.go @@ -329,6 +329,14 @@ func ScanCriticalPaymentLogs(ctx context.Context) (int, error) { AND an.booking_id IS NOT DISTINCT FROM src.booking_id AND an.acknowledged_at IS NULL ) + -- admin_notifications.booking_id is a hard FK to bookings(id); a + -- candidate whose booking was hard-deleted (7-year retention) would + -- otherwise fail the whole INSERT and silently kill ALL critical + -- alerts. Skip orphans: keep NULL booking_ids (till sales) and only + -- payments/refunds whose booking still exists. + AND (src.booking_id IS NULL OR EXISTS ( + SELECT 1 FROM bookings b WHERE b.id = src.booking_id + )) `) if err != nil { return 0, fmt.Errorf("failed to scan critical payment logs: %w", err) diff --git a/frontend/src/lib/components/payments/UserPaymentModal.svelte b/frontend/src/lib/components/payments/UserPaymentModal.svelte index 382c072..f86a665 100644 --- a/frontend/src/lib/components/payments/UserPaymentModal.svelte +++ b/frontend/src/lib/components/payments/UserPaymentModal.svelte @@ -404,8 +404,13 @@ } // Cache the idempotency key per amount+type+card so a lost-response - // retry reuses it (backend dedups) instead of double-charging. - const cardKey = cardId ?? `new:${newCardToken ?? ''}`; + // retry reuses it (backend dedups) instead of double-charging. The + // new-card identity is a STABLE sentinel, NOT the cnon: nonce: the + // nonce is one-shot and cleared on a failed charge, so keying on it + // would regenerate the key on retry and a network-timeout retry (where + // the charge actually landed) would double-charge. The nonce changes + // per tokenize but represents the same logical card intent. + const cardKey = cardId ?? 'new-card'; if ( !payIdempotencyKey || payKeyedAmount !== amountCents || diff --git a/frontend/src/routes/account/+page.svelte b/frontend/src/routes/account/+page.svelte index b310ca9..d8fdda3 100644 --- a/frontend/src/routes/account/+page.svelte +++ b/frontend/src/routes/account/+page.svelte @@ -317,7 +317,13 @@ // Cache the idempotency key per amount+card so a lost-response retry // reuses the same key (backend dedups) instead of double-charging. // Regenerate when the amount or card changes. - const cardKey = cardId || `new:${newCardToken ?? ''}`; + // Cache the idempotency key per amount+card so a lost-response + // retry reuses it (backend dedups) instead of double-charging. The + // new-card identity is a STABLE sentinel, NOT the cnon: nonce: the + // nonce is one-shot and cleared on a failed charge, so keying on it + // would regenerate the key on retry and a network-timeout retry + // (where the charge actually landed) would double-charge. + const cardKey = cardId || 'new-card'; if (!buyIdempotencyKey || buyKeyedAmount !== buyAmount || buyKeyedCard !== cardKey) { buyIdempotencyKey = generateIdempotencyKey(); buyKeyedAmount = buyAmount;