From 352f9e50d45bce76e30b245326198e3ef2202535 Mon Sep 17 00:00:00 2001 From: Stephen Adamson Date: Mon, 3 Aug 2026 19:25:17 +0100 Subject: [PATCH] Fix till-sale money safety: sweep gift-card clawback, orphaned-checkout cancel, cash-retry reconciliation HIGH-1: a till sale whose card-machine checkout is provably dead, or whose Square reconcile proves the charge never landed, now claws back the funded gift card atomically with the failed mark (claim-first gating UPDATE serializes against the admin retry; blind-fail and ambiguous/lost-response rows never claw back, and a bare CANCEL_REQUESTED is not treated as proof of non-completion). HIGH-2: a card-machine checkout created at Square but not committed is cancelled on any pre-commit failure. HIGH-3: a cash/on_the_house retry of a pending card sale is reconciled at Square first (COMPLETED rescues + refuses cash; NOT_FOUND/FAILED/CANCELED allows cash; lost-response forces the card-method retry; ambiguous rejects). GetTillCheckoutStatus no longer resurrects a swept-failed sale, and the cash-completion UPDATE checks RowsAffected so the admin is never told to take cash against an already-resolved sale. 23 new tests covering the clawback matrix, the reconcile-or-reject matrix, cancel-on-error, and a till concurrency test. --- backend/handlers/payments/concurrency_test.go | 93 +++ backend/handlers/payments/sweep.go | 205 +++++- backend/handlers/payments/sweep_test.go | 502 +++++++++++++++ backend/handlers/payments/till.go | 124 +++- backend/handlers/payments/till_test.go | 592 +++++++++++++++++- 5 files changed, 1478 insertions(+), 38 deletions(-) diff --git a/backend/handlers/payments/concurrency_test.go b/backend/handlers/payments/concurrency_test.go index 3902de5..1d0f311 100644 --- a/backend/handlers/payments/concurrency_test.go +++ b/backend/handlers/payments/concurrency_test.go @@ -522,6 +522,99 @@ func TestLoyaltyRedemption_ConcurrentSameBooking_SingleApply(t *testing.T) { }) } +// TestCreateTillSale_ConcurrentSameKey_SingleRecord proves the till-sale +// advisory lock (till.go): two goroutines POSTing the same idempotency key must +// produce exactly ONE till_sales row and ONE funded gift card — never 2× value +// for one charge. Without the lock, both goroutines pass the idempotency check, +// both fund a gift card, and one dies on the till_sales idempotency_key UNIQUE +// constraint after the funding already committed. +func TestCreateTillSale_ConcurrentSameKey_SingleRecord(t *testing.T) { + 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") + pool := context.Background() + cleanupConcurrentTestRows(t, pool, adminID, "") + key := "till-concurrent-same-key" + t.Cleanup(func() { + _, _ = db.Conn.Exec(pool, `DELETE FROM till_sales WHERE idempotency_key = $1`, key) + }) + + // Commit the setup so both goroutines operate at pool level — the advisory + // locks only serialize across independent connections, and a per-test tx + // would route both sides through a single shared connection. + 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) + } + + origClient := SquareClient + slow := &slowCreatePaymentClient{SquareClient: square.NewDevClient(), delay: 300 * time.Millisecond} + SquareClient = slow + defer func() { SquareClient = origClient }() + + reqBody := TillSaleRequest{ + ItemType: "gift_card", + Action: "create", + Amount: 50.00, + PaymentMethod: "online_square", + CardToken: "cnon:concurrent-till-card", + IdempotencyKey: key, + } + + 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(CreateTillSale, "POST", "/api/admin/till/sale", reqBody, adminToken, pool) + }(i) + } + close(startBoth) + wg.Wait() + + // Both requests must succeed — the lock serializes them and the second + // finds the completed record (idempotent dedup), so neither double-charges + // nor errors. + for i, rec := range recs { + if rec.Code != http.StatusCreated && rec.Code != http.StatusOK { + t.Errorf("request %d expected 201 (create) or 200 (dedup), got %d. body: %s", i, rec.Code, rec.Body.String()) + } + } + + // Exactly one till_sales row for this key. + var saleCount int + err = db.Conn.QueryRow(pool, + `SELECT COUNT(*) FROM till_sales WHERE idempotency_key = $1`, key).Scan(&saleCount) + if err != nil { + t.Fatalf("failed to count till_sales: %v", err) + } + if saleCount != 1 { + t.Errorf("expected exactly 1 till_sales row, got %d (double-charge!)", saleCount) + } + + // Exactly one funded gift card for this admin's till sale. + var gcCount int + err = db.Conn.QueryRow(pool, + `SELECT COUNT(*) FROM gift_cards WHERE total_funds_added = 50.00 AND created_by = $1`, adminID).Scan(&gcCount) + if err != nil { + t.Fatalf("failed to count gift cards: %v", err) + } + if gcCount != 1 { + t.Errorf("expected exactly 1 funded gift card, got %d (2× value!)", gcCount) + } +} + // TestTransferGiftCard_ConcurrentCrossTransfer_NoDeadlock proves the gift-card // transfer lock ordering (giftcards.go): two concurrent cross-transfers A→B and // B→A must both succeed. Locking "source first" (the caller's chosen order) diff --git a/backend/handlers/payments/sweep.go b/backend/handlers/payments/sweep.go index 3465c01..971ac7c 100644 --- a/backend/handlers/payments/sweep.go +++ b/backend/handlers/payments/sweep.go @@ -2,6 +2,7 @@ package payments import ( "context" + "database/sql" "errors" "fmt" "log" @@ -11,6 +12,8 @@ import ( "crussell/clock" "crussell/db" "crussell/internal/square" + + "github.com/jackc/pgx/v5" ) // SweepStalePendingPayments resolves pending payment records that are older @@ -75,17 +78,31 @@ func SweepStalePendingPayments(ctx context.Context) (int, error) { } // staleRow is one stale pending row read by the sweep so it can reconcile -// rows that carry a Square reference BEFORE failing them. +// rows that carry a Square reference BEFORE failing them. For till_sales rows +// the gift-card context needed to claw back funded money on a definitive +// failure is carried alongside: the card id, who it was redeemed to, whether +// this sale created the card (created_at equality — provable because both +// timestamps are the transaction-start NOW() in the same tx), and the sale +// amount (the exact funding this sale added). type staleRow struct { ID string SquarePaymentID string + ItemID string // till_sales.item_id — the gift card ("" when NULL / non-gift-card) + RedeemToUserID *string // gift_cards.redeemed_by — user credited by a create-with-redeem + IsCreate bool // true when this sale created the gift card (timestamps equal) + HasGiftCard bool // false when the LEFT JOIN found no gift_cards row (gc.id IS NULL) + TotalAmount float64 // till_sales.total_amount — the funding this sale added } // sweepStaleRows resolves the stale pending rows of one table. Rows with a // square_payment_id are reconciled at Square first (COMPLETED → 'completed', // anything else → 'failed' exactly as the legacy bulk UPDATE did); rows -// without one cannot be reconciled and are failed directly. Returns the total -// rows resolved and how many were rescued to 'completed'. +// without one cannot be reconciled and are failed directly. A till_sales row +// whose reconcile PROVES the charge never completed (NOT_FOUND / non-COMPLETED) +// also claws back the funded gift card atomically with the failed mark; the +// blind-fail path (no square_payment_id — the charge may have landed) never +// claws back. Returns the total rows resolved and how many were rescued to +// 'completed'. func sweepStaleRows(ctx context.Context, table string, cutoff time.Time) (resolved int, completed int, err error) { switch table { case "payments", "till_sales": @@ -99,20 +116,48 @@ func sweepStaleRows(ctx context.Context, table string, cutoff time.Time) (resolv if table == "till_sales" { methodFilter = ` AND payment_method IN ('online_square', 'in_person_card')` } - rows, err := db.Conn.Query(ctx, ` - SELECT id, COALESCE(square_payment_id, '') - FROM `+table+` - WHERE status = 'pending' AND created_at < $1`+methodFilter+` - `, cutoff) + var rows pgx.Rows + if table == "till_sales" { + rows, err = db.Conn.Query(ctx, ` + SELECT ts.id, COALESCE(ts.square_payment_id, ''), ts.item_id, gc.redeemed_by, + (ts.created_at = gc.created_at) AS is_create, + (gc.id IS NOT NULL) AS has_gift_card, ts.total_amount + FROM till_sales ts + LEFT JOIN gift_cards gc ON gc.id = ts.item_id + WHERE ts.status = 'pending' AND ts.created_at < $1`+methodFilter+` + `, cutoff) + } else { + rows, err = db.Conn.Query(ctx, ` + SELECT id, COALESCE(square_payment_id, '') + FROM `+table+` + WHERE status = 'pending' AND created_at < $1`+methodFilter+` + `, cutoff) + } if err != nil { return 0, 0, err } var stale []staleRow for rows.Next() { var r staleRow - if err := rows.Scan(&r.ID, &r.SquarePaymentID); err != nil { - log.Printf("Failed to scan stale pending row from %s: %v", table, err) - continue + if table == "till_sales" { + var itemID, redeemedBy sql.NullString + var isCreate *bool + var hasGiftCard bool + if err := rows.Scan(&r.ID, &r.SquarePaymentID, &itemID, &redeemedBy, &isCreate, &hasGiftCard, &r.TotalAmount); err != nil { + log.Printf("Failed to scan stale pending row from %s: %v", table, err) + continue + } + r.ItemID = itemID.String + if redeemedBy.Valid && redeemedBy.String != "" { + r.RedeemToUserID = &redeemedBy.String + } + r.IsCreate = isCreate != nil && *isCreate + r.HasGiftCard = hasGiftCard + } else { + if err := rows.Scan(&r.ID, &r.SquarePaymentID); err != nil { + log.Printf("Failed to scan stale pending row from %s: %v", table, err) + continue + } } stale = append(stale, r) } @@ -143,6 +188,27 @@ func sweepStaleRows(ctx context.Context, table string, cutoff time.Time) (resolv } // staleReconcileDefinitivelyFailed falls through to the fail path. } + // Fail path. A till-sale reconcile that PROVED the charge never + // completed (NOT_FOUND / non-COMPLETED) claws back the funded gift + // card atomically with the failed mark (HIGH-1). The blind-fail path + // (no square_payment_id — the charge outcome is unknown) NEVER claws + // back: the money may have landed at Square. + if table == "till_sales" && r.SquarePaymentID != "" && r.HasGiftCard { + action := "topup" + if r.IsCreate { + action = "create" + } + if revErr := revertGiftCardFunding(ctx, action, r.ItemID, r.TotalAmount, r.RedeemToUserID, r.ID); revErr != nil { + if errors.Is(revErr, errTillSaleNotPending) { + log.Printf("Stale pending till sale %s was already resolved (not pending) — skipping funding clawback", r.ID) + continue + } + log.Printf("CRITICAL: failed to claw back gift card %s funding for stale till sale %s: %v — MANUAL RECONCILIATION REQUIRED: gift card may still be funded", r.ItemID, r.ID, revErr) + continue + } + resolved++ + continue + } if tag, upErr := db.Conn.Exec(ctx, ` UPDATE `+table+` SET status = 'failed', updated_at = NOW() WHERE id = $1 AND status = 'pending' @@ -354,7 +420,14 @@ func SweepStaleTerminalCheckouts(ctx context.Context) (int, error) { // The cancel landed (CANCELED / cancel-requested / expired / // still-reporting-pending-but-now-cancelled) — it can never // complete, so resolve the row to the terminal 'failed' state. - if markTerminalCheckoutRowFailed(ctx, r) { + // A till sale's funded gift card is clawed back (a cancel that + // landed cannot later complete); a booking terminal_checkout + // has no gift card. + if r.Kind == "till_sale" { + if clawBackTillSaleFunding(ctx, r.RowID) { + resolved++ + } + } else if markTerminalCheckoutRowFailed(ctx, r) { resolved++ } log.Printf("Cancelled stale terminal checkout %s (%s %s, pending >%s) — marked failed", r.CheckoutID, r.Kind, r.RowID, staleTerminalCheckoutAge) @@ -380,12 +453,30 @@ func SweepStaleTerminalCheckouts(ctx context.Context) (int, error) { log.Printf("Terminal checkout %s already COMPLETED at Square — leaving sale %s pending (poll handler records it)", r.CheckoutID, r.RowID) } case isTerminalCheckoutError(gErr): - // Definitely cancelled / cancel-requested / expired — the checkout - // can never complete, so resolve it to the terminal 'failed' state. - if markTerminalCheckoutRowFailed(ctx, r) { - resolved++ + // The checkout is cancelled / cancel-requested / expired at Square. + switch { + case r.Kind == "till_sale" && isCheckoutDefinitivelyDead(gErr): + // The error PROVES the checkout can never complete (CANCELED / + // NOT_FOUND, and no bare CANCEL_REQUESTED) — claw back the + // funded gift card along with the failed mark. + if clawBackTillSaleFunding(ctx, r.RowID) { + resolved++ + } + log.Printf("Terminal checkout %s is definitively terminal (%v) — marked till sale %s failed, gift-card funding clawed back", r.CheckoutID, gErr, r.RowID) + case r.Kind == "till_sale": + // CANCEL_REQUESTED-only: Square does not promise non-completion + // in that state, so the charge may still land. Mark the sale + // failed but DO NOT claw back the funded gift card. + if markTerminalCheckoutRowFailed(ctx, r) { + resolved++ + } + log.Printf("CRITICAL: terminal checkout %s reports only CANCEL_REQUESTED (not provably dead) — till sale %s marked failed WITHOUT clawing back the funded gift card; verify at Square before re-issuing — MANUAL RECONCILIATION REQUIRED", r.CheckoutID, r.RowID) + default: + if markTerminalCheckoutRowFailed(ctx, r) { + resolved++ + } + log.Printf("Terminal checkout %s is definitively terminal (%v) — marked %s %s failed", r.CheckoutID, gErr, r.Kind, r.RowID) } - log.Printf("Terminal checkout %s is definitively terminal (%v) — marked %s %s failed", r.CheckoutID, gErr, r.Kind, r.RowID) default: // Ambiguous transport/unknown status — leave for a later run. log.Printf("Terminal checkout %s status unknown (%v) — leaving %s %s pending for a later sweep", r.CheckoutID, gErr, r.Kind, r.RowID) @@ -443,3 +534,83 @@ func isTerminalCheckoutError(err error) bool { strings.Contains(msg, "NOT_FOUND") || strings.Contains(msg, "NOT FOUND") } + +// 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. +func isCheckoutDefinitivelyDead(err error) bool { + if err == nil || errors.Is(err, square.ErrCheckoutPending) { + return false + } + msg := strings.ToUpper(err.Error()) + if !strings.Contains(msg, "CANCELED") && !strings.Contains(msg, "NOT_FOUND") && !strings.Contains(msg, "NOT FOUND") { + return false + } + if strings.Contains(msg, "CANCEL_REQUESTED") && !strings.Contains(msg, "CANCELED") { + return false + } + return true +} + +// clawBackTillSaleFunding reverts the gift-card funding of a till sale whose +// card-machine checkout is provably dead, marking the sale failed at the same +// time. The terminal-checkout sweep's rows carry no gift-card context, so the +// sale's item/amount/redeem target are looked up here (is_create via the +// created_at equality with the gift card) and handed to the claim-first +// revertGiftCardFunding. A sale with no gift card (future retail product / +// orphaned item) is only marked failed. Returns true when the sale was +// resolved to failed; false when it was already resolved by someone else, had +// no gift card, or the clawback failed (CRITICAL logged). +func clawBackTillSaleFunding(ctx context.Context, tillSaleID string) bool { + var itemType string + var itemID sql.NullString + var totalAmount float64 + var redeemedBy sql.NullString + var isCreate *bool + err := db.Conn.QueryRow(ctx, ` + SELECT ts.item_type, ts.item_id, ts.total_amount, gc.redeemed_by, + (ts.created_at = gc.created_at) AS is_create + FROM till_sales ts + LEFT JOIN gift_cards gc ON gc.id = ts.item_id + WHERE ts.id = $1 + `, tillSaleID).Scan(&itemType, &itemID, &totalAmount, &redeemedBy, &isCreate) + if err != nil { + log.Printf("CRITICAL: failed to look up till sale %s for funding clawback: %v — MANUAL RECONCILIATION REQUIRED", tillSaleID, err) + return false + } + if itemType != "gift_card" || !itemID.Valid || itemID.String == "" || isCreate == nil { + // No gift card to claw back — mark the sale failed without touching + // any card. + tag, upErr := db.Conn.Exec(ctx, ` + UPDATE till_sales SET status = 'failed', updated_at = NOW() + WHERE id = $1 AND status = 'pending' + `, tillSaleID) + if upErr != nil { + log.Printf("Failed to mark till sale %s failed: %v", tillSaleID, upErr) + return false + } + return int(tag.RowsAffected()) > 0 + } + action := "topup" + if *isCreate { + action = "create" + } + var redeem *string + if redeemedBy.Valid && redeemedBy.String != "" { + redeem = &redeemedBy.String + } + if revErr := revertGiftCardFunding(ctx, action, itemID.String, totalAmount, redeem, tillSaleID); revErr != nil { + if errors.Is(revErr, errTillSaleNotPending) { + log.Printf("Till sale %s was already resolved (not pending) — skipping funding clawback", tillSaleID) + return false + } + log.Printf("CRITICAL: funding clawback for till sale %s (gift card %s) failed: %v — MANUAL RECONCILIATION REQUIRED: gift card may still be funded", tillSaleID, itemID.String, revErr) + return false + } + return true +} diff --git a/backend/handlers/payments/sweep_test.go b/backend/handlers/payments/sweep_test.go index 861e293..5206e1f 100644 --- a/backend/handlers/payments/sweep_test.go +++ b/backend/handlers/payments/sweep_test.go @@ -563,3 +563,505 @@ func TestSweepStaleTerminalCheckouts_CompletedDuringCancel_MarkedCompleted(t *te t.Errorf("expected a checkout that completed during the cancel window marked 'COMPLETED', got %q", status) } } + +// ============================================================================= +// Sweep clawback — funded gift cards are reverted when the sale is provably +// dead (HIGH-1) +// ============================================================================= + +// seedStaleTillSaleWithCard seeds a stale pending till_sale (created 25h ago, +// past the 24h stale cutoff) with its gift card inside the caller's setup +// transaction. isCreate=true seeds the gift card with the SAME created_at +// timestamp so the sweep's created_at-equality discriminates a create (a real +// create sets both timestamps to the transaction-start NOW()); isCreate=false +// predates the card so the sweep treats the sale as a topup. Returns the sale +// and gift-card ids plus a pool-level cleanup closure. +func seedStaleTillSaleWithCard(t *testing.T, ctx context.Context, q db.Querier, adminID string, saleAmount float64, squarePaymentID string, isCreate bool) (saleID, giftCardID string) { + t.Helper() + pool := context.Background() + + gcAge := "NOW() - INTERVAL '30 hours'" + if isCreate { + // Identical to the sale's created_at — provably a create. + gcAge = "NOW() - INTERVAL '25 hours'" + } + if err := q.QueryRow(ctx, ` + INSERT INTO gift_cards (total_funds_added, amount_remaining, created_by, created_at) + VALUES ($1, $1, $2, `+gcAge+`) + RETURNING id + `, saleAmount, adminID).Scan(&giftCardID); err != nil { + t.Fatalf("failed to seed gift card: %v", err) + } + + sqParam := any(squarePaymentID) + if squarePaymentID == "" { + sqParam = nil + } + if err := q.QueryRow(ctx, ` + INSERT INTO till_sales (item_type, item_id, description, quantity, unit_price, total_amount, + payment_method, status, square_payment_id, created_by, created_at, updated_at) + VALUES ('gift_card', $1, 'Gift Card', 1, $2, $2, 'online_square', 'pending', $3, $4, + NOW() - INTERVAL '25 hours', NOW()) + RETURNING id + `, giftCardID, saleAmount, sqParam, adminID).Scan(&saleID); err != nil { + t.Fatalf("failed to seed stale pending till sale: %v", err) + } + + t.Cleanup(func() { + _, _ = db.Conn.Exec(pool, `DELETE FROM gift_card_transactions WHERE gift_card_id = $1`, giftCardID) + _, _ = db.Conn.Exec(pool, `DELETE FROM till_sales WHERE id = $1`, saleID) + _, _ = db.Conn.Exec(pool, `DELETE FROM gift_cards WHERE id = $1`, giftCardID) + _, _ = db.Conn.Exec(pool, `DELETE FROM users WHERE id = $1`, adminID) + }) + return saleID, giftCardID +} + +// TestSweepStalePendingPayments_TillCreateWithRedeem_Clawbacks locks the +// HIGH-1 create-with-redeem clawback: a stale pending till sale whose Square +// charge provably never completed (NOT_FOUND reconcile) is marked failed and +// its created gift card is DELETED while the user's redeemed balance is +// reversed back to zero. +func TestSweepStalePendingPayments_TillCreateWithRedeem_Clawbacks(t *testing.T) { + ctx, tx := testutils.SetupTestTx(t) + + adminID, err := fixtures.CreateTestAdminUser(tx) + if err != nil { + t.Fatalf("failed to create admin user: %v", err) + } + redeemerID, err := fixtures.CreateTestUser(tx) + if err != nil { + t.Fatalf("failed to create redeemer user: %v", err) + } + pool := context.Background() + t.Cleanup(func() { + _, _ = db.Conn.Exec(pool, `DELETE FROM user_giftcard_balances WHERE user_id = $1`, redeemerID) + _, _ = db.Conn.Exec(pool, `DELETE FROM users WHERE id = $1`, redeemerID) + }) + + saleID, giftCardID := seedStaleTillSaleWithCard(t, ctx, tx, adminID, 50.00, "sqp_till_clawback_create", true) + + // The card was redeemed to the user's account balance in the same sale. + if _, err := tx.Exec(ctx, ` + UPDATE gift_cards SET redeemed_by = $1, amount_remaining = 0.00 WHERE id = $2 + `, redeemerID, giftCardID); err != nil { + t.Fatalf("failed to mark gift card redeemed: %v", err) + } + if _, err := tx.Exec(ctx, ` + INSERT INTO user_giftcard_balances (user_id, balance, updated_at) + VALUES ($1, 50.00, NOW()) + `, redeemerID); err != nil { + t.Fatalf("failed to seed user gift card balance: %v", err) + } + if _, err := tx.Exec(ctx, ` + INSERT INTO gift_card_transactions (gift_card_id, transaction_type, amount, reference_type, reference_id) + VALUES ($1, 'purchase', 50.00, 'till_sale', $2) + `, giftCardID, saleID); err != nil { + t.Fatalf("failed to seed gift card transaction: %v", err) + } + + origClient := SquareClient + SquareClient = &staleGetPaymentClient{SquareClient: square.NewDevClient(), + err: fmt.Errorf("square: GET /v2/payments/sqp_till_clawback_create: [PAYMENT_NOT_FOUND/NOT_FOUND] payment does not exist")} + defer func() { SquareClient = origClient }() + + 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 setup tx: %v", err) + } + + if _, err := SweepStalePendingPayments(pool); err != nil { + t.Fatalf("sweep failed: %v", err) + } + + var status string + if err := db.Conn.QueryRow(pool, `SELECT status FROM till_sales WHERE id = $1`, saleID).Scan(&status); err != nil { + t.Fatalf("failed to query till sale: %v", err) + } + if status != "failed" { + t.Errorf("expected stale pending till sale marked failed after clawback, got %q", status) + } + + // The created card must be GONE. + var cardCount int + if err := db.Conn.QueryRow(pool, `SELECT COUNT(*) FROM gift_cards WHERE id = $1`, giftCardID).Scan(&cardCount); err != nil { + t.Fatalf("failed to count gift cards: %v", err) + } + if cardCount != 0 { + t.Errorf("expected the clawed-back created gift card deleted, got %d cards", cardCount) + } + + // The redeemed balance must be reversed to zero. + var balance float64 + if err := db.Conn.QueryRow(pool, `SELECT balance FROM user_giftcard_balances WHERE user_id = $1`, redeemerID).Scan(&balance); err != nil { + t.Fatalf("failed to query redeemed balance: %v", err) + } + if balance != 0.00 { + t.Errorf("expected redeemed balance reversed to 0.00, got %.2f", balance) + } +} + +// TestSweepStalePendingPayments_TillTopup_ClawbacksAmount_KeepsCard locks the +// HIGH-1 top-up clawback AND the is_create discrimination: a stale pending +// top-up sale whose charge provably never completed is failed and its top-up +// amount subtracted back out of the PRE-EXISTING card (which must NOT be +// deleted — is_create is false because the card predates the sale), and only +// this sale's top-up transaction is removed. +func TestSweepStalePendingPayments_TillTopup_ClawbacksAmount_KeepsCard(t *testing.T) { + ctx, tx := testutils.SetupTestTx(t) + + adminID, err := fixtures.CreateTestAdminUser(tx) + if err != nil { + t.Fatalf("failed to create admin user: %v", err) + } + pool := context.Background() + + saleID, giftCardID := seedStaleTillSaleWithCard(t, ctx, tx, adminID, 50.00, "sqp_till_clawback_topup", false) + + // The pre-existing card holds £100 (was topped up £50 by this sale). + if _, err := tx.Exec(ctx, ` + UPDATE gift_cards SET total_funds_added = 100.00, amount_remaining = 100.00 WHERE id = $1 + `, giftCardID); err != nil { + t.Fatalf("failed to set card balance: %v", err) + } + if _, err := tx.Exec(ctx, ` + INSERT INTO gift_card_transactions (gift_card_id, transaction_type, amount, reference_type, reference_id) + VALUES ($1, 'topup', 50.00, 'till_sale', $2) + `, giftCardID, saleID); err != nil { + t.Fatalf("failed to seed gift card transaction: %v", err) + } + + origClient := SquareClient + SquareClient = &staleGetPaymentClient{SquareClient: square.NewDevClient(), + err: fmt.Errorf("square: GET /v2/payments/sqp_till_clawback_topup: [PAYMENT_NOT_FOUND/NOT_FOUND] payment does not exist")} + defer func() { SquareClient = origClient }() + + 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 setup tx: %v", err) + } + + if _, err := SweepStalePendingPayments(pool); err != nil { + t.Fatalf("sweep failed: %v", err) + } + + var status string + if err := db.Conn.QueryRow(pool, `SELECT status FROM till_sales WHERE id = $1`, saleID).Scan(&status); err != nil { + t.Fatalf("failed to query till sale: %v", err) + } + if status != "failed" { + t.Errorf("expected stale pending till sale marked failed after top-up clawback, got %q", status) + } + + // The PRE-EXISTING card must survive (is_create discrimination) with the + // £50 top-up subtracted back out. + var totalAdded, remaining float64 + if err := db.Conn.QueryRow(pool, `SELECT total_funds_added, amount_remaining FROM gift_cards WHERE id = $1`, giftCardID).Scan(&totalAdded, &remaining); err != nil { + t.Fatalf("failed to query gift card: %v", err) + } + if totalAdded != 50.00 || remaining != 50.00 { + t.Errorf("expected top-up reversed out (100.00 -> 50.00), got total_funds_added=%.2f amount_remaining=%.2f", totalAdded, remaining) + } + + // This sale's top-up transaction must be removed. + var txCount int + if err := db.Conn.QueryRow(pool, `SELECT COUNT(*) FROM gift_card_transactions WHERE gift_card_id = $1 AND reference_type = 'till_sale' AND reference_id = $2`, giftCardID, saleID).Scan(&txCount); err != nil { + t.Fatalf("failed to count gift card transactions: %v", err) + } + if txCount != 0 { + t.Errorf("expected this sale's top-up transaction removed, got %d", txCount) + } +} + +// TestSweepStalePendingPayments_TillAmbiguous_NoClawback locks the HIGH-1 +// MUST-NOT: an ambiguous Square reconcile (transport error) leaves the sale +// pending AND the funded gift card untouched — the charge may still complete. +func TestSweepStalePendingPayments_TillAmbiguous_NoClawback(t *testing.T) { + ctx, tx := testutils.SetupTestTx(t) + + adminID, err := fixtures.CreateTestAdminUser(tx) + if err != nil { + t.Fatalf("failed to create admin user: %v", err) + } + pool := context.Background() + + saleID, giftCardID := seedStaleTillSaleWithCard(t, ctx, tx, adminID, 50.00, "sqp_till_ambiguous", true) + + origClient := SquareClient + SquareClient = &staleGetPaymentClient{SquareClient: square.NewDevClient(), err: fmt.Errorf("network error: connection reset by peer")} + defer func() { SquareClient = origClient }() + + 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 setup tx: %v", err) + } + + if _, err := SweepStalePendingPayments(pool); err != nil { + t.Fatalf("sweep failed: %v", err) + } + + var status string + if err := db.Conn.QueryRow(pool, `SELECT status FROM till_sales WHERE id = $1`, saleID).Scan(&status); err != nil { + t.Fatalf("failed to query till sale: %v", err) + } + if status != "pending" { + t.Errorf("expected ambiguous reconcile to leave the sale pending, got %q", status) + } + + // The funded card must be untouched (still exists, fully funded). + var cardCount int + var remaining float64 + if err := db.Conn.QueryRow(pool, `SELECT COUNT(*), COALESCE(MAX(amount_remaining), 0) FROM gift_cards WHERE id = $1`, giftCardID).Scan(&cardCount, &remaining); err != nil { + t.Fatalf("failed to query gift card: %v", err) + } + if cardCount != 1 || remaining != 50.00 { + t.Errorf("expected card untouched after ambiguous reconcile, got count=%d remaining=%.2f", cardCount, remaining) + } +} + +// TestSweepStalePendingPayments_TillBlindFail_NoClawback locks the HIGH-1 +// MUST-NOT: a stale pending till sale with NO square_payment_id (lost response) +// is marked failed WITHOUT clawing back — the charge may have landed at Square +// and the funding must stay put. +func TestSweepStalePendingPayments_TillBlindFail_NoClawback(t *testing.T) { + ctx, tx := testutils.SetupTestTx(t) + + adminID, err := fixtures.CreateTestAdminUser(tx) + if err != nil { + t.Fatalf("failed to create admin user: %v", err) + } + pool := context.Background() + + saleID, giftCardID := seedStaleTillSaleWithCard(t, ctx, tx, adminID, 50.00, "", true) + + origClient := SquareClient + SquareClient = square.NewDevClient() + defer func() { SquareClient = origClient }() + + 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 setup tx: %v", err) + } + + if _, err := SweepStalePendingPayments(pool); err != nil { + t.Fatalf("sweep failed: %v", err) + } + + var status string + if err := db.Conn.QueryRow(pool, `SELECT status FROM till_sales WHERE id = $1`, saleID).Scan(&status); err != nil { + t.Fatalf("failed to query till sale: %v", err) + } + if status != "failed" { + t.Errorf("expected blind-failed till sale marked failed, got %q", status) + } + + // The funded card must be untouched (charge outcome unknown). + var cardCount int + if err := db.Conn.QueryRow(pool, `SELECT COUNT(*) FROM gift_cards WHERE id = $1`, giftCardID).Scan(&cardCount); err != nil { + t.Fatalf("failed to count gift cards: %v", err) + } + if cardCount != 1 { + t.Errorf("expected blind-fail to leave the funded card in place, got %d cards", cardCount) + } +} + +// terminalErrorClient forces GetCheckout to return a fixed error for the target +// checkout while delegating everything else to the real mock — used to exercise +// the terminal sweep's definitively-dead vs CANCEL_REQUESTED-only branches. +type terminalErrorClient struct { + square.SquareClient + checkoutID string + err error +} + +func (c *terminalErrorClient) GetCheckout(ctx context.Context, checkoutID string) (*square.PaymentResult, error) { + if checkoutID == c.checkoutID { + return nil, c.err + } + return c.SquareClient.GetCheckout(ctx, checkoutID) +} + +// seedStaleTerminalTillSale seeds a stale pending card-machine till sale (with +// square_checkout_id, created 2h ago past the 1h terminal cutoff) and a funded +// gift card, returning ids and a pool-level cleanup closure. +func seedStaleTerminalTillSale(t *testing.T, ctx context.Context, q db.Querier, adminID string, isCreate bool) (saleID, giftCardID string) { + t.Helper() + pool := context.Background() + + gcAge := "NOW() - INTERVAL '3 hours'" + if isCreate { + gcAge = "NOW() - INTERVAL '2 hours'" + } + if err := q.QueryRow(ctx, ` + INSERT INTO gift_cards (total_funds_added, amount_remaining, created_by, created_at) + VALUES (50.00, 50.00, $1, `+gcAge+`) + RETURNING id + `, adminID).Scan(&giftCardID); err != nil { + t.Fatalf("failed to seed gift card: %v", err) + } + checkoutID := "chk_stale_terminal_till" + if err := q.QueryRow(ctx, ` + INSERT INTO till_sales (item_type, item_id, description, quantity, unit_price, total_amount, + payment_method, status, square_checkout_id, created_by, created_at, updated_at) + VALUES ('gift_card', $1, 'Gift Card create', 1, 50.00, 50.00, 'in_person_card', 'pending', $2, $3, + NOW() - INTERVAL '2 hours', NOW()) + RETURNING id + `, giftCardID, checkoutID, adminID).Scan(&saleID); err != nil { + t.Fatalf("failed to seed stale terminal till sale: %v", err) + } + + t.Cleanup(func() { + _, _ = db.Conn.Exec(pool, `DELETE FROM gift_card_transactions WHERE gift_card_id = $1`, giftCardID) + _, _ = db.Conn.Exec(pool, `DELETE FROM till_sales WHERE id = $1`, saleID) + _, _ = db.Conn.Exec(pool, `DELETE FROM gift_cards WHERE id = $1`, giftCardID) + _, _ = db.Conn.Exec(pool, `DELETE FROM users WHERE id = $1`, adminID) + }) + return saleID, giftCardID +} + +// TestSweepStaleTerminalCheckouts_TillDefinitivelyCanceled_Clawbacks locks the +// HIGH-1 terminal clawback: a till-sale checkout that reports CANCELED at +// Square is provably dead, so the sale is failed AND the funded gift card is +// clawed back (deleted for a create). +func TestSweepStaleTerminalCheckouts_TillDefinitivelyCanceled_Clawbacks(t *testing.T) { + ctx, tx := testutils.SetupTestTx(t) + + adminID, err := fixtures.CreateTestAdminUser(tx) + if err != nil { + t.Fatalf("failed to create admin user: %v", err) + } + pool := context.Background() + + const checkoutID = "chk_till_definitively_canceled" + saleID, giftCardID := seedStaleTerminalTillSale(t, ctx, tx, adminID, true) + if _, err := tx.Exec(ctx, `UPDATE till_sales SET square_checkout_id = $1 WHERE id = $2`, checkoutID, saleID); err != nil { + t.Fatalf("failed to set checkout id: %v", err) + } + if _, err := tx.Exec(ctx, ` + INSERT INTO gift_card_transactions (gift_card_id, transaction_type, amount, reference_type, reference_id) + VALUES ($1, 'purchase', 50.00, 'till_sale', $2) + `, giftCardID, saleID); err != nil { + t.Fatalf("failed to seed gift card transaction: %v", err) + } + + origClient := SquareClient + SquareClient = &terminalErrorClient{SquareClient: square.NewDevClient(), checkoutID: checkoutID, + err: fmt.Errorf("square: checkout %s is CANCELED (not COMPLETED)", checkoutID)} + defer func() { SquareClient = origClient }() + + 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 setup tx: %v", err) + } + + // Drop any other stale terminal rows left by parallel tests so the count is + // deterministic. + if _, err := db.Conn.Exec(pool, `DELETE FROM till_sales WHERE status = 'pending' AND square_checkout_id IS NOT NULL AND id <> $1`, saleID); err != nil { + t.Fatalf("failed to clean leftover stale terminal sales: %v", err) + } + if _, err := db.Conn.Exec(pool, `DELETE FROM terminal_checkouts WHERE status IN ('PENDING', 'IN_PROGRESS')`); err != nil { + t.Fatalf("failed to clean leftover stale booking terminal checkouts: %v", err) + } + + n, err := SweepStaleTerminalCheckouts(pool) + if err != nil { + t.Fatalf("sweep failed: %v", err) + } + if n != 1 { + t.Errorf("expected exactly 1 resolved stale terminal checkout, got %d", n) + } + + var status string + if err := db.Conn.QueryRow(pool, `SELECT status FROM till_sales WHERE id = $1`, saleID).Scan(&status); err != nil { + t.Fatalf("failed to query till sale: %v", err) + } + if status != "failed" { + t.Errorf("expected definitively-cancelled till sale marked failed, got %q", status) + } + + // The created gift card must have been clawed back (deleted). + var cardCount int + if err := db.Conn.QueryRow(pool, `SELECT COUNT(*) FROM gift_cards WHERE id = $1`, giftCardID).Scan(&cardCount); err != nil { + t.Fatalf("failed to count gift cards: %v", err) + } + if cardCount != 0 { + t.Errorf("expected the clawed-back created gift card deleted, got %d cards", cardCount) + } +} + +// TestSweepStaleTerminalCheckouts_TillCancelRequested_NoClawback locks the +// HIGH-1 MUST-NOT: a checkout that reports only CANCEL_REQUESTED is NOT +// provably dead (Square does not promise non-completion), so the sale is +// marked failed WITHOUT clawing back the funded gift card (CRITICAL logged for +// manual reconciliation). +func TestSweepStaleTerminalCheckouts_TillCancelRequested_NoClawback(t *testing.T) { + ctx, tx := testutils.SetupTestTx(t) + + adminID, err := fixtures.CreateTestAdminUser(tx) + if err != nil { + t.Fatalf("failed to create admin user: %v", err) + } + pool := context.Background() + + const checkoutID = "chk_till_cancel_requested" + saleID, giftCardID := seedStaleTerminalTillSale(t, ctx, tx, adminID, true) + if _, err := tx.Exec(ctx, `UPDATE till_sales SET square_checkout_id = $1 WHERE id = $2`, checkoutID, saleID); err != nil { + t.Fatalf("failed to set checkout id: %v", err) + } + + origClient := SquareClient + SquareClient = &terminalErrorClient{SquareClient: square.NewDevClient(), checkoutID: checkoutID, + err: fmt.Errorf("square: checkout %s is CANCEL_REQUESTED", checkoutID)} + defer func() { SquareClient = origClient }() + + 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 setup tx: %v", err) + } + + if _, err := db.Conn.Exec(pool, `DELETE FROM till_sales WHERE status = 'pending' AND square_checkout_id IS NOT NULL AND id <> $1`, saleID); err != nil { + t.Fatalf("failed to clean leftover stale terminal sales: %v", err) + } + if _, err := db.Conn.Exec(pool, `DELETE FROM terminal_checkouts WHERE status IN ('PENDING', 'IN_PROGRESS')`); err != nil { + t.Fatalf("failed to clean leftover stale booking terminal checkouts: %v", err) + } + + if _, err := SweepStaleTerminalCheckouts(pool); err != nil { + t.Fatalf("sweep failed: %v", err) + } + + var status string + if err := db.Conn.QueryRow(pool, `SELECT status FROM till_sales WHERE id = $1`, saleID).Scan(&status); err != nil { + t.Fatalf("failed to query till sale: %v", err) + } + if status != "failed" { + t.Errorf("expected CANCEL_REQUESTED-only till sale marked failed, got %q", status) + } + + // The funded gift card must NOT have been clawed back. + var cardCount int + var remaining float64 + if err := db.Conn.QueryRow(pool, `SELECT COUNT(*), COALESCE(MAX(amount_remaining), 0) FROM gift_cards WHERE id = $1`, giftCardID).Scan(&cardCount, &remaining); err != nil { + t.Fatalf("failed to query gift card: %v", err) + } + if cardCount != 1 || remaining != 50.00 { + t.Errorf("expected card untouched after CANCEL_REQUESTED-only, got count=%d remaining=%.2f", cardCount, remaining) + } +} diff --git a/backend/handlers/payments/till.go b/backend/handlers/payments/till.go index 6905a68..c5e80fe 100644 --- a/backend/handlers/payments/till.go +++ b/backend/handlers/payments/till.go @@ -89,14 +89,20 @@ func isDefinitiveChargeFailure(err error) bool { 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") + // revertGiftCardFunding undoes the gift-card funding performed earlier in the // SAME till-sale request after a definitive Square charge rejection, matching // the gift_card_transactions accounting: a created card is deleted (with its // purchase transaction) and any immediate redeem-to-account credit reversed; a // topped-up card has the amount subtracted back out and its top-up transaction -// removed. The till sale is marked 'failed' in the same compensating -// transaction so a late same-key retry cannot re-complete a sale whose gift -// card no longer exists. +// removed. The clawback is claim-first: it atomically claims the till sale +// with a gating `status='pending'` UPDATE whose row lock serializes against +// the handler's completion UPDATE, then runs the card mutation + failed-mark +// in the same transaction so a late same-key retry cannot re-complete a sale +// whose gift card no longer exists. func revertGiftCardFunding(ctx context.Context, action, giftCardID string, amount float64, redeemToUserID *string, tillSaleID string) error { tx, err := db.Conn.Begin(ctx) if err != nil { @@ -108,6 +114,18 @@ func revertGiftCardFunding(ctx context.Context, action, giftCardID string, amoun } }() + // Claim the sale first: the row lock serializes against the handler's + // completion UPDATE; a zero-row claim means the funding is not ours. + tag, err := tx.Exec(ctx, ` + UPDATE till_sales SET status = 'failed', updated_at = NOW() + WHERE id = $1 AND status = 'pending'`, tillSaleID) + if err != nil { + return fmt.Errorf("failed to claim till sale for clawback: %w", err) + } + if tag.RowsAffected() == 0 { + return errTillSaleNotPending + } + if action == "create" { // A newly created card has exactly one funding transaction (this // request's purchase) — remove it, then the card itself. @@ -158,12 +176,6 @@ func revertGiftCardFunding(ctx context.Context, action, giftCardID string, amoun } } - if _, err := tx.Exec(ctx, ` - UPDATE till_sales SET status = 'failed', updated_at = NOW() WHERE id = $1 AND status = 'pending' - `, tillSaleID); err != nil { - return fmt.Errorf("failed to mark till sale failed after clawback: %w", err) - } - if err := tx.Commit(ctx); err != nil { return fmt.Errorf("failed to commit clawback transaction: %w", err) } @@ -323,13 +335,34 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) { service := NewPaymentService() + // squarePaymentID/squareCheckoutID are set inside the payment-method switch + // below but must be declared before the deferred cancel-on-error closure + // (registered at tx creation) so it can read the live checkout id. + var squarePaymentID *string + var squareCheckoutID *string + tx, err := db.Conn.Begin(ctx) if err != nil { log.Printf("Failed to begin transaction: %v", err) http.Error(w, "internal server error", http.StatusInternalServerError) return } + // Cancel-on-error for an orphaned live checkout: a card_machine checkout + // created at Square inside this tx is cancelled if the request fails before + // commit (e.g. the till_sales INSERT dies on the idempotency_key UNIQUE + // constraint) — otherwise the checkout stays live at the terminal as an + // invisible, untracked charge. The pending-retry reuse branch never sets + // checkoutCreated (it reuses a live checkout and must never cancel it), + // and committed=true after a successful commit means a committed sale's + // checkout is never cancelled here. + var checkoutCreated bool + committed := false defer func() { + if !committed && checkoutCreated && squareCheckoutID != nil { + if cErr := SquareClient.CancelCheckout(context.Background(), *squareCheckoutID); cErr != nil { + log.Printf("CRITICAL: till checkout %s created at Square but request failed pre-commit; cancel failed: %v — MANUAL RECONCILIATION REQUIRED", *squareCheckoutID, cErr) + } + } if err := tx.Rollback(ctx); err != nil && !errors.Is(err, pgx.ErrTxClosed) { slog.Error("failed to rollback transaction", "err", err) } @@ -469,8 +502,6 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) { penceAmount := int64(math.Round(req.Amount * 100)) - var squarePaymentID *string - var squareCheckoutID *string var saleStatus string var dbPaymentMethod string @@ -503,12 +534,60 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) { // original checkout is still live — the customer could be charged at the // terminal AND by the new method (double charge). The terminal checkout // cannot be cancelled via this API, so reject the switch outright. + // NOTE: a future provisional "tmp-" till_sales row (pre-Square, as the + // booking path uses) must be treated as "no real checkout" by BOTH this + // reuse and the method-switch guard — a "tmp-" id is provably not live at + // Square. if existingPendingID != "" && existingPendingCheckoutID != "" && req.PaymentMethod != "card_machine" { log.Printf("Till-sale retry rejected: pending sale %s has a live card-machine checkout, cannot switch method from card_machine to %s", existingPendingID, req.PaymentMethod) http.Error(w, "This pending sale is tied to a live card-machine checkout — retry with card machine payment", http.StatusConflict) return } + // Reconcile-or-reject on a cash/on_the_house retry of a pending card sale. + // The gift card was funded by a Square charge whose outcome is unknown; a + // definitive answer from Square decides whether cash may be taken. This + // MUST run before the method switch below so a lost-response sale (no + // square_payment_id) forces the original card-method retry instead of + // taking cash on top of a charge that may have landed. + if existingPendingID != "" && (req.PaymentMethod == "cash" || req.PaymentMethod == "on_the_house") { + var sqPaymentID string + if err := tx.QueryRow(ctx, `SELECT COALESCE(square_payment_id, '') FROM till_sales WHERE id = $1`, + existingPendingID).Scan(&sqPaymentID); err != nil { + log.Printf("Failed to query pending sale %s square_payment_id: %v", existingPendingID, err) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + + if sqPaymentID == "" { + // Lost response: the charge may have landed at Square and cannot + // be looked up. Taking cash risks a double payment. Force the + // original card-method retry — Square dedups on the same + // idempotency key and resolves the lost response. + http.Error(w, "Original card charge outcome is unknown — retry with the original card method", http.StatusConflict) + return + } + + pr, gErr := SquareClient.GetPayment(ctx, sqPaymentID) + switch { + case gErr == nil && pr.Status == "COMPLETED": + // Money landed. Rescue the sale, refuse the cash. + if _, upErr := db.Conn.Exec(ctx, `UPDATE till_sales SET status='completed', updated_at=NOW() WHERE id=$1 AND status='pending'`, existingPendingID); upErr != nil { + log.Printf("CRITICAL: card payment %s for pending till sale %s is COMPLETED but the rescue UPDATE failed: %v — MANUAL RECONCILIATION REQUIRED", sqPaymentID, existingPendingID, upErr) + } + http.Error(w, "This sale was already paid by card — do not take cash", http.StatusConflict) + return + case squarePaymentErrorIsNotFound(gErr), gErr == nil && (pr.Status == "FAILED" || pr.Status == "CANCELED"): + // Provably no money landed — cash is safe; fall through. + case gErr != nil: + http.Error(w, "Unable to confirm the card payment status — try again", http.StatusServiceUnavailable) + return + default: + http.Error(w, "Card charge status not definitively failed — do not take cash", http.StatusConflict) + return + } + } + switch req.PaymentMethod { case "cash": saleStatus = "completed" @@ -606,6 +685,7 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) { } squareCheckoutID = &checkout.ID + checkoutCreated = true saleStatus = "pending" } case "online_square": @@ -688,6 +768,7 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) { http.Error(w, "internal server error", http.StatusInternalServerError) return } + committed = true // Step 2: DB transaction committed — safe to call Square now. // If Square fails, the till_sale record stays 'pending' for manual retry. @@ -794,7 +875,7 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) { // 'completed' explicitly — otherwise the row stays pending forever while // the response claims success. if existingPendingID != "" && !needsSquarePayment && req.PaymentMethod != "card_machine" { - _, upErr := db.Conn.Exec(ctx, + tag, upErr := db.Conn.Exec(ctx, `UPDATE till_sales SET status = 'completed', updated_at = NOW() WHERE id = $1 AND status = 'pending'`, tillSaleID, ) @@ -803,6 +884,13 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) { http.Error(w, "internal server error", http.StatusInternalServerError) return } + if tag.RowsAffected() == 0 { + // The sweep failed (or a clawback reverted the gift card) between + // this retry's read and this write — the cash must not be taken. + log.Printf("Pending till sale %s was already resolved before the %s retry could complete it", tillSaleID, req.PaymentMethod) + http.Error(w, "This sale was already resolved — do not take cash", http.StatusConflict) + return + } saleStatus = "completed" } @@ -915,18 +1003,26 @@ func GetTillCheckoutStatus(w http.ResponseWriter, r *http.Request) { } }() - _, err = tx.Exec(r.Context(), ` + tag, err := tx.Exec(r.Context(), ` UPDATE till_sales SET status = 'completed', square_payment_id = $1, updated_at = NOW() - WHERE id = $2 + WHERE id = $2 AND status = 'pending' `, paymentResult.SquarePayID, tillSaleID) if err != nil { log.Printf("Failed to update till sale: %v", err) http.Error(w, "internal server error", http.StatusInternalServerError) return } + if tag.RowsAffected() == 0 { + // The sweep already failed this sale (or a clawback reverted the + // gift card) — completing it now would resurrect a sale whose card + // no longer exists. Fail loudly for manual reconciliation. + log.Printf("CRITICAL: checkout %s reports COMPLETED but till sale %s is no longer pending — refusing to complete; MANUAL RECONCILIATION REQUIRED", checkoutID, tillSaleID) + http.Error(w, "Till sale no longer pending — manual reconciliation required", http.StatusNotFound) + return + } ApplyVATToTillSale(r.Context(), tx, tillSaleID) if err := tx.Commit(r.Context()); err != nil { diff --git a/backend/handlers/payments/till_test.go b/backend/handlers/payments/till_test.go index d227795..15c6790 100644 --- a/backend/handlers/payments/till_test.go +++ b/backend/handlers/payments/till_test.go @@ -6,6 +6,8 @@ import ( "bytes" "context" "encoding/json" + "errors" + "fmt" "net/http" "net/http/httptest" "testing" @@ -1422,11 +1424,12 @@ func TestCreateTillSale_PendingRetry_AmountMismatch_Rejected(t *testing.T) { } // TestCreateTillSale_PendingRetry_Cash_CompletesRow verifies that a same-key -// retry resolved by cash on a pending sale explicitly flips the row to -// 'completed' — previously the response claimed success while the DB row stayed -// pending forever (unreconciled). +// retry resolved by cash on a pending sale whose Square charge PROVABLY failed +// (HIGH-3: a FAILED status means no money landed, so cash is safe) explicitly +// flips the row to 'completed' — previously the response claimed success while +// the DB row stayed pending forever (unreconciled). func TestCreateTillSale_PendingRetry_Cash_CompletesRow(t *testing.T) { - t.Parallel() + // Not parallel: swaps the package-global SquareClient. ctx, tx := testutils.SetupTestTx(t) adminID, err := fixtures.CreateTestAdminUser(tx) @@ -1435,7 +1438,8 @@ func TestCreateTillSale_PendingRetry_Cash_CompletesRow(t *testing.T) { } adminToken := jwt.GenerateTestToken(adminID, "admin") - // Seed a PENDING till_sale (prior online_square attempt failed post-commit). + // Seed a PENDING till_sale (prior online_square attempt failed post-commit) + // whose Square charge is recorded as FAILED — provably no money landed. key := "till-pending-cash-retry-key" var giftCardID string err = tx.QueryRow(ctx, ` @@ -1448,14 +1452,18 @@ func TestCreateTillSale_PendingRetry_Cash_CompletesRow(t *testing.T) { } _, err = tx.Exec(ctx, ` INSERT INTO till_sales (item_type, item_id, description, quantity, unit_price, total_amount, - payment_method, status, user_id, idempotency_key, created_by, created_at, updated_at) + payment_method, status, user_id, square_payment_id, idempotency_key, created_by, created_at, updated_at) VALUES ('gift_card', $1, 'Gift Card create', 1, 50.00, 50.00, 'online_square', 'pending', - NULL, $2, $3, NOW(), NOW()) + NULL, 'sqp_cash_retry_failed', $2, $3, NOW(), NOW()) `, giftCardID, key, adminID) if err != nil { t.Fatalf("failed to seed pending till sale: %v", err) } + origClient := SquareClient + SquareClient = &staleGetPaymentClient{SquareClient: square.NewDevClient(), result: &square.PaymentResult{Status: "FAILED", SquarePayID: "sqp_cash_retry_failed"}} + defer func() { SquareClient = origClient }() + // Retry with the same key, same amount, cash payment. reqBody := TillSaleRequest{ ItemType: "gift_card", @@ -1495,6 +1503,376 @@ func TestCreateTillSale_PendingRetry_Cash_CompletesRow(t *testing.T) { } } +// TestCreateTillSale_PendingRetry_Cash_NotFound_CompletesRow verifies the +// HIGH-3 NOT_FOUND branch: a pending sale whose square_payment_id does NOT +// resolve at Square is provably never charged, so cash may complete it. +func TestCreateTillSale_PendingRetry_Cash_NotFound_CompletesRow(t *testing.T) { + // Not parallel: swaps the package-global SquareClient. + 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") + + key := "till-pending-cash-notfound-key" + var giftCardID string + 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) + if err != nil { + t.Fatalf("failed to create gift card: %v", err) + } + _, err = tx.Exec(ctx, ` + INSERT INTO till_sales (item_type, item_id, description, quantity, unit_price, total_amount, + payment_method, status, user_id, square_payment_id, idempotency_key, created_by, created_at, updated_at) + VALUES ('gift_card', $1, 'Gift Card create', 1, 50.00, 50.00, 'online_square', 'pending', + NULL, 'sqp_cash_retry_not_found', $2, $3, NOW(), NOW()) + `, giftCardID, key, adminID) + if err != nil { + t.Fatalf("failed to seed pending till sale: %v", err) + } + + origClient := SquareClient + SquareClient = &staleGetPaymentClient{SquareClient: square.NewDevClient(), + err: fmt.Errorf("square: GET /v2/payments/sqp_cash_retry_not_found: [PAYMENT_NOT_FOUND/NOT_FOUND] payment does not exist")} + defer func() { SquareClient = origClient }() + + reqBody := TillSaleRequest{ + ItemType: "gift_card", + Action: "create", + Amount: 50.00, + PaymentMethod: "cash", + IdempotencyKey: key, + } + bodyBytes, _ := json.Marshal(reqBody) + req := httptest.NewRequest("POST", "/api/admin/till/sale", bytes.NewReader(bodyBytes)) + req.Header.Set("Authorization", "Bearer "+adminToken) + req.Header.Set("Content-Type", "application/json") + req = req.WithContext(db.ContextWithTx(req.Context(), tx.(pgx.Tx))) + + w := httptest.NewRecorder() + r := chi.NewRouter() + r.Use(mw.RequireAuth) + r.Post("/api/admin/till/sale", CreateTillSale) + r.ServeHTTP(w, req) + + if w.Code != http.StatusOK && w.Code != http.StatusCreated { + t.Fatalf("expected 200/201, got %d. body: %s", w.Code, w.Body.String()) + } + + var saleStatus string + if err := tx.QueryRow(ctx, `SELECT status FROM till_sales WHERE idempotency_key = $1`, key).Scan(&saleStatus); err != nil { + t.Fatalf("failed to query till sale: %v", err) + } + if saleStatus != "completed" { + t.Errorf("expected pending sale to be completed by cash retry after NOT_FOUND reconcile, got %s", saleStatus) + } +} + +// TestCreateTillSale_PendingRetry_Cash_Rejected_LostResponse verifies the +// HIGH-3 lost-response branch: a pending sale with NO square_payment_id cannot +// be resolved with cash — the charge may have landed at Square and cannot be +// looked up, so the retry must stay on the original card method (Square dedups +// on the same idempotency key and resolves the lost response). +func TestCreateTillSale_PendingRetry_Cash_Rejected_LostResponse(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") + + key := "till-pending-cash-lost-response-key" + var giftCardID string + 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) + if err != nil { + t.Fatalf("failed to create gift card: %v", err) + } + // No square_payment_id — the lost-response case. + _, err = tx.Exec(ctx, ` + INSERT INTO till_sales (item_type, item_id, description, quantity, unit_price, total_amount, + payment_method, status, user_id, idempotency_key, created_by, created_at, updated_at) + VALUES ('gift_card', $1, 'Gift Card create', 1, 50.00, 50.00, 'online_square', 'pending', + NULL, $2, $3, NOW(), NOW()) + `, giftCardID, key, adminID) + if err != nil { + t.Fatalf("failed to seed pending till sale: %v", err) + } + + reqBody := TillSaleRequest{ + ItemType: "gift_card", + Action: "create", + Amount: 50.00, + PaymentMethod: "cash", + IdempotencyKey: key, + } + bodyBytes, _ := json.Marshal(reqBody) + req := httptest.NewRequest("POST", "/api/admin/till/sale", bytes.NewReader(bodyBytes)) + req.Header.Set("Authorization", "Bearer "+adminToken) + req.Header.Set("Content-Type", "application/json") + req = req.WithContext(db.ContextWithTx(req.Context(), tx.(pgx.Tx))) + + w := httptest.NewRecorder() + r := chi.NewRouter() + r.Use(mw.RequireAuth) + r.Post("/api/admin/till/sale", CreateTillSale) + r.ServeHTTP(w, req) + + if w.Code != http.StatusConflict { + t.Fatalf("expected 409 (force original card method), got %d. body: %s", w.Code, w.Body.String()) + } + + // The pending sale must be untouched. + var saleStatus string + if err := tx.QueryRow(ctx, `SELECT status FROM till_sales WHERE idempotency_key = $1`, key).Scan(&saleStatus); err != nil { + t.Fatalf("failed to query till sale: %v", err) + } + if saleStatus != "pending" { + t.Errorf("expected pending sale to remain pending after rejected cash retry, got %s", saleStatus) + } +} + +// TestCreateTillSale_PendingRetry_Cash_Rejected_AlreadyPaid verifies the +// HIGH-3 COMPLETED branch: a pending sale whose Square charge is COMPLETED has +// already been paid — the sale is rescued to 'completed' and the cash is +// refused (double payment). The setup is COMMITTED so the handler runs at pool +// level: the rescue UPDATE is issued via db.Conn.Exec on a connection +// independent of the handler's transaction (as in production) and must persist +// even though the handler returns 409 before committing its own tx. +func TestCreateTillSale_PendingRetry_Cash_Rejected_AlreadyPaid(t *testing.T) { + // Not parallel: swaps the package-global SquareClient. + 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") + + key := "till-pending-cash-already-paid-key" + var giftCardID string + 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) + if err != nil { + t.Fatalf("failed to create gift card: %v", err) + } + _, err = tx.Exec(ctx, ` + INSERT INTO till_sales (item_type, item_id, description, quantity, unit_price, total_amount, + payment_method, status, user_id, square_payment_id, idempotency_key, created_by, created_at, updated_at) + VALUES ('gift_card', $1, 'Gift Card create', 1, 50.00, 50.00, 'online_square', 'pending', + NULL, 'sqp_cash_retry_completed', $2, $3, NOW(), NOW()) + `, giftCardID, key, adminID) + if err != nil { + t.Fatalf("failed to seed pending till sale: %v", err) + } + + // Commit the setup so the handler runs at pool level — the rescue UPDATE + // (db.Conn.Exec) then commits on its own connection exactly as in prod. + 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) + } + + pool := context.Background() + t.Cleanup(func() { + _, _ = db.Conn.Exec(pool, `DELETE FROM gift_card_transactions WHERE gift_card_id = $1`, giftCardID) + _, _ = db.Conn.Exec(pool, `DELETE FROM till_sales WHERE idempotency_key = $1`, key) + _, _ = db.Conn.Exec(pool, `DELETE FROM gift_cards WHERE id = $1`, giftCardID) + _, _ = db.Conn.Exec(pool, `DELETE FROM users WHERE id = $1`, adminID) + }) + + origClient := SquareClient + SquareClient = &staleGetPaymentClient{SquareClient: square.NewDevClient(), result: &square.PaymentResult{Status: "COMPLETED", SquarePayID: "sqp_cash_retry_completed"}} + defer func() { SquareClient = origClient }() + + reqBody := TillSaleRequest{ + ItemType: "gift_card", + Action: "create", + Amount: 50.00, + PaymentMethod: "cash", + IdempotencyKey: key, + } + bodyBytes, _ := json.Marshal(reqBody) + req := httptest.NewRequest("POST", "/api/admin/till/sale", bytes.NewReader(bodyBytes)) + req.Header.Set("Authorization", "Bearer "+adminToken) + req.Header.Set("Content-Type", "application/json") + req = req.WithContext(pool) + + w := httptest.NewRecorder() + r := chi.NewRouter() + r.Use(mw.RequireAuth) + r.Post("/api/admin/till/sale", CreateTillSale) + r.ServeHTTP(w, req) + + if w.Code != http.StatusConflict { + t.Fatalf("expected 409 (already paid by card), got %d. body: %s", w.Code, w.Body.String()) + } + + // The sale must have been rescued to 'completed' at pool level. + var saleStatus string + if err := db.Conn.QueryRow(pool, `SELECT status FROM till_sales WHERE idempotency_key = $1`, key).Scan(&saleStatus); err != nil { + t.Fatalf("failed to query till sale: %v", err) + } + if saleStatus != "completed" { + t.Errorf("expected already-paid pending sale rescued to completed, got %s", saleStatus) + } +} + +// TestCreateTillSale_PendingRetry_Cash_Rejected_Ambiguous verifies the HIGH-3 +// ambiguous branch: a transport/server error from GetPayment leaves the charge +// outcome unknown — cash must not be taken and the retry gets 503. +func TestCreateTillSale_PendingRetry_Cash_Rejected_Ambiguous(t *testing.T) { + // Not parallel: swaps the package-global SquareClient. + 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") + + key := "till-pending-cash-ambiguous-key" + var giftCardID string + 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) + if err != nil { + t.Fatalf("failed to create gift card: %v", err) + } + _, err = tx.Exec(ctx, ` + INSERT INTO till_sales (item_type, item_id, description, quantity, unit_price, total_amount, + payment_method, status, user_id, square_payment_id, idempotency_key, created_by, created_at, updated_at) + VALUES ('gift_card', $1, 'Gift Card create', 1, 50.00, 50.00, 'online_square', 'pending', + NULL, 'sqp_cash_retry_ambiguous', $2, $3, NOW(), NOW()) + `, giftCardID, key, adminID) + if err != nil { + t.Fatalf("failed to seed pending till sale: %v", err) + } + + origClient := SquareClient + SquareClient = &staleGetPaymentClient{SquareClient: square.NewDevClient(), err: fmt.Errorf("network error: connection reset by peer")} + defer func() { SquareClient = origClient }() + + reqBody := TillSaleRequest{ + ItemType: "gift_card", + Action: "create", + Amount: 50.00, + PaymentMethod: "cash", + IdempotencyKey: key, + } + bodyBytes, _ := json.Marshal(reqBody) + req := httptest.NewRequest("POST", "/api/admin/till/sale", bytes.NewReader(bodyBytes)) + req.Header.Set("Authorization", "Bearer "+adminToken) + req.Header.Set("Content-Type", "application/json") + req = req.WithContext(db.ContextWithTx(req.Context(), tx.(pgx.Tx))) + + w := httptest.NewRecorder() + r := chi.NewRouter() + r.Use(mw.RequireAuth) + r.Post("/api/admin/till/sale", CreateTillSale) + r.ServeHTTP(w, req) + + if w.Code != http.StatusServiceUnavailable { + t.Fatalf("expected 503 (unable to confirm card status), got %d. body: %s", w.Code, w.Body.String()) + } + + // The pending sale must be untouched. + var saleStatus string + if err := tx.QueryRow(ctx, `SELECT status FROM till_sales WHERE idempotency_key = $1`, key).Scan(&saleStatus); err != nil { + t.Fatalf("failed to query till sale: %v", err) + } + if saleStatus != "pending" { + t.Errorf("expected pending sale to remain pending after ambiguous reconcile, got %s", saleStatus) + } +} + +// TestCreateTillSale_PendingRetry_OnTheHouse_DefinitivelyFailed_CompletesRow +// verifies the HIGH-3 guard also fires for on_the_house: a pending sale whose +// Square charge provably failed can be resolved with on_the_house. +func TestCreateTillSale_PendingRetry_OnTheHouse_DefinitivelyFailed_CompletesRow(t *testing.T) { + // Not parallel: swaps the package-global SquareClient. + 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") + + key := "till-pending-oth-retry-key" + var giftCardID string + 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) + if err != nil { + t.Fatalf("failed to create gift card: %v", err) + } + _, err = tx.Exec(ctx, ` + INSERT INTO till_sales (item_type, item_id, description, quantity, unit_price, total_amount, + payment_method, status, user_id, square_payment_id, idempotency_key, created_by, created_at, updated_at) + VALUES ('gift_card', $1, 'Gift Card create', 1, 50.00, 50.00, 'online_square', 'pending', + NULL, 'sqp_oth_retry_failed', $2, $3, NOW(), NOW()) + `, giftCardID, key, adminID) + if err != nil { + t.Fatalf("failed to seed pending till sale: %v", err) + } + + origClient := SquareClient + SquareClient = &staleGetPaymentClient{SquareClient: square.NewDevClient(), result: &square.PaymentResult{Status: "CANCELED", SquarePayID: "sqp_oth_retry_failed"}} + defer func() { SquareClient = origClient }() + + reqBody := TillSaleRequest{ + ItemType: "gift_card", + Action: "create", + Amount: 50.00, + PaymentMethod: "on_the_house", + IdempotencyKey: key, + } + bodyBytes, _ := json.Marshal(reqBody) + req := httptest.NewRequest("POST", "/api/admin/till/sale", bytes.NewReader(bodyBytes)) + req.Header.Set("Authorization", "Bearer "+adminToken) + req.Header.Set("Content-Type", "application/json") + req = req.WithContext(db.ContextWithTx(req.Context(), tx.(pgx.Tx))) + + w := httptest.NewRecorder() + r := chi.NewRouter() + r.Use(mw.RequireAuth) + r.Post("/api/admin/till/sale", CreateTillSale) + r.ServeHTTP(w, req) + + if w.Code != http.StatusOK && w.Code != http.StatusCreated { + t.Fatalf("expected 200/201, got %d. body: %s", w.Code, w.Body.String()) + } + + var saleStatus string + if err := tx.QueryRow(ctx, `SELECT status FROM till_sales WHERE idempotency_key = $1`, key).Scan(&saleStatus); err != nil { + t.Fatalf("failed to query till sale: %v", err) + } + if saleStatus != "completed" { + t.Errorf("expected pending sale completed by on_the_house retry after definitive failure, got %s", saleStatus) + } +} + // TestCreateTillSale_PendingRetry_CardMachineSwitch_Rejected verifies that a // pending card_machine sale with a live checkout cannot be retried via a // different method — the original terminal checkout is still live and could @@ -1573,6 +1951,11 @@ func TestCreateTillSale_PendingRetry_CardMachineSwitch_Rejected(t *testing.T) { // revertGiftCardFunding — top-up guard-blocked reversal still fails the sale // ============================================================================= +// TestRevertGiftCardFunding_TopupPartiallySpent_LogsCritical_StillFailsSale locks +// the claim-first top-up guard: the gating claim succeeds (sale pending), then +// the guarded reversal is blocked because some of the top-up was already spent. +// The clawback must NOT fail (the sale still has to be marked failed) and must +// leave the card amounts untouched. func TestRevertGiftCardFunding_TopupPartiallySpent_LogsCritical_StillFailsSale(t *testing.T) { ctx, tx := testutils.SetupTestTx(t) @@ -1641,3 +2024,198 @@ func TestRevertGiftCardFunding_TopupPartiallySpent_LogsCritical_StillFailsSale(t t.Errorf("expected this request's top-up transaction removed, got %d", txCount) } } + +// TestRevertGiftCardFunding_NotPending_LeavesCardUntouched locks the claim-first +// gate: when the till sale is no longer 'pending' (already completed/failed), +// revertGiftCardFunding returns errTillSaleNotPending and the gift card (and +// its transaction) is left exactly as it was — the funding is no longer ours +// to revert. +func TestRevertGiftCardFunding_NotPending_LeavesCardUntouched(t *testing.T) { + ctx, tx := testutils.SetupTestTx(t) + + adminID, err := fixtures.CreateTestAdminUser(tx) + if err != nil { + t.Fatalf("failed to create admin user: %v", err) + } + + var cardID string + if err := tx.QueryRow(ctx, ` + INSERT INTO gift_cards (total_funds_added, amount_remaining, created_by, is_inventory) + VALUES (50.00, 50.00, $1, FALSE) + RETURNING id + `, adminID).Scan(&cardID); err != nil { + t.Fatalf("failed to seed gift card: %v", err) + } + + // A COMPLETED sale — the claim-first gating UPDATE must match zero rows. + var saleID string + if err := tx.QueryRow(ctx, ` + INSERT INTO till_sales (item_type, item_id, description, quantity, unit_price, total_amount, payment_method, status, created_by, created_at, updated_at) + VALUES ('gift_card', $1, 'Gift Card topup', 1, 50.00, 50.00, 'online_square', 'completed', $2, NOW(), NOW()) + RETURNING id + `, cardID, adminID).Scan(&saleID); err != nil { + t.Fatalf("failed to seed till sale: %v", err) + } + if _, err := tx.Exec(ctx, ` + INSERT INTO gift_card_transactions (gift_card_id, transaction_type, amount, reference_type, reference_id) + VALUES ($1, 'topup', 50.00, 'till_sale', $2) + `, cardID, saleID); err != nil { + t.Fatalf("failed to seed gift card transaction: %v", err) + } + + err = revertGiftCardFunding(ctx, "topup", cardID, 50.00, nil, saleID) + if !errors.Is(err, errTillSaleNotPending) { + t.Fatalf("expected errTillSaleNotPending when the sale is not pending, got %v", err) + } + + var remaining, totalAdded float64 + if err := tx.QueryRow(ctx, `SELECT amount_remaining, total_funds_added FROM gift_cards WHERE id = $1`, cardID).Scan(&remaining, &totalAdded); err != nil { + t.Fatalf("failed to query gift card: %v", err) + } + if remaining != 50.00 || totalAdded != 50.00 { + t.Errorf("claim-failed clawback must leave the card untouched, got remaining=%.2f total_funds_added=%.2f", remaining, totalAdded) + } + var txCount int + if err := tx.QueryRow(ctx, `SELECT COUNT(*) FROM gift_card_transactions WHERE gift_card_id = $1`, cardID).Scan(&txCount); err != nil { + t.Fatalf("failed to count gift card transactions: %v", err) + } + if txCount != 1 { + t.Errorf("claim-failed clawback must leave the transaction untouched, got %d transactions", txCount) + } +} + +// TestCreateTillSale_PostCheckoutFailure_CancelsOrphanedCheckout locks the +// HIGH-2 fix: when a card_machine checkout is created at Square inside the tx +// but a later pre-commit failure aborts the request (here: the till_sales +// INSERT collides on the idempotency_key UNIQUE constraint), the orphaned live +// checkout must be cancelled at Square — otherwise it stays live at the +// terminal as an invisible, untracked charge. +func TestCreateTillSale_PostCheckoutFailure_CancelsOrphanedCheckout(t *testing.T) { + 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") + + // A till_sales row already holding the request's idempotency key makes the + // handler's till_sales INSERT collide on the UNIQUE constraint AFTER the + // Square CreateCheckout succeeded. Seeding item_id = NULL makes the dedup + // SELECT's NULL-into-string scan FAIL with a non-ErrNoRows error, so the + // dedup is skipped, existingPendingID stays empty, and the create path runs + // its INSERT into the collision. + const key = "till-high2-orphan-key" + if _, err := tx.Exec(ctx, ` + INSERT INTO till_sales (item_type, item_id, description, quantity, unit_price, total_amount, + payment_method, status, created_by, idempotency_key, created_at, updated_at) + VALUES ('gift_card', NULL, 'blocker row', 1, 50.00, 50.00, 'online_square', 'pending', + $1, $2, NOW(), NOW()) + `, adminID, key); err != nil { + t.Fatalf("failed to seed blocker till sale: %v", err) + } + + origClient := SquareClient + client := &fixedCheckoutClient{SquareClient: square.NewDevClient(), checkoutID: "chk_high2_orphaned"} + SquareClient = client + defer func() { SquareClient = origClient }() + + reqBody := TillSaleRequest{ + ItemType: "gift_card", + Action: "create", + Amount: 50.00, + PaymentMethod: "card_machine", + IdempotencyKey: key, + } + bodyBytes, _ := json.Marshal(reqBody) + req := httptest.NewRequest("POST", "/api/admin/till/sale", bytes.NewReader(bodyBytes)) + req.Header.Set("Authorization", "Bearer "+adminToken) + req.Header.Set("Content-Type", "application/json") + req = req.WithContext(db.ContextWithTx(req.Context(), tx.(pgx.Tx))) + + w := httptest.NewRecorder() + r := chi.NewRouter() + r.Use(mw.RequireAuth) + r.Post("/api/admin/till/sale", CreateTillSale) + r.ServeHTTP(w, req) + + if w.Code != http.StatusInternalServerError { + t.Fatalf("expected 500 from the failed till_sales INSERT, got %d. body: %s", w.Code, w.Body.String()) + } + + // The orphaned live checkout must have been cancelled at Square. + if calls := client.cancelCalls(); len(calls) != 1 || calls[0] != "chk_high2_orphaned" { + t.Errorf("expected exactly one CancelCheckout for the orphaned checkout %q, got %v", "chk_high2_orphaned", calls) + } +} + +// TestGetTillCheckoutStatus_AlreadyFailed_DoesNotResurrect locks the LOW +// resurrection guard: a poll of a checkout whose till sale the sweep already +// failed (e.g. a clawback reverted the gift card) must NOT complete the sale — +// the completion UPDATE's status='pending' guard matches zero rows and the +// poll fails loudly with 404 instead of reporting COMPLETED for a sale whose +// card no longer exists. +func TestGetTillCheckoutStatus_AlreadyFailed_DoesNotResurrect(t *testing.T) { + ctx, tx := testutils.SetupTestTx(t) + + adminID, err := fixtures.CreateTestAdminUser(tx) + if err != nil { + t.Fatalf("failed to create admin user: %v", err) + } + pool := context.Background() + const checkoutID = "chk_till_already_failed" + + var saleID string + if err := tx.QueryRow(ctx, ` + INSERT INTO till_sales (item_type, item_id, description, quantity, unit_price, total_amount, + payment_method, status, square_checkout_id, created_by, created_at, updated_at) + VALUES ('gift_card', NULL, 'Gift Card create', 1, 50.00, 50.00, 'in_person_card', 'failed', + $1, $2, NOW(), NOW()) + RETURNING id + `, checkoutID, adminID).Scan(&saleID); err != nil { + t.Fatalf("failed to seed failed till sale: %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 setup tx: %v", err) + } + + t.Cleanup(func() { + _, _ = db.Conn.Exec(pool, `DELETE FROM till_sales WHERE id = $1`, saleID) + _, _ = db.Conn.Exec(pool, `DELETE FROM users WHERE id = $1`, adminID) + }) + + origClient := SquareClient + SquareClient = &completedCheckoutClient{SquareClient: square.NewDevClient(), result: &square.PaymentResult{ + Status: "COMPLETED", + SquarePayID: "sqp_poll_already_failed", + }} + defer func() { SquareClient = origClient }() + + req := httptest.NewRequest("GET", "/api/admin/till/checkout/"+checkoutID+"/status", nil) + rctx := chi.NewRouteContext() + rctx.URLParams.Add("checkout_id", checkoutID) + reqCtx := context.WithValue(req.Context(), chi.RouteCtxKey, rctx) + req = req.WithContext(reqCtx) + req = adminRequestCtx(req) + + w := httptest.NewRecorder() + GetTillCheckoutStatus(w, req) + + if w.Code != http.StatusNotFound { + t.Fatalf("expected 404 for a poll of a failed sale, got %d. body: %s", w.Code, w.Body.String()) + } + + // The sale must STILL be failed — the poll must not have resurrected it. + var status string + if err := db.Conn.QueryRow(pool, `SELECT status FROM till_sales WHERE id = $1`, saleID).Scan(&status); err != nil { + t.Fatalf("failed to query till sale: %v", err) + } + if status != "failed" { + t.Errorf("expected failed sale to remain failed after the guarded poll, got %q", status) + } +}