From 47e0dfd34b71316d778df148e16a3fec827e181c Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sat, 7 Nov 2026 04:37:50 +0100 Subject: [PATCH 1/4] fix(purchase): record no_submission when every failed rec was refused before submission Per-rec purchase errors reach the execution only as strings, so a refusal raised before any purchase call (the beforeSubmission class) was lost in the aggregate "some purchases failed" error. A row whose every rec was refused therefore did not record no_submission or release its suppressions. The aggregation now tallies, while the error values are in hand, how many failed recs carried the beforeSubmission class without an unknown outcome. When every failure was such a refusal and no rec was purchased: - the single-account run returns the aggregate error with the beforeSubmission class, so saveTerminalExecution records no_submission and releases suppressions; - a per-account row in a fan-out is saved with NoSubmission set. Suppressions there belong to the root execution, as on the skipped-account path. Any purchased rec, any unknown-outcome rec or any other failure keeps today's behaviour. No rec-level refusal exists yet, so this changes nothing until the GCP recent-commitment check (#682 P1) adds one. Part of #682 Co-Authored-By: Claude Opus 5.5 --- .../azure_savings_plans_alias_proof_test.go | 2 +- internal/purchase/execution.go | 27 ++- internal/purchase/execution_test.go | 2 +- .../purchase/money_path_regression_test.go | 6 +- internal/purchase/plan_step_proofs_test.go | 2 +- internal/purchase/refusal_tally.go | 53 +++++ internal/purchase/refusal_tally_test.go | 193 ++++++++++++++++++ 7 files changed, 268 insertions(+), 17 deletions(-) create mode 100644 internal/purchase/refusal_tally.go create mode 100644 internal/purchase/refusal_tally_test.go diff --git a/internal/purchase/azure_savings_plans_alias_proof_test.go b/internal/purchase/azure_savings_plans_alias_proof_test.go index 41fb45609..653dbb3ae 100644 --- a/internal/purchase/azure_savings_plans_alias_proof_test.go +++ b/internal/purchase/azure_savings_plans_alias_proof_test.go @@ -108,7 +108,7 @@ func purchaseThroughPinnedAzureClient(t *testing.T, exec *config.PurchaseExecuti prov.On("GetServiceClient", mock.Anything, common.ServiceSavingsPlansAll, mock.Anything).Return(provider.ServiceClient(client), nil) m := &Manager{config: store, email: sender, providerFactory: factory} - _, _, errs, err := m.processPurchaseRecommendations(ctx, exec, &config.PurchasePlan{Name: "p"}, "acct", (*provider.Config)(nil), "") + _, _, errs, _, err := m.processPurchaseRecommendations(ctx, exec, &config.PurchasePlan{Name: "p"}, "acct", (*provider.Config)(nil), "") require.NoError(t, err) require.Empty(t, errs) for i, rec := range exec.Recommendations { diff --git a/internal/purchase/execution.go b/internal/purchase/execution.go index 1965e0f85..c2ec2cbd6 100644 --- a/internal/purchase/execution.go +++ b/internal/purchase/execution.go @@ -128,14 +128,14 @@ func (m *Manager) executeSingleAccount(ctx context.Context, exec *config.Purchas } } - totalSavings, totalUpfront, purchaseErrors, procErr := m.processPurchaseRecommendations(ctx, exec, plan, accountID, provCfg, target.awsPartition) + totalSavings, totalUpfront, purchaseErrors, allRefused, procErr := m.processPurchaseRecommendations(ctx, exec, plan, accountID, provCfg, target.awsPartition) if procErr != nil { return procErr } if len(purchaseErrors) > 0 && !anyRecPurchased(exec.Recommendations) { // Nothing committed — a clean total failure. - return fmt.Errorf("some purchases failed: %v", purchaseErrors) + return failedPurchasesError(purchaseErrors, allRefused) } // Confirm only when at least one rec committed (full or partial success). @@ -373,7 +373,7 @@ func (m *Manager) executeForAccountLate(ctx context.Context, baseExec *config.Pu } accountID := account.ExternalID - totalSavings, totalUpfront, purchaseErrors, procErr := m.processPurchaseRecommendations(ctx, &acctExec, plan, accountID, provCfg, account.AWSPartition()) + totalSavings, totalUpfront, purchaseErrors, allRefused, procErr := m.processPurchaseRecommendations(ctx, &acctExec, plan, accountID, provCfg, account.AWSPartition()) if procErr != nil { acctExec.Status = "failed" acctExec.Error = procErr.Error() @@ -384,6 +384,7 @@ func (m *Manager) executeForAccountLate(ctx context.Context, baseExec *config.Pu // Resolve the final status/error/completion stamp from the per-rec outcome // (#642), and learn whether any rec committed (#1014). _, committed = applyAccountOutcome(&acctExec, purchaseErrors) + markRefusedAccountRow(&acctExec, allRefused) if saveErr := m.saveAccountRow(ctx, &acctExec); saveErr != nil { return committed, fmt.Errorf("AUDIT LOSS: failed to save execution record for account %s: %w", account.ID, saveErr) @@ -744,7 +745,7 @@ type recPurchaseOutcome struct { index int } -func (m *Manager) processPurchaseRecommendations(ctx context.Context, exec *config.PurchaseExecution, plan *config.PurchasePlan, accountID string, provCfg *provider.Config, awsPartition string) (totalSavings, totalUpfront float64, purchaseErrors []string, procErr error) { +func (m *Manager) processPurchaseRecommendations(ctx context.Context, exec *config.PurchaseExecution, plan *config.PurchasePlan, accountID string, provCfg *provider.Config, awsPartition string) (totalSavings, totalUpfront float64, purchaseErrors []string, allRefused bool, procErr error) { // ExecutionID is carried into PurchaseOptions so executeSinglePurchase // can tag every per-rec log line with the owning exec UUID. Without // this, CloudWatch filtering by exec ID returns zero hits and a stuck @@ -766,7 +767,7 @@ func (m *Manager) processPurchaseRecommendations(ctx context.Context, exec *conf // settings (no-silent-fallbacks-on-money-paths rule). globalCfg, err := m.config.GetGlobalConfig(ctx) if err != nil { - return 0, 0, nil, fmt.Errorf("purchase[%s]: failed to load GlobalConfig for offering class: %w", + return 0, 0, nil, false, fmt.Errorf("purchase[%s]: failed to load GlobalConfig for offering class: %w", exec.ExecutionID, err) } if globalCfg != nil { @@ -774,7 +775,7 @@ func (m *Manager) processPurchaseRecommendations(ctx context.Context, exec *conf } offeringClass, err := resolveEC2OfferingClass(opts.OfferingClass) if err != nil { - return 0, 0, nil, fmt.Errorf("purchase[%s]: %w", exec.ExecutionID, err) + return 0, 0, nil, false, fmt.Errorf("purchase[%s]: %w", exec.ExecutionID, err) } // Build the list of selected indices once so the fan-out closure only @@ -783,7 +784,7 @@ func (m *Manager) processPurchaseRecommendations(ctx context.Context, exec *conf if len(selected) == 0 { logging.Infof("purchase[%s]: no selected recommendations, nothing to execute (account=%s plan=%q)", exec.ExecutionID, accountID, plan.Name) - return 0, 0, nil, nil + return 0, 0, nil, false, nil } logging.Infof("purchase[%s]: dispatching %d recommendation(s) for account=%s plan=%q", exec.ExecutionID, len(selected), accountID, plan.Name) @@ -841,8 +842,8 @@ func (m *Manager) processPurchaseRecommendations(ctx context.Context, exec *conf // there are no concurrent writes to totals, exec.Recommendations, or // purchaseErrors (05-N2). Do NOT move the aggregation inside the FanOut // closure or run it concurrently with the fan-out. - totalSavings, totalUpfront, purchaseErrors = m.aggregatePurchaseOutcomes(ctx, exec, plan, accountID, offeringClass, results) - return totalSavings, totalUpfront, purchaseErrors, nil + totalSavings, totalUpfront, purchaseErrors, allRefused = m.aggregatePurchaseOutcomes(ctx, exec, plan, accountID, offeringClass, results) + return totalSavings, totalUpfront, purchaseErrors, allRefused, nil } // aggregatePurchaseOutcomes walks the fan-out results serially and writes @@ -851,9 +852,10 @@ func (m *Manager) processPurchaseRecommendations(ctx context.Context, exec *conf // so the aggregation logic is single-threaded — no concurrent writes to // totals, purchaseErrors, or exec.Recommendations[i] regardless of how // many recs ran in parallel. -func (m *Manager) aggregatePurchaseOutcomes(ctx context.Context, exec *config.PurchaseExecution, plan *config.PurchasePlan, accountID string, offeringClass ec2types.OfferingClassType, results []execution.Result[recPurchaseOutcome]) (float64, float64, []string) { //nolint:gocritic // unnamedResult: return names would conflict with body locals +func (m *Manager) aggregatePurchaseOutcomes(ctx context.Context, exec *config.PurchaseExecution, plan *config.PurchasePlan, accountID string, offeringClass ec2types.OfferingClassType, results []execution.Result[recPurchaseOutcome]) (float64, float64, []string, bool) { //nolint:gocritic // unnamedResult: return names would conflict with body locals var totalSavings, totalUpfront float64 var purchaseErrors []string + var tally failureTally for _, r := range results { // Closure-level error (parse / bounds / framework). r.Value is the // zero recPurchaseOutcome here — its index is 0 and would mis-target @@ -861,6 +863,7 @@ func (m *Manager) aggregatePurchaseOutcomes(ctx context.Context, exec *config.Pu // generic execution failure and skip the per-rec writes. if r.Err != nil { logging.Errorf("Internal fan-out failure during purchase execution: %v", r.Err) + tally.add(r.Err) purchaseErrors = append(purchaseErrors, fmt.Sprintf("internal fan-out error: %v", r.Err)) continue } @@ -872,6 +875,7 @@ func (m *Manager) aggregatePurchaseOutcomes(ctx context.Context, exec *config.Pu // this otherwise). if i < 0 || i >= len(exec.Recommendations) { logging.Errorf("Aggregator received out-of-range index %d (len=%d)", i, len(exec.Recommendations)) + tally.add(nil) purchaseErrors = append(purchaseErrors, fmt.Sprintf("aggregator index %d out of range", i)) continue } @@ -886,6 +890,7 @@ func (m *Manager) aggregatePurchaseOutcomes(ctx context.Context, exec *config.Pu if v.purchase.CommitmentID != "" { exec.Recommendations[i].PurchaseID = v.purchase.CommitmentID } + tally.add(v.err) purchaseErrors = append(purchaseErrors, fmt.Sprintf("%s: %v", rec.ResourceType, v.err)) continue } @@ -904,7 +909,7 @@ func (m *Manager) aggregatePurchaseOutcomes(ctx context.Context, exec *config.Pu recordHistoryAuditGap(exec, v.purchase.CommitmentID, histErr) } } - return totalSavings, totalUpfront, purchaseErrors + return totalSavings, totalUpfront, purchaseErrors, tally.allRefused(exec.Recommendations) } // recordHistoryAuditGap stamps exec.Error with a note that a successful diff --git a/internal/purchase/execution_test.go b/internal/purchase/execution_test.go index eed8fc74a..392a3de45 100644 --- a/internal/purchase/execution_test.go +++ b/internal/purchase/execution_test.go @@ -2248,7 +2248,7 @@ func TestProcessPurchaseRecommendations_GlobalConfigError_FailsInsteadOfDefaulti dashboardURL: "https://dashboard.example.com", } - _, _, purchaseErrors, procErr := manager.processPurchaseRecommendations(ctx, exec, plan, "111111111111", nil, "") + _, _, purchaseErrors, _, procErr := manager.processPurchaseRecommendations(ctx, exec, plan, "111111111111", nil, "") // The function must return an error derived from the DB failure. require.Error(t, procErr, "GlobalConfig load failure must abort processing, not silently default to convertible") diff --git a/internal/purchase/money_path_regression_test.go b/internal/purchase/money_path_regression_test.go index 4fdfeaca1..1f6ac3878 100644 --- a/internal/purchase/money_path_regression_test.go +++ b/internal/purchase/money_path_regression_test.go @@ -77,7 +77,7 @@ func captureIdempotencyTokens(t *testing.T, exec *config.PurchaseExecution) map[ // provCfg is nil: the mock factory matches mock.Anything for it and the // real factory is never reached. - _, _, errs, procErr := manager.processPurchaseRecommendations(ctx, exec, plan, "111111111111", nil, "") + _, _, errs, _, procErr := manager.processPurchaseRecommendations(ctx, exec, plan, "111111111111", nil, "") require.NoError(t, procErr, "processPurchaseRecommendations must not fail in the happy path") require.Empty(t, errs, "all recs must commit so the captured token reflects a real purchase") return tokens @@ -134,7 +134,7 @@ func TestOfferingClassStampedOnHistoryMatchesPurchase(t *testing.T) { }, } - _, _, errs, procErr := manager.processPurchaseRecommendations(ctx, exec, &config.PurchasePlan{Name: "Direct purchase"}, "111111111111", nil, "") + _, _, errs, _, procErr := manager.processPurchaseRecommendations(ctx, exec, &config.PurchasePlan{Name: "Direct purchase"}, "111111111111", nil, "") require.NoError(t, procErr) require.Empty(t, errs) require.NotNil(t, history) @@ -162,7 +162,7 @@ func TestUnknownOfferingClassRefusesBeforePurchase(t *testing.T) { }, } - _, _, errs, procErr := manager.processPurchaseRecommendations(ctx, exec, &config.PurchasePlan{Name: "Direct purchase"}, "111111111111", nil, "") + _, _, errs, _, procErr := manager.processPurchaseRecommendations(ctx, exec, &config.PurchasePlan{Name: "Direct purchase"}, "111111111111", nil, "") require.ErrorIs(t, procErr, errUnknownOfferingClass) assert.Nil(t, errs) mockFactory.AssertNotCalled(t, "CreateAndValidateProvider", mock.Anything, mock.Anything, mock.Anything) diff --git a/internal/purchase/plan_step_proofs_test.go b/internal/purchase/plan_step_proofs_test.go index 80479ed7e..8e7b9dff5 100644 --- a/internal/purchase/plan_step_proofs_test.go +++ b/internal/purchase/plan_step_proofs_test.go @@ -105,7 +105,7 @@ func tokenCapture(t *testing.T, providerName string, exec *config.PurchaseExecut mu.Unlock() }).Return(common.PurchaseResult{Success: true, CommitmentID: "c"}, nil) m := &Manager{config: store, email: sender, providerFactory: factory} - _, _, errs, err := m.processPurchaseRecommendations(ctx, exec, &config.PurchasePlan{Name: "p"}, "acct", (*provider.Config)(nil), "") + _, _, errs, _, err := m.processPurchaseRecommendations(ctx, exec, &config.PurchasePlan{Name: "p"}, "acct", (*provider.Config)(nil), "") require.NoError(t, err) require.Empty(t, errs) return tokens diff --git a/internal/purchase/refusal_tally.go b/internal/purchase/refusal_tally.go new file mode 100644 index 000000000..f1947eb17 --- /dev/null +++ b/internal/purchase/refusal_tally.go @@ -0,0 +1,53 @@ +package purchase + +import ( + "errors" + "fmt" + + "github.com/LeanerCloud/cloud-commitments-go/pkg/common" + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" +) + +// failureTally counts one aggregation's failed recs and how many of them were +// refused before any purchase call. Per-rec errors reach the callers only as +// strings, so the beforeSubmission class has to be read here, while the error +// values are still in hand. +type failureTally struct{ failed, refused int } + +// add records one failed rec. A rec counts as refused only when its error +// carries the beforeSubmission class and no unknown outcome: an error that +// also wraps ErrOutcomeUnknown may have reached the cloud. +func (t *failureTally) add(err error) { + t.failed++ + var notSubmitted *beforeSubmissionError + if errors.As(err, ¬Submitted) && !errors.Is(err, common.ErrOutcomeUnknown) { + t.refused++ + } +} + +// allRefused reports that every failed rec was refused before submission and +// no rec holds a commitment, so the whole row proves nothing was submitted. +func (t failureTally) allRefused(recs []config.RecommendationRecord) bool { + return t.failed > 0 && t.failed == t.refused && !anyRecPurchased(recs) +} + +// failedPurchasesError is the "some purchases failed" error of a run where +// nothing committed. When every failure was a pre-submission refusal it carries +// the beforeSubmission class, so saveTerminalExecution records no_submission +// and releases the row's suppressions. +func failedPurchasesError(purchaseErrors []string, allRefused bool) error { + err := fmt.Errorf("some purchases failed: %v", purchaseErrors) + if allRefused { + return beforeSubmission(err) + } + return err +} + +// markRefusedAccountRow stamps NoSubmission on a failed per-account row whose +// every rec was refused before submission. The fan-out saves that row itself, +// bypassing saveTerminalExecution. +func markRefusedAccountRow(row *config.PurchaseExecution, allRefused bool) { + if allRefused && row.Status == config.StatusFailed { + row.NoSubmission = true + } +} diff --git a/internal/purchase/refusal_tally_test.go b/internal/purchase/refusal_tally_test.go new file mode 100644 index 000000000..d70e17b1c --- /dev/null +++ b/internal/purchase/refusal_tally_test.go @@ -0,0 +1,193 @@ +package purchase + +// #682 P0: a run whose every failed rec was refused before submission (and +// nothing bought, nothing of unknown outcome) ends no_submission and releases +// its suppressions. Fake provider and service client; no cloud call. A rec-level +// refusal reaches the aggregator as a beforeSubmission error out of +// PurchaseCommitment, which is how the GCP recent-commitment refusal (#682 P1) +// will surface. + +import ( + "context" + "errors" + "fmt" + "sync" + "testing" + + "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" +) + +type recBehavior int + +const ( + recBuys recBehavior = iota + recRefused + recUnknown + recPlainFail +) + +func (b recBehavior) result() (common.PurchaseResult, error) { + switch b { + case recRefused: + return common.PurchaseResult{}, beforeSubmission(errors.New("refused before submission")) + case recUnknown: + return common.PurchaseResult{}, fmt.Errorf("lost response: %w", common.ErrOutcomeUnknown) + case recPlainFail: + return common.PurchaseResult{}, errors.New("offering not available") + } + return common.PurchaseResult{Success: true, CommitmentID: "ri-ok"}, nil +} + +// refusalRecs builds one selected EC2 rec per behavior, keyed by ResourceType. +func refusalRecs(behaviors map[string]recBehavior) []config.RecommendationRecord { + recs := make([]config.RecommendationRecord, 0, len(behaviors)) + for rt := range behaviors { + recs = append(recs, config.RecommendationRecord{Provider: "aws", Service: "ec2", ResourceType: rt, + Region: "us-east-1", Count: 1, UpfrontCost: 100, Savings: 10, Selected: true}) + } + return recs +} + +// refusalManager wires fakes whose PurchaseCommitment answers by ResourceType. +func refusalManager(t *testing.T, behaviors map[string]recBehavior, saved *sync.Map) (*Manager, *MockConfigStore) { + t.Helper() + store := new(MockConfigStore) + mailer := allowFailedNotification(new(MockEmailSender)) + factory, prov, client := new(MockProviderFactory), new(MockProvider), new(MockServiceClient) + store.SavePurchaseExecutionFn = func(_ context.Context, e *config.PurchaseExecution) error { + c := *e + key := "" // single-account rows carry no CloudAccountID + if e.CloudAccountID != nil { + key = *e.CloudAccountID + } + saved.Store(key, &c) + 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) + prov.On("GetServiceClient", mock.Anything, common.ServiceEC2, "us-east-1").Return(client, nil) + for rt, b := range behaviors { + res, err := b.result() + client.On("PurchaseCommitment", mock.Anything, + mock.MatchedBy(func(r common.Recommendation) bool { return r.ResourceType == rt }), + mock.AnythingOfType("common.PurchaseOptions")).Return(res, err) + } + return &Manager{config: store, email: mailer, providerFactory: factory, credStore: awsAccessKeyCredStore()}, store +} + +func refusalCases() map[string]struct { + behaviors map[string]recBehavior + wantNoSubmit bool + wantProvedFail bool +} { + return map[string]struct { + behaviors map[string]recBehavior + wantNoSubmit bool + wantProvedFail bool + }{ + "all refused": {map[string]recBehavior{"a": recRefused, "b": recRefused}, true, true}, + "purchased plus refused": {map[string]recBehavior{"a": recBuys, "b": recRefused}, false, false}, + "unknown outcome plus refused": {map[string]recBehavior{"a": recUnknown, "b": recRefused}, false, false}, + "plain failure plus refused": {map[string]recBehavior{"a": recPlainFail, "b": recRefused}, false, false}, + } +} + +func TestSingleAccount_AllRefusedBeforeSubmission_EndsNoSubmissionAndReleasesSuppressions(t *testing.T) { + for name, tc := range refusalCases() { + if !tc.wantProvedFail { + continue + } + t.Run(name, func(t *testing.T) { + var saved sync.Map + m, store := refusalManager(t, tc.behaviors, &saved) + exec := &config.PurchaseExecution{ExecutionID: "e", PlanID: "p", Status: "running", + Recommendations: refusalRecs(tc.behaviors)} + store.On("DeleteSuppressionsByExecutionTx", mock.Anything, mock.Anything, "e").Return(nil).Maybe() + + err := m.executeSingleAccount(context.Background(), exec, &config.PurchasePlan{ID: "p", Name: "P"}) + require.Error(t, err) + var notSubmitted *beforeSubmissionError + require.True(t, errors.As(err, ¬Submitted), "beforeSubmission class on the returned error") + exec.Status = "failed" + require.NoError(t, m.saveTerminalExecution(context.Background(), exec, err)) + assert.True(t, exec.NoSubmission) + store.AssertCalled(t, "DeleteSuppressionsByExecutionTx", mock.Anything, mock.Anything, "e") + }) + } +} + +func TestSingleAccount_MixedOutcomesAreNotAllRefused(t *testing.T) { + for name, tc := range refusalCases() { + if tc.wantProvedFail { + continue + } + t.Run(name, func(t *testing.T) { + var saved sync.Map + m, store := refusalManager(t, tc.behaviors, &saved) + exec := &config.PurchaseExecution{ExecutionID: "e", PlanID: "p", Status: "running", + Recommendations: refusalRecs(tc.behaviors)} + _, _, errs, allRefused, procErr := m.processPurchaseRecommendations(context.Background(), exec, &config.PurchasePlan{Name: "P"}, "111111111111", nil, "") + require.NoError(t, procErr) + require.NotEmpty(t, errs) + assert.False(t, allRefused) + store.AssertNotCalled(t, "DeleteSuppressionsByExecutionTx", mock.Anything, mock.Anything, mock.Anything) + }) + } +} + +func TestMultiAccount_RefusedAccountRowIsNoSubmission(t *testing.T) { + const refusedAcct, boughtAcct = "aaaaaaaa-0000-0000-0000-000000000001", "bbbbbbbb-0000-0000-0000-000000000002" + behaviors := map[string]recBehavior{"r1": recRefused, "r2": recRefused, "ok": recBuys} + var saved sync.Map + m, _ := refusalManager(t, behaviors, &saved) + plan := &config.PurchasePlan{ID: "p", Name: "P"} + base := func(types ...string) *config.PurchaseExecution { + sub := map[string]recBehavior{} + for _, ty := range types { + sub[ty] = behaviors[ty] + } + return &config.PurchaseExecution{ExecutionID: "root", PlanID: "p", IdempotencyKey: "lineage", Recommendations: refusalRecs(sub)} + } + acct := func(id, ext string) config.CloudAccount { + return config.CloudAccount{ID: id, Name: id, Provider: "aws", ExternalID: ext, AWSAuthMode: "access_keys"} + } + _, err := m.executeForAccount(context.Background(), base("r1", "r2"), plan, acct(refusedAcct, "111111111111")) + require.Error(t, err) + _, err = m.executeForAccount(context.Background(), base("ok"), plan, acct(boughtAcct, "222222222222")) + require.NoError(t, err) + + row := func(id string) *config.PurchaseExecution { + v, ok := saved.Load(id) + require.True(t, ok) + return v.(*config.PurchaseExecution) + } + assert.Equal(t, "failed", row(refusedAcct).Status) + assert.True(t, row(refusedAcct).NoSubmission) + assert.Equal(t, "completed", row(boughtAcct).Status) + assert.False(t, row(boughtAcct).NoSubmission) +} + +func TestMultiAccount_MixedOutcomesAreNotNoSubmission(t *testing.T) { + for name, tc := range refusalCases() { + if tc.wantNoSubmit { + continue + } + t.Run(name, func(t *testing.T) { + const acctID = "aaaaaaaa-0000-0000-0000-000000000001" + var saved sync.Map + m, _ := refusalManager(t, tc.behaviors, &saved) + base := &config.PurchaseExecution{ExecutionID: "root", PlanID: "p", IdempotencyKey: "lineage", Recommendations: refusalRecs(tc.behaviors)} + account := config.CloudAccount{ID: acctID, Name: "A", Provider: "aws", ExternalID: "111111111111", AWSAuthMode: "access_keys"} + _, err := m.executeForAccount(context.Background(), base, &config.PurchasePlan{ID: "p", Name: "P"}, account) + require.Error(t, err) + v, ok := saved.Load(acctID) + require.True(t, ok) + assert.False(t, v.(*config.PurchaseExecution).NoSubmission) + }) + } +} From 95c7d791af6f29499549787fda459831e072551b Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sat, 7 Nov 2026 04:54:06 +0100 Subject: [PATCH 2/4] test(purchase): pin that a refusal wrapping an unknown outcome is not counted as refused An error with the beforeSubmission class that also wraps ErrOutcomeUnknown may have reached the cloud, so the row must keep today's handling. Without this case, dropping the ErrOutcomeUnknown check from failureTally.add went unnoticed. Part of #682 Co-Authored-By: Claude Opus 5.5 --- internal/purchase/refusal_tally_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/internal/purchase/refusal_tally_test.go b/internal/purchase/refusal_tally_test.go index d70e17b1c..5632a07be 100644 --- a/internal/purchase/refusal_tally_test.go +++ b/internal/purchase/refusal_tally_test.go @@ -28,6 +28,7 @@ const ( recRefused recUnknown recPlainFail + recRefusedUnknown ) func (b recBehavior) result() (common.PurchaseResult, error) { @@ -36,6 +37,10 @@ func (b recBehavior) result() (common.PurchaseResult, error) { return common.PurchaseResult{}, beforeSubmission(errors.New("refused before submission")) case recUnknown: return common.PurchaseResult{}, fmt.Errorf("lost response: %w", common.ErrOutcomeUnknown) + case recRefusedUnknown: + // A pre-submission class that still wraps an unknown outcome may have + // reached the cloud, so it must not count as refused. + return common.PurchaseResult{}, beforeSubmission(fmt.Errorf("guard: %w", common.ErrOutcomeUnknown)) case recPlainFail: return common.PurchaseResult{}, errors.New("offering not available") } @@ -94,6 +99,7 @@ func refusalCases() map[string]struct { "purchased plus refused": {map[string]recBehavior{"a": recBuys, "b": recRefused}, false, false}, "unknown outcome plus refused": {map[string]recBehavior{"a": recUnknown, "b": recRefused}, false, false}, "plain failure plus refused": {map[string]recBehavior{"a": recPlainFail, "b": recRefused}, false, false}, + "refused with unknown outcome": {map[string]recBehavior{"a": recRefusedUnknown, "b": recRefused}, false, false}, } } From 67ba0c6b54e4d0e2fb75486032e234d69bd08b08 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sat, 7 Nov 2026 22:07:27 +0100 Subject: [PATCH 3/4] test(purchase): pin that a panic after submission is not counted as refused The fan-out recovers a panic inside the per-rec closure into a closure-level error, possibly after the provider call. failureTally must count it as a non-refused failure; without this case, deleting tally.add(r.Err) left every test green. A rec that panics in PurchaseCommitment next to a refused rec now keeps the row off no_submission and its suppressions in place, single-account and fan-out. Part of #682 Co-Authored-By: Claude Opus 5.5 --- internal/purchase/refusal_tally_test.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/internal/purchase/refusal_tally_test.go b/internal/purchase/refusal_tally_test.go index 5632a07be..2c8320326 100644 --- a/internal/purchase/refusal_tally_test.go +++ b/internal/purchase/refusal_tally_test.go @@ -29,6 +29,7 @@ const ( recUnknown recPlainFail recRefusedUnknown + recPanics ) func (b recBehavior) result() (common.PurchaseResult, error) { @@ -78,9 +79,15 @@ func refusalManager(t *testing.T, behaviors map[string]recBehavior, saved *sync. prov.On("GetServiceClient", mock.Anything, common.ServiceEC2, "us-east-1").Return(client, nil) for rt, b := range behaviors { res, err := b.result() - client.On("PurchaseCommitment", mock.Anything, + call := client.On("PurchaseCommitment", mock.Anything, mock.MatchedBy(func(r common.Recommendation) bool { return r.ResourceType == rt }), - mock.AnythingOfType("common.PurchaseOptions")).Return(res, err) + mock.AnythingOfType("common.PurchaseOptions")) + if b == recPanics { + // The fan-out recovers this into a closure-level error after the + // call may have reached the provider. + call = call.Run(func(mock.Arguments) { panic("lost after submitting") }) + } + call.Return(res, err) } return &Manager{config: store, email: mailer, providerFactory: factory, credStore: awsAccessKeyCredStore()}, store } @@ -100,6 +107,7 @@ func refusalCases() map[string]struct { "unknown outcome plus refused": {map[string]recBehavior{"a": recUnknown, "b": recRefused}, false, false}, "plain failure plus refused": {map[string]recBehavior{"a": recPlainFail, "b": recRefused}, false, false}, "refused with unknown outcome": {map[string]recBehavior{"a": recRefusedUnknown, "b": recRefused}, false, false}, + "panic plus refused": {map[string]recBehavior{"a": recPanics, "b": recRefused}, false, false}, } } From eb72026282efbc5a0961b43ca3af57cdb3245823 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sat, 7 Nov 2026 22:50:16 +0100 Subject: [PATCH 4/4] test(purchase): update the Azure Redis routing test for the four-value result processPurchaseRecommendations returns allRefused since this branch; the routing test that #1837 added on main used the three-value form. Part of #682 Co-Authored-By: Claude Opus 5.5 --- internal/purchase/azure_redis_routing_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/purchase/azure_redis_routing_test.go b/internal/purchase/azure_redis_routing_test.go index 91a8cb0e6..9f2b6c867 100644 --- a/internal/purchase/azure_redis_routing_test.go +++ b/internal/purchase/azure_redis_routing_test.go @@ -32,7 +32,7 @@ func purchaseCacheRecThrough(t *testing.T, prov *MockProvider, providerName, reg Recommendations: []config.RecommendationRecord{{ID: "rec-1", Provider: providerName, Service: "cache", ResourceType: "cache.r6g.large", Region: region, Count: 1, Term: 1, Selected: true}}} m := &Manager{config: store, email: sender, providerFactory: factory} - _, _, errs, err := m.processPurchaseRecommendations(ctx, exec, &config.PurchasePlan{Name: "p"}, "acct", (*provider.Config)(nil), "") + _, _, errs, _, err := m.processPurchaseRecommendations(ctx, exec, &config.PurchasePlan{Name: "p"}, "acct", (*provider.Config)(nil), "") require.NoError(t, err) require.Empty(t, errs) require.True(t, exec.Recommendations[0].Purchased, "the purchase must reach the %s client", want)