fix(purchase): record fan-out accounts skipped before they started

internal/ctxutil/detached.go

@@ -17,3 +17,13 @@ import (
 func DetachedWithTimeout(ctx context.Context, d time.Duration) (context.Context, context.CancelFunc) {
 	return context.WithTimeout(context.WithoutCancel(ctx), d)
 }
+
+// DetachedWithDeadline is DetachedWithTimeout with an absolute deadline: ctx's
+// values are kept, its cancellation and deadline are dropped, and the result
+// ends at deadline. Use it when several detached phases must share one
+// schedule anchored on an earlier deadline instead of each starting a fresh
+// timeout from "now". A deadline already in the past yields a context that is
+// already done. Always defer the returned CancelFunc.
+func DetachedWithDeadline(ctx context.Context, deadline time.Time) (context.Context, context.CancelFunc) {
+	return context.WithDeadline(context.WithoutCancel(ctx), deadline)
+}

internal/ctxutil/detached_test.go

@@ -35,3 +35,29 @@ func TestDetachedWithTimeout(t *testing.T) {
 		t.Fatal("the returned CancelFunc must cancel the detached context")
 	}
 }
+
+func TestDetachedWithDeadline(t *testing.T) {
+	parent, cancelParent := context.WithTimeout(context.WithValue(context.Background(), ctxKey{}, "req-2"), time.Millisecond)
+	cancelParent()
+
+	want := time.Now().Add(time.Hour)
+	ctx, cancel := DetachedWithDeadline(parent, want)
+	defer cancel()
+
+	if err := ctx.Err(); err != nil {
+		t.Fatalf("parent cancellation must not reach the detached context, got %v", err)
+	}
+	got, ok := ctx.Deadline()
+	if !ok || !got.Equal(want) {
+		t.Fatalf("deadline must be exactly the absolute one given: got %v ok=%v want %v", got, ok, want)
+	}
+	if v := ctx.Value(ctxKey{}); v != "req-2" {
+		t.Fatalf("detached context must keep parent values, got %v", v)
+	}
+
+	past, cancelPast := DetachedWithDeadline(context.Background(), time.Now().Add(-time.Second))
+	defer cancelPast()
+	if past.Err() == nil {
+		t.Fatal("a deadline in the past must give an already-done context")
+	}
+}

internal/purchase/account_row_deadline_test.go

@@ -0,0 +1,111 @@
+package purchase
+
+import (
+	"context"
+	"errors"
+	"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/mock"
+	"github.com/stretchr/testify/require"
+)
+
+const lateTerminalT = 200 * time.Millisecond // T; the account slice T/2 is 100ms
+
+// accountSaveCase triggers the run context ending at one of the three places
+// executeForAccount saves its row.
+type accountSaveCase struct {
+	name    string
+	trigger func(f *lateFixtureOneAccount)
+	status  string
+}
+
+type lateFixtureOneAccount struct {
+	cancel  context.CancelFunc
+	store   *MockConfigStore
+	client  *MockServiceClient
+	loadRaw func(context.Context, string, string) ([]byte, error)
+}
+
+// Each of executeForAccount's three row saves must run on a live context whose
+// deadline is the run's deadline plus T/2, however the run context ended.
+func TestExecuteForAccount_RowSavesAreLiveAndAnchoredOnTheRunDeadline(t *testing.T) {
+	keys := func(context.Context, string, string) ([]byte, error) {
+		return []byte(`{"access_key_id":"AKIAIOSFODNN7EXAMPLE","secret_access_key":"wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY"}`), nil
+	}
+	cases := []accountSaveCase{
+		{"processing error", func(f *lateFixtureOneAccount) {
+			f.store.On("GetGlobalConfig", mock.Anything).Run(func(mock.Arguments) { f.cancel() }).Return(nil, errors.New("db down"))
+		}, "failed"},
+		{"outcome", func(f *lateFixtureOneAccount) {
+			f.client.On("PurchaseCommitment", mock.Anything, mock.Anything, mock.Anything).
+				Run(func(mock.Arguments) { f.cancel() }).Return(common.PurchaseResult{Success: true, CommitmentID: "ri-1"}, nil)
+		}, "completed"},
+		{"credential failure", func(f *lateFixtureOneAccount) {
+			f.loadRaw = func(context.Context, string, string) ([]byte, error) { f.cancel(); return nil, nil }
+		}, "failed"},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			store := new(MockConfigStore)
+			mailer := allowFailedNotification(new(MockEmailSender))
+			factory := new(MockProviderFactory)
+			prov := new(MockProvider)
+			client := new(MockServiceClient)
+			runCtx, runCancel := context.WithTimeout(context.Background(), time.Hour)
+			defer runCancel()
+			f := &lateFixtureOneAccount{cancel: runCancel, store: store, client: client, loadRaw: keys}
+
+			var saved []config.PurchaseExecution
+			var live []bool
+			var deadlines []time.Time
+			store.SavePurchaseExecutionFn = func(ctx context.Context, e *config.PurchaseExecution) error {
+				saved = append(saved, *e)
+				live = append(live, ctx.Err() == nil)
+				dl, _ := ctx.Deadline()
+				deadlines = append(deadlines, dl)
+				return nil
+			}
+			store.On("SavePurchaseHistory", mock.Anything, mock.Anything).Return(nil).Maybe()
+			mailer.On("SendPurchaseConfirmation", mock.Anything, mock.Anything).Return(nil).Maybe()
+			factory.On("CreateAndValidateProvider", mock.Anything, "aws", mock.Anything).Return(prov, nil).Maybe()
+			prov.On("GetServiceClient", mock.Anything, common.ServiceEC2, "us-east-1").Return(client, nil).Maybe()
+			tc.trigger(f)
+
+			m := &Manager{config: store, email: mailer, providerFactory: factory,
+				budgets: purchaseBudgets{terminalSave: lateTerminalT},
+				credStore: &MockCredentialStore{LoadRawFn: func(c context.Context, a, k string) ([]byte, error) {
+					return f.loadRaw(c, a, k)
+				}}}
+			account := config.CloudAccount{ID: skipAcctA, Name: "A", Provider: "aws", ExternalID: "111111111111", AWSAuthMode: "access_keys"}
+			base := &config.PurchaseExecution{ExecutionID: "exec-pin", PlanID: "plan-pin", IdempotencyKey: "lineage-pin",
+				Recommendations: []config.RecommendationRecord{withAccount(config.RecommendationRecord{Provider: "aws", Service: "ec2",
+					ResourceType: "m5.large", Region: "us-east-1", Count: 1, Selected: true}, skipAcctA)}}
+			runDeadline, _ := runCtx.Deadline()
+
+			_, _ = m.executeForAccount(runCtx, base, &config.PurchasePlan{ID: "plan-pin", Name: "pin"}, account)
+
+			require.Len(t, saved, 1)
+			assert.Equal(t, tc.status, saved[0].Status)
+			assert.True(t, live[0], "the row is saved on a context the run's end did not cancel")
+			assert.True(t, deadlines[0].Equal(runDeadline.Add(lateTerminalT/2)), "deadline is E+T/2, got %v want %v", deadlines[0], runDeadline.Add(lateTerminalT/2))
+		})
+	}
+}
+
+// Without a run deadline the account row falls back to a fresh full budget.
+func TestSaveAccountRow_NoRunDeadlineFallsBackToFullBudget(t *testing.T) {
+	store := new(MockConfigStore)
+	var deadline time.Time
+	store.SavePurchaseExecutionFn = func(ctx context.Context, _ *config.PurchaseExecution) error {
+		deadline, _ = ctx.Deadline()
+		return nil
+	}
+	m := &Manager{config: store, budgets: purchaseBudgets{terminalSave: time.Second}}
+	start := time.Now()
+	require.NoError(t, m.saveAccountRow(context.Background(), &config.PurchaseExecution{}))
+	assert.WithinDuration(t, start.Add(time.Second), deadline, 100*time.Millisecond)
+}

internal/purchase/errnotes.go

@@ -31,6 +31,8 @@ var serverNotePatterns = []string{
 	`target already met: target \d+, committed \d+(?:; ` + skippedNotePattern + `)?; \d+ failed row\(s\) of unknown outcome counted as committed`,
 	`selected recommendations do not resolve to a single cloud account: (?:\d+ selected recommendation\(s\) target \d+ cloud accounts \(` + idListPattern + `\); submit one purchase per account|\d+ selected recommendation\(s\) carry no cloud_account_id while \d+ target account ` + idPattern + `)`,
 	`purchase was not successful`,
+	regexp.QuoteMeta(skippedAccountRetryNote),
+	regexp.QuoteMeta(skippedAccountNoRetryNote),
 	regexp.QuoteMeta(redshiftConflictRefusal),
 	regexp.QuoteMeta(redshiftHorizonRefusal),
 	regexp.QuoteMeta(redshiftNoStampRefusal),

internal/purchase/errnotes_test.go

@@ -50,6 +50,8 @@ func TestSanitizeStoredError_AllowlistedNotesPassThroughVerbatim(t *testing.T) {
 		fmt.Errorf("%w: 2 selected recommendation(s) target 2 cloud accounts (a, b); submit one purchase per account", errAmbiguousAccountScope).Error(),
 		fmt.Errorf("%w: 1 selected recommendation(s) carry no cloud_account_id while 2 target account a", errAmbiguousAccountScope).Error(),
 		"purchase was not successful",
+		skippedAccountRetryNote,
+		skippedAccountNoRetryNote,
 		redshiftConflictRefusal,
 		redshiftHorizonRefusal,
 		redshiftNoStampRefusal,

internal/purchase/execute_detached_test.go

@@ -90,10 +90,13 @@ func TestExecuteAndFinalize_SlowRunGetsFreshTerminalSaveBudget(t *testing.T) {
 	const (
 		budget   = 2 * time.Second         // terminal save and per-rec cap
 		callTime = 1200 * time.Millisecond // each call fits its cap; two exceed the save budget
+		// The root save deadline is execDeadline+budget (#1672), so the
+		// execution budget is scaled down too; the run ends well inside it.
+		execTimeout = 10 * time.Second
 	)
 	t.Setenv("CUDLY_MAX_ACCOUNT_PARALLELISM", "1")
 	f := newDetachFixture(t)
-	f.manager.budgets = purchaseBudgets{terminalSave: budget, perRec: budget}
+	f.manager.budgets = purchaseBudgets{execTimeout: execTimeout, terminalSave: budget, perRec: budget}
 	f.exec.Recommendations = append(f.exec.Recommendations, f.exec.Recommendations[0])
 	f.exec.Recommendations[1].ResourceType = "m5.xlarge"
 	var active atomic.Int32
@@ -124,7 +127,7 @@ func TestExecuteAndFinalize_SlowRunGetsFreshTerminalSaveBudget(t *testing.T) {
 	require.NoError(t, err)
 	assert.Equal(t, []string{"completed"}, f.saved)
 	assert.Greater(t, f.saveRemaining, budget-time.Second)
-	assert.LessOrEqual(t, f.saveRemaining, budget)
+	assert.LessOrEqual(t, f.saveRemaining, execTimeout+budget)
 }
 
 var stsOut = sts.GetCallerIdentityOutput{Account: aws.String("123456789012")}
@@ -171,8 +174,10 @@ func TestExecuteAndFinalize_ExpiredCallerDeadline_UsesOwnBudgets(t *testing.T) {
 	require.True(t, ok, "terminal save needs a deadline")
 	// Literal durations on purpose: comparing against the constants would
 	// pass whatever their values are.
-	assert.LessOrEqual(t, f.saveRemaining, 30*time.Second)
-	assert.Greater(t, f.saveRemaining, 29*time.Second)
+	// The root save ends at the run deadline plus the 30s save budget (#1672),
+	// so a run that finishes at once has almost 3m30s + 30s left.
+	assert.LessOrEqual(t, f.saveRemaining, 4*time.Minute)
+	assert.Greater(t, f.saveRemaining, 3*time.Minute+59*time.Second)
 	// SavePurchaseHistory runs inside executePurchase, so it sees the run ctx.
 	_, ok = f.histCtx.Deadline()
 	require.True(t, ok, "run ctx needs a deadline")