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.
This commit is contained in:
2026-08-22 00:34:49 +01:00
parent 8416f033d6
commit 352f9e50d4
5 changed files with 1478 additions and 38 deletions
@@ -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 // TestTransferGiftCard_ConcurrentCrossTransfer_NoDeadlock proves the gift-card
// transfer lock ordering (giftcards.go): two concurrent cross-transfers A→B and // 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) // B→A must both succeed. Locking "source first" (the caller's chosen order)
+188 -17
View File
@@ -2,6 +2,7 @@ package payments
import ( import (
"context" "context"
"database/sql"
"errors" "errors"
"fmt" "fmt"
"log" "log"
@@ -11,6 +12,8 @@ import (
"crussell/clock" "crussell/clock"
"crussell/db" "crussell/db"
"crussell/internal/square" "crussell/internal/square"
"github.com/jackc/pgx/v5"
) )
// SweepStalePendingPayments resolves pending payment records that are older // 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 // 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 { type staleRow struct {
ID string ID string
SquarePaymentID 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 // sweepStaleRows resolves the stale pending rows of one table. Rows with a
// square_payment_id are reconciled at Square first (COMPLETED → 'completed', // square_payment_id are reconciled at Square first (COMPLETED → 'completed',
// anything else → 'failed' exactly as the legacy bulk UPDATE did); rows // anything else → 'failed' exactly as the legacy bulk UPDATE did); rows
// without one cannot be reconciled and are failed directly. Returns the total // without one cannot be reconciled and are failed directly. A till_sales row
// rows resolved and how many were rescued to 'completed'. // 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) { func sweepStaleRows(ctx context.Context, table string, cutoff time.Time) (resolved int, completed int, err error) {
switch table { switch table {
case "payments", "till_sales": case "payments", "till_sales":
@@ -99,20 +116,48 @@ func sweepStaleRows(ctx context.Context, table string, cutoff time.Time) (resolv
if table == "till_sales" { if table == "till_sales" {
methodFilter = ` AND payment_method IN ('online_square', 'in_person_card')` methodFilter = ` AND payment_method IN ('online_square', 'in_person_card')`
} }
rows, err := db.Conn.Query(ctx, ` var rows pgx.Rows
SELECT id, COALESCE(square_payment_id, '') if table == "till_sales" {
FROM `+table+` rows, err = db.Conn.Query(ctx, `
WHERE status = 'pending' AND created_at < $1`+methodFilter+` SELECT ts.id, COALESCE(ts.square_payment_id, ''), ts.item_id, gc.redeemed_by,
`, cutoff) (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 { if err != nil {
return 0, 0, err return 0, 0, err
} }
var stale []staleRow var stale []staleRow
for rows.Next() { for rows.Next() {
var r staleRow var r staleRow
if err := rows.Scan(&r.ID, &r.SquarePaymentID); err != nil { if table == "till_sales" {
log.Printf("Failed to scan stale pending row from %s: %v", table, err) var itemID, redeemedBy sql.NullString
continue 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) 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. // 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, ` if tag, upErr := db.Conn.Exec(ctx, `
UPDATE `+table+` SET status = 'failed', updated_at = NOW() UPDATE `+table+` SET status = 'failed', updated_at = NOW()
WHERE id = $1 AND status = 'pending' WHERE id = $1 AND status = 'pending'
@@ -354,7 +420,14 @@ func SweepStaleTerminalCheckouts(ctx context.Context) (int, error) {
// The cancel landed (CANCELED / cancel-requested / expired / // The cancel landed (CANCELED / cancel-requested / expired /
// still-reporting-pending-but-now-cancelled) — it can never // still-reporting-pending-but-now-cancelled) — it can never
// complete, so resolve the row to the terminal 'failed' state. // 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++ resolved++
} }
log.Printf("Cancelled stale terminal checkout %s (%s %s, pending >%s) — marked failed", r.CheckoutID, r.Kind, r.RowID, staleTerminalCheckoutAge) 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) log.Printf("Terminal checkout %s already COMPLETED at Square — leaving sale %s pending (poll handler records it)", r.CheckoutID, r.RowID)
} }
case isTerminalCheckoutError(gErr): case isTerminalCheckoutError(gErr):
// Definitely cancelled / cancel-requested / expired — the checkout // The checkout is cancelled / cancel-requested / expired at Square.
// can never complete, so resolve it to the terminal 'failed' state. switch {
if markTerminalCheckoutRowFailed(ctx, r) { case r.Kind == "till_sale" && isCheckoutDefinitivelyDead(gErr):
resolved++ // 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: default:
// Ambiguous transport/unknown status — leave for a later run. // 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) 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") ||
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
}
+502
View File
@@ -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) 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)
}
}
+110 -14
View File
@@ -89,14 +89,20 @@ func isDefinitiveChargeFailure(err error) bool {
return false 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 // revertGiftCardFunding undoes the gift-card funding performed earlier in the
// SAME till-sale request after a definitive Square charge rejection, matching // SAME till-sale request after a definitive Square charge rejection, matching
// the gift_card_transactions accounting: a created card is deleted (with its // the gift_card_transactions accounting: a created card is deleted (with its
// purchase transaction) and any immediate redeem-to-account credit reversed; a // 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 // 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 // removed. The clawback is claim-first: it atomically claims the till sale
// transaction so a late same-key retry cannot re-complete a sale whose gift // with a gating `status='pending'` UPDATE whose row lock serializes against
// card no longer exists. // 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 { func revertGiftCardFunding(ctx context.Context, action, giftCardID string, amount float64, redeemToUserID *string, tillSaleID string) error {
tx, err := db.Conn.Begin(ctx) tx, err := db.Conn.Begin(ctx)
if err != nil { 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" { if action == "create" {
// A newly created card has exactly one funding transaction (this // A newly created card has exactly one funding transaction (this
// request's purchase) — remove it, then the card itself. // 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 { if err := tx.Commit(ctx); err != nil {
return fmt.Errorf("failed to commit clawback transaction: %w", err) return fmt.Errorf("failed to commit clawback transaction: %w", err)
} }
@@ -323,13 +335,34 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) {
service := NewPaymentService() 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) tx, err := db.Conn.Begin(ctx)
if err != nil { if err != nil {
log.Printf("Failed to begin transaction: %v", err) log.Printf("Failed to begin transaction: %v", err)
http.Error(w, "internal server error", http.StatusInternalServerError) http.Error(w, "internal server error", http.StatusInternalServerError)
return 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() { 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) { if err := tx.Rollback(ctx); err != nil && !errors.Is(err, pgx.ErrTxClosed) {
slog.Error("failed to rollback transaction", "err", err) 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)) penceAmount := int64(math.Round(req.Amount * 100))
var squarePaymentID *string
var squareCheckoutID *string
var saleStatus string var saleStatus string
var dbPaymentMethod 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 // original checkout is still live — the customer could be charged at the
// terminal AND by the new method (double charge). The terminal checkout // terminal AND by the new method (double charge). The terminal checkout
// cannot be cancelled via this API, so reject the switch outright. // 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" { 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) 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) http.Error(w, "This pending sale is tied to a live card-machine checkout — retry with card machine payment", http.StatusConflict)
return 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 { switch req.PaymentMethod {
case "cash": case "cash":
saleStatus = "completed" saleStatus = "completed"
@@ -606,6 +685,7 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) {
} }
squareCheckoutID = &checkout.ID squareCheckoutID = &checkout.ID
checkoutCreated = true
saleStatus = "pending" saleStatus = "pending"
} }
case "online_square": case "online_square":
@@ -688,6 +768,7 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) {
http.Error(w, "internal server error", http.StatusInternalServerError) http.Error(w, "internal server error", http.StatusInternalServerError)
return return
} }
committed = true
// Step 2: DB transaction committed — safe to call Square now. // Step 2: DB transaction committed — safe to call Square now.
// If Square fails, the till_sale record stays 'pending' for manual retry. // 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 // 'completed' explicitly — otherwise the row stays pending forever while
// the response claims success. // the response claims success.
if existingPendingID != "" && !needsSquarePayment && req.PaymentMethod != "card_machine" { 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'`, `UPDATE till_sales SET status = 'completed', updated_at = NOW() WHERE id = $1 AND status = 'pending'`,
tillSaleID, tillSaleID,
) )
@@ -803,6 +884,13 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) {
http.Error(w, "internal server error", http.StatusInternalServerError) http.Error(w, "internal server error", http.StatusInternalServerError)
return 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" 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 UPDATE till_sales
SET status = 'completed', SET status = 'completed',
square_payment_id = $1, square_payment_id = $1,
updated_at = NOW() updated_at = NOW()
WHERE id = $2 WHERE id = $2 AND status = 'pending'
`, paymentResult.SquarePayID, tillSaleID) `, paymentResult.SquarePayID, tillSaleID)
if err != nil { if err != nil {
log.Printf("Failed to update till sale: %v", err) log.Printf("Failed to update till sale: %v", err)
http.Error(w, "internal server error", http.StatusInternalServerError) http.Error(w, "internal server error", http.StatusInternalServerError)
return 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) ApplyVATToTillSale(r.Context(), tx, tillSaleID)
if err := tx.Commit(r.Context()); err != nil { if err := tx.Commit(r.Context()); err != nil {
+585 -7
View File
@@ -6,6 +6,8 @@ import (
"bytes" "bytes"
"context" "context"
"encoding/json" "encoding/json"
"errors"
"fmt"
"net/http" "net/http"
"net/http/httptest" "net/http/httptest"
"testing" "testing"
@@ -1422,11 +1424,12 @@ func TestCreateTillSale_PendingRetry_AmountMismatch_Rejected(t *testing.T) {
} }
// TestCreateTillSale_PendingRetry_Cash_CompletesRow verifies that a same-key // TestCreateTillSale_PendingRetry_Cash_CompletesRow verifies that a same-key
// retry resolved by cash on a pending sale explicitly flips the row to // retry resolved by cash on a pending sale whose Square charge PROVABLY failed
// 'completed' — previously the response claimed success while the DB row stayed // (HIGH-3: a FAILED status means no money landed, so cash is safe) explicitly
// pending forever (unreconciled). // 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) { func TestCreateTillSale_PendingRetry_Cash_CompletesRow(t *testing.T) {
t.Parallel() // Not parallel: swaps the package-global SquareClient.
ctx, tx := testutils.SetupTestTx(t) ctx, tx := testutils.SetupTestTx(t)
adminID, err := fixtures.CreateTestAdminUser(tx) adminID, err := fixtures.CreateTestAdminUser(tx)
@@ -1435,7 +1438,8 @@ func TestCreateTillSale_PendingRetry_Cash_CompletesRow(t *testing.T) {
} }
adminToken := jwt.GenerateTestToken(adminID, "admin") 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" key := "till-pending-cash-retry-key"
var giftCardID string var giftCardID string
err = tx.QueryRow(ctx, ` err = tx.QueryRow(ctx, `
@@ -1448,14 +1452,18 @@ func TestCreateTillSale_PendingRetry_Cash_CompletesRow(t *testing.T) {
} }
_, err = tx.Exec(ctx, ` _, err = tx.Exec(ctx, `
INSERT INTO till_sales (item_type, item_id, description, quantity, unit_price, total_amount, 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', 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) `, giftCardID, key, adminID)
if err != nil { if err != nil {
t.Fatalf("failed to seed pending till sale: %v", err) 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. // Retry with the same key, same amount, cash payment.
reqBody := TillSaleRequest{ reqBody := TillSaleRequest{
ItemType: "gift_card", 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 // TestCreateTillSale_PendingRetry_CardMachineSwitch_Rejected verifies that a
// pending card_machine sale with a live checkout cannot be retried via 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 // 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 // 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) { func TestRevertGiftCardFunding_TopupPartiallySpent_LogsCritical_StillFailsSale(t *testing.T) {
ctx, tx := testutils.SetupTestTx(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) 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)
}
}