fix(purchase): refuse two identical Redshift recs in one execution

internal/purchase/errnotes.go

@@ -44,6 +44,7 @@ var serverNotePatterns = []string{
 	regexp.QuoteMeta(redshiftRecentBuyRefusal),
 	regexp.QuoteMeta(redshiftRecentBuyRefusalLegacy),
 	regexp.QuoteMeta(redshiftSettleRefusal),
+	regexp.QuoteMeta(redshiftDuplicateTupleRefusal),
 	regexp.QuoteMeta(gcpDuplicateCheckFailedNote),
 	regexp.QuoteMeta(gcpRecentCommitmentNote),
 	regexp.QuoteMeta(elastiCacheDuplicateCheckFailedNote),

internal/purchase/errnotes_test.go

@@ -64,6 +64,7 @@ func TestSanitizeStoredError_AllowlistedNotesPassThroughVerbatim(t *testing.T) {
 		redshiftRecentBuyRefusal,
 		redshiftRecentBuyRefusalLegacy,
 		redshiftSettleRefusal,
+		redshiftDuplicateTupleRefusal,
 		gcpDuplicateCheckFailedNote,
 		gcpRecentCommitmentNote,
 		elastiCacheDuplicateCheckFailedNote,
@@ -220,7 +221,7 @@ func TestSanitizeStoredError_ResolveNoteExchangeIDIsClosed(t *testing.T) {
 // The Redshift gate texts are fixed: an appended execution id or raw text must
 // not pass verbatim.
 func TestSanitizeStoredError_RedshiftGateRefusalsAreClosed(t *testing.T) {
-	for _, text := range []string{redshiftConflictRefusal, redshiftHorizonRefusal, redshiftNoStampRefusal, redshiftFailedConflictRefusal, redshiftNoJournalRefusal, redshiftRecentBuyRefusal, redshiftSettleRefusal, gcpDuplicateCheckFailedNote, gcpRecentCommitmentNote, elastiCacheDuplicateCheckFailedNote, elastiCacheRecentReservationNote} {
+	for _, text := range []string{redshiftConflictRefusal, redshiftHorizonRefusal, redshiftNoStampRefusal, redshiftFailedConflictRefusal, redshiftNoJournalRefusal, redshiftRecentBuyRefusal, redshiftSettleRefusal, redshiftDuplicateTupleRefusal, gcpDuplicateCheckFailedNote, gcpRecentCommitmentNote, elastiCacheDuplicateCheckFailedNote, elastiCacheRecentReservationNote} {
 		assert.Equal(t, text, SanitizeStoredError(text))
 		for _, variant := range []string{
 			text + " (execution 00000000-0000-0000-0000-000000000000, running)",

internal/purchase/manager.go

@@ -435,6 +435,9 @@ func (m *Manager) preExecutionRefusal(ctx context.Context, exec *config.Purchase
 	if refusal := m.ladderExecutionRefusal(ctx, exec); refusal != nil {
 		return beforeSubmission(refusal)
 	}
+	if hasDuplicateRedshiftTuple(exec) {
+		return beforeSubmission(errors.New(redshiftDuplicateTupleRefusal))
+	}
 	// A Redshift retry reuses the first attempt's request time; only the journal lets go's dedupe tell its node from another's.
 	if hasRedshiftRec(exec) && exec.RetryAttemptN > 0 && m.attemptJournal == nil {
 		return errors.New(redshiftNoJournalRefusal)

internal/purchase/redshift_duplicate_tuple_test.go

@@ -0,0 +1,95 @@
+package purchase
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"github.com/LeanerCloud/cloud-commitments-go/pkg/common"
+	"github.com/LeanerCloud/cloud-commitments-platform/internal/config"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func dupTupleExec(recs ...config.RecommendationRecord) *config.PurchaseExecution {
+	return &config.PurchaseExecution{
+		ExecutionID: "rs-dup", IdempotencyKey: "rs-dup-lineage", Status: "running", Recommendations: recs,
+	}
+}
+
+func TestRedshiftDuplicateTuple_RefusedBeforeSubmission(t *testing.T) {
+	at := time.Now().UTC().Add(-time.Minute)
+	exec := dupTupleExec(redshiftScopedRec("acct-A"), redshiftScopedRec("acct-A"))
+	run := runRedshiftFinalize(t, exec, nil, common.ServiceRedshift, stampAt(at))
+
+	require.Error(t, run.err)
+	assert.Equal(t, redshiftDuplicateTupleRefusal, run.err.Error())
+	assert.True(t, isBeforeSubmission(run.err))
+	assert.True(t, exec.NoSubmission, "nothing was submitted, so the row is retryable from scratch")
+	assert.Equal(t, "failed", exec.Status)
+	assert.Empty(t, run.opts, "no PurchaseCommitment call")
+	assert.Empty(t, run.stampedIDs, "refused before the dispatch gate stamps")
+}
+
+func TestRedshiftDuplicateTuple_Detection(t *testing.T) {
+	a := redshiftScopedRec("acct-A")
+	diff := func(f func(*config.RecommendationRecord)) config.RecommendationRecord {
+		r := redshiftScopedRec("acct-A")
+		f(&r)
+		return r
+	}
+	unselected := redshiftScopedRec("acct-A")
+	unselected.Selected = false
+	ec2 := scopedTestRec("acct-A")
+	noAcct := redshiftScopedRec("acct-A")
+	noAcct.CloudAccountID = nil
+
+	cases := map[string]struct {
+		recs    []config.RecommendationRecord
+		refused bool
+	}{
+		"identical tuple and account":   {[]config.RecommendationRecord{a, a}, true},
+		"identical, third one differs":  {[]config.RecommendationRecord{a, diff(func(r *config.RecommendationRecord) { r.Region = "eu-west-1" }), a}, true},
+		"different node type":           {[]config.RecommendationRecord{a, diff(func(r *config.RecommendationRecord) { r.ResourceType = "ra3.4xlarge" })}, false},
+		"different region":              {[]config.RecommendationRecord{a, diff(func(r *config.RecommendationRecord) { r.Region = "eu-west-1" })}, false},
+		"different term":                {[]config.RecommendationRecord{a, diff(func(r *config.RecommendationRecord) { r.Term = 3 })}, false},
+		"different payment":             {[]config.RecommendationRecord{a, diff(func(r *config.RecommendationRecord) { r.Payment = "no_upfront" })}, false},
+		"different count":               {[]config.RecommendationRecord{a, diff(func(r *config.RecommendationRecord) { r.Count = 2 })}, false},
+		"different account":             {[]config.RecommendationRecord{a, redshiftScopedRec("acct-B")}, false},
+		"duplicate is not selected":     {[]config.RecommendationRecord{a, unselected}, false},
+		"non-redshift twins":            {[]config.RecommendationRecord{ec2, ec2}, false},
+		"single rec":                    {[]config.RecommendationRecord{a}, false},
+		"both without an account":       {[]config.RecommendationRecord{noAcct, noAcct}, true},
+		"one scoped, one without scope": {[]config.RecommendationRecord{a, noAcct}, false},
+	}
+	for name, tc := range cases {
+		t.Run(name, func(t *testing.T) {
+			m := &Manager{config: new(MockConfigStore), attemptJournal: &fakeAttemptJournal{}}
+			err := m.preExecutionRefusal(context.Background(), dupTupleExec(tc.recs...))
+			if tc.refused {
+				require.Error(t, err)
+				assert.Equal(t, redshiftDuplicateTupleRefusal, err.Error())
+				assert.True(t, isBeforeSubmission(err))
+				return
+			}
+			assert.NoError(t, err)
+		})
+	}
+}
+
+// A rec with no account of its own buys in the execution's account, so it duplicates a scoped twin there.
+func TestRedshiftDuplicateTuple_UnscopedRecUsesTheExecutionAccount(t *testing.T) {
+	acct := "acct-A"
+	noAcct := redshiftScopedRec("acct-A")
+	noAcct.CloudAccountID = nil
+	exec := dupTupleExec(redshiftScopedRec("acct-A"), noAcct)
+	exec.CloudAccountID = &acct
+
+	err := (&Manager{config: new(MockConfigStore)}).preExecutionRefusal(context.Background(), exec)
+	require.Error(t, err)
+	assert.Equal(t, redshiftDuplicateTupleRefusal, err.Error())
+
+	other := "acct-B"
+	exec.CloudAccountID = &other
+	assert.NoError(t, (&Manager{config: new(MockConfigStore)}).preExecutionRefusal(context.Background(), exec))
+}

internal/purchase/redshift_lineage.go

@@ -36,6 +36,32 @@ func hasRedshiftRec(exec *config.PurchaseExecution) bool {
 	return false
 }
 
+// hasDuplicateRedshiftTuple reports whether two selected Redshift recs share a tuple and account. The
+// go client adopts a node it finds for the same offering and count, so the second would buy nothing.
+func hasDuplicateRedshiftTuple(exec *config.PurchaseExecution) bool {
+	type key struct {
+		tuple   config.RedshiftTuple
+		account string
+	}
+	seen := map[key]bool{}
+	for _, t := range redshiftTuples(exec) {
+		account := ""
+		switch {
+		case t.CloudAccountID != nil:
+			account = *t.CloudAccountID
+		case exec.CloudAccountID != nil:
+			account = *exec.CloudAccountID
+		}
+		t.CloudAccountID = nil
+		k := key{t, account}
+		if seen[k] {
+			return true
+		}
+		seen[k] = true
+	}
+	return false
+}
+
 // afterRedshiftStampHook runs inside the gate transaction right after the
 // stamp. Tests set it to observe the lock wait; it is nil in production.
 var afterRedshiftStampHook func()
@@ -51,6 +77,7 @@ const (
 	redshiftRecentBuyRefusal      = "refusing to dispatch: another purchase of the same Redshift reservation (region, node type, term, payment, count) ended too close to this purchase's first dispatch for AWS to tell the two apart, so start a new purchase at least 10 minutes after the other one ended, or, if it failed and can still be retried, retry or settle it"
 	// redshiftRecentBuyRefusalLegacy is the text stored before failed rows joined the adoption window.
 	redshiftRecentBuyRefusalLegacy = "refusing to dispatch: another purchase of the same Redshift reservation (region, node type, term, payment, count) completed too close to this purchase's first dispatch for AWS to tell the two apart, so start a new purchase at least 10 minutes after the other one completed"
+	redshiftDuplicateTupleRefusal  = "refusing to dispatch: this purchase selects the same Redshift reservation (region, node type, term, payment, count) twice for one account, and AWS would treat the second as the first, so keep one of them in the selection and start a new purchase"
 	redshiftSettleRefusal          = "refusing to dispatch: another attempt of this Redshift purchase ended less than 10 minutes ago and may have bought, so wait 10 minutes and then retry"
 )
 

known_issues/39_redshift_dedupe_window.md

@@ -12,10 +12,11 @@
 
 **Mitigation (platform-recorded buys):** The Redshift dispatch gate now refuses a purchase when another lineage's completed, partially completed or revocation-requested row, or a failed row without the no-submission proof (settled or not, and whether or not Retry is refused), for the same tuple was last saved within 10 minutes of the new lineage's first dispatch time. The refusal is recorded before submission with a fixed text that names no execution, and the user starts a new purchase at least 10 minutes after the other one ended, or retries or settles it if it failed and can still be retried.
 
+**Two identical-tuple recs in one execution** (same region, node type, term, payment, count and account) are refused before submission with a fixed text (`redshiftDuplicateTupleRefusal`, checked in `preExecutionRefusal`), since the go client would adopt the first node for the second. The user keeps one of them and starts a new purchase.
+
 **Remaining:**
 
 - Buys made outside the platform are invisible to the gate.
-- Two identical-tuple recs in one execution still adopt each other (platform #1809).
 - A lineage whose purchase record never landed (the child bought but its row save failed, `execution.go:351` and `:383`) leaves no completed row for the gate to see.
 - The go-side narrowing in LeanerCloud/cloud-commitments-go#700 and the real fix in #515 are open.
 - The window is measured from the other row's last save, so a later save of that row (for example a revocation request) restarts it. That only refuses longer; it never allows a double buy.