fix(sca): fix stuff the review commented on

api/handlers/cart.go

@@ -539,9 +539,10 @@ func (h *CartHandler) ConvertCart(w http.ResponseWriter, r *http.Request) {
 	})
 }
 
-// respondRequiresAction handles a PaymentIntent that needs 3DS auth
-// The order stays pending. the webhook flips it to paid once the customer
-// completes the bank-hosted challenge, the tui polls /api/v1/orders/{id}/status
+// respondRequiresAction handles a PaymentIntent that needs 3DS auth. The order
+// transitions to requires_action (see caveat #20 in sca-psd2-compliance.md);
+// the payment_intent.succeeded webhook flips it to paid once the customer
+// completes the bank-hosted challenge. The TUI polls /api/v1/orders/{id}/status.
 func (h *CartHandler) respondRequiresAction(w http.ResponseWriter, order *models.Order, paymentMethodID string, pi *stripe.PaymentIntent) {
 	db := database.GetDB()
 
@@ -568,13 +569,15 @@ func (h *CartHandler) respondRequiresAction(w http.ResponseWriter, order *models
 		return
 	}
 
-	// Persist the paymentIntent id AND reset status to pending. The
-	// payment_intent.payment_failed webhook from the initial off-session decline
-	// often arrives before this response and flips the order to failed —
-	// override it back so the 3DS-in-flight order shows the correct state.
+	// Persist the paymentIntent id AND set status to requires_action so list
+	// UIs can distinguish "charging" from "awaiting customer 3DS". The
+	// payment_intent.payment_failed webhook from the initial off-session
+	// decline often arrives before this response and flips the order to
+	// failed - override it back so the 3DS-in-flight order shows the
+	// correct state
 	if err := db.Model(order).Updates(map[string]any{
 		"stripe_payment_id": pi.ID,
-		"status":            models.OrderStatusPending,
+		"status":            models.OrderStatusRequiresAction,
 	}).Error; err != nil {
 		utils.RespondError(w, http.StatusInternalServerError, "DATABASE_ERROR", "failed to save payment id", nil)
 		return
@@ -658,10 +661,14 @@ func (h *CartHandler) RetryAuth(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	// Only pending (initial attempt mid-flight) and failed (webhook flipped it
-	// after a non-recoverable error) are retryable. paid, refundedn shipped,
-	// delivered, cancelled are terminal - refuse
-	if order.Status != models.OrderStatusPending && order.Status != models.OrderStatusFailed {
+	// Retryable source states:
+	//	pending 		- initial attempt mid-flight (pre-PI confirm page)
+	//	requires_action - customer abandoned or failed the 3DS challenge
+	//	failed 			- webhook flipped after a non-recoverable error
+	// terminal states (paid, shipped, delivered, cancelled, refunded) refuse.
+	if order.Status != models.OrderStatusPending &&
+		order.Status != models.OrderStatusRequiresAction &&
+		order.Status != models.OrderStatusFailed {
 		utils.RespondError(w, http.StatusConflict, "INVALID_STATE", "order is already finalized", nil)
 		return
 	}

api/handlers/cart_test.go

@@ -11,6 +11,7 @@ import (
 	"time"
 
 	"github.com/go-chi/chi/v5"
+	"github.com/stripe/stripe-go/v78"
 
 	"terminalShop/api/middleware"
 	"terminalShop/pkg/audit"
@@ -919,6 +920,12 @@ func TestRetryAuth(t *testing.T) {
 			seedPayment: "pi_test_refunded", seedEvents: 1,
 			wantStatus: http.StatusConflict, wantErrCode: "INVALID_STATE",
 		},
+		{
+			name:        "requires_action honors retry cap",
+			ownedByUser: true, seedStatus: models.OrderStatusRequiresAction,
+			seedPayment: "pi_test_req_action_cap", seedEvents: maxAuthIssuances,
+			wantStatus: http.StatusTooManyRequests, wantErrCode: "RETRY_LIMIT",
+		},
 		{
 			name:        "retry cap reached returns 429",
 			ownedByUser: true, seedStatus: models.OrderStatusFailed,
@@ -1022,3 +1029,137 @@ func TestRetryAuth_OrderNotFound(t *testing.T) {
 		t.Fatalf("want 404, got %d body=%s", w.Code, w.Body.String())
 	}
 }
+
+func TestRespondRequiresAction_SetStatusRequiresAction(t *testing.T) {
+	testDB, user := setupCartTestDB(t)
+	defer func() { _ = os.Remove(testDB) }()
+	defer database.ResetForTesting()
+
+	db := database.GetDB()
+	card := models.Card{
+		UserID: user.ID, StripePaymentID: "pm_test_req_action",
+		Last4: "4242", Brand: "Visa", ExpMonth: 12, ExpYear: 2030,
+	}
+	if err := db.Create(&card).Error; err != nil {
+		t.Fatalf("seed card: %v", err)
+	}
+
+	order := models.Order{
+		UserID: user.ID, CardID: card.ID,
+		Status:   models.OrderStatusPending,
+		Subtotal: 500, Total: 500,
+		ShippingName: "Test", ShippingStreet: "1 Main St",
+		ShippingCity: "PDX", ShippingState: "OR",
+		ShippingZip: "97201", ShippingCountry: "US",
+	}
+	if err := db.Create(&order).Error; err != nil {
+		t.Fatalf("seed order: %v", err)
+	}
+
+	pi := &stripe.PaymentIntent{
+		ID: "pi_test_action_set",
+		NextAction: &stripe.PaymentIntentNextAction{
+			RedirectToURL: &stripe.PaymentIntentNextActionRedirectToURL{
+				URL: "https://hooks.stripe.com/redirect/test/test_3ds",
+			},
+		},
+	}
+
+	handler := NewCartHandler("", "http://localhost", 0)
+	w := httptest.NewRecorder()
+	handler.respondRequiresAction(w, &order, card.StripePaymentID, pi)
+
+	if w.Code != http.StatusAccepted {
+		t.Fatalf("want 202, got %d body=%s", w.Code, w.Body.String())
+	}
+
+	var resp struct {
+		Data struct {
+			OrderID         uint   `json:"order_id"`
+			PaymentIntentID string `json:"payment_intent_id"`
+			Status          string `json:"status"`
+			RedirectURL     string `json:"redirect_url"`
+		} `json:"data"`
+	}
+	if err := json.NewDecoder(w.Body).Decode(&resp); err != nil {
+		t.Fatalf("decode response: %v", err)
+	}
+	if resp.Data.Status != "requires_action" {
+		t.Fatalf("response status: %q, got %q", "requires_action", resp.Data.Status)
+	}
+	if resp.Data.RedirectURL == "" {
+		t.Fatalf("expected redirect url, got empty")
+	}
+	if resp.Data.PaymentIntentID != pi.ID {
+		t.Fatalf("payment intent id: want %q, got %q", pi.ID, resp.Data.PaymentIntentID)
+	}
+
+	var reloaded models.Order
+	if err := db.First(&reloaded, order.ID).Error; err != nil {
+		t.Fatalf("reload order: %v", err)
+	}
+	if reloaded.Status != models.OrderStatusRequiresAction {
+		t.Fatalf("order.Status: want %q, got %q", models.OrderStatusRequiresAction, reloaded.Status)
+	}
+	if reloaded.StripePaymentID != pi.ID {
+		t.Fatalf("order.StripePaymentID: want %q, got %q", pi.ID, reloaded.StripePaymentID)
+	}
+}
+
+// TestRespondRequiresAction_OverridesFailedFromWebhookRace simulates the race
+// described in sca-psd2-compliance.md note #3 / caveat #4: the
+// payment_intent.payment_failed webhook arrives BEFORE respondRequiresAction
+// returns and flips the row to OrderStatusFailed. The helper must override
+// that back to OrderStatusRequiresAction so the in-flight 3DS state surfaces
+// correctly to the TUI.
+func TestRespondRequiresAction_OverridesFailedFromWebhookRace(t *testing.T) {
+	testDB, user := setupCartTestDB(t)
+	defer func() { _ = os.Remove(testDB) }()
+	defer database.ResetForTesting()
+
+	db := database.GetDB()
+	card := models.Card{
+		UserID: user.ID, StripePaymentID: "pm_test_race",
+		Last4: "4242", Brand: "Visa", ExpMonth: 12, ExpYear: 2030,
+	}
+	if err := db.Create(&card).Error; err != nil {
+		t.Fatalf("seed card: %v", err)
+	}
+
+	order := models.Order{
+		UserID: user.ID, CardID: card.ID,
+		Status:   models.OrderStatusFailed,
+		Subtotal: 500, Total: 500,
+		ShippingName: "Test", ShippingStreet: "1 Main St",
+		ShippingCity: "PDX", ShippingState: "OR",
+		ShippingZip: "97201", ShippingCountry: "US",
+	}
+	if err := db.Create(&order).Error; err != nil {
+		t.Fatalf("seed order: %v", err)
+	}
+
+	pi := &stripe.PaymentIntent{
+		ID: "pi_test_race",
+		NextAction: &stripe.PaymentIntentNextAction{
+			RedirectToURL: &stripe.PaymentIntentNextActionRedirectToURL{
+				URL: "https://hooks.stripe.com/redirect/test_race",
+			},
+		},
+	}
+
+	handler := NewCartHandler("", "http://localhost", 0)
+	w := httptest.NewRecorder()
+	handler.respondRequiresAction(w, &order, card.StripePaymentID, pi)
+
+	if w.Code != http.StatusAccepted {
+		t.Fatalf("want 202, got %d body=%s", w.Code, w.Body.String())
+	}
+
+	var reloaded models.Order
+	if err := db.First(&reloaded, order.ID).Error; err != nil {
+		t.Fatalf("reload order: %v", err)
+	}
+	if reloaded.Status != models.OrderStatusRequiresAction {
+		t.Fatalf("override failed: want %q, got %q", models.OrderStatusRequiresAction, reloaded.Status)
+	}
+}

api/handlers/webhook.go

@@ -184,7 +184,7 @@ func (h *WebhookHandler) handlePaymentIntentFailed(ctx context.Context, event st
 	}
 
 	db := database.GetDB().WithContext(ctx)
-	if err := db.Model(&models.Order{}).Where("id = ? AND status = ?", orderIDStr, models.OrderStatusPending).
+	if err := db.Model(&models.Order{}).Where("id = ? AND status IN (?, ?)", orderIDStr, models.OrderStatusPending, models.OrderStatusRequiresAction).
 		Update("status", models.OrderStatusFailed).Error; err != nil {
 		webhookLog().Error("failed to mark order failed", "order_id", orderIDStr, "error", err)
 	}

api/handlers/webhook_test.go

@@ -0,0 +1,224 @@
+package handlers
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+	"os"
+	"testing"
+
+	"github.com/stripe/stripe-go/v78"
+
+	"terminalShop/pkg/database"
+	"terminalShop/pkg/models"
+)
+
+// buildPaymentFailedEvent constructs a stripe.Event whose Data.Raw decodes to
+// a PaymentIntent referencing the given order via metadata. errCode is set on
+// last_payment_error to drive the authentication_required short-circuit
+// branch in handlePaymentIntentFailed (pass "" to omit the error block).
+func buildPaymentFailedEvent(t *testing.T, piID string, orderID uint, errCode string) stripe.Event {
+	t.Helper()
+	pi := map[string]any{
+		"id":       piID,
+		"metadata": map[string]string{"order_id": fmt.Sprintf("%d", orderID)},
+	}
+	if errCode != "" {
+		pi["last_payment_error"] = map[string]any{"code": errCode}
+	}
+	raw, err := json.Marshal(pi)
+	if err != nil {
+		t.Fatalf("marshal pi: %v", err)
+	}
+	return stripe.Event{
+		Type: "payment_intent.payment_failed",
+		Data: &stripe.EventData{Raw: raw},
+	}
+}
+
+func seedOrderForWebhook(t *testing.T, userID uint, status models.OrderStatus, piID string) models.Order {
+	t.Helper()
+	db := database.GetDB()
+	card := models.Card{
+		UserID:          userID,
+		StripePaymentID: fmt.Sprintf("pm_test_%s", piID),
+		Last4:           "4242", Brand: "Visa", ExpMonth: 12, ExpYear: 2030,
+	}
+	if err := db.Create(&card).Error; err != nil {
+		t.Fatalf("seed card: %v", err)
+	}
+	order := models.Order{
+		UserID: userID, CardID: card.ID,
+		StripePaymentID: piID,
+		Status:          status,
+		Subtotal:        500, Total: 500,
+		ShippingName: "Test", ShippingStreet: "1 Main St",
+		ShippingCity: "PDX", ShippingState: "OR",
+		ShippingZip: "97201", ShippingCountry: "US",
+	}
+	if err := db.Create(&order).Error; err != nil {
+		t.Fatalf("seed order: %v", err)
+	}
+	return order
+}
+
+// TestHandlePaymentIntentFailed_RequiresActionFlipsToFailed verifies the
+// widened predicate (IN (pending, requires_action)) catches a row in the new
+// requires_action state. Without this case, a card_declined webhook arriving
+// after respondRequiresAction wrote requires_action would leave the row
+// stuck forever.
+func TestHandlePaymentIntentFailed_RequiresActionFlipsToFailed(t *testing.T) {
+	testDB, user := setupCartTestDB(t)
+	defer func() { _ = os.Remove(testDB) }()
+	defer database.ResetForTesting()
+
+	order := seedOrderForWebhook(t, user.ID, models.OrderStatusRequiresAction, "pi_test_req_action_failed")
+
+	h := NewWebhookHandler("", "", "")
+	evt := buildPaymentFailedEvent(t, order.StripePaymentID, order.ID, "card_declined")
+	h.handlePaymentIntentFailed(context.Background(), evt)
+
+	db := database.GetDB()
+	var reloaded models.Order
+	if err := db.First(&reloaded, order.ID).Error; err != nil {
+		t.Fatalf("reload order: %v", err)
+	}
+	if reloaded.Status != models.OrderStatusFailed {
+		t.Fatalf("status: want %q, got %q", models.OrderStatusFailed, reloaded.Status)
+	}
+}
+
+// TestHandlePaymentIntentFailed_RequiresActionAuthRequiredShortCircuits
+// verifies the existing short-circuit (note #3 in sca-psd2-compliance.md)
+// still fires when the row is in requires_action and the error code is
+// authentication_required. Without the short-circuit, the recovery flow
+// would race the webhook and surface a momentarily-failed state to the TUI.
+func TestHandlePaymentIntentFailed_RequiresActionAuthRequiredShortCircuits(t *testing.T) {
+	testDB, user := setupCartTestDB(t)
+	defer func() { _ = os.Remove(testDB) }()
+	defer database.ResetForTesting()
+
+	order := seedOrderForWebhook(t, user.ID, models.OrderStatusRequiresAction, "pi_test_req_action_authreq")
+
+	h := NewWebhookHandler("", "", "")
+	evt := buildPaymentFailedEvent(t, order.StripePaymentID, order.ID, "authentication_required")
+	h.handlePaymentIntentFailed(context.Background(), evt)
+
+	db := database.GetDB()
+	var reloaded models.Order
+	if err := db.First(&reloaded, order.ID).Error; err != nil {
+		t.Fatalf("reload order: %v", err)
+	}
+	if reloaded.Status != models.OrderStatusRequiresAction {
+		t.Fatalf("auth_required must not flip requires_action; got %q", reloaded.Status)
+	}
+}
+
+// TestHandlePaymentIntentFailed_PendingStillFlips guards the pre-existing
+// pending -> failed transition against a regression where the widened
+// predicate accidentally dropped pending from the source set.
+func TestHandlePaymentIntentFailed_PendingStillFlips(t *testing.T) {
+	testDB, user := setupCartTestDB(t)
+	defer func() { _ = os.Remove(testDB) }()
+	defer database.ResetForTesting()
+
+	order := seedOrderForWebhook(t, user.ID, models.OrderStatusPending, "pi_test_pending_failed")
+
+	h := NewWebhookHandler("", "", "")
+	evt := buildPaymentFailedEvent(t, order.StripePaymentID, order.ID, "card_declined")
+	h.handlePaymentIntentFailed(context.Background(), evt)
+
+	db := database.GetDB()
+	var reloaded models.Order
+	if err := db.First(&reloaded, order.ID).Error; err != nil {
+		t.Fatalf("reload order: %v", err)
+	}
+	if reloaded.Status != models.OrderStatusFailed {
+		t.Fatalf("status: want %q, got %q", models.OrderStatusFailed, reloaded.Status)
+	}
+}
+
+// TestHandlePaymentIntentFailed_PaidNotTouched locks the upper boundary of
+// the predicate: a paid row must NEVER be flipped to failed by a stale
+// payment_failed webhook. Without this guard, a future maintainer could
+// widen the IN clause to include paid (e.g. for some refund-retry flow)
+// and silently break idempotency.
+func TestHandlePaymentIntentFailed_PaidNotTouched(t *testing.T) {
+	testDB, user := setupCartTestDB(t)
+	defer func() { _ = os.Remove(testDB) }()
+	defer database.ResetForTesting()
+
+	order := seedOrderForWebhook(t, user.ID, models.OrderStatusPaid, "pi_test_paid_untouched")
+
+	h := NewWebhookHandler("", "", "")
+	evt := buildPaymentFailedEvent(t, order.StripePaymentID, order.ID, "card_declined")
+	h.handlePaymentIntentFailed(context.Background(), evt)
+
+	db := database.GetDB()
+	var reloaded models.Order
+	if err := db.First(&reloaded, order.ID).Error; err != nil {
+		t.Fatalf("reload order: %v", err)
+	}
+	if reloaded.Status != models.OrderStatusPaid {
+		t.Fatalf("paid order was modified by payment_failed webhook: got %q", reloaded.Status)
+	}
+}
+
+// TestHandlePaymentIntentFailed_MissingMetadataNoOp covers the no-order
+// short-circuit so the test file exercises every early-return branch in
+// handlePaymentIntentFailed touched by the predicate work.
+func TestHandlePaymentIntentFailed_MissingMetadataNoOp(t *testing.T) {
+	testDB, _ := setupCartTestDB(t)
+	defer func() { _ = os.Remove(testDB) }()
+	defer database.ResetForTesting()
+
+	pi := map[string]any{"id": "pi_test_no_meta"}
+	raw, err := json.Marshal(pi)
+	if err != nil {
+		t.Fatalf("marshal pi: %v", err)
+	}
+	evt := stripe.Event{
+		Type: "payment_intent.payment_failed",
+		Data: &stripe.EventData{Raw: raw},
+	}
+
+	h := NewWebhookHandler("", "", "")
+	h.handlePaymentIntentFailed(context.Background(), evt) // must not panic
+}
+
+func TestHandlePaymentIntentSucceeded_RequiresActionFlipsToPaid(t *testing.T) {
+	testDB, user := setupCartTestDB(t)
+	defer func() { _ = os.Remove(testDB) }()
+	defer database.ResetForTesting()
+
+	order := seedOrderForWebhook(t, user.ID, models.OrderStatusRequiresAction, "pi_test_req_action_paid")
+
+	pi := map[string]any{
+		"id":       order.StripePaymentID,
+		"amount":   order.Total,
+		"metadata": map[string]string{"order_id": fmt.Sprintf("%d", order.ID)},
+	}
+	raw, err := json.Marshal(pi)
+	if err != nil {
+		t.Fatalf("marshal pi: %v", err)
+	}
+	evt := stripe.Event{
+		Type: "payment_intent.succeeded",
+		Data: &stripe.EventData{Raw: raw},
+	}
+
+	h := NewWebhookHandler("", "", "")
+	h.handlePaymentIntentSucceeded(context.Background(), evt)
+
+	db := database.GetDB()
+	var reloaded models.Order
+	if err := db.First(&reloaded, order.ID).Error; err != nil {
+		t.Fatalf("reload order: %v", err)
+	}
+	if reloaded.Status != models.OrderStatusPaid {
+		t.Fatalf("status: want %q, got %q", models.OrderStatusPaid, reloaded.Status)
+	}
+	if reloaded.StripePaymentID != order.StripePaymentID {
+		t.Fatalf("stripe_payment_id: want %q, got %q", order.StripePaymentID, reloaded.StripePaymentID)
+	}
+}

pkg/models/order.go

@@ -10,13 +10,14 @@ import (
 type OrderStatus string
 
 const (
-	OrderStatusPending   OrderStatus = "pending"
-	OrderStatusPaid      OrderStatus = "paid"
-	OrderStatusShipped   OrderStatus = "shipped"
-	OrderStatusDelivered OrderStatus = "delivered"
-	OrderStatusCancelled OrderStatus = "cancelled"
-	OrderStatusRefunded  OrderStatus = "refunded"
-	OrderStatusFailed    OrderStatus = "failed"
+	OrderStatusPending        OrderStatus = "pending"
+	OrderStatusRequiresAction OrderStatus = "requires_action"
+	OrderStatusPaid           OrderStatus = "paid"
+	OrderStatusShipped        OrderStatus = "shipped"
+	OrderStatusDelivered      OrderStatus = "delivered"
+	OrderStatusCancelled      OrderStatus = "cancelled"
+	OrderStatusRefunded       OrderStatus = "refunded"
+	OrderStatusFailed         OrderStatus = "failed"
 )
 
 type TrackingStatus string

pkg/models/order_display.go

@@ -35,6 +35,8 @@ func (order *Order) DisplayState() string {
 	switch order.Status {
 	case OrderStatusPending:
 		return "Awaiting payment"
+	case OrderStatusRequiresAction:
+		return "Awaiting authentication"
 	case OrderStatusPaid:
 		if order.TrackingNumber == "" {
 			return "Paid * awaiting label"
@@ -77,6 +79,8 @@ func (order *Order) DisplayKind() DisplayKind {
 	switch order.Status {
 	case OrderStatusPending:
 		return DisplayKindNeutral
+	case OrderStatusRequiresAction:
+		return DisplayKindNeutral
 	case OrderStatusPaid:
 		return DisplayKindAccent
 	case OrderStatusShipped:

pkg/models/order_display_test.go

@@ -14,6 +14,7 @@ func TestOrderDisplayState(t *testing.T) {
 		kind  DisplayKind
 	}{
 		{"pending", Order{Status: OrderStatusPending}, "Awaiting payment", DisplayKindNeutral},
+		{"requires_action", Order{Status: OrderStatusRequiresAction}, "Awaiting authentication", DisplayKindNeutral},
 		{"paid no tracking", Order{Status: OrderStatusPaid}, "Paid * awaiting label", DisplayKindAccent},
 		{"paid with tracking, no event", Order{Status: OrderStatusPaid, TrackingNumber: "12999"}, "Paid", DisplayKindAccent},
 		{"Paid pre-transit", Order{Status: OrderStatusPaid, TrackingNumber: "12999", TrackingStatus: TrackingStatusPreTransit}, "Paid * ready to ship", DisplayKindAccent},