Till-sale: resolve pending top-ups by gift card to reuse stored idempotency key

A lost-response retry carrying a fresh or absent idempotency key (an old
frontend, or a key regenerated for a changed cart) now reuses the pending
till_sale row by resolving the gift card itself and adopts the row's STORED
key — so Square dedups the retry against the original charge and the card is
never funded twice. An amount mismatch proves a genuinely different sale (same
card, new amount) and leaves the pending row untouched. 'create' actions carry
no gift-card id and continue to rely on the client-supplied key cached per
cart line in the till frontend.

backend/handlers/payments/till.go

@@ -370,6 +370,42 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) {
 		}
 	}
 
+	// Defense-in-depth fallback: resolve a pending TOP-UP by the gift card
+	// itself, not just the idempotency key. A lost-response retry carrying a
+	// fresh/absent key (an old frontend, or a key regenerated for a changed
+	// cart) must still reuse the pending till_sale row and adopt its STORED
+	// idempotency key — otherwise the retry would issue a second Square charge
+	// and fund the card twice. Only the row's own key may be charged: adopting
+	// it makes Square dedup the retry against the original charge. An amount
+	// mismatch proves this is a genuinely different sale (same card, new
+	// amount), so the pending row is left untouched and the request proceeds
+	// as a new charge below. 'create' cannot be resolved this way (the request
+	// carries no gift-card id — the card is created by the first attempt), so
+	// create retries rely on the client-supplied key, which the till frontend
+	// caches per cart line.
+	if existingPendingID == "" && req.Action == "topup" && req.GiftCardID != nil && *req.GiftCardID != "" {
+		cardID := validators.NormalizeGiftCardCode(*req.GiftCardID)
+		var existingID, existingItemID, existingKey string
+		var existingTotal float64
+		err := db.Conn.QueryRow(ctx, `
+			SELECT id, item_id, total_amount, idempotency_key
+			FROM till_sales
+			WHERE item_id = $1 AND status = 'pending'
+			ORDER BY created_at DESC
+			LIMIT 1
+		`, cardID).Scan(&existingID, &existingItemID, &existingTotal, &existingKey)
+		if err == nil && existingKey != "" {
+			if int64(math.Round(existingTotal*100)) == int64(math.Round(req.Amount*100)) {
+				log.Printf("Till-sale retry resolved by gift card %s: reusing pending sale %s with stored idempotency key", cardID, existingID)
+				existingPendingID = existingID
+				existingPendingGiftCard = existingItemID
+				req.IdempotencyKey = existingKey
+			} else {
+				log.Printf("Till-sale retry by gift card %s skipped: pending sale %s has %.2f, request has %.2f — treated as a new sale", cardID, existingID, existingTotal, req.Amount)
+			}
+		}
+	}
+
 	service := NewPaymentService()
 
 	// squarePaymentID/squareCheckoutID are set inside the payment-method switch

backend/handlers/payments/till_test.go

@@ -10,6 +10,7 @@ import (
 	"fmt"
 	"net/http"
 	"net/http/httptest"
+	"sync"
 	"testing"
 	"time"
 
@@ -2219,3 +2220,341 @@ func TestGetTillCheckoutStatus_AlreadyFailed_DoesNotResurrect(t *testing.T) {
 		t.Errorf("expected failed sale to remain failed after the guarded poll, got %q", status)
 	}
 }
+
+// countingPaymentClient wraps the Square client and records every CreatePayment
+// invocation (its idempotency key and the returned payment) so tests can prove
+// a retry issued exactly ONE charge (Square dedup returns the original payment)
+// and that the charge reused the expected idempotency key.
+type countingPaymentClient struct {
+	square.SquareClient
+	mu       sync.Mutex
+	keys     []string
+	payments []*square.PaymentResult
+}
+
+func (c *countingPaymentClient) CreatePayment(ctx context.Context, req square.CreatePaymentReq) (*square.PaymentResult, error) {
+	res, err := c.SquareClient.CreatePayment(ctx, req)
+	c.mu.Lock()
+	defer c.mu.Unlock()
+	c.keys = append(c.keys, req.IdempotencyKey)
+	if err == nil {
+		c.payments = append(c.payments, res)
+	}
+	return res, err
+}
+
+func (c *countingPaymentClient) callCount() int {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+	return len(c.keys)
+}
+
+// makeTillSaleRequest POSTs a till sale through the real router with the test
+// transaction embedded in the request context.
+func makeTillSaleRequest(t *testing.T, req TillSaleRequest, adminToken string, ctx context.Context, tx pgx.Tx) *httptest.ResponseRecorder {
+	t.Helper()
+	bodyBytes, _ := json.Marshal(req)
+	r := httptest.NewRequest("POST", "/api/admin/till/sale", bytes.NewReader(bodyBytes))
+	r.Header.Set("Authorization", "Bearer "+adminToken)
+	r.Header.Set("Content-Type", "application/json")
+	r = r.WithContext(db.ContextWithTx(r.Context(), tx))
+	w := httptest.NewRecorder()
+	router := chi.NewRouter()
+	router.Use(mw.RequireAuth)
+	router.Post("/api/admin/till/sale", CreateTillSale)
+	router.ServeHTTP(w, r)
+	return w
+}
+
+// TestCreateTillSale_PendingRetry_LostResponse_SingleSquareCharge simulates
+// the P0 double-charge window: a charge lands at Square but the response is
+// lost, leaving the till_sale row 'pending'. A same-key retry must re-attempt
+// the charge with the SAME idempotency key — Square dedups on the key and
+// returns the ORIGINAL payment — so the customer is charged exactly once, the
+// same row is reused, and the gift card is funded exactly once.
+func TestCreateTillSale_PendingRetry_LostResponse_SingleSquareCharge(t *testing.T) {
+	t.Parallel()
+	ctx, tx := testutils.SetupTestTx(t)
+
+	adminID, err := fixtures.CreateTestAdminUser(tx)
+	if err != nil {
+		t.Fatalf("failed to create admin user: %v", err)
+	}
+	adminToken := jwt.GenerateTestToken(adminID, "admin")
+
+	origClient := SquareClient
+	rec := &countingPaymentClient{SquareClient: square.NewDevClient()}
+	SquareClient = rec
+	defer func() { SquareClient = origClient }()
+
+	key := "till-lost-response-single-charge"
+	reqBody := TillSaleRequest{
+		ItemType:       "gift_card",
+		Action:         "create",
+		Amount:         50.00,
+		PaymentMethod:  "online_square",
+		CardToken:      "cnon:lost-response-card",
+		IdempotencyKey: key,
+	}
+
+	// First attempt: the charge succeeds at Square and the sale completes.
+	first := makeTillSaleRequest(t, reqBody, adminToken, ctx, tx.(pgx.Tx))
+	if first.Code != http.StatusCreated {
+		t.Fatalf("first attempt: expected 201, got %d. body: %s", first.Code, first.Body.String())
+	}
+	var firstResp TillSaleResponse
+	if err := json.NewDecoder(first.Body).Decode(&firstResp); err != nil {
+		t.Fatalf("failed to decode first response: %v", err)
+	}
+
+	// Lost-response window: the charge keyed on `key` is live at Square (the
+	// mock keeps it in paymentByKey) but the response never reached the
+	// handler, so the sale row is stuck 'pending' with no payment id recorded.
+	if _, err := tx.Exec(ctx, `UPDATE till_sales SET status = 'pending', square_payment_id = NULL WHERE id = $1`, firstResp.ID); err != nil {
+		t.Fatalf("failed to simulate the lost response: %v", err)
+	}
+
+	// Retry with the same key: the pending-reuse path re-charges with the SAME
+	// key, Square dedups to the original payment, and the same row completes.
+	second := makeTillSaleRequest(t, reqBody, adminToken, ctx, tx.(pgx.Tx))
+	if second.Code != http.StatusOK && second.Code != http.StatusCreated {
+		t.Fatalf("retry: expected 200/201, got %d. body: %s", second.Code, second.Body.String())
+	}
+	var secondResp TillSaleResponse
+	if err := json.NewDecoder(second.Body).Decode(&secondResp); err != nil {
+		t.Fatalf("failed to decode retry response: %v", err)
+	}
+
+	// Exactly ONE till_sales row — the retry reused the pending row.
+	if secondResp.ID != firstResp.ID {
+		t.Errorf("expected the SAME till_sale row on retry, got %s then %s", firstResp.ID, secondResp.ID)
+	}
+	var saleCount int
+	if err := tx.QueryRow(ctx, `SELECT COUNT(*) FROM till_sales WHERE id = $1`, firstResp.ID).Scan(&saleCount); err != nil {
+		t.Fatalf("failed to count till_sales: %v", err)
+	}
+	if saleCount != 1 {
+		t.Errorf("expected 1 till_sale row (reused), got %d", saleCount)
+	}
+
+	// Square was invoked twice (attempt + retry) but the retry deduped on the
+	// key: both invocations returned the SAME payment — one charge in total.
+	if rec.callCount() != 2 {
+		t.Errorf("expected 2 CreatePayment invocations (attempt + dedup retry), got %d", rec.callCount())
+	}
+	rec.mu.Lock()
+	oneCharge := len(rec.payments) == 2 && rec.payments[0].SquarePayID == rec.payments[1].SquarePayID
+	rec.mu.Unlock()
+	if !oneCharge {
+		t.Errorf("expected the retry to dedup to the original payment (single charge), got %d distinct payments", len(rec.payments))
+	}
+
+	// Exactly one funded gift card for this sale, still at the single value.
+	var giftCardCount int
+	if err := tx.QueryRow(ctx, `SELECT COUNT(*) FROM gift_cards WHERE total_funds_added = 50.00`).Scan(&giftCardCount); err != nil {
+		t.Fatalf("failed to count gift cards: %v", err)
+	}
+	if giftCardCount != 1 {
+		t.Errorf("expected 1 funded gift card (single funding), got %d", giftCardCount)
+	}
+
+	// The reused row must be completed with the payment id recorded.
+	var saleStatus, sqPaymentID string
+	if err := tx.QueryRow(ctx, `SELECT status, COALESCE(square_payment_id, '') FROM till_sales WHERE id = $1`, firstResp.ID).Scan(&saleStatus, &sqPaymentID); err != nil {
+		t.Fatalf("failed to read sale status: %v", err)
+	}
+	if saleStatus != "completed" {
+		t.Errorf("expected sale to be completed after the retry, got %s", saleStatus)
+	}
+	if sqPaymentID == "" {
+		t.Error("expected square_payment_id to be recorded after the retry")
+	}
+}
+
+// TestCreateTillSale_PendingRetry_FreshKey_ReusesPendingRowViaGiftCard proves
+// the backend defense-in-depth: even when a retry arrives with a FRESH
+// idempotency key (an old frontend, or a regenerated key), a same-amount
+// top-up on the SAME gift card reuses the pending till_sale row and adopts its
+// STORED idempotency key — the Square charge dedups on the stored key instead
+// of charging the customer twice.
+func TestCreateTillSale_PendingRetry_FreshKey_ReusesPendingRowViaGiftCard(t *testing.T) {
+	t.Parallel()
+	ctx, tx := testutils.SetupTestTx(t)
+
+	adminID, err := fixtures.CreateTestAdminUser(tx)
+	if err != nil {
+		t.Fatalf("failed to create admin user: %v", err)
+	}
+	adminToken := jwt.GenerateTestToken(adminID, "admin")
+
+	// Seed a funded gift card with a pending top-up till_sale (the first
+	// attempt's Square charge failed after the DB commit — card already funded).
+	var giftCardID string
+	if err := tx.QueryRow(ctx, `
+		INSERT INTO gift_cards (total_funds_added, amount_remaining, created_by, is_inventory, voucher_type_at_purchase)
+		VALUES (50.00, 50.00, $1, FALSE, 'SPV')
+		RETURNING id
+	`, adminID).Scan(&giftCardID); err != nil {
+		t.Fatalf("failed to create gift card: %v", err)
+	}
+	storedKey := "till-original-topup-key"
+	if _, err := tx.Exec(ctx, `
+		INSERT INTO till_sales (item_type, item_id, description, quantity, unit_price, total_amount,
+			payment_method, status, user_id, user_saved_card_id, idempotency_key, created_by, created_at, updated_at)
+		VALUES ('gift_card', $1, 'Gift Card topup', 1, 50.00, 50.00, 'online_square', 'pending',
+			NULL, NULL, $2, $3, NOW(), NOW())
+	`, giftCardID, storedKey, adminID); err != nil {
+		t.Fatalf("failed to seed pending top-up: %v", err)
+	}
+
+	origClient := SquareClient
+	rec := &countingPaymentClient{SquareClient: square.NewDevClient()}
+	SquareClient = rec
+	defer func() { SquareClient = origClient }()
+
+	// Retry with a GENUINELY FRESH key: same gift card, same amount, but a key
+	// the backend has never seen (simulating an old/regenerated key).
+	reqBody := TillSaleRequest{
+		ItemType:       "gift_card",
+		Action:         "topup",
+		Amount:         50.00,
+		GiftCardID:     &giftCardID,
+		PaymentMethod:  "online_square",
+		CardToken:      "cnon:fresh-key-topup-card",
+		IdempotencyKey: "till-fresh-key-topup",
+	}
+	w := makeTillSaleRequest(t, reqBody, adminToken, ctx, tx.(pgx.Tx))
+	if w.Code != http.StatusOK && w.Code != http.StatusCreated {
+		t.Fatalf("expected 200/201, got %d. body: %s", w.Code, w.Body.String())
+	}
+
+	// The pending row was reused, not duplicated, and keeps its stored key.
+	var saleCount int
+	var saleStatus, usedKey string
+	if err := tx.QueryRow(ctx, `
+		SELECT COUNT(*), MAX(status), MAX(idempotency_key) FROM till_sales WHERE item_id = $1
+	`, giftCardID).Scan(&saleCount, &saleStatus, &usedKey); err != nil {
+		t.Fatalf("failed to query till_sales: %v", err)
+	}
+	if saleCount != 1 {
+		t.Errorf("expected 1 till_sale row (reused), got %d", saleCount)
+	}
+	if saleStatus != "completed" {
+		t.Errorf("expected the reused row to complete, got %s", saleStatus)
+	}
+	if usedKey != storedKey {
+		t.Errorf("expected the sale row to keep its stored key %q, got %q", storedKey, usedKey)
+	}
+
+	// The Square charge reused the STORED key (the dedup target), not the
+	// fresh request key — a second charge on the fresh key would double-charge.
+	rec.mu.Lock()
+	gotKey := rec.keys[0]
+	rec.mu.Unlock()
+	if gotKey != storedKey {
+		t.Errorf("expected Square charge to use the stored key %q, got %q", storedKey, gotKey)
+	}
+
+	// The gift card was funded exactly once — a second charge would have funded
+	// it again.
+	var remaining float64
+	if err := tx.QueryRow(ctx, `SELECT amount_remaining FROM gift_cards WHERE id = $1`, giftCardID).Scan(&remaining); err != nil {
+		t.Fatalf("failed to read gift card: %v", err)
+	}
+	if remaining != 50.00 {
+		t.Errorf("expected gift card funded once (£50.00), got £%.2f (double-funding!)", remaining)
+	}
+}
+
+// TestCreateTillSale_PendingRetry_FreshKey_DifferentAmount_NewCharge proves a
+// retry with a genuinely different amount on the same gift card is a NEW sale:
+// the pending row is left untouched and a new Square charge is issued (the
+// amount mismatch is the only signal distinguishing a retry from a fresh
+// top-up).
+func TestCreateTillSale_PendingRetry_FreshKey_DifferentAmount_NewCharge(t *testing.T) {
+	t.Parallel()
+	ctx, tx := testutils.SetupTestTx(t)
+
+	adminID, err := fixtures.CreateTestAdminUser(tx)
+	if err != nil {
+		t.Fatalf("failed to create admin user: %v", err)
+	}
+	adminToken := jwt.GenerateTestToken(adminID, "admin")
+
+	var giftCardID string
+	if err := tx.QueryRow(ctx, `
+		INSERT INTO gift_cards (total_funds_added, amount_remaining, created_by, is_inventory, voucher_type_at_purchase)
+		VALUES (50.00, 50.00, $1, FALSE, 'SPV')
+		RETURNING id
+	`, adminID).Scan(&giftCardID); err != nil {
+		t.Fatalf("failed to create gift card: %v", err)
+	}
+	if _, err := tx.Exec(ctx, `
+		INSERT INTO till_sales (item_type, item_id, description, quantity, unit_price, total_amount,
+			payment_method, status, user_id, user_saved_card_id, idempotency_key, created_by, created_at, updated_at)
+		VALUES ('gift_card', $1, 'Gift Card topup', 1, 50.00, 50.00, 'online_square', 'pending',
+			NULL, NULL, 'till-original-topup-key-2', $2, NOW(), NOW())
+	`, giftCardID, adminID); err != nil {
+		t.Fatalf("failed to seed pending top-up: %v", err)
+	}
+
+	origClient := SquareClient
+	rec := &countingPaymentClient{SquareClient: square.NewDevClient()}
+	SquareClient = rec
+	defer func() { SquareClient = origClient }()
+
+	// Different amount on the same card with a fresh key: a genuinely
+	// different sale — a new charge is allowed.
+	reqBody := TillSaleRequest{
+		ItemType:       "gift_card",
+		Action:         "topup",
+		Amount:         25.00,
+		GiftCardID:     &giftCardID,
+		PaymentMethod:  "online_square",
+		CardToken:      "cnon:new-amount-topup-card",
+		IdempotencyKey: "till-fresh-key-topup-25",
+	}
+	w := makeTillSaleRequest(t, reqBody, adminToken, ctx, tx.(pgx.Tx))
+	if w.Code != http.StatusOK && w.Code != http.StatusCreated {
+		t.Fatalf("expected 200/201 for the new sale, got %d. body: %s", w.Code, w.Body.String())
+	}
+
+	// Two till_sales rows now: the untouched pending one and the new completed
+	// one.
+	var saleCount int
+	if err := tx.QueryRow(ctx, `SELECT COUNT(*) FROM till_sales WHERE item_id = $1`, giftCardID).Scan(&saleCount); err != nil {
+		t.Fatalf("failed to count till_sales: %v", err)
+	}
+	if saleCount != 2 {
+		t.Errorf("expected 2 till_sale rows (pending original + new sale), got %d", saleCount)
+	}
+
+	// The new sale charged Square exactly once, with its own fresh key.
+	if rec.callCount() != 1 {
+		t.Errorf("expected exactly 1 new Square charge, got %d", rec.callCount())
+	}
+	rec.mu.Lock()
+	gotKey := rec.keys[0]
+	rec.mu.Unlock()
+	if gotKey != "till-fresh-key-topup-25" {
+		t.Errorf("expected the new charge to use the request's fresh key, got %q", gotKey)
+	}
+
+	// The card now holds £50 (original pending funding) + £25 (new top-up).
+	var totalAdded float64
+	if err := tx.QueryRow(ctx, `SELECT total_funds_added FROM gift_cards WHERE id = $1`, giftCardID).Scan(&totalAdded); err != nil {
+		t.Fatalf("failed to read gift card: %v", err)
+	}
+	if totalAdded != 75.00 {
+		t.Errorf("expected gift card total funded to £75.00, got £%.2f", totalAdded)
+	}
+
+	// The original pending row is untouched.
+	var origStatus string
+	if err := tx.QueryRow(ctx, `SELECT status FROM till_sales WHERE idempotency_key = 'till-original-topup-key-2'`).Scan(&origStatus); err != nil {
+		t.Fatalf("failed to read original row: %v", err)
+	}
+	if origStatus != "pending" {
+		t.Errorf("expected the original pending row to be untouched, got %s", origStatus)
+	}
+}