From 08d8e49f872d723318a451ba2092c6fe1dff97af Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 16 Jul 2026 16:01:13 +0300 Subject: [PATCH] fix(server): rename cap->capability, fix cadence comment, add multi-account skip test - handler_ladder.go ~256: rename `cap` to `capability` (shadows builtin) - handler_ladder.go ~437: correct comment -- unknown cadence surfaces in ladderConfigToEngine (pre-persist, fail-loud), not during Allocate - handler_ladder_test.go: add TestHandleLadderRun_MultiAccountSkip_CountedAndIsolated which asserts SkippedMultiAccount==1 for a foreign-ExternalID config, nothing persisted for it, and a co-present healthy config still plans --- internal/server/handler_ladder.go | 7 +-- internal/server/handler_ladder_test.go | 59 ++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 3 deletions(-) diff --git a/internal/server/handler_ladder.go b/internal/server/handler_ladder.go index a7e74fcf6..7691a73e7 100644 --- a/internal/server/handler_ladder.go +++ b/internal/server/handler_ladder.go @@ -253,14 +253,14 @@ func (app *Application) processOneLadderConfig( log.Printf("ladder_run: config %s: LadderCapabilityFactory is nil (not wired), erroring", dbCfg.ID) return outcomeErrored } - cap, err := app.LadderCapabilityFactory(ctx, region, cloudAcct.ExternalID) + capability, err := app.LadderCapabilityFactory(ctx, region, cloudAcct.ExternalID) if err != nil { log.Printf("ladder_run: config %s: failed to build ladder capability: %v", dbCfg.ID, err) return outcomeErrored } // Run the plan engine and persist the result. - if err := app.executeLadderRun(ctx, dbCfg, cap, cloudAcct.ExternalID, term, paymentOpt, now); err != nil { + if err := app.executeLadderRun(ctx, dbCfg, capability, cloudAcct.ExternalID, term, paymentOpt, now); err != nil { log.Printf("ladder_run: config %s: planning failed: %v", dbCfg.ID, err) return outcomeErrored } @@ -435,7 +435,8 @@ func ladderWithinCadenceWindow(ctx context.Context, store config.StoreInterface, } default: // Unknown cadence: log but do not block the run. The LadderConfig - // validator will surface this as an error during Allocate. + // validator surfaces this as an error in ladderConfigToEngine (pre-persist, + // fail-loud), so the run will fail before any money action is taken. log.Printf("ladder_run: config %s: unknown cadence=%q, proceeding without cadence gate", dbCfg.ID, dbCfg.Cadence) } return false, "", nil diff --git a/internal/server/handler_ladder_test.go b/internal/server/handler_ladder_test.go index 77a206245..2a319ab42 100644 --- a/internal/server/handler_ladder_test.go +++ b/internal/server/handler_ladder_test.go @@ -418,6 +418,65 @@ func TestHandleLadderRun_AllConfigsDisabled(t *testing.T) { assert.Nil(t, store.savedRun) } +// TestHandleLadderRun_MultiAccountSkip_CountedAndIsolated verifies that: +// (a) an enabled ladder config whose cloud account ExternalID does NOT match the +// +// resolved caller account is counted as SkippedMultiAccount (not Errored), +// +// (b) nothing is persisted for that config, and +// (c) a second healthy config present in the same run is not affected -- it still +// +// plans successfully (per-config isolation). +func TestHandleLadderRun_MultiAccountSkip_CountedAndIsolated(t *testing.T) { + ctx := testutil.TestContext(t) + now := time.Date(2026, 7, 1, 12, 0, 0, 0, time.UTC) + const ownAccount = "123456789012" + const foreignAccount = "999999999999" + + // cfgForeign: enabled, but its cloud account ExternalID is a different AWS account. + cfgForeign := validTestDBConfig("cfg-foreign") + cfgForeign.CloudAccountID = "acct-foreign" + + // cfgHealthy: enabled, cloud account matches the caller account. + cfgHealthy := validTestDBConfig("cfg-healthy-ma") + cfgHealthy.CloudAccountID = "acct-own" + + store := &ladderTestStore{ + cloudAcctByID: map[string]*config.CloudAccount{ + "acct-foreign": { + ID: "acct-foreign", + Provider: "aws", + ExternalID: foreignAccount, + Enabled: true, + }, + "acct-own": validTestCloudAccount(ownAccount), + }, + } + app := &Application{ + Config: store, + LadderCapabilityFactory: func(_ context.Context, _, _ string) (pkgladder.LadderCapability, error) { + return &fakeLadderCapability{t: t, baseline: testBaseline(10.0)}, nil + }, + } + + // Put the foreign config first to prove isolation is order-independent. + configs := []config.LadderConfigDB{cfgForeign, cfgHealthy} + result := app.runLadderConfigs(ctx, configs, ownAccount, "us-east-1", pkgladder.Term1Year, pkgladder.PaymentNoUpfront, now) + + require.NotNil(t, result) + // (a) The foreign config must be counted as SkippedMultiAccount. + assert.Equal(t, 1, result.SkippedMultiAccount, "foreign-account config must be counted SkippedMultiAccount") + assert.Equal(t, 0, result.Errored, "a multi-account skip is not an error") + // (b) Nothing persisted for the foreign config; only the healthy config run. + require.Len(t, store.savedRuns, 1, "only the healthy config may persist a run") + require.NotNil(t, store.savedRuns[0].ConfigID) + assert.Equal(t, "cfg-healthy-ma", *store.savedRuns[0].ConfigID, "persisted run must be from the healthy config, not the skipped one") + // (c) The healthy config still processes successfully despite the skip. + assert.Equal(t, 1, result.Planned, "the healthy config must still be planned") + assert.Equal(t, 0, result.SkippedDisabled) + assert.Equal(t, 0, result.SkippedCadence) +} + // ============================================================ // executeLadderRun: healthy single-config run // ============================================================