From a8d54f1e2a20d699de6e09b5046a2f5917c55f9c Mon Sep 17 00:00:00 2001
From: Stephen Adamson <popertots@gmail.com>
Date: Mon, 3 Aug 2026 21:10:17 +0100
Subject: [PATCH] Fix review findings:
 aggregated-refund/saved-card/legacy-refund idempotency keys, structured
 Square error classification, CSP for Square SDK
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Money-safety idempotency fixes (external review bugs 1-3):
- processChargeGroup: aggregated refund key now hashes the sorted pending-row
  set (chargeID-square-agg-<sha256 suffix>) so a changed group can never mark
  a new row completed against an old smaller refund; >45-char chargeIDs use a
  hashed prefix instead of verbatim truncation (which would collide charges on
  Square's global key dedup). Same-set crash-retry keeps Square's dedup.
- CreateTerminalPayment saved_card: two-tier idempotency key — client-supplied
  per-attempt UUID preferred (distinct identical charges no longer collapse),
  deterministic booking+type+amount+card fallback for no-key retry safety.
  PaymentModal sends a per-charge UUID cleared after success.
- ensureRefundKey: legacy NULL-key manual refunds persist a generated key to
  the row BEFORE the Square call (race-safe AND idempotency_key IS NULL guard),
  so a lost-response retry reuses the key and never double-refunds. Wired into
  resumeManualPendingRefund and the sweep's manual-retry loop.

Classification + money-safety hardening:
- till.go/sweep.go: structured square.ErrorCode/IsNotFound are authoritative
  when present; message-substring matching only for non-structured errors
  (dev mock, client-side status errors). Fixes fragile string-matching driving
  sweep retries and gift-card clawbacks.
- SaveCardForUser: ON CONFLICT (user_id, square_card_id) DO NOTHING + re-select
  (was a latent UNIQUE-violation 500 on save-card retry).
- CreateBookingPayment: partial payments re-validated against remaining balance
  inside the advisory lock (closes concurrent-overpayment race).
- InvalidateSquareCustomerCache on GDPR erasure paths (account.go,
  time-blockers.go stale-guest anonymization).
- GetUserGiftCardBalanceAdmin: in-handler admin check (defense-in-depth).
- getCheckoutHTTP: warn on multi-payment checkouts instead of dropping
  payments[1:].
- Cash/giftcard terminal branch: removed dead idempotency SELECT, "tip-" ->
  "till-" prefix.
- UserPaymentModal: removed vestigial polling state; proper interval cleanup.
- account/+page.svelte: gift-card redeem dialog links /terms.
- nginx CSP: allow *.squarecdn.com and js.squareup.com so the Square Web
  Payments SDK + card iframe can tokenize behind the proxy.

Tests: +8 regression tests covering changed-set refund keys, legacy NULL-key
single-refund, saved-card client-key dedup/no-dedup, concurrent partials, and
cache invalidation. Full suite + race detector clean via run-tests.sh lockfile.
---
 backend/handlers/payments/concurrency_test.go | 132 ++++++
 backend/handlers/payments/giftcards.go        |   5 +
 backend/handlers/payments/handlers.go         | 115 +++--
 .../payments/p14_payment_fixes_test.go        |  76 ++++
 backend/handlers/payments/payments_test.go    | 139 +++++++
 backend/handlers/payments/refunds.go          | 114 ++++-
 backend/handlers/payments/refunds_test.go     | 392 +++++++++++++++++-
 backend/handlers/payments/service.go          |  28 ++
 backend/handlers/payments/sweep.go            |  78 +++-
 backend/handlers/payments/till.go             |  28 +-
 backend/handlers/scheduling/time-blockers.go  |  44 +-
 .../handlers/scheduling/time_blockers_test.go |  63 +++
 backend/handlers/user/account.go              |   6 +
 backend/handlers/user/user_coverage_test.go   |  50 +++
 backend/internal/square/square_dev.go         |  10 +
 backend/internal/square/square_http_client.go |   7 +
 backend/run-tests.sh                          |  25 ++
 .../components/payments/PaymentModal.svelte   |  38 +-
 .../payments/UserPaymentModal.svelte          |  25 +-
 frontend/src/routes/account/+page.svelte      |  10 +-
 nginx/conf.d/default.conf                     |   5 +-
 21 files changed, 1279 insertions(+), 111 deletions(-)
 create mode 100755 backend/run-tests.sh

diff --git a/backend/handlers/payments/concurrency_test.go b/backend/handlers/payments/concurrency_test.go
index 1d0f311..0f7f59d 100644
--- a/backend/handlers/payments/concurrency_test.go
+++ b/backend/handlers/payments/concurrency_test.go
@@ -314,6 +314,138 @@ func TestBookingPayment_ConcurrentSameKey_SingleRecord(t *testing.T) {
 	}
 }
 
+// countingCreatePaymentClient delays the Square charge (widening the advisory
+// lock race window) and counts every successful CreatePayment call so the test
+// can assert exactly one charge reaches Square.
+type countingCreatePaymentClient struct {
+	square.SquareClient
+	mu      sync.Mutex
+	charges int
+}
+
+func (c *countingCreatePaymentClient) CreatePayment(ctx context.Context, req square.CreatePaymentReq) (*square.PaymentResult, error) {
+	c.mu.Lock()
+	c.charges++
+	c.mu.Unlock()
+	time.Sleep(300 * time.Millisecond)
+	return c.SquareClient.CreatePayment(ctx, req)
+}
+
+func (c *countingCreatePaymentClient) chargeCount() int {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+	return c.charges
+}
+
+// TestBookingPayment_ConcurrentPartials_SingleCharge proves the in-lock
+// remaining-balance re-check (handlers.go): two concurrent 'partial' payments
+// whose combined amount exceeds the booking's remaining balance must yield ONE
+// successful charge — the loser is rejected with 4xx inside the advisory lock
+// BEFORE inserting a pending record or hitting Square. Without the re-check
+// both pass the pre-lock ValidatePartialAmount against the same balance, both
+// charge, and the overflow is silently recorded as a tip.
+func TestBookingPayment_ConcurrentPartials_SingleCharge(t *testing.T) {
+	ctx, tx := testutils.SetupTestTx(t)
+
+	userID, bookingID, _ := setupTestDataPast(t, ctx, tx)
+	token := jwt.GenerateUserToken(userID)
+	cleanupConcurrentTestRows(t, context.Background(), userID, bookingID)
+
+	innerTx := db.TxFromContext(ctx)
+	if innerTx == nil {
+		t.Fatal("no transaction in context")
+	}
+	if err := innerTx.Commit(ctx); err != nil {
+		t.Fatalf("failed to commit setup tx: %v", err)
+	}
+
+	// The fixture service costs £50, so the booking's remaining balance is 5000
+	// pence. Two £30 partials sum to £60 > £50 — only one may succeed.
+	var remainingCents int64
+	if err := db.Conn.QueryRow(context.Background(),
+		`SELECT ROUND(total_amount * 100)::bigint FROM bookings WHERE id = $1`, bookingID).Scan(&remainingCents); err != nil {
+		t.Fatalf("failed to read booking total: %v", err)
+	}
+	if remainingCents != 5000 {
+		t.Fatalf("expected fixture booking total of 5000 pence, got %d", remainingCents)
+	}
+
+	origClient := SquareClient
+	slow := &countingCreatePaymentClient{SquareClient: square.NewDevClient()}
+	SquareClient = slow
+	defer func() { SquareClient = origClient }()
+
+	pool := context.Background()
+	cardToken := "cnon:concurrent-partial-card"
+	reqBody := CreateBookingPaymentRequest{
+		Amount:         3000,
+		PaymentType:    "partial",
+		NewCardToken:   &cardToken,
+		IdempotencyKey: "partial-concurrent-" + bookingID,
+	}
+
+	var wg sync.WaitGroup
+	startBoth := make(chan struct{})
+	recs := make([]*httptest.ResponseRecorder, 2)
+
+	for i := 0; i < 2; i++ {
+		wg.Add(1)
+		go func(idx int) {
+			defer wg.Done()
+			<-startBoth
+			recs[idx] = makePaymentRequest(CreateBookingPayment, "POST", "/api/bookings/"+bookingID+"/payment", reqBody, token, pool)
+		}(i)
+	}
+	close(startBoth)
+	wg.Wait()
+
+	// Exactly one request wins; the loser is rejected by the in-lock balance
+	// re-check (409) — or by the pre-lock filter if it read the reduced balance
+	// after the winner committed (400). Either way, never a second charge.
+	okCount, rejectedCount := 0, 0
+	for i, rec := range recs {
+		switch {
+		case rec.Code == http.StatusOK:
+			okCount++
+		case rec.Code == http.StatusBadRequest || rec.Code == http.StatusConflict:
+			rejectedCount++
+		default:
+			t.Errorf("request %d unexpected status %d: %s", i, rec.Code, rec.Body.String())
+		}
+	}
+	if okCount != 1 {
+		t.Errorf("expected exactly 1 successful partial payment, got %d", okCount)
+	}
+	if rejectedCount != 1 {
+		t.Errorf("expected exactly 1 rejected partial payment, got %d", rejectedCount)
+	}
+
+	// Exactly one charge reached Square.
+	if n := slow.chargeCount(); n != 1 {
+		t.Errorf("expected exactly 1 Square charge, got %d (double-charge!)", n)
+	}
+
+	// Exactly one completed payment for the booking, and no overpayment: the
+	// recorded total must not exceed the booking's remaining balance.
+	var payCount int
+	if err := db.Conn.QueryRow(pool,
+		`SELECT COUNT(*) FROM payments WHERE booking_id = $1 AND status = 'completed'`, bookingID).Scan(&payCount); err != nil {
+		t.Fatalf("failed to count payments: %v", err)
+	}
+	if payCount != 1 {
+		t.Errorf("expected exactly 1 completed payment, got %d (double-charge!)", payCount)
+	}
+	var paidPence int64
+	if err := db.Conn.QueryRow(pool,
+		`SELECT ROUND(COALESCE(SUM(amount), 0) * 100)::bigint FROM payments WHERE booking_id = $1 AND status = 'completed' AND payment_type <> 'tip'`,
+		bookingID).Scan(&paidPence); err != nil {
+		t.Fatalf("failed to sum paid amount: %v", err)
+	}
+	if paidPence > remainingCents {
+		t.Errorf("overpayment recorded: paid %d pence exceeds remaining balance %d pence", paidPence, remainingCents)
+	}
+}
+
 // completedCheckoutClient forces GetCheckout to return a fixed COMPLETED
 // payment for any checkout id, deterministically exercising the terminal
 // completion dedup+insert path (the mock's real async goroutine would be
diff --git a/backend/handlers/payments/giftcards.go b/backend/handlers/payments/giftcards.go
index 93991a7..faf4dc4 100644
--- a/backend/handlers/payments/giftcards.go
+++ b/backend/handlers/payments/giftcards.go
@@ -844,6 +844,11 @@ func GetGiftCardBalance(w http.ResponseWriter, r *http.Request) {
 
 // GetUserGiftCardBalanceAdmin Handler returns any user's balance for the admin.
 func GetUserGiftCardBalanceAdmin(w http.ResponseWriter, r *http.Request) {
+	if !isAdminRequest(r) {
+		http.Error(w, "Unauthorized", http.StatusForbidden)
+		return
+	}
+
 	ctx := r.Context()
 	userID := chi.URLParam(r, "id")
 	if userID == "" || !validators.IsValidID(userID) {
diff --git a/backend/handlers/payments/handlers.go b/backend/handlers/payments/handlers.go
index a5fab2f..39becf4 100644
--- a/backend/handlers/payments/handlers.go
+++ b/backend/handlers/payments/handlers.go
@@ -36,6 +36,14 @@ type CreateTerminalPaymentRequest struct {
 	// directly, bypassing the terminal. The frontend sends this for the admin
 	// "Charge Saved Card" action.
 	UserSavedCardID *string `json:"saved_card_id,omitempty"`
+	// idempotency_key: optional client-generated per-attempt UUID for saved-card
+	// charges. The frontend generates one per distinct charge and reuses it
+	// across retries of the SAME charge, so two DISTINCT identical charges on
+	// one booking (e.g. a second £50 'full' charge for a second service) get
+	// different keys and never collapse on the deterministic fallback key.
+	// When absent, the handler falls back to the deterministic booking+type+
+	// amount+card key for no-client-key retry safety.
+	IdempotencyKey string `json:"idempotency_key,omitempty"`
 }
 
 type CreateBookingPaymentRequest struct {
@@ -287,8 +295,10 @@ func CreateTerminalPayment(w http.ResponseWriter, r *http.Request) {
 	// dedup two legitimate identical payments (e.g. two £50 cash receipts on
 	// one booking). Use a unique key per payment: retries of a lost response
 	// are handled by the Square-side key for card payments, and cash/giftcard
-	// are DB-committed synchronously.
-	idempotencyKey := uniqueChargeKey("tip-")
+	// are DB-committed synchronously. There is deliberately NO idempotency
+	// dedup check here — each request inserts its own row, and two identical
+	// cash receipts are legitimate distinct payments.
+	idempotencyKey := uniqueChargeKey("till-")
 
 	// Route based on payment method
 	if req.PaymentMethod != nil && (*req.PaymentMethod == "cash" || *req.PaymentMethod == "giftcard") {
@@ -322,23 +332,6 @@ func CreateTerminalPayment(w http.ResponseWriter, r *http.Request) {
 			return
 		}
 
-		// Check idempotency inside the transaction.
-		var existingID string
-		var existingStatus string
-		if err := tx.QueryRow(r.Context(), `
-			SELECT id, status FROM payments WHERE booking_id = $1 AND idempotency_key = $2
-		`, bookingID, idempotencyKey).Scan(&existingID, &existingStatus); err == nil {
-			if err := json.NewEncoder(w).Encode(CheckoutResponse{
-				CheckoutID: existingID,
-				Status:     existingStatus,
-			}); err != nil {
-				log.Printf("Failed to encode JSON response: %v", err)
-			}
-			return
-		} else if !errors.Is(err, pgx.ErrNoRows) {
-			log.Printf("Failed to check idempotency: %v", err)
-		}
-
 		amountPounds := float64(amount) / 100.0
 		var paymentID string
 
@@ -535,10 +528,25 @@ func CreateTerminalPayment(w http.ResponseWriter, r *http.Request) {
 		}
 		defer releaseBookingPaymentLock(pinConn, "crussell:payment:"+bookingID)
 
-		// Deterministic idempotency key: booking+type+amount+card. A network
-		// retry with the same inputs derives the same key → dedup, never a
-		// second charge. ≤45 chars for Square's limit.
-		scKey := bookingID + "-sc-" + req.PaymentType + "-" + strconv.FormatInt(amount, 10) + "-" + *req.UserSavedCardID
+		// Idempotency key — two tiers:
+		// 1. Client-supplied per-attempt UUID (preferred): the frontend
+		//    generates one per DISTINCT charge and reuses it across retries of
+		//    the same charge. Two distinct identical charges on one booking
+		//    (e.g. a second £50 'full' charge for a second service) send
+		//    different UUIDs → no dedup, each becomes its own payment. The
+		//    UUID is globally unique so it is NOT namespaced with the booking
+		//    id (the UNIQUE(idempotency_key) constraint is global); the dedup
+		//    SELECT matches on booking_id + key, so a same-booking retry of
+		//    the same UUID still dedups.
+		// 2. Deterministic booking+type+amount+card fallback when the client
+		//    sends no key: a no-key network retry derives the same key → dedup,
+		//    never a second charge (old-client retry safety).
+		// Both stay ≤45 chars for Square's limit (36-char UUID / ~38-char
+		// deterministic key).
+		scKey := req.IdempotencyKey
+		if scKey == "" {
+			scKey = bookingID + "-sc-" + req.PaymentType + "-" + strconv.FormatInt(amount, 10) + "-" + *req.UserSavedCardID
+		}
 
 		// Idempotency switch inside the lock: completed → dedup; pending →
 		// reuse (re-attempt Square with the same key, which dedups Square-side);
@@ -1299,6 +1307,28 @@ func CreateBookingPayment(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	// Authoritative remaining-balance re-check for 'partial' payments, inside
+	// the advisory lock. The cheap pre-lock ValidatePartialAmount above can
+	// race a concurrent partial payment on the same booking: both pass against
+	// the same remaining balance, then both charge at Square, and the overflow
+	// is silently recorded as a tip by buildSplitRecords. The lock serializes
+	// payment attempts, so by the time we re-read here a competing payment has
+	// already committed — reject before any pending record is inserted or
+	// Square is hit.
+	if req.PaymentType == "partial" {
+		remainingCents, err := service.GetBookingRemainingBalanceCents(r.Context(), bookingID)
+		if err != nil {
+			log.Printf("Failed to get remaining balance: %v", err)
+			http.Error(w, "internal server error", http.StatusInternalServerError)
+			return
+		}
+		if err := ValidatePartialAmount(req.Amount, remainingCents); err != nil {
+			log.Printf("Payment rejected: %v", err)
+			http.Error(w, "Partial amount exceeds remaining balance", http.StatusConflict)
+			return
+		}
+	}
+
 	// Check idempotency inside the transaction.
 	// Only short-circuit when the existing record is 'completed'. A 'pending'
 	// record means the previous Square call failed — returning it as 200 would
@@ -2101,10 +2131,18 @@ func RefundPayment(w http.ResponseWriter, r *http.Request) {
 				}
 				return
 			}
+			// Persist a fallback key (legacy NULL-key rows) before re-issuing so
+			// a lost-response retry reuses it — see ensureRefundKey.
+			reissueKey, keyErr := ensureRefundKey(r.Context(), existingRefundID.String, paymentID, resumeAmount, existingRefundKey.String)
+			if keyErr != nil {
+				log.Printf("Failed to ensure refund key for refund %s before re-issue: %v", existingRefundID.String, keyErr)
+				http.Error(w, "Unable to verify refund status with Square, please retry", http.StatusServiceUnavailable)
+				return
+			}
 			reissueReq := square.RefundPaymentReq{
 				PaymentID:      refundSqPaymentID,
 				Amount:         resumeAmount,
-				IdempotencyKey: refundResumeKey(paymentID, resumeAmount, existingRefundKey.String),
+				IdempotencyKey: reissueKey,
 				Reason:         existingRefundReason.String,
 			}
 			reissueResult, reissueErr := SquareClient.RefundPayment(r.Context(), reissueReq)
@@ -2388,29 +2426,22 @@ func RefundPayment(w http.ResponseWriter, r *http.Request) {
 // row. Called from RefundPayment's exact-key dedup and the (payment, amount)
 // pending fallback. Using the stored key lets Square return the original
 // refund if the prior attempt actually completed (response loss), so no second
-// refund can ever be issued for a row whose money state is unknown.
-// refundResumeKey returns the row's OWN stored idempotency key when present, or
-// a fresh deterministic fallback when the row predates keyed refunds (legacy
-// NULL idempotency_key). Square's RefundPayment REQUIRES a non-empty
-// idempotency key — re-issuing with "" returns a 400 INVALID_REQUEST_ERROR,
-// which classifies as ambiguous and leaves the refund pending forever. The
-// fallback mirrors the no-client-key shape (paymentID + "-refund-" + amount +
-// "-" + hex) and stays ≤45 chars (12 + 8 + up-to-9 + 1 + 12 ≈ 42), so the
-// re-issue is never rejected for length either. A legit stored key is ALWAYS
-// reused so Square's same-key dedup keeps returning the original refund.
-func refundResumeKey(paymentID string, amount int64, storedKey string) string {
-	if storedKey != "" {
-		return storedKey
-	}
-	return paymentID + "-refund-" + strconv.FormatInt(amount, 10) + "-" + randomHexSuffix(6)
-}
-
+// refund can ever be issued for a row whose money state is unknown. A legacy
+// NULL-key row gets a fallback key persisted to the row FIRST (ensureRefundKey),
+// so a lost-response retry reuses it instead of double-refunding with a fresh
+// random suffix.
 func resumeManualPendingRefund(w http.ResponseWriter, r *http.Request, paymentID string, payment *PaymentRecord, refundID string, refundAmount float64, refundReason, refundKey string) {
 	resumeAmount := int64(math.Round(refundAmount * 100))
+	resumeKey, keyErr := ensureRefundKey(r.Context(), refundID, paymentID, resumeAmount, refundKey)
+	if keyErr != nil {
+		log.Printf("Failed to ensure refund key for refund %s before resume: %v", refundID, keyErr)
+		http.Error(w, "Unable to verify refund status with Square, please retry", http.StatusServiceUnavailable)
+		return
+	}
 	resumeReq := square.RefundPaymentReq{
 		PaymentID:      *payment.SquarePaymentID,
 		Amount:         resumeAmount,
-		IdempotencyKey: refundResumeKey(paymentID, resumeAmount, refundKey),
+		IdempotencyKey: resumeKey,
 		Reason:         refundReason,
 	}
 	resumeResult, resumeErr := SquareClient.RefundPayment(r.Context(), resumeReq)
diff --git a/backend/handlers/payments/p14_payment_fixes_test.go b/backend/handlers/payments/p14_payment_fixes_test.go
index 665de32..9a758ab 100644
--- a/backend/handlers/payments/p14_payment_fixes_test.go
+++ b/backend/handlers/payments/p14_payment_fixes_test.go
@@ -443,6 +443,82 @@ func TestCreatePaymentMethodFromToken_ProvisionsCustomerOnceAndReuses(t *testing
 	require.Equal(t, cid1, cid2, "both saved cards must share the user's Square customer id")
 }
 
+// alwaysNewCustomerClient counts every CreateCustomer call and always returns a
+// fresh customer id (unlike recordingCustomerClient, which dedups by email and
+// would mask a second mint for the same user).
+type alwaysNewCustomerClient struct {
+	square.SquareClient
+	mu          sync.Mutex
+	createCalls int
+}
+
+func (c *alwaysNewCustomerClient) CreateCustomer(ctx context.Context, name, email string) (*square.CustomerResult, error) {
+	c.mu.Lock()
+	c.createCalls++
+	id := fmt.Sprintf("cus_cache_%d", c.createCalls)
+	c.mu.Unlock()
+	return &square.CustomerResult{ID: id, Email: email}, nil
+}
+
+func (c *alwaysNewCustomerClient) customerCallCount() int {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+	return c.createCalls
+}
+
+// TestInvalidateSquareCustomerCache_DropsCachedID proves the exported cache
+// invalidation: after GDPR erasure NULLs the DB square_customer_id and the
+// customer is deleted at Square, the process-local cache must not keep serving
+// the erased user's stale customer id. Without invalidation a later
+// ensureSquareCustomer would return the cached id without re-minting; after
+// invalidation it re-queries the (NULLed) DB and mints a fresh customer.
+func TestInvalidateSquareCustomerCache_DropsCachedID(t *testing.T) {
+	ctx, tx := testutils.SetupTestTx(t)
+	userID, err := fixtures.CreateTestUser(tx)
+	require.NoError(t, err)
+
+	// A saved-card row is the persistence point for the customer id; start
+	// with a NULL square_customer_id so ensureSquareCustomer must mint one.
+	_, err = tx.Exec(ctx, `
+		INSERT INTO user_saved_cards (user_id, square_card_id, brand, last_4, exp_month, exp_year, fingerprint, is_default)
+		VALUES ($1, 'sq_card_cache_test', 'Visa', '4242', 12, 2030, 'fp_cache', true)
+	`, userID)
+	require.NoError(t, err)
+
+	origClient := SquareClient
+	rec := &alwaysNewCustomerClient{SquareClient: square.NewDevClient()}
+	SquareClient = rec
+	defer func() { SquareClient = origClient }()
+	t.Cleanup(func() { InvalidateSquareCustomerCache(userID) })
+
+	svc := NewPaymentService()
+
+	// 1. First ensure mints a customer and caches it.
+	c1, err := svc.EnsureSquareCustomer(ctx, userID)
+	require.NoError(t, err)
+	require.NotEmpty(t, c1)
+	require.Equal(t, 1, rec.customerCallCount())
+
+	// Simulate GDPR erasure NULLing the saved-card square_customer_id.
+	_, err = tx.Exec(ctx, `UPDATE user_saved_cards SET square_customer_id = NULL WHERE user_id = $1`, userID)
+	require.NoError(t, err)
+
+	// 2. WITHOUT invalidation the stale cached id is still served (no re-mint).
+	c2, err := svc.EnsureSquareCustomer(ctx, userID)
+	require.NoError(t, err)
+	require.Equal(t, c1, c2, "stale cached customer id must be served when the cache is NOT invalidated")
+	require.Equal(t, 1, rec.customerCallCount())
+
+	// 3. Invalidate, then re-ensure: the entry is gone, so the NULLed DB is
+	// re-queried and a fresh customer is minted — the stale identity must not
+	// resurface.
+	InvalidateSquareCustomerCache(userID)
+	c3, err := svc.EnsureSquareCustomer(ctx, userID)
+	require.NoError(t, err)
+	require.NotEqual(t, c1, c3, "after invalidation a fresh customer must be minted, not the stale cached id")
+	require.Equal(t, 2, rec.customerCallCount())
+}
+
 func TestBuyGiftCard_NoSaveCard_NoCustomerProvisioned(t *testing.T) {
 	ctx, tx := testutils.SetupTestTx(t)
 	userID, err := fixtures.CreateTestUser(tx)
diff --git a/backend/handlers/payments/payments_test.go b/backend/handlers/payments/payments_test.go
index 243449f..b477178 100644
--- a/backend/handlers/payments/payments_test.go
+++ b/backend/handlers/payments/payments_test.go
@@ -3953,6 +3953,145 @@ func TestSavedCardPayment_SweptFailed_Rejected(t *testing.T) {
 	}
 }
 
+// TestSavedCardPayment_ClientKey_DistinctCharges_NoDedup verifies the Bug 2
+// fix: the frontend sends a per-attempt idempotency UUID, so two DISTINCT
+// identical saved-card charges on the same booking (same amount, same card,
+// same payment type) must NOT collapse on the old deterministic
+// booking+type+amount+card key. Each becomes its own payment.
+func TestSavedCardPayment_ClientKey_DistinctCharges_NoDedup(t *testing.T) {
+	t.Parallel()
+	ctx, tx := testutils.SetupTestTx(t)
+
+	userID, bookingID, _ := setupTestData(t, ctx, tx)
+	adminID, err := fixtures.CreateTestAdminUser(tx)
+	if err != nil {
+		t.Fatalf("failed to create admin user: %v", err)
+	}
+	adminToken := jwt.GenerateTestToken(adminID, "admin")
+
+	cardID, err := fixtures.CreateTestPaymentMethod(tx, userID, "ccof:distinct-key-card", "VISA", "4242")
+	if err != nil {
+		t.Fatalf("failed to create saved card: %v", err)
+	}
+
+	handler := CreateTerminalPayment
+
+	// Two legitimately distinct £50 'full' charges on the same booking — the
+	// frontend sends a different per-attempt UUID for each.
+	for i, key := range []string{"saved-card-uuid-0001", "saved-card-uuid-0002"} {
+		reqBody := CreateTerminalPaymentRequest{
+			Amount:          5000,
+			PaymentType:     "full",
+			PaymentMethod:   strPtr("saved_card"),
+			UserSavedCardID: &cardID,
+			IdempotencyKey:  key,
+		}
+		w := makePaymentRequest(handler, "POST", "/api/admin/bookings/"+bookingID+"/payment", reqBody, adminToken, ctx)
+		if w.Code != http.StatusOK {
+			t.Fatalf("saved-card charge %d: expected 200, got %d. body: %s", i+1, w.Code, w.Body.String())
+		}
+	}
+
+	// Exactly TWO payment records — the second charge must not be swallowed by
+	// the dedup branch.
+	var payCount int
+	if err := tx.QueryRow(ctx, `SELECT COUNT(*) FROM payments WHERE booking_id = $1 AND payment_method = 'online_square'`, bookingID).Scan(&payCount); err != nil {
+		t.Fatalf("failed to count payments: %v", err)
+	}
+	if payCount != 2 {
+		t.Errorf("expected exactly 2 payment records (no dedup), got %d", payCount)
+	}
+}
+
+// TestSavedCardPayment_ClientKey_SameKeyRetry_Dedups verifies that a lost-
+// response retry carrying the SAME client UUID still dedups to a single
+// payment — the client key does not disable retry safety.
+func TestSavedCardPayment_ClientKey_SameKeyRetry_Dedups(t *testing.T) {
+	t.Parallel()
+	ctx, tx := testutils.SetupTestTx(t)
+
+	userID, bookingID, _ := setupTestData(t, ctx, tx)
+	adminID, err := fixtures.CreateTestAdminUser(tx)
+	if err != nil {
+		t.Fatalf("failed to create admin user: %v", err)
+	}
+	adminToken := jwt.GenerateTestToken(adminID, "admin")
+
+	cardID, err := fixtures.CreateTestPaymentMethod(tx, userID, "ccof:same-key-card", "VISA", "4242")
+	if err != nil {
+		t.Fatalf("failed to create saved card: %v", err)
+	}
+
+	handler := CreateTerminalPayment
+	reqBody := CreateTerminalPaymentRequest{
+		Amount:          5000,
+		PaymentType:     "full",
+		PaymentMethod:   strPtr("saved_card"),
+		UserSavedCardID: &cardID,
+		IdempotencyKey:  "saved-card-uuid-0001",
+	}
+
+	// First charge.
+	w1 := makePaymentRequest(handler, "POST", "/api/admin/bookings/"+bookingID+"/payment", reqBody, adminToken, ctx)
+	if w1.Code != http.StatusOK {
+		t.Fatalf("first saved-card charge: expected 200, got %d. body: %s", w1.Code, w1.Body.String())
+	}
+
+	// Same-input retry (lost response) — same client UUID → dedup, not a
+	// second charge.
+	w2 := makePaymentRequest(handler, "POST", "/api/admin/bookings/"+bookingID+"/payment", reqBody, adminToken, ctx)
+	if w2.Code != http.StatusOK {
+		t.Fatalf("retry saved-card charge: expected 200, got %d. body: %s", w2.Code, w2.Body.String())
+	}
+
+	// Exactly ONE payment record.
+	var payCount int
+	if err := tx.QueryRow(ctx, `SELECT COUNT(*) FROM payments WHERE booking_id = $1 AND payment_method = 'online_square'`, bookingID).Scan(&payCount); err != nil {
+		t.Fatalf("failed to count payments: %v", err)
+	}
+	if payCount != 1 {
+		t.Errorf("expected exactly 1 payment record (same-key dedup), got %d — double-charge!", payCount)
+	}
+}
+
+// TestTerminalPayment_TwoIdenticalCashReceipts_NoDedup verifies the cash/
+// giftcard branch: two identical £50 cash receipts on the same booking are
+// legitimate distinct payments and must each insert their own row (there is
+// deliberately no idempotency dedup in this branch).
+func TestTerminalPayment_TwoIdenticalCashReceipts_NoDedup(t *testing.T) {
+	t.Parallel()
+	ctx, tx := testutils.SetupTestTx(t)
+
+	_, bookingID, _ := setupTestData(t, ctx, tx)
+	adminID, err := fixtures.CreateTestAdminUser(tx)
+	if err != nil {
+		t.Fatalf("failed to create admin user: %v", err)
+	}
+	adminToken := jwt.GenerateTestToken(adminID, "admin")
+
+	handler := CreateTerminalPayment
+	reqBody := CreateTerminalPaymentRequest{
+		Amount:        5000,
+		PaymentType:   "full",
+		PaymentMethod: strPtr("cash"),
+	}
+
+	for i := 0; i < 2; i++ {
+		w := makePaymentRequest(handler, "POST", "/api/admin/bookings/"+bookingID+"/payment", reqBody, adminToken, ctx)
+		if w.Code != http.StatusOK {
+			t.Fatalf("cash receipt %d: expected 200, got %d. body: %s", i+1, w.Code, w.Body.String())
+		}
+	}
+
+	var payCount int
+	if err := tx.QueryRow(ctx, `SELECT COUNT(*) FROM payments WHERE booking_id = $1 AND payment_method = 'cash'`, bookingID).Scan(&payCount); err != nil {
+		t.Fatalf("failed to count payments: %v", err)
+	}
+	if payCount != 2 {
+		t.Errorf("expected exactly 2 cash payment rows (no dedup), got %d", payCount)
+	}
+}
+
 // TestSweepStalePendingPayments_CoversTillSales verifies the R3 fix: the sweep
 // also marks stale pending till_sales rows (card payments) as failed, so a
 // lost-response till sale can't stay pending past Square's key retention.
diff --git a/backend/handlers/payments/refunds.go b/backend/handlers/payments/refunds.go
index 7407898..311b145 100644
--- a/backend/handlers/payments/refunds.go
+++ b/backend/handlers/payments/refunds.go
@@ -2,6 +2,7 @@ package payments
 
 import (
 	"context"
+	"crypto/sha256"
 	"errors"
 	"fmt"
 	"log"
@@ -9,6 +10,7 @@ import (
 	"math"
 	"sort"
 	"strconv"
+	"strings"
 	"time"
 
 	"crussell/clock"
@@ -871,14 +873,29 @@ func processChargeGroup(ctx context.Context, chargeID string, rows []pendingChar
 		}
 	}
 
-	// ONE Square refund per charge with a STABLE charge-level idempotency key.
-	// The key is used ONLY for the Square call — never stored in
-	// refunds.idempotency_key (the per-row keys remain the audit trail). Square
-	// dedups same-key retries, so crash-retry amounts stay identical.
+	// ONE Square refund per charge with a SET-STABLE idempotency key. The key
+	// is derived from the sha256 of the SORTED ids of the pending rows being
+	// aggregated — computed from `pending`, the rows re-read UNDER the lock,
+	// NEVER the caller-supplied `rows` slice (which may be stale by the time
+	// the lock is held). The key is used ONLY for the Square call — never
+	// stored in refunds.idempotency_key (the per-row keys remain the audit
+	// trail).
+	//
+	// SAME-set retry (a crash/response-loss where the pending set is unchanged)
+	// hashes to the SAME key → Square's idempotency dedup returns the ORIGINAL
+	// refund, so a retry can never double-refund. A CHANGED set (a new
+	// cancellation refund row joined the group while the old rows still sit
+	// 'pending' — the CRITICAL-log path where the post-refund DB UPDATE failed)
+	// hashes to a NEW key → Square issues a fresh refund covering the new
+	// total, so the new row is NEVER marked 'completed' against an old smaller
+	// refund with no money actually moved (the under-refund / lost-money
+	// bookkeeping bug). The 23h age-guard reconcile above still protects the
+	// cross-sweep case where Square's finite (~24h) key retention may have
+	// lapsed.
 	sqResult, sqErr := SquareClient.RefundPayment(ctx, square.RefundPaymentReq{
 		PaymentID:      chargeID,
 		Amount:         totalCents,
-		IdempotencyKey: chargeID + "-square-agg",
+		IdempotencyKey: chargeAggKey(chargeID, idsOf(pending)),
 		Reason:         reason,
 	})
 	switch {
@@ -997,6 +1014,38 @@ func idsOf(rows []pendingChargeRow) []string {
 	return ids
 }
 
+// aggRefundKeySuffix returns a deterministic 12-hex-char suffix identifying
+// the exact set of pending refund rows being aggregated. The sorted row IDs
+// (CHAR(12), so plain concatenation is unambiguous) are sha256'd and truncated:
+// the SAME set always yields the SAME suffix — keeping Square's idempotency-key
+// dedup for a same-set crash-retry — while a CHANGED set yields a DIFFERENT
+// suffix, so a new row can never resolve against an old smaller refund.
+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]
+}
+
+// chargeAggKey builds the charge-level idempotency key for an aggregated
+// refund as <chargeID>-square-agg-<set-suffix>. Square's idempotency-key limit
+// is 45 chars; the verbatim form needs the chargeID ≤21 chars. square_payment_id
+// is an arbitrary TEXT column holding Square's real payment ID (typically 20-28
+// chars), so the verbatim form can exceed the limit — and a >45-char key is
+// rejected with a 400 INVALID_REQUEST_ERROR (classified ambiguous → stuck
+// pending forever). When the verbatim form does not fit, the chargeID is
+// sha256'd into a fixed-width prefix instead — NEVER truncated verbatim: two
+// charges sharing a truncated prefix would collide on Square's global key dedup
+// and silently swallow the second charge's refund (lost money).
+func chargeAggKey(chargeID string, ids []string) string {
+	suffix := aggRefundKeySuffix(ids)
+	key := chargeID + "-square-agg-" + suffix
+	if len(key) <= 45 {
+		return key
+	}
+	return aggRefundKeySuffix([]string{chargeID}) + "-square-agg-" + suffix
+}
+
 // manualPendingRow is one stale manual refund row eligible for the sweep's
 // resolution. It covers BOTH pending shapes the RefundPayment handler can
 // leave behind:
@@ -1085,6 +1134,44 @@ func sweepManualPendingSquareRefunds(ctx context.Context) (int, error) {
 	return processed, nil
 }
 
+// ensureRefundKey returns the idempotency key to use when re-issuing a manual
+// refund, persisting a generated fallback to the refunds row BEFORE Square is
+// called so every retry reuses the SAME key — Square dedups same-key retries,
+// so a lost-response retry can never issue a SECOND refund.
+//
+// A legacy refund row with a NULL idempotency_key used to get a fresh random
+// suffix on every resume; if the Square call succeeded but the follow-up DB
+// UPDATE to 'completed' failed (the CRITICAL-log path), the row stayed
+// 'pending' with its key STILL NULL and the next resume generated a NEW random
+// key → a second Square refund (double refund). Persisting the key first
+// closes that hole: a crash-retry re-reads the persisted key and reuses it.
+//
+// The `AND idempotency_key IS NULL` guard makes the persist race-safe: only
+// one concurrent caller wins the UPDATE; a loser (0 rows affected) re-reads
+// and returns the winner's key. The generated shape (paymentID + "-refund-" +
+// amount + "-" + 12 hex chars) stays ≤45 chars: 12 + 8 + up-to-9 + 1 + 12 ≈ 42.
+func ensureRefundKey(ctx context.Context, refundID, paymentID string, amount int64, storedKey string) (string, error) {
+	if storedKey != "" {
+		return storedKey, nil
+	}
+	key := paymentID + "-refund-" + strconv.FormatInt(amount, 10) + "-" + randomHexSuffix(6)
+	tag, err := db.Conn.Exec(ctx, `
+		UPDATE refunds SET idempotency_key = $1
+		WHERE id = $2 AND idempotency_key IS NULL
+	`, key, refundID)
+	if err != nil {
+		return "", fmt.Errorf("failed to persist refund idempotency key for %s: %w", refundID, err)
+	}
+	if tag.RowsAffected() == 0 {
+		var existing string
+		if err := db.Conn.QueryRow(ctx, `SELECT idempotency_key FROM refunds WHERE id = $1`, refundID).Scan(&existing); err != nil {
+			return "", fmt.Errorf("failed to re-read persisted refund idempotency key for %s: %w", refundID, err)
+		}
+		return existing, nil
+	}
+	return key, nil
+}
+
 // processManualPaymentGroup retries one payment's stale manual pending refunds
 // under the SAME per-payment advisory lock the manual RefundPayment handler
 // holds across its guard read — so a re-issued refund can never double-spend
@@ -1239,10 +1326,25 @@ func processManualPaymentGroup(ctx context.Context, paymentID string, rows []man
 			continue
 		}
 
+		// Route the stored key through ensureRefundKey: a legacy row with a
+		// NULL idempotency_key must NOT reach Square with "" (a 400
+		// INVALID_REQUEST_ERROR, classified ambiguous → stuck forever). The
+		// helper persists a generated fallback to the row first, so every retry
+		// reuses the SAME key — a lost-response retry can never issue a second
+		// refund (Square dedups same-key retries).
+		idemKey, keyErr := ensureRefundKey(ctx, pr.ID, pr.PaymentID, amountCents, pr.IdempotencyKey)
+		if keyErr != nil {
+			// The key could not be persisted — Square must not be called with an
+			// empty/unknown key. Leave the row pending for the next sweep (never
+			// mark failed on an unknown state); the 23h age guard above will
+			// eventually reconcile it.
+			log.Printf("Failed to ensure refund key for manual refund %s before re-issue: %v", pr.ID, keyErr)
+			continue
+		}
 		sqResult, sqErr := SquareClient.RefundPayment(ctx, square.RefundPaymentReq{
 			PaymentID:      pr.SquarePaymentID,
 			Amount:         amountCents,
-			IdempotencyKey: pr.IdempotencyKey,
+			IdempotencyKey: idemKey,
 			Reason:         pr.Reason,
 		})
 		switch {
diff --git a/backend/handlers/payments/refunds_test.go b/backend/handlers/payments/refunds_test.go
index 2c0e501..bda9ceb 100644
--- a/backend/handlers/payments/refunds_test.go
+++ b/backend/handlers/payments/refunds_test.go
@@ -40,6 +40,21 @@ func (c *countingRefundClient) refundCalls() []square.RefundPaymentReq {
 	return append([]square.RefundPaymentReq(nil), c.calls...)
 }
 
+// assertAggKey asserts the charge-level aggregated refund key carries the
+// "-square-agg-" segment and stays within Square's 45-char idempotency-key
+// limit. The prefix is the verbatim chargeID for short square_payment_ids and a
+// hashed fixed-width form for long ones (see chargeAggKey), so only the shared
+// segment + length bound are asserted here.
+func assertAggKey(t *testing.T, got, chargeID string) {
+	t.Helper()
+	if !strings.Contains(got, "-square-agg-") {
+		t.Errorf("expected charge-level idempotency key containing \"-square-agg-\", got %q", got)
+	}
+	if len(got) > 45 {
+		t.Errorf("expected idempotency key within Square's 45-char limit, got %d chars: %q", len(got), got)
+	}
+}
+
 // ambiguousRefundClient simulates a transport-level failure (no sentinel
 // error) — Square may or may not have processed the refund.
 type ambiguousRefundClient struct {
@@ -1002,9 +1017,7 @@ func TestProcessCancellationRefund_SplitPayment_SingleSquareRefund(t *testing.T)
 	if calls[0].Amount != 5000 {
 		t.Errorf("expected aggregated Square refund of 5000 pence, got %d", calls[0].Amount)
 	}
-	if calls[0].IdempotencyKey != sameSquareID+"-square-agg" {
-		t.Errorf("expected charge-level idempotency key %q, got %q", sameSquareID+"-square-agg", calls[0].IdempotencyKey)
-	}
+	assertAggKey(t, calls[0].IdempotencyKey, sameSquareID)
 
 	// Both refund records must be completed and share ONE square_refund_id —
 	// the old bug left one record completed with square_refund_id NULL (no
@@ -1353,9 +1366,7 @@ func TestProcessPendingSquareRefunds_SameChargePending_IssuesRefund(t *testing.T
 	if calls[0].Amount != 2500 {
 		t.Errorf("expected Square refund of 2500 pence (the residual), got %d", calls[0].Amount)
 	}
-	if calls[0].IdempotencyKey != sameSquareID+"-square-agg" {
-		t.Errorf("expected charge-level idempotency key %q, got %q", sameSquareID+"-square-agg", calls[0].IdempotencyKey)
-	}
+	assertAggKey(t, calls[0].IdempotencyKey, sameSquareID)
 
 	// The balance pending refund must be completed WITH a square_refund_id —
 	// money moved, NOT left NULL.
@@ -1858,9 +1869,7 @@ func TestProcessPendingSquareRefunds_SplitCharge_OneAggregateRefund(t *testing.T
 	if calls[0].Amount != 5000 {
 		t.Errorf("expected aggregated amount 5000 pence (25+25), got %d", calls[0].Amount)
 	}
-	if calls[0].IdempotencyKey != sameSquareID+"-square-agg" {
-		t.Errorf("expected idempotency key %q, got %q", sameSquareID+"-square-agg", calls[0].IdempotencyKey)
-	}
+	assertAggKey(t, calls[0].IdempotencyKey, sameSquareID)
 
 	// Both rows completed with the same square_refund_id, amounts preserved.
 	rows, err := db.Conn.Query(freshCtx, `
@@ -2361,9 +2370,7 @@ func TestProcessPendingSquareRefunds_PartialManualThenCancel_IssuesResidual(t *t
 	if calls[0].Amount != 7000 {
 		t.Errorf("expected aggregated residual of 7000 pence (£70), got %d", calls[0].Amount)
 	}
-	if calls[0].IdempotencyKey != sameSquareID+"-square-agg" {
-		t.Errorf("expected idempotency key %q, got %q", sameSquareID+"-square-agg", calls[0].IdempotencyKey)
-	}
+	assertAggKey(t, calls[0].IdempotencyKey, sameSquareID)
 
 	// Two pending rows created for the residual: deposit £20 + balance £50.
 	rows, err := db.Conn.Query(freshCtx, `
@@ -3250,3 +3257,364 @@ func TestSweepPendingSquareRefunds_ManualWithSquareRefundID_NoMatch_Failed(t *te
 		t.Errorf("expected at least 1 admin_notification with reason 'refund_failed', got %d", notifCount)
 	}
 }
+
+// =============================================================================
+// Bug fix — set-stable charge-level idempotency key (refunds.go processChargeGroup)
+// =============================================================================
+
+// TestChargeAggKey_AlwaysFitsAndDeterministic locks the charge-level idempotency
+// key builder: the key must stay within Square's 45-char limit for ANY
+// square_payment_id length (the verbatim form only fits chargeIDs ≤21 chars;
+// longer ones fall back to a hashed fixed-width prefix — see chargeAggKey), be
+// deterministic for a given (charge, set), and differ across charges/sets.
+func TestChargeAggKey_AlwaysFitsAndDeterministic(t *testing.T) {
+	ids := []string{"id1", "id2"}
+	longCharge := "sqp_real_square_payment_id_that_is_quite_long"
+
+	// Short chargeID → verbatim form.
+	shortKey := chargeAggKey("sqp_short", ids)
+	if !strings.HasPrefix(shortKey, "sqp_short-square-agg-") {
+		t.Errorf("expected verbatim prefix for a short chargeID, got %q", shortKey)
+	}
+	if len(shortKey) > 45 {
+		t.Errorf("expected short-charge key within 45 chars, got %d: %q", len(shortKey), shortKey)
+	}
+
+	// Long chargeID → still ≤45, never the verbatim form.
+	longKey := chargeAggKey(longCharge, ids)
+	if strings.HasPrefix(longKey, longCharge+"-square-agg-") {
+		t.Errorf("expected long chargeID NOT embedded verbatim, got %q", longKey)
+	}
+	if len(longKey) > 45 {
+		t.Errorf("expected long-charge key within 45 chars, got %d: %q", len(longKey), longKey)
+	}
+
+	// Same (charge, set) → same key; different set → different key; different
+	// charge → different key (no cross-charge dedup collision).
+	if chargeAggKey("sqp_short", ids) != shortKey {
+		t.Error("expected the same (charge, set) to produce the same key")
+	}
+	if chargeAggKey("sqp_short", []string{"id1"}) == shortKey {
+		t.Error("expected a changed set to produce a different key")
+	}
+	if chargeAggKey(longCharge, []string{"id1"}) == longKey {
+		t.Error("expected a changed set to produce a different key (long charge)")
+	}
+	if chargeAggKey("sqp_other", ids) == shortKey {
+		t.Error("expected a different charge to produce a different key")
+	}
+	if chargeAggKey(longCharge+"x", ids) == longKey {
+		t.Error("expected a different long charge to produce a different key")
+	}
+}
+
+// TestProcessChargeGroup_ChangedPendingSet_NewKey verifies the idempotency-key
+// bug fix: the charge-level aggregated refund key is derived from the SET of
+// pending row ids. A SAME-set retry (the CRITICAL-log path where the post-refund
+// DB UPDATE failed, leaving the rows 'pending') reuses the SAME key → Square's
+// dedup returns the ORIGINAL refund. When a NEW cancellation refund row joins
+// the group, the set changes → a NEW key → Square issues a fresh refund covering
+// the new total, so the new row is NEVER marked 'completed' against the old
+// smaller refund (the under-refund / lost-money bookkeeping bug).
+func TestProcessChargeGroup_ChangedPendingSet_NewKey(t *testing.T) {
+	ctx, tx := testutils.SetupTestTx(t)
+
+	userID, err := fixtures.CreateTestUser(tx)
+	if err != nil {
+		t.Fatalf("failed to create user: %v", err)
+	}
+	serviceID, err := fixtures.CreateTestService(tx)
+	if err != nil {
+		t.Fatalf("failed to create service: %v", err)
+	}
+	bookingID, err := fixtures.CreateTestBookingAtTime(tx, userID, serviceID,
+		time.Date(2099, 12, 31, 10, 0, 0, 0, time.UTC))
+	if err != nil {
+		t.Fatalf("failed to create booking: %v", err)
+	}
+
+	// Split payment records sharing one square_payment_id (deposit + balance).
+	depositID, err := fixtures.CreateTestPayment(tx, bookingID, 25.00, "online_square", "deposit", "completed")
+	if err != nil {
+		t.Fatalf("failed to create deposit payment: %v", err)
+	}
+	balanceID, err := fixtures.CreateTestPayment(tx, bookingID, 25.00, "online_square", "balance", "completed")
+	if err != nil {
+		t.Fatalf("failed to create balance payment: %v", err)
+	}
+	chargeID := "sqp_changed_set"
+	if _, err := tx.Exec(ctx, "UPDATE payments SET square_payment_id = $1 WHERE id IN ($2, $3)", chargeID, depositID, balanceID); err != nil {
+		t.Fatalf("failed to set square_payment_id: %v", err)
+	}
+
+	insertRefund := func(pid string, amount float64, key string) string {
+		t.Helper()
+		var id string
+		if err := tx.QueryRow(ctx, `
+			INSERT INTO refunds (payment_id, booking_id, amount, status, reason, idempotency_key, origin, created_at)
+			VALUES ($1, $2, $3, 'pending', 'client_cancelled', $4, 'cancellation', NOW())
+			RETURNING id
+		`, pid, bookingID, amount, key).Scan(&id); err != nil {
+			t.Fatalf("failed to insert pending refund: %v", err)
+		}
+		return id
+	}
+	row1 := insertRefund(depositID, 25, depositID+"-square-2500")
+	row2 := insertRefund(balanceID, 25, balanceID+"-square-2500")
+
+	pgxTx := db.TxFromContext(ctx)
+	if pgxTx == nil {
+		t.Fatal("no transaction in context")
+	}
+	if err := pgxTx.Commit(ctx); err != nil {
+		t.Fatalf("failed to commit test tx: %v", err)
+	}
+	freshCtx := context.Background()
+	t.Cleanup(func() {
+		_, _ = db.Conn.Exec(context.Background(), `DELETE FROM admin_notifications WHERE booking_id = $1`, bookingID)
+		_, _ = db.Conn.Exec(context.Background(), `DELETE FROM refunds WHERE booking_id = $1`, bookingID)
+		_, _ = db.Conn.Exec(context.Background(), `DELETE FROM payments WHERE booking_id = $1`, bookingID)
+		_, _ = db.Conn.Exec(context.Background(), `DELETE FROM bookings WHERE id = $1`, bookingID)
+		_, _ = db.Conn.Exec(context.Background(), `DELETE FROM services WHERE id = $1`, serviceID)
+		_, _ = db.Conn.Exec(context.Background(), `DELETE FROM users WHERE id = $1`, userID)
+	})
+
+	origClient := SquareClient
+	mock := square.NewDevClient().(*square.MockClient)
+	counting := &countingRefundClient{SquareClient: mock}
+	SquareClient = counting
+	defer func() { SquareClient = origClient }()
+
+	runGroup := func() string {
+		t.Helper()
+		if _, err := processChargeGroup(freshCtx, chargeID, fetchPendingChargeRows(freshCtx, chargeID), "client_cancelled"); err != nil {
+			t.Fatalf("processChargeGroup failed: %v", err)
+		}
+		calls := counting.refundCalls()
+		return calls[len(calls)-1].IdempotencyKey
+	}
+
+	// Run 1: the two-row set → key K1 covering £50.
+	key1 := runGroup()
+	assertAggKey(t, key1, chargeID)
+	calls := counting.refundCalls()
+	if len(calls) != 1 || calls[0].Amount != 5000 {
+		t.Fatalf("expected exactly 1 Square refund of 5000 pence, got %d call(s) (amount %d)", len(calls), calls[0].Amount)
+	}
+	var oldRefundID string
+	if err := db.Conn.QueryRow(freshCtx, `SELECT square_refund_id FROM refunds WHERE id = $1`, row1).Scan(&oldRefundID); err != nil {
+		t.Fatalf("failed to read refund id: %v", err)
+	}
+
+	// SAME-set retry (the CRITICAL-log path: Square committed but the DB UPDATE
+	// failed, rows still 'pending'): reset to pending and re-run → the SAME key
+	// K1, and Square's dedup returns the ORIGINAL refund (no second refund).
+	_, err = db.Conn.Exec(freshCtx, `UPDATE refunds SET status = 'pending', square_refund_id = NULL WHERE id = ANY($1)`, []string{row1, row2})
+	if err != nil {
+		t.Fatalf("failed to reset refund rows pending: %v", err)
+	}
+	keyRetry := runGroup()
+	if keyRetry != key1 {
+		t.Errorf("expected SAME-set retry to reuse key %q, got %q", key1, keyRetry)
+	}
+	if n := mock.RefundKeyCount(); n != 1 {
+		t.Errorf("expected Square to have issued exactly 1 refund after the same-set retry (dedup), got %d", n)
+	}
+
+	// A NEW cancellation refund row joins the group (e.g. an admin re-cancels
+	// the residual after run 1's DB UPDATE failed) → the set changes.
+	var row3 string
+	err = db.Conn.QueryRow(freshCtx, `
+		INSERT INTO refunds (payment_id, booking_id, amount, status, reason, idempotency_key, origin, created_at)
+		VALUES ($1, $2, 25, 'pending', 'client_cancelled', $3, 'cancellation', NOW())
+		RETURNING id
+	`, balanceID, bookingID, balanceID+"-square-2500-2").Scan(&row3)
+	if err != nil {
+		t.Fatalf("failed to insert third pending refund: %v", err)
+	}
+
+	// Reset the original two rows to pending again (they completed in run 2) so
+	// the group is once more the FULL set {row1, row2, row3}.
+	_, err = db.Conn.Exec(freshCtx, `UPDATE refunds SET status = 'pending', square_refund_id = NULL WHERE id = ANY($1)`, []string{row1, row2})
+	if err != nil {
+		t.Fatalf("failed to reset refund rows pending: %v", err)
+	}
+
+	// Run 3: the three-row set → key K2 ≠ K1 → a NEW Square refund for the full
+	// £75 — never deduped against the old £50 refund.
+	key2 := runGroup()
+	if key2 == key1 {
+		t.Errorf("expected CHANGED set to yield a DIFFERENT key, got the same %q", key2)
+	}
+	calls = counting.refundCalls()
+	if len(calls) != 3 {
+		t.Fatalf("expected 3 total Square refund calls, got %d", len(calls))
+	}
+	if calls[2].Amount != 7500 {
+		t.Errorf("expected the changed-set refund of 7500 pence (£25+£25+£25), got %d", calls[2].Amount)
+	}
+	if n := mock.RefundKeyCount(); n != 2 {
+		t.Errorf("expected Square to have issued exactly 2 distinct refunds, got %d", n)
+	}
+
+	// All three rows resolve to completed; the new row is completed against the
+	// NEW refund, never the old smaller one.
+	var newRefundID string
+	var newStatus string
+	if err := db.Conn.QueryRow(freshCtx, `SELECT status, square_refund_id FROM refunds WHERE id = $1`, row3).Scan(&newStatus, &newRefundID); err != nil {
+		t.Fatalf("failed to query third refund: %v", err)
+	}
+	if newStatus != "completed" {
+		t.Errorf("expected third refund status 'completed', got %q", newStatus)
+	}
+	if newRefundID == "" || newRefundID == oldRefundID {
+		t.Errorf("expected third refund completed against the NEW refund (not the old %q), got %q", oldRefundID, newRefundID)
+	}
+	var completedCount int
+	if err := db.Conn.QueryRow(freshCtx, `SELECT COUNT(*) FROM refunds WHERE id = ANY($1) AND status = 'completed'`, []string{row1, row2, row3}).Scan(&completedCount); err != nil {
+		t.Fatalf("failed to count completed refunds: %v", err)
+	}
+	if completedCount != 3 {
+		t.Errorf("expected all 3 refund rows completed, got %d", completedCount)
+	}
+}
+
+// =============================================================================
+// Bug fix — NULL-key manual refund rows are keyed ONCE before re-issue
+// =============================================================================
+
+// TestSweepManualRetry_NullKey_SingleSquareRefund verifies the idempotency-key
+// bug fix for the manual-refund sweep loop: a manual refund row with a NULL
+// idempotency_key (legacy) gets a fallback key PERSISTED to the row before the
+// Square call, so a retry after the CRITICAL-log path (Square committed, DB
+// UPDATE to 'completed' failed, row still 'pending') reuses the SAME key and
+// Square issues exactly ONE refund (the mock dedups same-key retries). The old
+// code generated a fresh random suffix per resume → a second Square refund.
+func TestSweepManualRetry_NullKey_SingleSquareRefund(t *testing.T) {
+	ctx, tx := testutils.SetupTestTx(t)
+
+	userID, err := fixtures.CreateTestUser(tx)
+	if err != nil {
+		t.Fatalf("failed to create user: %v", err)
+	}
+	serviceID, err := fixtures.CreateTestService(tx)
+	if err != nil {
+		t.Fatalf("failed to create service: %v", err)
+	}
+	bookingID, err := fixtures.CreateTestBookingAtTime(tx, userID, serviceID,
+		time.Date(2099, 12, 31, 10, 0, 0, 0, time.UTC))
+	if err != nil {
+		t.Fatalf("failed to create booking: %v", err)
+	}
+
+	paymentID, err := fixtures.CreateTestPayment(tx, bookingID, 50.00, "online_square", "full", "completed")
+	if err != nil {
+		t.Fatalf("failed to create card payment: %v", err)
+	}
+	if _, err := tx.Exec(ctx, "UPDATE payments SET square_payment_id = 'sqp_null_key_manual' WHERE id = $1", paymentID); err != nil {
+		t.Fatalf("failed to set square_payment_id: %v", err)
+	}
+
+	// A legacy pending manual refund row with NO idempotency_key column value →
+	// NULL (the root source of the bug: the next sweep's refundResumeKey would
+	// generate a fresh random suffix per resume).
+	var refundID string
+	err = tx.QueryRow(ctx, `
+		INSERT INTO refunds (payment_id, booking_id, amount, status, reason, origin, created_at)
+		VALUES ($1, $2, 50, 'pending', 'customer request', 'manual', NOW())
+		RETURNING id
+	`, paymentID, bookingID).Scan(&refundID)
+	if err != nil {
+		t.Fatalf("failed to insert legacy NULL-key manual pending refund: %v", err)
+	}
+
+	pgxTx := db.TxFromContext(ctx)
+	if pgxTx == nil {
+		t.Fatal("no transaction in context")
+	}
+	if err := pgxTx.Commit(ctx); err != nil {
+		t.Fatalf("failed to commit test tx: %v", err)
+	}
+	freshCtx := context.Background()
+	t.Cleanup(func() {
+		_, _ = db.Conn.Exec(context.Background(), `DELETE FROM admin_notifications WHERE booking_id = $1`, bookingID)
+		_, _ = db.Conn.Exec(context.Background(), `DELETE FROM refunds WHERE id = $1`, refundID)
+		_, _ = db.Conn.Exec(context.Background(), `DELETE FROM payments WHERE id = $1`, paymentID)
+		_, _ = db.Conn.Exec(context.Background(), `DELETE FROM bookings WHERE id = $1`, bookingID)
+		_, _ = db.Conn.Exec(context.Background(), `DELETE FROM services WHERE id = $1`, serviceID)
+		_, _ = db.Conn.Exec(context.Background(), `DELETE FROM users WHERE id = $1`, userID)
+	})
+
+	origClient := SquareClient
+	mock := square.NewDevClient().(*square.MockClient)
+	counting := &countingRefundClient{SquareClient: mock}
+	SquareClient = counting
+	defer func() { SquareClient = origClient }()
+
+	runSweep := func() {
+		t.Helper()
+		n, err := processManualPaymentGroup(freshCtx, paymentID, []manualPendingRow{{ID: refundID}})
+		if err != nil {
+			t.Fatalf("processManualPaymentGroup failed: %v", err)
+		}
+		if n != 1 {
+			t.Fatalf("expected 1 manual refund processed, got %d", n)
+		}
+	}
+
+	// Run 1: ensureRefundKey generates a fallback key, PERSISTS it to the row,
+	// and Square issues refund R1 under that key.
+	runSweep()
+	calls := counting.refundCalls()
+	if len(calls) != 1 {
+		t.Fatalf("expected exactly 1 Square refund call, got %d", len(calls))
+	}
+	key1 := calls[0].IdempotencyKey
+	if key1 == "" {
+		t.Fatal("expected a NON-EMPTY idempotency key (Square rejects empty keys with 400 INVALID_REQUEST_ERROR)")
+	}
+	if len(key1) > 45 {
+		t.Errorf("expected key within Square's 45-char limit, got %d chars: %q", len(key1), key1)
+	}
+	var storedKey string
+	if err := db.Conn.QueryRow(freshCtx, `SELECT idempotency_key FROM refunds WHERE id = $1`, refundID).Scan(&storedKey); err != nil {
+		t.Fatalf("failed to read stored key: %v", err)
+	}
+	if storedKey != key1 {
+		t.Errorf("expected the generated key %q persisted to the row, got %q", key1, storedKey)
+	}
+
+	// Simulate the CRITICAL-log path: the Square refund committed but the DB
+	// UPDATE to 'completed' failed → row back to 'pending', square_refund_id NULL.
+	if _, err := db.Conn.Exec(freshCtx, `UPDATE refunds SET status = 'pending', square_refund_id = NULL WHERE id = $1`, refundID); err != nil {
+		t.Fatalf("failed to reset refund row pending: %v", err)
+	}
+
+	// Run 2: the retry reuses the PERSISTED key → Square dedups → the SAME
+	// single refund is returned; NO second refund is ever issued.
+	runSweep()
+	calls = counting.refundCalls()
+	if len(calls) != 2 {
+		t.Fatalf("expected 2 Square refund calls across both runs, got %d", len(calls))
+	}
+	if calls[1].IdempotencyKey != key1 {
+		t.Errorf("expected the retry to reuse the persisted key %q, got %q", key1, calls[1].IdempotencyKey)
+	}
+	if n := mock.RefundKeyCount(); n != 1 {
+		t.Errorf("expected exactly ONE distinct Square refund issued (dedup), got %d", n)
+	}
+	var keyAfter string
+	if err := db.Conn.QueryRow(freshCtx, `SELECT idempotency_key FROM refunds WHERE id = $1`, refundID).Scan(&keyAfter); err != nil {
+		t.Fatalf("failed to read stored key: %v", err)
+	}
+	if keyAfter != key1 {
+		t.Errorf("expected the row's key unchanged across both runs (%q), got %q", key1, keyAfter)
+	}
+	var status string
+	if err := db.Conn.QueryRow(freshCtx, `SELECT status FROM refunds WHERE id = $1`, refundID).Scan(&status); err != nil {
+		t.Fatalf("failed to read refund status: %v", err)
+	}
+	if status != "completed" {
+		t.Errorf("expected refund status 'completed', got %q", status)
+	}
+}
diff --git a/backend/handlers/payments/service.go b/backend/handlers/payments/service.go
index c46c1bf..3ed0204 100644
--- a/backend/handlers/payments/service.go
+++ b/backend/handlers/payments/service.go
@@ -749,14 +749,31 @@ func (s *PaymentService) EnsureSquareCustomerForSavedCard(ctx context.Context, s
 // the DB and, on a first-save flow, re-run CreateCustomer).
 func (s *PaymentService) SaveCardForUser(ctx context.Context, userID, squareCustomerID, squareCardID, brand, last4 string, expMonth, expYear int, fingerprint string) (string, error) {
 	var id string
+	// ON CONFLICT (user_id, square_card_id) DO NOTHING: resolveChargeSource
+	// runs CreateCardOnFile (deterministic sha256 key) + SaveCardForUser on a
+	// same-key retry of a save_card=true charge. Square returns the SAME ccof:
+	// id on the retry, so a plain INSERT would violate the per-user UNIQUE
+	// constraint (N-8). Upsert instead so the retry returns the existing row;
+	// a distinct card id is a brand-new row, never a mutation of another
+	// user's card.
 	err := db.Conn.QueryRow(ctx, `
 		INSERT INTO user_saved_cards (
 			user_id, square_card_id, square_customer_id, brand, last_4, exp_month, exp_year, fingerprint, is_default, created_at
 		) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, false, NOW())
+		ON CONFLICT (user_id, square_card_id) DO NOTHING
 		RETURNING id
 	`, userID, squareCardID, squareCustomerID, brand, last4, expMonth, expYear, fingerprint).Scan(&id)
 
 	if err != nil {
+		if errors.Is(err, pgx.ErrNoRows) {
+			if err := db.Conn.QueryRow(ctx, `
+				SELECT id FROM user_saved_cards
+				WHERE user_id = $1 AND square_card_id = $2 AND deleted_at IS NULL
+			`, userID, squareCardID).Scan(&id); err != nil {
+				return "", err
+			}
+			return id, nil
+		}
 		return "", err
 	}
 	return id, nil
@@ -790,4 +807,15 @@ func (s *PaymentService) GetCardByIDQuerier(ctx context.Context, q db.Querier, c
 // customer. The durable record remains the user_saved_cards row.
 var squareCustomerCache sync.Map
 
+// InvalidateSquareCustomerCache drops a user's cached Square customer id,
+// e.g. after GDPR erasure deletes the customer at Square and NULLs the DB
+// columns. Without it, the process-local cache would keep serving the erased
+// user's stale customer id on a later save-card flow — reusing a deleted
+// Square customer id (fail-closed at Square, but incorrect) and letting the
+// erased identity resurface in memory. Called by the erasure handlers
+// (handlers/user, handlers/scheduling) after the local anonymization commits.
+func InvalidateSquareCustomerCache(userID string) {
+	squareCustomerCache.Delete(userID)
+}
+
 var SquareClient square.SquareClient
diff --git a/backend/handlers/payments/sweep.go b/backend/handlers/payments/sweep.go
index 595c348..4d65c27 100644
--- a/backend/handlers/payments/sweep.go
+++ b/backend/handlers/payments/sweep.go
@@ -284,20 +284,41 @@ func reconcileStalePaymentAtSquare(ctx context.Context, table, squarePaymentID s
 }
 
 // squarePaymentErrorIsNotFound reports whether a GetPayment error proves the
-// payment does not exist at Square. The structured Square error code is the
-// primary check (square.ErrorCode); the message fallback also covers the dev
-// mock (a plain "payment not found" error) and a non-JSON 404 response.
+// payment does not exist at Square. The structured not-found check is primary
+// (square.IsNotFound: NOT_FOUND code / HTTP 404 status / plain "HTTP 404" body);
+// the message fallback covers only the dev mock's plain "payment not found"
+// error, which carries no structured code.
 func squarePaymentErrorIsNotFound(err error) bool {
 	if err == nil {
 		return false
 	}
-	if square.ErrorCode(err) == "NOT_FOUND" {
+	if square.IsNotFound(err) {
 		return true
 	}
+	// Any other structured Square error code is authoritative — never
+	// substring-match its message.
+	if square.ErrorCode(err) != "" {
+		return false
+	}
 	msg := strings.ToUpper(err.Error())
 	return strings.Contains(msg, "NOT_FOUND") ||
-		strings.Contains(msg, "NOT FOUND") ||
-		strings.Contains(msg, "HTTP 404")
+		strings.Contains(msg, "NOT FOUND")
+}
+
+// squareHasCode reports whether err carries a structured Square error code
+// equal to any of codes. Errors without a structured code (the dev mock's
+// plain errors) match nothing.
+func squareHasCode(err error, codes ...string) bool {
+	if err == nil {
+		return false
+	}
+	code := square.ErrorCode(err)
+	for _, c := range codes {
+		if code == c {
+			return true
+		}
+	}
+	return false
 }
 
 // staleTerminalCheckoutAge is how old a still-pending terminal checkout must
@@ -538,13 +559,28 @@ func markTerminalCheckoutRowFailed(ctx context.Context, r staleTerminalCheckoutR
 // checkout can never complete. Square's HTTP client returns ErrCheckoutPending
 // for a still-live checkout and surfaces a definitively CANCELED status as a
 // "square: checkout <id> is CANCELED (not COMPLETED)" error; an expired
-// checkout returns a NOT_FOUND API error (the mock uses "checkout not found").
-// Any other error (timeout, 5xx) leaves the money state ambiguous, so the
-// checkout must stay in flight.
+// checkout returns a structured NOT_FOUND API error (the mock uses a plain
+// "checkout not found"). Any other error (timeout, 5xx) leaves the money state
+// ambiguous, so the checkout must stay in flight.
+//
+// Structured codes are authoritative when present: NOT_FOUND (via
+// square.IsNotFound) and an explicit CANCELED / CANCEL_REQUESTED code classify
+// as terminal. The formatted-message match applies only to errors that carry
+// no structured code — the dev mock's plain errors and the real client's
+// client-side "is <status> (not COMPLETED)" error, which it synthesizes
+// without a squareAPIError.
 func isTerminalCheckoutError(err error) bool {
 	if err == nil || errors.Is(err, square.ErrCheckoutPending) {
 		return false
 	}
+	if square.IsNotFound(err) || squareHasCode(err, "CANCELED", "CANCEL_REQUESTED") {
+		return true
+	}
+	// Any other structured Square error code is authoritative — never
+	// substring-match its message.
+	if square.ErrorCode(err) != "" {
+		return false
+	}
 	msg := strings.ToUpper(err.Error())
 	return strings.Contains(msg, "CANCELED") ||
 		strings.Contains(msg, "CANCEL_REQUESTED") ||
@@ -555,15 +591,29 @@ func isTerminalCheckoutError(err error) bool {
 // isCheckoutDefinitivelyDead reports whether a GetCheckout error PROVES the
 // checkout can never complete — the condition under which a till sale's funded
 // gift card may be clawed back. It is stricter than isTerminalCheckoutError:
-// a message that only reports CANCEL_REQUESTED (Square does not promise
-// non-completion in that state) is NOT definitive proof, so the charge may
-// still land and the funding must stay put. Only an explicit CANCELED or
-// NOT_FOUND status is definitive; a CANCEL_REQUESTED message counts only when
-// it ALSO carries CANCELED.
+// a CANCEL_REQUESTED-only classification (Square does not promise non-completion
+// in that state) is NOT definitive proof, so the charge may still land and the
+// funding must stay put. Only an explicit CANCELED or NOT_FOUND status is
+// definitive; a CANCEL_REQUESTED message counts only when it ALSO carries
+// CANCELED.
+//
+// Structured codes are authoritative when present: CANCELED and NOT_FOUND are
+// definitive, a bare CANCEL_REQUESTED code is not. The formatted-message match
+// applies only to errors that carry no structured code (dev mock plain errors,
+// and the real client's client-side "is <status> (not COMPLETED)" error).
 func isCheckoutDefinitivelyDead(err error) bool {
 	if err == nil || errors.Is(err, square.ErrCheckoutPending) {
 		return false
 	}
+	if code := square.ErrorCode(err); code != "" {
+		return code == "CANCELED" || code == "NOT_FOUND"
+	}
+	// Non-structured errors: an HTTP 404 / plain 404 body is definitive
+	// (IsNotFound); the message match covers the client-side CANCELED status
+	// error and the mock's plain not-found errors.
+	if square.IsNotFound(err) {
+		return true
+	}
 	msg := strings.ToUpper(err.Error())
 	if !strings.Contains(msg, "CANCELED") && !strings.Contains(msg, "NOT_FOUND") && !strings.Contains(msg, "NOT FOUND") {
 		return false
diff --git a/backend/handlers/payments/till.go b/backend/handlers/payments/till.go
index da5b752..d6939b1 100644
--- a/backend/handlers/payments/till.go
+++ b/backend/handlers/payments/till.go
@@ -73,13 +73,24 @@ var definitivePaymentDeclineCodes = []string{
 
 // isDefinitiveChargeFailure reports whether a Square CreatePayment error is a
 // definitive business rejection (declined/expired) rather than an ambiguous
-// transport/server error. The real HTTP client formats declines as
-// "square: POST /v2/payments: [CATEGORY/CODE] ...", so the code is matched
-// against the uppercased error message.
+// transport/server error. The real HTTP client surfaces declines as a
+// structured squareAPIError carrying the Square error Code (and Category), so
+// the classification matches those EXACTLY against definitivePaymentDeclineCodes
+// — a Square message-wording change can never silently flip the
+// definitive↔retryable decision that drives the gift-card funding clawback.
+// Only errors that carry NO structured code (the dev mock's plain errors, or a
+// non-JSON failure body) fall back to the legacy formatted-message match
+// ("square: POST /v2/payments: [CATEGORY/CODE] ..."), which is the only signal
+// available for them.
 func isDefinitiveChargeFailure(err error) bool {
 	if err == nil {
 		return false
 	}
+	// The formatted message carries both [CATEGORY/CODE] and the legacy check
+	// matched either, so compare the Code AND the Category exactly.
+	if code := square.ErrorCode(err); code != "" {
+		return declineCodeListContains(code) || declineCodeListContains(square.ErrorCategory(err))
+	}
 	msg := strings.ToUpper(err.Error())
 	for _, code := range definitivePaymentDeclineCodes {
 		if strings.Contains(msg, code) {
@@ -89,6 +100,17 @@ func isDefinitiveChargeFailure(err error) bool {
 	return false
 }
 
+// declineCodeListContains reports whether s is exactly one of the definitive
+// payment decline codes.
+func declineCodeListContains(s string) bool {
+	for _, code := range definitivePaymentDeclineCodes {
+		if s == code {
+			return true
+		}
+	}
+	return false
+}
+
 // errTillSaleNotPending: the claim-first gating UPDATE matched zero rows, so
 // the sale is no longer 'pending' and the gift card must be left untouched.
 var errTillSaleNotPending = errors.New("till sale is not pending")
diff --git a/backend/handlers/scheduling/time-blockers.go b/backend/handlers/scheduling/time-blockers.go
index 025c9cf..33e3247 100644
--- a/backend/handlers/scheduling/time-blockers.go
+++ b/backend/handlers/scheduling/time-blockers.go
@@ -416,6 +416,10 @@ func CleanupOldReservations(ctx context.Context) (int, error) {
 // Financial records (bookings, payments) remain intact — only PII is scrubbed.
 // Active/pending bookings are excluded so the salon can still contact the guest.
 func AnonymizeStaleGuestAccounts(ctx context.Context) (int, error) {
+	// Stale guests whose Square customers are deleted below — their
+	// process-local cache entries must be invalidated after the anonymization.
+	var staleGuestUserIDs []string
+
 	// Best-effort: disable stale-guests' saved cards at Square BEFORE the SQL
 	// below NULLs square_card_id, so those cards can't keep accepting ccof:
 	// charges after anonymization (GDPR erasure completeness). A Square failure
@@ -431,7 +435,7 @@ func AnonymizeStaleGuestAccounts(ctx context.Context) (int, error) {
 		// local anonymization must never be blocked by Square. Rows are
 		// selected with the same stale-guest predicate the users UPDATE uses.
 		rows, err := db.Conn.Query(ctx, `
-			SELECT usc.square_card_id, usc.square_customer_id
+			SELECT usc.user_id, usc.square_card_id, usc.square_customer_id
 			FROM user_saved_cards usc
 			JOIN users u ON u.id = usc.user_id
 			WHERE u.account_role = 'guest'
@@ -449,12 +453,17 @@ func AnonymizeStaleGuestAccounts(ctx context.Context) (int, error) {
 			// customer) are skipped.
 			customerSeen := map[string]bool{}
 			var customerIDs []string
+			userSeen := map[string]bool{}
 			for rows.Next() {
-				var cardID, customerID sql.NullString
-				if err := rows.Scan(&cardID, &customerID); err != nil {
+				var userID, cardID, customerID sql.NullString
+				if err := rows.Scan(&userID, &cardID, &customerID); err != nil {
 					log.Printf("Warning: Failed to scan stale-guest saved card: %v", err)
 					continue
 				}
+				if userID.Valid && userID.String != "" && !userSeen[userID.String] {
+					userSeen[userID.String] = true
+					staleGuestUserIDs = append(staleGuestUserIDs, userID.String)
+				}
 				if cardID.Valid && cardID.String != "" {
 					cardIDs = append(cardIDs, cardID.String)
 				}
@@ -607,7 +616,18 @@ func AnonymizeStaleGuestAccounts(ctx context.Context) (int, error) {
 	}
 	totalRows += int(tag.RowsAffected())
 
-	return totalRows, tx.Commit(ctx)
+	if err := tx.Commit(ctx); err != nil {
+		return 0, err
+	}
+
+	// The guests' Square customer ids were deleted above and the DB columns are
+	// now NULLed — drop their process-local cache entries so an erased guest's
+	// stale Square customer id cannot resurface on a later save-card flow.
+	for _, uid := range staleGuestUserIDs {
+		payments.InvalidateSquareCustomerCache(uid)
+	}
+
+	return totalRows, nil
 }
 
 func CleanupExpiredLoyaltyRedemptions(ctx context.Context) (int, error) {
@@ -1111,7 +1131,21 @@ func CleanupIdleAccounts(ctx context.Context) (int, error) {
 		return 0, fmt.Errorf("failed to anonymize idle accounts: %w", err)
 	}
 
-	return len(accountsWithBalance) + len(accountsNoBalance), tx.Commit(ctx)
+	if err := tx.Commit(ctx); err != nil {
+		return 0, err
+	}
+
+	// The anonymize_user(unnest(...)) calls above erased these accounts — drop
+	// their process-local Square customer cache entries so a stale id cannot
+	// resurface for an erased user.
+	for _, acc := range accountsWithBalance {
+		payments.InvalidateSquareCustomerCache(acc.id)
+	}
+	for _, id := range accountsNoBalance {
+		payments.InvalidateSquareCustomerCache(id)
+	}
+
+	return len(accountsWithBalance) + len(accountsNoBalance), nil
 }
 
 // CleanupOldIdempotencyKeys clears idempotency keys from bookings, payments, and
diff --git a/backend/handlers/scheduling/time_blockers_test.go b/backend/handlers/scheduling/time_blockers_test.go
index ac62dce..46dfd14 100644
--- a/backend/handlers/scheduling/time_blockers_test.go
+++ b/backend/handlers/scheduling/time_blockers_test.go
@@ -3797,3 +3797,66 @@ func TestTimeBlockers_List_WithDateFilter_RecurringOnly(t *testing.T) {
 		t.Errorf("expected 'Only Recurring', got %s", response[0].Description)
 	}
 }
+
+// TestAnonymizeStaleGuestAccounts_InvalidatesSquareCustomerCache verifies the
+// stale-guest anonymization drops each erased guest's process-local Square
+// customer cache entry (GDPR erasure completeness): after the guest is
+// anonymized (email → anon-{id}@anon.invalid, square_customer_id NULLed), a
+// later save-card flow must re-mint a fresh Square customer instead of reusing
+// the deleted one's stale cached id. Deliberately NOT t.Parallel: it swaps the
+// package-level payments.SquareClient.
+func TestAnonymizeStaleGuestAccounts_InvalidatesSquareCustomerCache(t *testing.T) {
+	ctx, tx := resetTestData(t)
+
+	guestID, err := fixtures.CreateTestUser(tx)
+	if err != nil {
+		t.Fatalf("failed to create guest user: %v", err)
+	}
+	if _, err := tx.Exec(ctx, `UPDATE users SET account_role = 'guest' WHERE id = $1`, guestID); err != nil {
+		t.Fatalf("failed to set guest role: %v", err)
+	}
+	// Stale booking (> 6 months) so the stale-guest predicate matches.
+	if _, err := tx.Exec(ctx, `
+		INSERT INTO bookings (user_id, start_time, status, deposit_required)
+		VALUES ($1, NOW() - INTERVAL '7 months', 'completed', false)
+	`, guestID); err != nil {
+		t.Fatalf("failed to create stale booking: %v", err)
+	}
+	// A saved card so the provisioning path has a persistence point.
+	if _, err := tx.Exec(ctx, `
+		INSERT INTO user_saved_cards (user_id, square_card_id, brand, last_4, exp_month, exp_year, fingerprint, is_default)
+		VALUES ($1, 'ccof:cache_stale_card', 'Visa', '4242', 12, 2030, 'fp1', true)
+	`, guestID); err != nil {
+		t.Fatalf("failed to insert saved card: %v", err)
+	}
+
+	origSquare := payments.SquareClient
+	payments.SquareClient = square.NewDevClient()
+	defer func() { payments.SquareClient = origSquare }()
+	t.Cleanup(func() { payments.InvalidateSquareCustomerCache(guestID) })
+
+	svc := payments.NewPaymentService()
+	originalID, err := svc.EnsureSquareCustomer(ctx, guestID)
+	if err != nil {
+		t.Fatalf("failed to provision Square customer: %v", err)
+	}
+	if originalID == "" {
+		t.Fatal("expected a provisioned Square customer id")
+	}
+
+	if _, err := AnonymizeStaleGuestAccounts(ctx); err != nil {
+		t.Fatalf("AnonymizeStaleGuestAccounts failed: %v", err)
+	}
+
+	// The guest is anonymized and the cache must have been invalidated:
+	// re-provisioning re-queries the NULLed DB and mints a fresh customer from
+	// the anonymized email — a different id. Without the invalidation it would
+	// return the stale originalID for the erased guest.
+	reprovisioned, err := svc.EnsureSquareCustomer(ctx, guestID)
+	if err != nil {
+		t.Fatalf("failed to re-provision Square customer: %v", err)
+	}
+	if reprovisioned == originalID {
+		t.Errorf("anonymized stale guest must not reuse the deleted Square customer id %q from the cache", originalID)
+	}
+}
diff --git a/backend/handlers/user/account.go b/backend/handlers/user/account.go
index 1f6e305..5a89b84 100644
--- a/backend/handlers/user/account.go
+++ b/backend/handlers/user/account.go
@@ -167,6 +167,12 @@ func DeleteAccountHandler(w http.ResponseWriter, r *http.Request) {
 		// TODO: Create 'user_anonymized' notification for admin audit trail
 	}
 
+	// The local erasure committed: drop the user's cached Square customer id so
+	// the erased identity cannot resurface from the process-local cache on a
+	// later save-card flow (the Square customer is deleted below and the DB
+	// columns are NULLed, but the cache is never touched by either).
+	payments.InvalidateSquareCustomerCache(userID)
+
 	// external Square cleanup fires only after local anonymization/deletion
 	// commits, so a failed local tx leaves external state intact for retry.
 	if sqClient != nil && (len(cardIDs) > 0 || len(customerIDs) > 0) {
diff --git a/backend/handlers/user/user_coverage_test.go b/backend/handlers/user/user_coverage_test.go
index 3d1995e..3c96e17 100644
--- a/backend/handlers/user/user_coverage_test.go
+++ b/backend/handlers/user/user_coverage_test.go
@@ -875,3 +875,53 @@ func TestDeleteAccount_SkipsSharedSquareCustomer(t *testing.T) {
 	require.True(t, bCustomerID.Valid && bCustomerID.String == "cus_shared_cross_user", "user B's row must keep the shared square_customer_id")
 	require.False(t, bDeletedAt.Valid, "user B's row must not be soft-deleted")
 }
+
+// =============================================================================
+// DeleteAccountHandler — process-local Square customer cache invalidation
+// =============================================================================
+
+// TestDeleteAccount_InvalidatesSquareCustomerCache verifies the GDPR erasure
+// flow drops the user's process-local Square customer cache entry: after
+// DeleteAccountHandler anonymizes the account (NULLing square_customer_id on
+// saved cards and deleting the customer at Square), a later save-card flow for
+// the same (anonymized) user must re-mint a fresh Square customer instead of
+// reusing the deleted one's stale cached id.
+func TestDeleteAccount_InvalidatesSquareCustomerCache(t *testing.T) {
+	savedSquareClient := payments.SquareClient
+	payments.SquareClient = square.NewDevClient()
+	t.Cleanup(func() { payments.SquareClient = savedSquareClient })
+
+	ctx, tx := testutils.SetupTestTx(t)
+	userID, err := fixtures.CreateTestUser(tx)
+	require.NoError(t, err)
+
+	// A saved card gives the provisioning path a persistence point.
+	_, err = tx.Exec(ctx, `
+		INSERT INTO user_saved_cards (user_id, square_card_id, brand, last_4, exp_month, exp_year, fingerprint, is_default)
+		VALUES ($1, 'ccof:cache_erasure_card', 'Visa', '4242', 12, 2030, 'fp1', true)
+	`, userID)
+	require.NoError(t, err)
+	t.Cleanup(func() { payments.InvalidateSquareCustomerCache(userID) })
+
+	svc := payments.NewPaymentService()
+	originalID, err := svc.EnsureSquareCustomer(ctx, userID)
+	require.NoError(t, err)
+	require.NotEmpty(t, originalID, "a Square customer must be provisioned and cached for the save-card user")
+
+	handler := http.HandlerFunc(DeleteAccountHandler)
+	req := httptest.NewRequest(http.MethodDelete, "/api/user/account", nil)
+	req = req.WithContext(context.WithValue(ctx, mw.UserIDKey, userID))
+	w := httptest.NewRecorder()
+	handler.ServeHTTP(w, req)
+	require.Equal(t, http.StatusNoContent, w.Code)
+
+	// The account is anonymized (email → anon-{id}@anon.invalid) and
+	// square_customer_id is NULLed; the handler must also have invalidated the
+	// process-local cache. A subsequent ensureSquareCustomer therefore
+	// re-queries the NULLed DB and mints a fresh customer from the anonymized
+	// email — a different id. Without the invalidation it would return the
+	// stale originalID, resurrecting the erased identity in memory.
+	reprovisioned, err := svc.EnsureSquareCustomer(ctx, userID)
+	require.NoError(t, err)
+	require.NotEqual(t, originalID, reprovisioned, "an erased user must not reuse the deleted Square customer id from the cache")
+}
diff --git a/backend/internal/square/square_dev.go b/backend/internal/square/square_dev.go
index 1b29fed..84bb5a3 100644
--- a/backend/internal/square/square_dev.go
+++ b/backend/internal/square/square_dev.go
@@ -447,6 +447,16 @@ func (m *MockClient) RefundPayment(ctx context.Context, req RefundPaymentReq) (*
 	return result, nil
 }
 
+// RefundKeyCount returns the number of distinct idempotency keys this mock has
+// recorded refunds against (the refundByKey dedup map). Test accessor for
+// asserting that same-key retries issue exactly ONE Square refund, never a
+// second.
+func (m *MockClient) RefundKeyCount() int {
+	m.mu.RLock()
+	defer m.mu.RUnlock()
+	return len(m.refundByKey)
+}
+
 func (m *MockClient) CreateCardOnFile(ctx context.Context, userID, cardToken, customerID string) (*CardOnFile, error) {
 	log.Printf("[SQUARE-MOCK] CreateCardOnFile: user=%s", userID)
 
diff --git a/backend/internal/square/square_http_client.go b/backend/internal/square/square_http_client.go
index d4e608c..e6e6b0d 100644
--- a/backend/internal/square/square_http_client.go
+++ b/backend/internal/square/square_http_client.go
@@ -534,6 +534,13 @@ func getCheckoutHTTPWithClient(ctx context.Context, checkoutID string, hc *httpC
 	if len(tc.PaymentIDs) == 0 {
 		return nil, fmt.Errorf("square: checkout %s has no payment IDs", checkoutID)
 	}
+	if len(tc.PaymentIDs) > 1 {
+		// Terminal checkouts are expected to produce a single payment. If a
+		// future checkout ever returns multiple, record only the first and
+		// surface the rest — silently dropping payments[1:] would under-record
+		// money taken at Square.
+		log.Printf("WARN: checkout %s returned %d payments — recording only the first (%s), manual review advised", checkoutID, len(tc.PaymentIDs), tc.PaymentIDs[0])
+	}
 	var payResp sqGetPaymentResponse
 	if err := hc.doJSON(ctx, http.MethodGet, "/v2/payments/"+tc.PaymentIDs[0], nil, &payResp); err != nil {
 		return nil, err
diff --git a/backend/run-tests.sh b/backend/run-tests.sh
new file mode 100755
index 0000000..b57cb15
--- /dev/null
+++ b/backend/run-tests.sh
@@ -0,0 +1,25 @@
+#!/usr/bin/env bash
+# run-tests.sh — runs the backend test suite behind an advisory lockfile.
+#
+# WHY: every test package drops/recreates a FIXED test database name
+# (crussell_test_handlers_payments, crussell_test_internal_square, etc.) in its
+# TestMain. Two concurrent `go test` invocations targeting the same package
+# therefore destroy each other's database mid-run (SQLSTATE 3D000 / 57P01
+# "terminating connection due to administrator command"). This wrapper
+# serializes runs on a flock(1) lockfile so the test DBs are never clobbered.
+#
+# USAGE:
+#   ./run-tests.sh [go test args...]    # e.g. ./run-tests.sh -tags "test,dev" -count=1 -parallel 8 ./...
+#   LOCK_TIMEOUT=600 ./run-tests.sh ...  # max wait for the lock (default 300s)
+#
+# Agents and humans: ALWAYS run backend tests through this script (or take the
+# lockfile yourself: `flock /tmp/crussell-tests.lock -c '<cmd>'`).
+
+set -u
+
+LOCKFILE="${LOCKFILE:-/tmp/crussell-tests.lock}"
+LOCK_TIMEOUT="${LOCK_TIMEOUT:-300}"
+
+cd "$(dirname "$0")"
+
+exec flock -w "$LOCK_TIMEOUT" "$LOCKFILE" go test "$@"
diff --git a/frontend/src/lib/components/payments/PaymentModal.svelte b/frontend/src/lib/components/payments/PaymentModal.svelte
index 442a636..ed4e92d 100644
--- a/frontend/src/lib/components/payments/PaymentModal.svelte
+++ b/frontend/src/lib/components/payments/PaymentModal.svelte
@@ -616,6 +616,16 @@
 	let loadingSavedCards = $state(false);
 	let selectedSavedCardId = $state<string | null>(null);
 
+	// Per-attempt idempotency key for saved-card charges: regenerated whenever
+	// the (booking.id, selectedSavedCardId, charge amount) tuple changes, so
+	// two DISTINCT identical charges get different UUIDs, but reused across
+	// retries of the SAME charge so a lost-response retry dedups server-side.
+	// Mirrors the TipPayment.svelte tipIdempotencyKey/tipKeyedAmount pattern.
+	let savedCardIdempotencyKey = $state('');
+	let savedCardKeyedBookingId = $state('');
+	let savedCardKeyedCardId = $state('');
+	let savedCardKeyedAmount = $state(0);
+
 	async function fetchSavedCards() {
 		const targetUserId = booking.user_id ?? booking.user?.id;
 		if (!targetUserId) return;
@@ -641,13 +651,30 @@
 			return;
 		}
 
+		const chargeAmount = Math.round(totalDue * 100) - loyaltyDiscount;
+
 		// totalDue can be £0 (fully discounted) and the loyalty discount is
 		// applied on top — the effective charge could otherwise be 0 or negative.
-		if (Math.round(totalDue * 100) - loyaltyDiscount <= 0) {
+		if (chargeAmount <= 0) {
 			toast.error('Nothing to charge — the booking is fully covered by discounts');
 			return;
 		}
 
+		// Reuse the key while the charge context is unchanged (retry of the
+		// same charge → server-side dedup); regenerate when the card or amount
+		// changes so distinct charges never collapse on one key.
+		if (
+			!savedCardIdempotencyKey ||
+			savedCardKeyedBookingId !== booking.id ||
+			savedCardKeyedCardId !== selectedSavedCardId ||
+			savedCardKeyedAmount !== chargeAmount
+		) {
+			savedCardIdempotencyKey = crypto.randomUUID();
+			savedCardKeyedBookingId = booking.id;
+			savedCardKeyedCardId = selectedSavedCardId;
+			savedCardKeyedAmount = chargeAmount;
+		}
+
 		isProcessingPaymentSync = true;
 		status = 'saved-card-processing';
 		error = null;
@@ -659,10 +686,11 @@
 				method: 'POST',
 				headers: { 'Content-Type': 'application/json' },
 				body: JSON.stringify({
-					amount: Math.round(totalDue * 100) - loyaltyDiscount,
+					amount: chargeAmount,
 					payment_type: 'full',
 					payment_method: 'saved_card',
-					saved_card_id: selectedSavedCardId
+					saved_card_id: selectedSavedCardId,
+					idempotency_key: savedCardIdempotencyKey
 				})
 			});
 
@@ -680,6 +708,10 @@
 				last4: data.card_last4,
 				amount: data.amount
 			};
+			// The charge succeeded — clear the cached key so the next (distinct)
+			// charge gets a fresh UUID and can't be deduped against this one.
+			savedCardIdempotencyKey = '';
+			savedCardKeyedAmount = 0;
 			toast.success('Saved card payment successful');
 			onComplete(paymentResult);
 		} catch (_err) {
diff --git a/frontend/src/lib/components/payments/UserPaymentModal.svelte b/frontend/src/lib/components/payments/UserPaymentModal.svelte
index f86a665..e4e0626 100644
--- a/frontend/src/lib/components/payments/UserPaymentModal.svelte
+++ b/frontend/src/lib/components/payments/UserPaymentModal.svelte
@@ -35,7 +35,7 @@
 	// the checkbox inside CardSelection; defaults to false (opt-in).
 	let saveCard = $state(false);
 
-	type PaymentStatus = 'idle' | 'processing' | 'polling' | 'success' | 'error';
+	type PaymentStatus = 'idle' | 'processing' | 'success' | 'error';
 
 	let status = $state<PaymentStatus>('idle');
 	let error = $state<string | null>(null);
@@ -102,8 +102,6 @@
 		paymentType = defaultType as 'full' | 'partial' | 'deposit';
 	});
 
-	let pollingInterval: ReturnType<typeof setInterval> | null = null;
-
 	// Payment lock state
 	let lockTimer = $state(-1);
 	let lockAcquired = $state(false);
@@ -506,17 +504,9 @@
 
 	function handleClose() {
 		releaseLock();
-		stopPolling();
 		onClose();
 	}
 
-	function stopPolling() {
-		if (pollingInterval) {
-			clearInterval(pollingInterval);
-			pollingInterval = null;
-		}
-	}
-
 	// Fetch payment methods on mount if authenticated
 	$effect(() => {
 		if (authStore.isAuthenticated) {
@@ -528,7 +518,8 @@
 	// Cleanup on unmount
 	$effect(() => {
 		return () => {
-			stopPolling();
+			if (countdownInterval) clearInterval(countdownInterval);
+			if (lockInterval) clearInterval(lockInterval);
 		};
 	});
 
@@ -927,16 +918,6 @@
 					Close
 				</Button>
 			</div>
-		{:else if status === 'polling'}
-			<!-- Polling State -->
-			<div class="flex flex-col items-center justify-center py-8">
-				<div
-					class="mb-4 h-12 w-12 animate-spin rounded-full border-4 border-gray-200 border-t-green-600"
-				></div>
-				<p class="text-lg font-medium text-gray-700">Processing payment...</p>
-				<p class="mt-2 text-sm text-gray-500">This may take a few moments</p>
-				<Button variant="ghost" onclick={handleClose} class="mt-6">Cancel</Button>
-			</div>
 		{:else if status === 'success' && paymentResult}
 			<!-- Success State -->
 			<div class="space-y-4">
diff --git a/frontend/src/routes/account/+page.svelte b/frontend/src/routes/account/+page.svelte
index d8fdda3..1bd252d 100644
--- a/frontend/src/routes/account/+page.svelte
+++ b/frontend/src/routes/account/+page.svelte
@@ -2070,10 +2070,14 @@
 										</li>
 									</ul>
 								</div>
-								<!-- TODO: Link full Terms & Conditions once the T&Cs page is created -->
 								<p class="text-xs text-muted-foreground italic">
-									By redeeming this gift card, you agree to our Terms &amp; Conditions.
-									<em>Link T&amp;Cs here once available.</em>
+									By redeeming this gift card, you agree to our
+									<a
+										href="/terms"
+										class="font-semibold text-primary hover:underline"
+										target="_blank"
+										rel="noopener noreferrer external">Terms &amp; Conditions</a
+									>.
 								</p>
 							</div>
 							<AlertDialog.Footer>
diff --git a/nginx/conf.d/default.conf b/nginx/conf.d/default.conf
index 7f8ef61..fdcb2d7 100644
--- a/nginx/conf.d/default.conf
+++ b/nginx/conf.d/default.conf
@@ -19,7 +19,10 @@ server {
     add_header X-Content-Type-Options nosniff;
     add_header X-Frame-Options DENY;
     add_header X-XSS-Protection "1; mode=block";
-    add_header Content-Security-Policy "default-src 'self'; script-src 'self' 'unsafe-inline'; style-src 'self' 'unsafe-inline'; img-src 'self' data: https:; font-src 'self'; connect-src 'self'; frame-ancestors 'none';" always;
+    # Square Web Payments SDK: script from *.squarecdn.com, card-entry iframe
+    # from js.squareup.com (frame-src; without it the payment form cannot
+    # tokenize behind this proxy). connect-src allows the SDK's own network calls.
+    add_header Content-Security-Policy "default-src 'self'; script-src 'self' 'unsafe-inline' https://*.squarecdn.com; style-src 'self' 'unsafe-inline'; img-src 'self' data: https:; font-src 'self'; connect-src 'self' https://*.squareup.com https://*.squarecdn.com; frame-src https://js.squareup.com https://*.squareup.com; frame-ancestors 'none';" always;
 
     # Serve static frontend
     root /usr/share/nginx/html;