fix(purchase): scope plan fan-out recommendations to each account

internal/api/handler_purchases_retry_fanout_test.go

@@ -63,6 +63,18 @@ func fanoutRecs() []config.RecommendationRecord {
 	}
 }
 
+// fanoutRootRecs is a root step's rec set: one rec per plan account, because
+// the fan-out buys each account's own recs only (platform#631).
+func fanoutRootRecs() []config.RecommendationRecord {
+	recs := make([]config.RecommendationRecord, 0, 3)
+	for _, id := range []string{fanoutAcctA, fanoutAcctB, fanoutAcctC} {
+		rec := fanoutRecs()[0]
+		rec.CloudAccountID = strPtr(id)
+		recs = append(recs, rec)
+	}
+	return recs
+}
+
 // fanoutCredStore resolves a static AWS access-key blob so per-account
 // credential resolution succeeds without touching the cloud.
 type fanoutCredStore struct {
@@ -329,7 +341,7 @@ func TestRetryOfFailedRootRowStillFansOutToEveryAccount(t *testing.T) {
 		CreatedByUserID: &creator,
 		CapacityPercent: 100,
 		Source:          common.PurchaseSourceWeb,
-		Recommendations: fanoutRecs(),
+		Recommendations: fanoutRootRecs(),
 	}
 
 	successor := retryFailedRow(t, failedRoot)

internal/purchase/account_scope.go

@@ -0,0 +1,63 @@
+package purchase
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/LeanerCloud/cloud-commitments-platform/internal/config"
+)
+
+// scopeExecutionsByAccount splits a plan step's recommendations across the
+// plan's accounts so each account buys only its own (platform#631). The
+// returned executions are per-account copies of base whose Recommendations
+// and money totals cover only that account's recs; accounts with no recs of
+// their own are omitted.
+//
+// It fails loudly rather than dropping anything: a rec with no account, or an
+// account outside the plan, would otherwise be silently unbought (or, before
+// this scoping existed, bought by every account). An empty step returns
+// ErrPlanStepNoRecommendations so it cannot complete and advance the ramp.
+func scopeExecutionsByAccount(base *config.PurchaseExecution, accounts []config.CloudAccount) (map[string]*config.PurchaseExecution, error) {
+	if len(base.Recommendations) == 0 {
+		return nil, fmt.Errorf("execution %s: %w", base.ExecutionID, ErrPlanStepNoRecommendations)
+	}
+	planAccounts := make(map[string]bool, len(accounts))
+	for i := range accounts {
+		planAccounts[accounts[i].ID] = true
+	}
+	byAccount := make(map[string][]config.RecommendationRecord, len(accounts))
+	var stray []string
+	for i := range base.Recommendations {
+		rec := base.Recommendations[i]
+		if rec.CloudAccountID == nil || !planAccounts[*rec.CloudAccountID] {
+			stray = append(stray, describeScopeRec(&rec))
+			continue
+		}
+		byAccount[*rec.CloudAccountID] = append(byAccount[*rec.CloudAccountID], rec)
+	}
+	if len(stray) > 0 {
+		return nil, fmt.Errorf("execution %s: %d recommendation(s) are not attributed to one of the plan's accounts and cannot be bought: %s",
+			base.ExecutionID, len(stray), strings.Join(stray, ", "))
+	}
+
+	scoped := make(map[string]*config.PurchaseExecution, len(byAccount))
+	for id, recs := range byAccount {
+		e := *base
+		e.Recommendations = recs
+		e.TotalUpfrontCost, e.EstimatedSavings = 0, 0
+		for i := range recs {
+			e.TotalUpfrontCost += recs[i].UpfrontCost
+			e.EstimatedSavings += recs[i].Savings
+		}
+		scoped[id] = &e
+	}
+	return scoped, nil
+}
+
+func describeScopeRec(rec *config.RecommendationRecord) string {
+	acct := "no account"
+	if rec.CloudAccountID != nil {
+		acct = "account " + *rec.CloudAccountID
+	}
+	return fmt.Sprintf("%s/%s %s (%s)", rec.Provider, rec.Service, rec.ResourceType, acct)
+}

internal/purchase/account_scope_test.go

@@ -0,0 +1,79 @@
+package purchase
+
+import (
+	"context"
+	"testing"
+
+	"github.com/LeanerCloud/cloud-commitments-platform/internal/config"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func withAccount(rec config.RecommendationRecord, id string) config.RecommendationRecord {
+	rec.CloudAccountID = &id
+	return rec
+}
+
+func scopedTestRec(accountID string) config.RecommendationRecord {
+	return withAccount(config.RecommendationRecord{
+		Provider: "aws", Service: "ec2", ResourceType: "m5.large", Region: "us-east-1",
+		Count: 1, UpfrontCost: 300, Savings: 75, Selected: true,
+	}, accountID)
+}
+
+func scopeFixture() (*config.PurchaseExecution, []config.CloudAccount) {
+	a, b := scopedTestRec("acct-A"), scopedTestRec("acct-B")
+	b.ResourceType, b.UpfrontCost, b.Savings = "c5.large", 100, 20
+	return &config.PurchaseExecution{
+		ExecutionID: "root", PlanID: "plan-x", StepNumber: 1,
+		TotalUpfrontCost: 400, EstimatedSavings: 95,
+		Recommendations: []config.RecommendationRecord{a, b},
+	}, []config.CloudAccount{{ID: "acct-A"}, {ID: "acct-B"}}
+}
+
+// Each account gets only its own recs, and its row carries its own totals
+// (not the step's, which would show N times the money in History).
+func TestScopeExecutionsByAccount_OwnRecsAndTotals(t *testing.T) {
+	base, accounts := scopeFixture()
+	scoped, err := scopeExecutionsByAccount(base, accounts)
+	require.NoError(t, err)
+	require.Len(t, scoped, 2)
+	assert.Len(t, scoped["acct-A"].Recommendations, 1)
+	assert.Equal(t, "m5.large", scoped["acct-A"].Recommendations[0].ResourceType)
+	assert.Equal(t, 300.0, scoped["acct-A"].TotalUpfrontCost)
+	assert.Equal(t, 75.0, scoped["acct-A"].EstimatedSavings)
+	assert.Equal(t, 100.0, scoped["acct-B"].TotalUpfrontCost)
+	assert.Equal(t, 20.0, scoped["acct-B"].EstimatedSavings)
+	assert.Len(t, base.Recommendations, 2, "base execution must be untouched")
+}
+
+// A rec with no account, or one outside the plan, fails the step loudly.
+func TestScopeExecutionsByAccount_StrayRecFailsLoudly(t *testing.T) {
+	for name, stray := range map[string]config.RecommendationRecord{
+		"nil account":     {Provider: "azure", Service: "compute", ResourceType: "D2"},
+		"foreign account": scopedTestRec("acct-other"),
+	} {
+		t.Run(name, func(t *testing.T) {
+			base, accounts := scopeFixture()
+			base.Recommendations = append(base.Recommendations, stray)
+			scoped, err := scopeExecutionsByAccount(base, accounts)
+			require.Error(t, err)
+			assert.Nil(t, scoped)
+			assert.Contains(t, err.Error(), "not attributed to one of the plan's accounts")
+		})
+	}
+}
+
+// executeMultiAccount: an empty step must fail with ErrPlanStepNoRecommendations
+// (nil would be recorded as completed and advance the ramp), buying nothing.
+func TestExecuteMultiAccount_EmptyStepFailsAndBuysNothing(t *testing.T) {
+	base, accounts := scopeFixture()
+	base.Recommendations = nil
+	store := new(MockConfigStore)
+	m := &Manager{config: store}
+
+	err := m.executeMultiAccount(context.Background(), base, &config.PurchasePlan{ID: "plan-x"}, accounts)
+
+	require.ErrorIs(t, err, ErrPlanStepNoRecommendations)
+	store.AssertNotCalled(t, "SavePurchaseExecution")
+}

internal/purchase/execution.go

@@ -191,8 +191,21 @@ var errAllAccountsFailed = errors.New("multi-account execution: all accounts fai
 // callers ACK rather than redeliver, issue #1014); and errAllAccountsFailed
 // (wrapping the per-account errors) when no account committed anything.
 func (m *Manager) executeMultiAccount(ctx context.Context, baseExec *config.PurchaseExecution, plan *config.PurchasePlan, accounts []config.CloudAccount) error {
-	results := execution.RunForAccountsWithConcurrency(ctx, accounts, func(ctx context.Context, account config.CloudAccount) (bool, error) {
-		return m.executeForAccount(ctx, baseExec, plan, account)
+	// Checked before any account runs: every account would otherwise buy the
+	// whole step, and a step with nothing to buy would return nil below and
+	// be recorded as completed, advancing the ramp (platform#631).
+	scoped, scopeErr := scopeExecutionsByAccount(baseExec, accounts)
+	if scopeErr != nil {
+		return scopeErr
+	}
+	buying := make([]config.CloudAccount, 0, len(scoped))
+	for i := range accounts {
+		if _, ok := scoped[accounts[i].ID]; ok {
+			buying = append(buying, accounts[i])
+		}
+	}
+	results := execution.RunForAccountsWithConcurrency(ctx, buying, func(ctx context.Context, account config.CloudAccount) (bool, error) {
+		return m.executeForAccount(ctx, scoped[account.ID], plan, account)
 	}, getMaxAccountParallelism())
 
 	committed := 0

internal/purchase/execution_test.go

@@ -743,7 +743,7 @@ func TestManager_ExecutePurchase_MultiAccount(t *testing.T) {
 		ExecutionID:     "exec-multi",
 		PlanID:          "plan-multi",
 		Status:          "pending",
-		Recommendations: []config.RecommendationRecord{rec},
+		Recommendations: []config.RecommendationRecord{withAccount(rec, "aaaaaaaa-0000-0000-0000-000000000001"), withAccount(rec, "aaaaaaaa-0000-0000-0000-000000000002")},
 	}
 
 	accounts := []config.CloudAccount{
@@ -797,6 +797,14 @@ func TestManager_ExecutePurchase_MultiAccount(t *testing.T) {
 		"aaaaaaaa-0000-0000-0000-000000000002",
 	}, accountIDs)
 
+	// platform#631: each account buys only its own rec (2 calls, not 4) and
+	// its row carries its own totals, not the whole step's.
+	mockServiceClient.AssertNumberOfCalls(t, "PurchaseCommitment", 2)
+	for _, e := range savedExecs {
+		require.Len(t, e.Recommendations, 1)
+		assert.Equal(t, *e.CloudAccountID, *e.Recommendations[0].CloudAccountID)
+	}
+
 	mockStore.AssertExpectations(t)
 	mockEmail.AssertExpectations(t)
 	mockFactory.AssertExpectations(t)
@@ -871,7 +879,7 @@ func TestExecuteForAccount_CredentialFailure_MarksFailed(t *testing.T) {
 				ExecutionID:     "exec-credfail",
 				PlanID:          "plan-credfail",
 				Status:          "pending",
-				Recommendations: []config.RecommendationRecord{rec},
+				Recommendations: []config.RecommendationRecord{withAccount(rec, tt.account.ID)},
 			}
 
 			mockStore.On("GetPurchasePlan", ctx, "plan-credfail").Return(plan, nil)
@@ -1032,7 +1040,7 @@ func TestExecuteMultiAccount_PartialFailure_IsolatesAccounts(t *testing.T) {
 		ExecutionID:     "exec-partial",
 		PlanID:          "plan-partial",
 		Status:          "pending",
-		Recommendations: []config.RecommendationRecord{rec},
+		Recommendations: []config.RecommendationRecord{withAccount(rec, accountVID), withAccount(rec, accountIID)},
 	}
 
 	plan := &config.PurchasePlan{
@@ -1203,7 +1211,7 @@ func TestExecuteMultiAccount_RunsAccountsInParallel(t *testing.T) {
 		ExecutionID:     "exec-parallel",
 		PlanID:          "plan-parallel",
 		Status:          "pending",
-		Recommendations: []config.RecommendationRecord{rec},
+		Recommendations: []config.RecommendationRecord{withAccount(rec, accountAID), withAccount(rec, accountBID)},
 	}
 
 	plan := &config.PurchasePlan{

internal/purchase/money_path_regression_test.go

@@ -240,7 +240,8 @@ func TestMultiAccountSeedsStablePerAccountKey(t *testing.T) {
 			PlanID:         "plan-x",
 			Source:         common.PurchaseSourceWeb,
 			Recommendations: []config.RecommendationRecord{
-				{Provider: "aws", Service: "ec2", ResourceType: "m5.large", Region: "us-east-1", Count: 1, UpfrontCost: 300, Selected: true},
+				scopedTestRec("acct-A"),
+				scopedTestRec("acct-B"),
 			},
 		}
 		plan := &config.PurchasePlan{ID: "plan-x", Name: "Plan X"}
@@ -385,7 +386,8 @@ func TestMultiAccountPartialSuccessIsAcked(t *testing.T) {
 		Status:         "pending",
 		PlanID:         "plan-x",
 		Recommendations: []config.RecommendationRecord{
-			{Provider: "aws", Service: "ec2", ResourceType: "m5.large", Region: "us-east-1", Count: 1, UpfrontCost: 300, Selected: true},
+			scopedTestRec("acct-ok"),
+			scopedTestRec("acct-bad"),
 		},
 	}
 	plan := &config.PurchasePlan{ID: "plan-x", Name: "Plan X", AutoPurchase: true}
@@ -484,7 +486,8 @@ func TestExecuteForAccount_CommittedButUnsavedIsAcked(t *testing.T) {
 		Status:         "pending",
 		PlanID:         "plan-y",
 		Recommendations: []config.RecommendationRecord{
-			{Provider: "aws", Service: "ec2", ResourceType: "m5.large", Region: "us-east-1", Count: 1, UpfrontCost: 300, Selected: true},
+			scopedTestRec("acct-bad"),
+			scopedTestRec("acct-unsaved"),
 		},
 	}
 	plan := &config.PurchasePlan{ID: "plan-y", Name: "Plan Y", AutoPurchase: true}
@@ -786,7 +789,8 @@ func TestRootKeyStillFansOutThroughExecutePurchase(t *testing.T) {
 		PlanID:         "plan-x",
 		Source:         common.PurchaseSourceWeb,
 		Recommendations: []config.RecommendationRecord{
-			{Provider: "aws", Service: "ec2", ResourceType: "m5.large", Region: "us-east-1", Count: 1, UpfrontCost: 300, Selected: true},
+			scopedTestRec("acct-A"),
+			scopedTestRec("acct-B"),
 		},
 	}
 	require.NoError(t, manager.executePurchase(ctx, root),

internal/purchase/ramp_step_progress_integration_test.go

@@ -199,6 +199,19 @@ func rampStepRecommendation() []config.RecommendationRecord {
 	}
 }
 
+// rampStepRecommendationsPerAccount is the step's rec set: one rec per plan
+// account, since the fan-out buys each account's own recs only (platform#631).
+func (f *rampStepFixture) rampStepRecommendationsPerAccount() []config.RecommendationRecord {
+	recs := make([]config.RecommendationRecord, 0, len(f.accounts))
+	for i := range f.accounts {
+		id := f.accounts[i].ID
+		rec := rampStepRecommendation()[0]
+		rec.CloudAccountID = &id
+		recs = append(recs, rec)
+	}
+	return recs
+}
+
 // saveRootExecution persists the plan-scheduled root row for ramp step
 // stepNumber: no cloud_account_id, so the executor fans it out.
 func (f *rampStepFixture) saveRootExecution(ctx context.Context, t *testing.T, stepNumber int) *config.PurchaseExecution {
@@ -210,7 +223,7 @@ func (f *rampStepFixture) saveRootExecution(ctx context.Context, t *testing.T, s
 		Status:          "pending",
 		StepNumber:      stepNumber,
 		ScheduledDate:   time.Now(),
-		Recommendations: rampStepRecommendation(),
+		Recommendations: f.rampStepRecommendationsPerAccount(),
 	}
 	require.NoError(t, f.store.SavePurchaseExecution(ctx, exec))
 	return exec