From ade6d8ce18d66b183497f0624b19a93c7a80e11e Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 9 Nov 2026 16:42:20 +0100 Subject: [PATCH 1/3] test(purchase): add a per-account fixture for fan-out tests The fan-out tests for carrying the no_submission proof to the root row need accounts that behave differently: a credential failure, a row-save failure, a provider factory failure and a refused account followed by a cancelled run. The fixture keys each hook by account ID and runs two accounts through executeMultiAccount. A table-driven smoke test, one case per hook, asserts only today's per-account rows and the returned error. Part of #1844 Co-Authored-By: Claude Sonnet 5.5 --- .../purchase/fanout_refusal_fixture_test.go | 158 ++++++++++++++++++ 1 file changed, 158 insertions(+) create mode 100644 internal/purchase/fanout_refusal_fixture_test.go diff --git a/internal/purchase/fanout_refusal_fixture_test.go b/internal/purchase/fanout_refusal_fixture_test.go new file mode 100644 index 000000000..62a2debf7 --- /dev/null +++ b/internal/purchase/fanout_refusal_fixture_test.go @@ -0,0 +1,158 @@ +package purchase + +// #1844 S0: a two-account fixture for tests that go through executeMultiAccount +// (the existing multi-account tests call executeForAccount directly). It builds +// on refusalManager: each account owns one rec whose ResourceType is the account +// ID, so recBehavior is chosen per account, and the hooks below fail one account +// at the credential, provider-factory or row-save step. + +import ( + "context" + "fmt" + "sync" + "testing" + + "github.com/LeanerCloud/cloud-commitments-go/pkg/provider" + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// fanOutFixture runs accounts A then B (parallelism 1) through +// executeMultiAccount. Every map is keyed by account ID. +type fanOutFixture struct { + behaviors map[string]recBehavior // the account's single rec; zero value buys + credFail map[string]bool // credential load fails + factoryFail map[string]bool // provider factory fails + saveFail map[string]bool // the account's row save fails (AUDIT LOSS) + // cancelAfterSave cancels the run context once that account's row saved, so + // the accounts after it never start. + cancelAfterSave string +} + +// keyedFactory fails the provider factory for the accounts in failKeys. The +// factory only sees the provider config, so the account is recognized by the +// access key the credential hook issues: the account ID. +type keyedFactory struct { + inner provider.FactoryInterface + failKeys map[string]bool +} + +func (f *keyedFactory) CreateAndValidateProvider(ctx context.Context, name string, cfg *provider.Config) (provider.Provider, error) { + creds, err := cfg.AWSCredentialsProvider.Retrieve(ctx) + if err != nil { + return nil, err + } + if f.failKeys[creds.AccessKeyID] { + return nil, fmt.Errorf("factory down for %s", creds.AccessKeyID) + } + return f.inner.CreateAndValidateProvider(ctx, name, cfg) +} + +// run returns the saved account rows (root rows are not saved by the fan-out) +// and executeMultiAccount's error. +func (f fanOutFixture) run(t *testing.T) (*sync.Map, error) { + t.Helper() + t.Setenv("CUDLY_MAX_ACCOUNT_PARALLELISM", "1") + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + var saved sync.Map + m, store := refusalManager(t, f.behaviors, &saved) + + record := store.SavePurchaseExecutionFn + store.SavePurchaseExecutionFn = func(c context.Context, e *config.PurchaseExecution) error { + id := accountIDOf(e) + if f.saveFail[id] { + return fmt.Errorf("database down saving %s", id) + } + err := record(c, e) + if f.cancelAfterSave != "" && id == f.cancelAfterSave { + cancel() + } + return err + } + m.providerFactory = &keyedFactory{inner: m.providerFactory, failKeys: f.factoryFail} + m.credStore = &MockCredentialStore{LoadRawFn: func(_ context.Context, id, _ string) ([]byte, error) { + if f.credFail[id] { + return nil, fmt.Errorf("secret store down for %s", id) + } + return []byte(fmt.Sprintf(`{"access_key_id":%q,"secret_access_key":"wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY"}`, id)), nil + }} + + accounts := []config.CloudAccount{ + {ID: skipAcctA, Name: "A", Provider: "aws", ExternalID: "111111111111", AWSAuthMode: "access_keys"}, + {ID: skipAcctB, Name: "B", Provider: "aws", ExternalID: "222222222222", AWSAuthMode: "access_keys"}, + } + recs := make([]config.RecommendationRecord, 0, len(accounts)) + for _, a := range accounts { + recs = append(recs, withAccount(refusalRecs(map[string]recBehavior{a.ID: f.behaviors[a.ID]})[0], a.ID)) + } + exec := &config.PurchaseExecution{ExecutionID: "root", PlanID: "p", IdempotencyKey: "lineage", Recommendations: recs} + err := m.executeMultiAccount(ctx, exec, &config.PurchasePlan{ID: "p", Name: "P"}, accounts) + return &saved, err +} + +func savedAccountRow(saved *sync.Map, id string) *config.PurchaseExecution { + v, ok := saved.Load(id) + if !ok { + return nil + } + return v.(*config.PurchaseExecution) +} + +// rowWant is an account row's expected end state; status "" means no row. +type rowWant struct { + status string + noSubmission bool +} + +func TestExecuteMultiAccount_FanOutFixtureHooks(t *testing.T) { + refusedBoth := map[string]recBehavior{skipAcctA: recRefused, skipAcctB: recRefused} + cases := map[string]struct { + fx fanOutFixture + errContain string + a, b rowWant + }{ + "credential failure on A, B refused": { + fx: fanOutFixture{behaviors: refusedBoth, credFail: map[string]bool{skipAcctA: true}}, + errContain: "credential resolution failed for account " + skipAcctA, + a: rowWant{"failed", false}, // the unflagged row #1844 S2 will flag + b: rowWant{"failed", true}, + }, + "row save failure on A (AUDIT LOSS), B refused": { + fx: fanOutFixture{behaviors: refusedBoth, saveFail: map[string]bool{skipAcctA: true}}, + errContain: "AUDIT LOSS: failed to save execution record for account " + skipAcctA, + a: rowWant{}, + b: rowWant{"failed", true}, + }, + "factory failure on A, B refused": { + fx: fanOutFixture{behaviors: refusedBoth, factoryFail: map[string]bool{skipAcctA: true}}, + errContain: "failed to create aws provider", + a: rowWant{"failed", false}, + b: rowWant{"failed", true}, + }, + "A refused then cancels the run, B never starts": { + fx: fanOutFixture{behaviors: refusedBoth, cancelAfterSave: skipAcctA}, + errContain: "some purchases failed", + a: rowWant{"failed", true}, + b: rowWant{}, + }, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + saved, err := tc.fx.run(t) + require.ErrorIs(t, err, errAllAccountsFailed) + assert.ErrorContains(t, err, tc.errContain) + for id, want := range map[string]rowWant{skipAcctA: tc.a, skipAcctB: tc.b} { + row := savedAccountRow(saved, id) + if want.status == "" { + assert.Nil(t, row, "account %s has no row", id) + continue + } + require.NotNil(t, row, "account %s row", id) + assert.Equal(t, want.status, row.Status, id) + assert.Equal(t, want.noSubmission, row.NoSubmission, id) + } + }) + } +} From 16284b5e07c40265ae168dfd71a2e02fab6eb4e4 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 9 Nov 2026 17:02:32 +0100 Subject: [PATCH 2/3] fix(purchase): carry the no_submission proof to the root of an all-refused fan-out When no account of a multi-account run committed and every account was refused before submission, never started, or finished with nothing bought, the root error now carries the before-submission class, so the root row is stamped no_submission and releases its suppressions. The decision reads the in-memory per-account results, not saved rows. A failed account-row save strips the class so a lost audit row never counts as proof. Part of #1844 Co-Authored-By: Claude Sonnet 5.5 --- internal/purchase/execution.go | 15 +- .../purchase/fanout_refusal_fixture_test.go | 80 ++++++++++- internal/purchase/fanout_root_proof_test.go | 135 ++++++++++++++++++ internal/purchase/refusal_tally.go | 38 ++++- internal/purchase/refusal_tally_test.go | 4 + 5 files changed, 261 insertions(+), 11 deletions(-) create mode 100644 internal/purchase/fanout_root_proof_test.go diff --git a/internal/purchase/execution.go b/internal/purchase/execution.go index 120856a54..8d45c7458 100644 --- a/internal/purchase/execution.go +++ b/internal/purchase/execution.go @@ -285,7 +285,13 @@ func summarizeFanOut(scoped map[string]*config.PurchaseExecution, results []exec // ack and never mislabel the run as a flat failure (issue #1014). return &multiAccountPartialError{committed: committed, errors: errs, skipped: skippedAccountRows(scoped, results, started)} } - return fmt.Errorf("%w: %s", errAllAccountsFailed, strings.Join(errs, "; ")) + err := fmt.Errorf("%w: %s", errAllAccountsFailed, strings.Join(errs, "; ")) + if fanOutNotSubmitted(results, started) { + // Nothing was submitted by any account (#1844), so the root carries the + // proof saveTerminalExecution reads; the %w chain is unchanged. + return beforeSubmission(err) + } + return err } // saveExecutionStatusBestEffort saves the execution record and logs any error. @@ -406,7 +412,7 @@ func (m *Manager) executeForAccountLate(ctx context.Context, baseExec *config.Pu // authoritative per-account row was already saved as partially_completed // (not failed) and the confirmation already went out. if len(purchaseErrors) > 0 { - return committed, fmt.Errorf("some purchases failed: %v", purchaseErrors) + return committed, failedPurchasesError(purchaseErrors, allRefused) } return committed, nil } @@ -494,9 +500,12 @@ func skippedAccountRows(scoped map[string]*config.PurchaseExecution, results []e // that function under the gocyclo:10 budget. func (m *Manager) persistFailedAccountExecution(ctx context.Context, acctExec *config.PurchaseExecution, account config.CloudAccount, cause error) error { if saveErr := m.saveAccountRow(ctx, acctExec); saveErr != nil { + // The cause is joined as text: a lost audit row means the outcome is + // unknown, so a before-submission class on the cause must not survive + // into the fan-out proof (errors.As walks every Join branch). return errors.Join( fmt.Errorf("AUDIT LOSS: failed to save execution record for account %s: %w", account.ID, saveErr), - cause, + errors.New(cause.Error()), ) } return cause diff --git a/internal/purchase/fanout_refusal_fixture_test.go b/internal/purchase/fanout_refusal_fixture_test.go index 62a2debf7..f2c86de04 100644 --- a/internal/purchase/fanout_refusal_fixture_test.go +++ b/internal/purchase/fanout_refusal_fixture_test.go @@ -8,13 +8,16 @@ package purchase import ( "context" + "errors" "fmt" "sync" "testing" + "github.com/LeanerCloud/cloud-commitments-go/pkg/common" "github.com/LeanerCloud/cloud-commitments-go/pkg/provider" "github.com/LeanerCloud/cloud-commitments-platform/internal/config" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" ) @@ -25,6 +28,12 @@ type fanOutFixture struct { credFail map[string]bool // credential load fails factoryFail map[string]bool // provider factory fails saveFail map[string]bool // the account's row save fails (AUDIT LOSS) + clientFail map[string]bool // the provider's service client lookup fails + pinFail bool // the fan-out shape pin fails for every account (before submission) + precancel bool // the run context is already done, so no account starts + // cancelAtCred cancels the run context while that account's credentials + // load (the load also fails): a started account cut off mid-run. + cancelAtCred string // cancelAfterSave cancels the run context once that account's row saved, so // the accounts after it never start. cancelAfterSave string @@ -34,8 +43,16 @@ type fanOutFixture struct { // factory only sees the provider config, so the account is recognized by the // access key the credential hook issues: the account ID. type keyedFactory struct { - inner provider.FactoryInterface - failKeys map[string]bool + inner provider.FactoryInterface + failKeys map[string]bool + clientFail map[string]bool +} + +// noClientProvider is a provider whose service client lookup fails. +type noClientProvider struct{ provider.Provider } + +func (noClientProvider) GetServiceClient(context.Context, common.ServiceType, string) (provider.ServiceClient, error) { + return nil, errors.New("service client down") } func (f *keyedFactory) CreateAndValidateProvider(ctx context.Context, name string, cfg *provider.Config) (provider.Provider, error) { @@ -46,18 +63,59 @@ func (f *keyedFactory) CreateAndValidateProvider(ctx context.Context, name strin if f.failKeys[creds.AccessKeyID] { return nil, fmt.Errorf("factory down for %s", creds.AccessKeyID) } - return f.inner.CreateAndValidateProvider(ctx, name, cfg) + prov, err := f.inner.CreateAndValidateProvider(ctx, name, cfg) + if err == nil && f.clientFail[creds.AccessKeyID] { + return noClientProvider{prov}, nil + } + return prov, err +} + +// fanOutOutcome is one executeMultiAccount run: the root it ran, the store behind +// it, the saved account rows and the returned error. +type fanOutOutcome struct { + root *config.PurchaseExecution + store *MockConfigStore + m *Manager + saved *sync.Map + err error +} + +// saveTerminal replays the root's terminal save with the fan-out's error, the +// way executeAndFinalize does after executeMultiAccount returns. +func (r *fanOutOutcome) saveTerminal(t *testing.T) { + t.Helper() + r.root.Status = "failed" + var partial *multiAccountPartialError + if errors.As(r.err, &partial) { + r.root.Status = "partially_completed" + } + require.NoError(t, r.m.saveTerminalExecution(context.Background(), r.root, r.err)) } // run returns the saved account rows (root rows are not saved by the fan-out) // and executeMultiAccount's error. func (f fanOutFixture) run(t *testing.T) (*sync.Map, error) { + t.Helper() + r := f.runRoot(t) + return r.saved, r.err +} + +func (f fanOutFixture) runRoot(t *testing.T) *fanOutOutcome { t.Helper() t.Setenv("CUDLY_MAX_ACCOUNT_PARALLELISM", "1") ctx, cancel := context.WithCancel(context.Background()) t.Cleanup(cancel) + if f.precancel { + cancel() + } var saved sync.Map m, store := refusalManager(t, f.behaviors, &saved) + store.On("DeleteSuppressionsByExecutionTx", mock.Anything, mock.Anything, "root").Return(nil).Maybe() + if f.pinFail { + store.PinExecutionFanOutShapeFn = func(context.Context, string, string) (string, error) { + return "", errors.New("pin store down") + } + } record := store.SavePurchaseExecutionFn store.SavePurchaseExecutionFn = func(c context.Context, e *config.PurchaseExecution) error { @@ -71,8 +129,12 @@ func (f fanOutFixture) run(t *testing.T) (*sync.Map, error) { } return err } - m.providerFactory = &keyedFactory{inner: m.providerFactory, failKeys: f.factoryFail} + m.providerFactory = &keyedFactory{inner: m.providerFactory, failKeys: f.factoryFail, clientFail: f.clientFail} m.credStore = &MockCredentialStore{LoadRawFn: func(_ context.Context, id, _ string) ([]byte, error) { + if f.cancelAtCred == id { + cancel() + return nil, fmt.Errorf("run ended while loading credentials for %s", id) + } if f.credFail[id] { return nil, fmt.Errorf("secret store down for %s", id) } @@ -89,7 +151,7 @@ func (f fanOutFixture) run(t *testing.T) (*sync.Map, error) { } exec := &config.PurchaseExecution{ExecutionID: "root", PlanID: "p", IdempotencyKey: "lineage", Recommendations: recs} err := m.executeMultiAccount(ctx, exec, &config.PurchasePlan{ID: "p", Name: "P"}, accounts) - return &saved, err + return &fanOutOutcome{root: exec, store: store, m: m, saved: &saved, err: err} } func savedAccountRow(saved *sync.Map, id string) *config.PurchaseExecution { @@ -111,7 +173,10 @@ func TestExecuteMultiAccount_FanOutFixtureHooks(t *testing.T) { cases := map[string]struct { fx fanOutFixture errContain string - a, b rowWant + // wantProof is whether the returned error proves nothing was submitted + // (#1844 S1): only the refused-then-never-started run does today. + wantProof bool + a, b rowWant }{ "credential failure on A, B refused": { fx: fanOutFixture{behaviors: refusedBoth, credFail: map[string]bool{skipAcctA: true}}, @@ -134,6 +199,7 @@ func TestExecuteMultiAccount_FanOutFixtureHooks(t *testing.T) { "A refused then cancels the run, B never starts": { fx: fanOutFixture{behaviors: refusedBoth, cancelAfterSave: skipAcctA}, errContain: "some purchases failed", + wantProof: true, a: rowWant{"failed", true}, b: rowWant{}, }, @@ -143,6 +209,8 @@ func TestExecuteMultiAccount_FanOutFixtureHooks(t *testing.T) { saved, err := tc.fx.run(t) require.ErrorIs(t, err, errAllAccountsFailed) assert.ErrorContains(t, err, tc.errContain) + var notSubmitted *beforeSubmissionError + assert.Equal(t, tc.wantProof, errors.As(err, ¬Submitted), "before-submission class on the returned error") for id, want := range map[string]rowWant{skipAcctA: tc.a, skipAcctB: tc.b} { row := savedAccountRow(saved, id) if want.status == "" { diff --git a/internal/purchase/fanout_root_proof_test.go b/internal/purchase/fanout_root_proof_test.go new file mode 100644 index 000000000..82ae38c69 --- /dev/null +++ b/internal/purchase/fanout_root_proof_test.go @@ -0,0 +1,135 @@ +package purchase + +// #1844 S1: a fan-out root records no_submission, and releases its +// suppressions, only when no account committed and every account provably +// submitted nothing (refused before submission, never started, or nothing to +// buy). Each case runs the real executeMultiAccount through the two-account +// fixture, then replays the root's terminal save. A wrong "proven" releases +// suppressions for a commitment that may exist and lets the next step buy it +// again, so every ending that may have reached the cloud must stay unproven. + +import ( + "context" + "errors" + "sync" + "testing" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" + "github.com/LeanerCloud/cloud-commitments-platform/internal/execution" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +func TestExecuteMultiAccount_RootProof(t *testing.T) { + bothRefused := map[string]recBehavior{skipAcctA: recRefused, skipAcctB: recRefused} + withA := func(a recBehavior) map[string]recBehavior { + return map[string]recBehavior{skipAcctA: a, skipAcctB: recRefused} + } + cases := map[string]struct { + fx fanOutFixture + wantProof bool + partial bool + }{ + "1 all accounts refused": {fx: fanOutFixture{behaviors: bothRefused}, wantProof: true}, + "2 one bought, one refused": {fx: fanOutFixture{behaviors: withA(recBuys)}, partial: true}, + "3 one unknown outcome, one refused": {fx: fanOutFixture{behaviors: withA(recUnknown)}}, + "4 refused, the other never started": {fx: fanOutFixture{behaviors: bothRefused, cancelAfterSave: skipAcctA}, wantProof: true}, + "5 all never started": {fx: fanOutFixture{behaviors: bothRefused, precancel: true}, wantProof: true}, + "5b started account cut off by the context": {fx: fanOutFixture{behaviors: bothRefused, cancelAtCred: skipAcctA}}, + "7 one panics after submitting, one refused": {fx: fanOutFixture{behaviors: withA(recPanics)}}, + "8 refused wrapping unknown, one refused": {fx: fanOutFixture{behaviors: withA(recRefusedUnknown)}}, + "9 audit loss on a refused account's row": {fx: fanOutFixture{behaviors: bothRefused, saveFail: map[string]bool{skipAcctA: true}}}, + "9b audit loss on a pin-refused account": {fx: fanOutFixture{behaviors: bothRefused, pinFail: true, saveFail: map[string]bool{skipAcctA: true}}}, + "pin refused for every account": {fx: fanOutFixture{behaviors: bothRefused, pinFail: true}, wantProof: true}, + "9c provider factory failure, one refused": {fx: fanOutFixture{behaviors: bothRefused, factoryFail: map[string]bool{skipAcctA: true}}}, + "9c service client failure, one refused": {fx: fanOutFixture{behaviors: bothRefused, clientFail: map[string]bool{skipAcctA: true}}}, + "9c success=false result, one refused": {fx: fanOutFixture{behaviors: withA(recSuccessFalse)}}, + "credential failure, one refused (until S2)": {fx: fanOutFixture{behaviors: bothRefused, credFail: map[string]bool{skipAcctA: true}}}, + "plain purchase failure, one refused": {fx: fanOutFixture{behaviors: withA(recPlainFail)}}, + "never started, the started one failed plain": {fx: fanOutFixture{behaviors: withA(recPlainFail), cancelAfterSave: skipAcctA}}, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + r := tc.fx.runRoot(t) + require.Error(t, r.err) + var partial *multiAccountPartialError + if tc.partial { + require.ErrorAs(t, r.err, &partial) + } else { + require.ErrorIs(t, r.err, errAllAccountsFailed) + } + + r.saveTerminal(t) + + assert.Equal(t, tc.wantProof, r.root.NoSubmission, "root no_submission") + if tc.wantProof { + r.store.AssertCalled(t, "DeleteSuppressionsByExecutionTx", mock.Anything, mock.Anything, "root") + } else { + r.store.AssertNotCalled(t, "DeleteSuppressionsByExecutionTx", mock.Anything, mock.Anything, "root") + } + }) + } +} + +// A pin-refused account whose row save failed is AUDIT LOSS, not proof: the +// before-submission class must not survive the errors.Join, while the save +// error and the cause text stay visible. +func TestPersistFailedAccountExecution_SaveFailureStripsBeforeSubmissionClass(t *testing.T) { + saveErr := errors.New("database down") + store := new(MockConfigStore) + store.SavePurchaseExecutionFn = func(context.Context, *config.PurchaseExecution) error { return saveErr } + m := &Manager{config: store} + account := config.CloudAccount{ID: skipAcctA} + cause := beforeSubmission(errors.New("pin refused")) + + err := m.persistFailedAccountExecution(context.Background(), &config.PurchaseExecution{}, account, cause) + + var notSubmitted *beforeSubmissionError + assert.False(t, errors.As(err, ¬Submitted), "a lost audit row is not proof") + assert.ErrorIs(t, err, saveErr) + assert.ErrorContains(t, err, "AUDIT LOSS: failed to save execution record for account "+skipAcctA) + assert.ErrorContains(t, err, "pin refused") + + store.SavePurchaseExecutionFn = func(context.Context, *config.PurchaseExecution) error { return nil } + err = m.persistFailedAccountExecution(context.Background(), &config.PurchaseExecution{}, account, cause) + assert.ErrorAs(t, err, ¬Submitted, "a saved row keeps the cause, class included") +} + +// #1844 9d: no real rec-level refusal exists for a Redshift rec on main +// (preSubmitChecks only refuses GCP), so the refusal is injected through the +// fixture's EC2-shaped recs and the root is then read as a Redshift row. This +// pins only the Go-side consequence: a proven root is NeverSubmitted, which +// lets a retry skip the stamp-horizon refusal. The SQL that mints the fresh +// stamp (StampFirstDispatchTx) is covered by integration tests, not here. +func TestExecuteMultiAccount_AllRefusedRedshiftRootIsNeverSubmitted(t *testing.T) { + r := fanOutFixture{behaviors: map[string]recBehavior{skipAcctA: recRefused, skipAcctB: recRefused}}.runRoot(t) + r.saveTerminal(t) + require.True(t, r.root.NoSubmission) + for i := range r.root.Recommendations { + r.root.Recommendations[i].Service = "redshift" + } + + assert.True(t, IsRedshiftExecution(r.root)) + assert.True(t, NeverSubmitted(r.root)) + assert.Empty(t, RedshiftRetryPermanentRefusal(r.root, nil), "no stamp needed: the retry mints a fresh one") + + r.root.NoSubmission = false + assert.NotEmpty(t, RedshiftRetryPermanentRefusal(r.root, nil), "an unproven root keeps the no-stamp refusal") +} + +// summarizeFanOut never reaches fanOutNotSubmitted with an empty or committed +// result set, so the guards that keep the function safe on its own are pinned +// directly. +func TestFanOutNotSubmitted_GuardsAreSafeOnTheirOwn(t *testing.T) { + var none sync.Map + assert.False(t, fanOutNotSubmitted(nil, &none), "no accounts proves nothing") + assert.False(t, fanOutNotSubmitted([]execution.Result[bool]{{AccountID: "a", Value: true, Err: beforeSubmission(errors.New("x"))}}, &none), + "an account that bought is never proven, whatever its error says") + var ran sync.Map + ran.Store("b", struct{}{}) + assert.True(t, fanOutNotSubmitted([]execution.Result[bool]{ + {AccountID: "a", Err: beforeSubmission(errors.New("refused"))}, + {AccountID: "b"}, + }, &ran), "a started account that finished with no error and bought nothing does not block the proof") +} diff --git a/internal/purchase/refusal_tally.go b/internal/purchase/refusal_tally.go index f1947eb17..9115b5123 100644 --- a/internal/purchase/refusal_tally.go +++ b/internal/purchase/refusal_tally.go @@ -3,9 +3,11 @@ package purchase import ( "errors" "fmt" + "sync" "github.com/LeanerCloud/cloud-commitments-go/pkg/common" "github.com/LeanerCloud/cloud-commitments-platform/internal/config" + "github.com/LeanerCloud/cloud-commitments-platform/internal/execution" ) // failureTally counts one aggregation's failed recs and how many of them were @@ -19,12 +21,44 @@ type failureTally struct{ failed, refused int } // 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) { + if refusedBeforeSubmission(err) { t.refused++ } } +// refusedBeforeSubmission reports an error that proves nothing reached the +// cloud: the beforeSubmission class without an unknown outcome. +func refusedBeforeSubmission(err error) bool { + var notSubmitted *beforeSubmissionError + return errors.As(err, ¬Submitted) && !errors.Is(err, common.ErrOutcomeUnknown) +} + +// fanOutNotSubmitted reports that a fan-out where no account committed provably +// submitted nothing, so the root may carry the no_submission proof and release +// its suppressions. It reads the in-memory results, not saved rows: a panicked +// account leaves no row and a lost row would read as "nothing submitted". Every +// account must be refused before submission, never started (the run context +// ended first, as skippedAccountRows decides), or have bought nothing without +// error; any other ending (panic, plain error, unknown outcome, a bought rec) +// may have reached the cloud, so it keeps today's unproven failure. +func fanOutNotSubmitted(results []execution.Result[bool], started *sync.Map) bool { + if len(results) == 0 { + return false + } + for _, r := range results { + if r.Value { + return false + } + if r.Err == nil || refusedBeforeSubmission(r.Err) { + continue + } + if _, ran := started.Load(r.AccountID); ran { + return false + } + } + return true +} + // 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 { diff --git a/internal/purchase/refusal_tally_test.go b/internal/purchase/refusal_tally_test.go index 2c8320326..aab93b4d7 100644 --- a/internal/purchase/refusal_tally_test.go +++ b/internal/purchase/refusal_tally_test.go @@ -30,6 +30,7 @@ const ( recPlainFail recRefusedUnknown recPanics + recSuccessFalse ) func (b recBehavior) result() (common.PurchaseResult, error) { @@ -42,6 +43,9 @@ func (b recBehavior) result() (common.PurchaseResult, error) { // 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 recSuccessFalse: + // The provider reports a failed purchase without an error value. + return common.PurchaseResult{Success: false}, nil case recPlainFail: return common.PurchaseResult{}, errors.New("offering not available") } From 51810d263ebddbbcab503de0c885e2c5743a4bdf Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 9 Nov 2026 19:24:12 +0100 Subject: [PATCH 3/3] test(purchase): use a plain receiver for the suppression-release mock assertions The mock assertion guard cannot check a call on a nested selector, so the root proof test copies the store into a local before asserting on it. Part of #1844 Co-Authored-By: Claude Sonnet 5.5 --- internal/purchase/fanout_root_proof_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/purchase/fanout_root_proof_test.go b/internal/purchase/fanout_root_proof_test.go index 82ae38c69..704a8fa4c 100644 --- a/internal/purchase/fanout_root_proof_test.go +++ b/internal/purchase/fanout_root_proof_test.go @@ -63,10 +63,11 @@ func TestExecuteMultiAccount_RootProof(t *testing.T) { r.saveTerminal(t) assert.Equal(t, tc.wantProof, r.root.NoSubmission, "root no_submission") + store := r.store if tc.wantProof { - r.store.AssertCalled(t, "DeleteSuppressionsByExecutionTx", mock.Anything, mock.Anything, "root") + store.AssertCalled(t, "DeleteSuppressionsByExecutionTx", mock.Anything, mock.Anything, "root") } else { - r.store.AssertNotCalled(t, "DeleteSuppressionsByExecutionTx", mock.Anything, mock.Anything, "root") + store.AssertNotCalled(t, "DeleteSuppressionsByExecutionTx", mock.Anything, mock.Anything, "root") } }) }