feat(purchase): retry a failed first attempt that provably sent nothing (EC2, Azure savings plans)

internal/api/handler_history.go

@@ -407,11 +407,12 @@ func executionToHistoryRow(exec config.PurchaseExecution, approver, createdByEma
 	// inputs, so this is safe to call unconditionally.
 	if exec.Status == "failed" {
 		row.OpsHint = purchase.ResolveOpsHint(exec.Error)
-		// A re-drive the backend will always refuse is permanent, so it wins
+		// A retry the backend will always refuse is permanent, so it wins
 		// over the operator-fixable failure-reason hint (which promises that
 		// fixing the config makes Retry work). Same call and input as the
-		// retry handler's gate so the two cannot disagree (issue #161).
-		if reason := purchase.RedriveRefusalReason(&exec); reason != "" {
+		// retry handler's gate so the two cannot disagree (issue #161); a
+		// proven-unsent first attempt keeps Retry (issue #1803).
+		if reason := purchase.RetryRefusalReason(&exec); reason != "" {
 			row.RedriveUnsafe = true
 			// Only a proven pre-send AWS refusal on every rec means nothing was
 			// bought; for any other outcome the duplicate-purchase warning stays.

internal/api/handler_purchases.go

@@ -2428,8 +2428,9 @@ const retryThreshold = 5
 //   - failedExec.Status must be "failed" → 409 otherwise.
 //   - a plan-step row (PlanID set) must carry at least one recommendation
 //     → 409 otherwise, nothing to buy (platform#609).
-//   - every rec must be safe to re-drive per purchase.RedriveRefusalReason
-//     → 409 with ops_hint + redrive_unsafe when any is not, NOT
+//   - every rec must be safe to re-drive per purchase.RetryRefusalReason
+//     (a proven-unsent attempt-0 EC2 or Azure savings-plan row is lifted,
+//     #1803) → 409 with ops_hint + redrive_unsafe when any is not, NOT
 //     overridable by ?force=true (issue #1668).
 //   - failedExec.Error must NOT match the persistent-failure map →
 //     409 with ops_hint when it does (Q3).
@@ -2603,15 +2604,17 @@ func checkRetryEligibilityGates(failedExec *config.PurchaseExecution, req *event
 	// Provider re-drive safety (issue #1668). A "failed" row may in fact
 	// have landed its commitment at the provider (a timeout, a lost
 	// response, a post-purchase write that failed), so every retry is
-	// potentially a re-drive. purchase.RedriveRefusalReason is the same
-	// predicate the reaper's automatic re-drive gates on: it returns a
-	// reason exactly when the provider offers nothing that would collapse
-	// the second attempt onto the first, which today means AWS EC2 (no
-	// client token; a lost response is reported as outcome unknown), Azure
-	// savings-plans (no server-side idempotency key, timestamp-derived order
-	// alias) plus any provider the predicate does not recognize. A failed EC2
-	// row therefore cannot be retried; the operator checks the account and
-	// starts a new purchase. Redshift is gated by checkRedshiftRetryGate.
+	// potentially a re-drive. purchase.RetryRefusalReason is the reaper's
+	// RedriveRefusalReason minus the EC2 and Azure savings-plans refusals for a
+	// row proven to have sent nothing (no_submission at attempt 0). The
+	// remaining reasons exist exactly when the provider offers nothing that
+	// would collapse the second attempt onto the first: AWS EC2 (no client
+	// token; a lost response is reported as outcome unknown), Azure
+	// savings-plans (no server-side idempotency key, no tag-based lookup) plus
+	// any provider the predicate does not recognize. A failed EC2 or Azure
+	// savings-plans row that may have sent therefore cannot be retried; the
+	// operator checks the account and starts a new purchase. Redshift is gated
+	// by checkRedshiftRetryGate.
 	// If plan steps ever carry recommendations (platform#609), a failed EC2
 	// step will be un-retryable too and needs its own recovery path.
 	//
@@ -2625,15 +2628,15 @@ func checkRetryEligibilityGates(failedExec *config.PurchaseExecution, req *event
 	// A refusal here is permanent: nothing about the row can change to make
 	// it retryable. So the gate must fire only where the duplicate hazard is
 	// real. An execution carrying no recommendations buys nothing and cannot
-	// double-buy, so RedriveRefusalReason stays silent on the empty case. The
+	// double-buy, so the predicate stays silent on the empty case. The
 	// rec-less plan steps that createPurchaseExecutionsTx (handler_plans.go) and
 	// getOrCreateExecution (purchase/notifications.go) used to create are
 	// refused earlier in this handler instead (platform#609), because a retry of
 	// one can only fail the same way. Empty here always means empty as created,
 	// never "we could not load them": GetExecutionByID propagates a
 	// recommendations unmarshal failure as an error (config/store_postgres.go),
 	// which this handler has already turned into a 500 well before this gate.
-	if reason := purchase.RedriveRefusalReason(failedExec); reason != "" {
+	if reason := purchase.RetryRefusalReason(failedExec); reason != "" {
 		return NewClientErrorWithDetails(409,
 			"this purchase cannot be retried safely: "+reason,
 			// ops_hint reuses the key the History UI already renders in
@@ -2642,11 +2645,10 @@ func checkRetryEligibilityGates(failedExec *config.PurchaseExecution, req *event
 			// the operator-fixable hints below, which clear once the
 			// configuration is fixed.
 			//
-			// No frontend reads redrive_unsafe yet, so it looks unused: issue
-			// #1714 is its intended consumer, where History will render a
-			// terminal badge instead of offering a Retry button that always
-			// 409s. That is presentation only. This refusal is enforced here,
-			// server-side, and does not depend on any client honoring it.
+			// History also sets redrive_unsafe on the row (handler_history.go)
+			// to hide Retry. That is presentation only. This refusal is
+			// enforced here, server-side, and does not depend on any client
+			// honoring it.
 			map[string]any{"ops_hint": reason, "redrive_unsafe": true})
 	}
 
@@ -2761,15 +2763,15 @@ func (h *Handler) persistRetryExecution(ctx context.Context, failedExec *config.
 	// against the identical lineage key, which is the precondition for the
 	// provider-side dedupe to engage at all.
 	//
-	// Whether it then DOES engage is per-provider and not universal: AWS (all
-	// services), Azure reservations and GCP CUDs reproduce the token and
-	// dedupe, but Azure savings-plans has no server-side idempotency key and
-	// names its order alias from time.Now().UnixNano(), so a re-drive of a
-	// landed Azure SP order still duplicates it. Scope propagation is therefore
-	// necessary for dedupe everywhere and sufficient everywhere except Azure SP.
-	// purchase.RedriveRefusalReason encodes exactly that exclusion and now gates
-	// this handler too (checkRetryEligibilityGates, issue #1668), so an Azure SP
-	// row never reaches this function in the first place.
+	// Whether it then DOES engage is per-provider and not universal: most AWS
+	// services, Azure reservations and GCP CUDs reproduce the token and
+	// dedupe, but AWS EC2 takes no client token and Azure savings-plans has no
+	// tag-based lookup, so a re-drive of a landed order of either may still
+	// duplicate it. Scope propagation is therefore necessary for dedupe
+	// everywhere and sufficient nowhere for those two.
+	// purchase.RetryRefusalReason gates this handler on that exclusion
+	// (checkRetryEligibilityGates, issue #1668) and lifts it only for a
+	// proven-unsent first attempt, which cannot have landed.
 	//
 	// Copied by value rather than by pointer so the successor and the
 	// historical failed row never share a *string, matching the defensive
@@ -2816,8 +2818,9 @@ func (h *Handler) persistRetryExecution(ctx context.Context, failedExec *config.
 // Order matters: the FK on retry_execution_id requires the successor to
 // exist before the original can point at it.
 //
-// The linkage write is an atomic CAS (LinkRetryExecutionAtomic, conditional
-// on status='failed' AND retry_execution_id IS NULL), not a full-row
+// The linkage write is an atomic CAS (LinkUnsentRetryExecutionAtomic when the
+// no-guard refusal was lifted by the proven-unsent proof, LinkRetryExecutionAtomic
+// otherwise; conditional on status='failed' AND retry_execution_id IS NULL), not a full-row
 // upsert of a stale in-memory copy of the failed row. This is the
 // authoritative, race-safe guard: the pre-tx already-retried check in
 // loadAndValidateRetryRequest is only a fast-path optimization, since two
@@ -2880,7 +2883,7 @@ func (h *Handler) writeRetrySuccessorTx(ctx context.Context, tx pgx.Tx, failedEx
 	if err := h.config.SavePurchaseExecutionTx(ctx, tx, newExecution); err != nil {
 		return err
 	}
-	linked, linkErr := h.config.LinkRetryExecutionAtomic(ctx, tx, failedExec.ExecutionID, newExecution.ExecutionID)
+	linked, linkErr := h.linkRetrySuccessor(ctx, tx, failedExec, newExecution.ExecutionID)
 	if linkErr != nil {
 		return linkErr
 	}
@@ -2898,6 +2901,16 @@ func (h *Handler) writeRetrySuccessorTx(ctx context.Context, tx pgx.Tx, failedEx
 	return nil
 }
 
+// linkRetrySuccessor links the failed row to its successor. A row retryable
+// only on its no-submission proof links through the CAS that re-checks the
+// proof, so a row that stopped being one after the gate read it is not linked.
+func (h *Handler) linkRetrySuccessor(ctx context.Context, tx pgx.Tx, failedExec *config.PurchaseExecution, successorID string) (bool, error) {
+	if purchase.RedriveRefusalReason(failedExec) != "" && purchase.RetryRefusalReason(failedExec) == "" {
+		return h.config.LinkUnsentRetryExecutionAtomic(ctx, tx, failedExec.ExecutionID, successorID)
+	}
+	return h.config.LinkRetryExecutionAtomic(ctx, tx, failedExec.ExecutionID, successorID)
+}
+
 // checkRetryDuplicateSubmit returns a *retryDuplicateSubmitError when recent
 // (the creator's live web submits, read under the submit lock) already holds
 // an execution with the successor's submit fingerprint. The successor's own
@@ -2927,7 +2940,7 @@ func (e *retryDuplicateSubmitError) Error() string {
 
 // errRetryLinkRaceLost is returned from persistRetrySuccessorTx's tx
 // closure to force a rollback (via WithTx's error path) when
-// LinkRetryExecutionAtomic matches zero rows -- a concurrent retry already
+// the link CAS matches zero rows -- a concurrent retry already
 // claimed the failed row between the pre-tx already-retried check and this
 // tx's UPDATE. Never returned to a caller directly; persistRetrySuccessorTx
 // translates it into a 409 ClientError.

internal/api/handler_purchases_retry_unsent_test.go

@@ -0,0 +1,122 @@
+package api
+
+import (
+	"context"
+	"testing"
+
+	"github.com/LeanerCloud/cloud-commitments-platform/internal/config"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
+)
+
+// Issue #1803: a failed EC2 or Azure savings-plans row that is a proven-unsent
+// first attempt (no_submission at attempt 0) cannot have bought anything, so
+// Retry is allowed and links through the CAS that re-checks that proof.
+
+const redriveUnsentExecID = "22222222-3333-4444-5555-666666666609"
+
+func unsentRow(provider, service string) *config.PurchaseExecution {
+	row := redriveFailedRow(redriveUnsentExecID, provider, service)
+	row.NoSubmission = true
+	return row
+}
+
+// retryWithLinkMocks runs the real retry handler with the successor insert
+// mocked and returns the store so the caller registers the link outcome first.
+func retryWithLinkMocks(t *testing.T, failed *config.PurchaseExecution) (*Handler, *MockConfigStore) {
+	t.Helper()
+	session := &Session{UserID: retryCallerID, Email: "operator@example.com"}
+	handler, mockConfig, _ := buildSessionRetryHandler(t, failed, session, false, true)
+	mockConfig.On("SavePurchaseExecution", mock.Anything, mock.AnythingOfType("*config.PurchaseExecution")).Return(nil).Maybe()
+	return handler, mockConfig
+}
+
+func TestRetryOfProvenUnsentRowLinksViaUnsentCAS(t *testing.T) {
+	for _, tc := range []struct{ name, provider, service string }{
+		{"aws ec2", "aws", "ec2"},
+		{"azure savings plans", "azure", "savingsplans"},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			failed := unsentRow(tc.provider, tc.service)
+			handler, store := retryWithLinkMocks(t, failed)
+			store.On("LinkUnsentRetryExecutionAtomic", mock.Anything, mock.Anything, failed.ExecutionID, mock.AnythingOfType("string")).Return(true, nil).Once()
+
+			result, err := handler.retryPurchase(context.Background(), sessionRetryReq(), failed.ExecutionID)
+
+			require.NoError(t, err, "a proven-unsent first attempt must be retryable")
+			assert.NotEmpty(t, result.(map[string]any)["execution_id"])
+			store.AssertNotCalled(t, "LinkRetryExecutionAtomic")
+		})
+	}
+}
+
+func TestRetryOfProvenUnsentRowLosingUnsentLinkIs409(t *testing.T) {
+	failed := unsentRow("aws", "ec2")
+	handler, store := retryWithLinkMocks(t, failed)
+	store.On("LinkUnsentRetryExecutionAtomic", mock.Anything, mock.Anything, failed.ExecutionID, mock.AnythingOfType("string")).Return(false, nil).Once()
+
+	result, err := handler.retryPurchase(context.Background(), sessionRetryReq(), failed.ExecutionID)
+
+	require.Error(t, err)
+	assert.Nil(t, result)
+	ce, ok := IsClientError(err)
+	require.True(t, ok)
+	assert.Equal(t, 409, ce.code)
+	assert.Contains(t, err.Error(), "already retried or settled")
+	store.AssertNotCalled(t, "CreateSuppressionTx")
+}
+
+// The proof is per row: a submitted EC2 row and a no_submission successor
+// (attempt 1) stay refused, whatever the flag says about later attempts.
+func TestRetryOfEC2RowWithoutAttemptZeroProofIsRefused(t *testing.T) {
+	submitted := redriveFailedRow(redriveUnsentExecID, "aws", "ec2")
+	attempt1 := unsentRow("aws", "ec2")
+	attempt1.RetryAttemptN = 1
+	for name, failed := range map[string]*config.PurchaseExecution{"submitted": submitted, "no_submission attempt 1": attempt1} {
+		t.Run(name, func(t *testing.T) {
+			tokens, err := purchasesFiredByRetry(t, failed, sessionRetryReq())
+
+			assert.Empty(t, tokens)
+			assertRedriveRefused(t, err, "Check EC2 Reserved Instances in the target account")
+		})
+	}
+}
+
+// Rows that were never refused keep the plain CAS and never touch the new one.
+func TestRetryOfRDSRowStillLinksViaPlainCAS(t *testing.T) {
+	failed := unsentRow("aws", "rds")
+	handler, store := retryWithLinkMocks(t, failed)
+	store.On("LinkRetryExecutionAtomic", mock.Anything, mock.Anything, failed.ExecutionID, mock.AnythingOfType("string")).Return(true, nil).Once()
+
+	_, err := handler.retryPurchase(context.Background(), sessionRetryReq(), failed.ExecutionID)
+
+	require.NoError(t, err)
+	store.AssertNotCalled(t, "LinkUnsentRetryExecutionAtomic")
+}
+
+func TestExecutionToHistoryRow_RetryHintForUnsentRows(t *testing.T) {
+	for _, tc := range []struct {
+		name       string
+		provider   string
+		service    string
+		unsent     bool
+		wantUnsafe bool
+	}{
+		{"unsent ec2 shows Retry", "aws", "ec2", true, false},
+		{"unsent azure savings plans shows Retry", "azure", "savingsplans", true, false},
+		{"submitted ec2 is not retryable", "aws", "ec2", false, true},
+		{"submitted azure savings plans is not retryable", "azure", "savingsplans", false, true},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			exec := redriveFailedRow(redriveUnsentExecID, tc.provider, tc.service)
+			exec.Error = "boom"
+			exec.NoSubmission = tc.unsent
+
+			row := executionToHistoryRow(*exec, "", "")
+
+			assert.Equal(t, tc.wantUnsafe, row.RedriveUnsafe)
+			assert.Equal(t, tc.wantUnsafe, row.OpsHint != "", "ops_hint is what hides the Retry button")
+		})
+	}
+}

internal/api/openapi.yaml

@@ -1471,7 +1471,9 @@ paths:
             known; a concurrent retry also answers this way), it is a plan step
             with no recommendations, it belongs to a settled ladder tranche,
             a provider re-drive of it is unsafe (ops_hint plus
-            redrive_unsafe, permanent and not overridable by force), it is a
+            redrive_unsafe, permanent and not overridable by force; an AWS
+            EC2 or Azure savings-plans row proven to have sent nothing, with
+            no_submission at attempt 0, is retryable), it is a
             Redshift purchase that failed too recently for AWS to have settled
             (retry_after_seconds and a Retry-After header, not overridable by
             force), its failure is operator-fixable (ops_hint plus
@@ -7731,7 +7733,9 @@ components:
           description: >-
             Execution-only. True when ops_hint is a permanent re-drive refusal,
             including a Redshift purchase with no first dispatch time or one
-            dispatched at least 24h ago.
+            dispatched at least 24h ago. False for an AWS EC2 or Azure
+            savings-plans row proven to have sent nothing (no_submission at
+            attempt 0), which stays retryable.
         is_audit_gap:
           type: boolean
           description: >-

internal/config/interfaces.go

@@ -313,6 +313,10 @@ type StoreInterface interface {
 	// DB error. Must be called inside the same tx that inserts the
 	// successor row, after that insert.
 	LinkRetryExecutionAtomic(ctx context.Context, tx pgx.Tx, executionID, retryExecutionID string) (linked bool, err error)
+	// LinkUnsentRetryExecutionAtomic is LinkRetryExecutionAtomic with the extra
+	// condition no_submission AND retry_attempt_n = 0 (a proven-unsent first
+	// attempt). The handler uses it for retries it allowed only on that proof.
+	LinkUnsentRetryExecutionAtomic(ctx context.Context, tx pgx.Tx, executionID, retryExecutionID string) (linked bool, err error)
 	// LockLadderTrancheForRetryTx row-locks the ladder tranche whose id equals
 	// idempotencyKey (matched as text; found=false for any non-ladder key) so
 	// Retry can refuse a settled tranche before inserting a successor.
@@ -821,6 +825,10 @@ type StoreInterface interface {
 	// whether it is within horizon on the database clock.
 	StampFirstDispatchTx(ctx context.Context, tx pgx.Tx, executionID string, horizon time.Duration) (StampResult, error)
 
+	// PredecessorProvenUnsent reports whether executionID's retry predecessor
+	// is a no_submission attempt 0 row (the lineage provably sent nothing).
+	PredecessorProvenUnsent(ctx context.Context, executionID string) (bool, error)
+
 	// LockRedshiftTuplesTx takes a transaction-scoped advisory lock per
 	// distinct tuple, in sorted order, serializing dispatches that share one.
 	LockRedshiftTuplesTx(ctx context.Context, tx pgx.Tx, tuples []RedshiftTuple) error

internal/config/store_postgres.go

@@ -1290,6 +1290,19 @@ func (s *PostgresStore) SavePurchaseExecutionTx(ctx context.Context, tx pgx.Tx,
 // successor row, after that insert, so the FK on retry_execution_id is
 // satisfied.
 func (s *PostgresStore) LinkRetryExecutionAtomic(ctx context.Context, tx pgx.Tx, executionID, retryExecutionID string) (linked bool, err error) {
+	return s.linkRetryExecution(ctx, tx, executionID, retryExecutionID, "")
+}
+
+// LinkUnsentRetryExecutionAtomic is LinkRetryExecutionAtomic for a retry the
+// handler allowed only because the row is a proven-unsent first attempt: the
+// same conditional UPDATE plus no_submission AND retry_attempt_n = 0, so a row
+// that stopped being that proof between the gate and the link is not linked.
+func (s *PostgresStore) LinkUnsentRetryExecutionAtomic(ctx context.Context, tx pgx.Tx, executionID, retryExecutionID string) (linked bool, err error) {
+	return s.linkRetryExecution(ctx, tx, executionID, retryExecutionID, "AND no_submission AND retry_attempt_n = 0")
+}
+
+// linkRetryExecution is the shared link CAS; extraPredicate narrows it.
+func (s *PostgresStore) linkRetryExecution(ctx context.Context, tx pgx.Tx, executionID, retryExecutionID, extraPredicate string) (bool, error) {
 	// Before the settlement migration no row can be settled (#999).
 	notSettled := ""
 	if ready, readyErr := settlementSchemaReady(ctx, tx); readyErr != nil {
@@ -1304,7 +1317,8 @@ func (s *PostgresStore) LinkRetryExecutionAtomic(ctx context.Context, tx pgx.Tx,
 		 WHERE execution_id = $1
 		   AND status = 'failed'
 		   AND retry_execution_id IS NULL
-		   ` + notSettled
+		   ` + notSettled + `
+		   ` + extraPredicate
 	tag, err := tx.Exec(ctx, q, executionID, retryExecutionID)
 	if err != nil {
 		return false, fmt.Errorf("failed to link retry execution: %w", err)