From ac24de108c70cca55744f9ee1692ca1a7e49044d Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 12 Oct 2026 23:16:46 +0200 Subject: [PATCH] feat(purchase): per-plan step commitment cap, auto-purchase kill switch and notified veto Safety layer for plan-step auto-purchase (platform#631 PR2c). It only adds refusals; nothing here makes a plan step buy by itself. - max_step_commitment (nullable, migration 000112) on purchase_plans. NULL refuses auto-purchase of recommendation-bearing steps. The cap is measured on total commitment (upfront + monthly * term * 12), and zero-commitment recommendations are refused. CheckStepCommitmentCap is enforced before the claim on the scheduler and SQS execute paths. - global_config.auto_purchase_enabled (default false) backs the previously unread scheduling.auto_purchase default as a fail-closed kill switch, read in shouldNotifyPlan and the scheduler/SQS gates. Manual and approved purchases are unaffected. - Recommendation-bearing plan rows must be "notified" to auto-execute, so a row that never got its notification email cannot buy. - PlanRequest.max_step_commitment (validated > 0) and openapi. Refs #631 Co-Authored-By: Claude Sonnet 5.5 --- internal/api/handler_plans.go | 4 + internal/api/handler_plans_step_cap_test.go | 36 +++ internal/api/handler_purchases.go | 6 +- ..._purchases_pause_claim_integration_test.go | 50 ++-- internal/api/openapi.yaml | 12 + internal/api/types.go | 9 +- internal/config/commitment.go | 16 ++ internal/config/store_postgres.go | 27 +- .../store_postgres_complete_step_test.go | 5 +- .../config/store_postgres_coverage_test.go | 3 +- internal/config/store_postgres_db_test.go | 47 ++++ .../config/store_postgres_pgxmock_test.go | 31 ++- .../config/store_postgres_plan_update_test.go | 4 +- internal/config/types.go | 10 + internal/config/validation.go | 18 +- ...lan_step_cap_auto_purchase_switch.down.sql | 5 + ..._plan_step_cap_auto_purchase_switch.up.sql | 18 ++ ...plan_step_cap_auto_purchase_switch_test.go | 64 +++++ internal/purchase/coverage_extra_test.go | 11 +- internal/purchase/manager.go | 44 +++- internal/purchase/manager_test.go | 11 +- internal/purchase/messages.go | 2 +- .../purchase/money_path_regression_test.go | 15 +- internal/purchase/notifications.go | 14 +- internal/purchase/notifications_test.go | 4 +- internal/purchase/plan_step_no_recs_test.go | 15 +- .../ramp_step_progress_integration_test.go | 15 +- internal/purchase/step_cap.go | 45 ++++ internal/purchase/step_cap_helpers_test.go | 11 + internal/purchase/step_cap_test.go | 232 ++++++++++++++++++ 30 files changed, 712 insertions(+), 72 deletions(-) create mode 100644 internal/api/handler_plans_step_cap_test.go create mode 100644 internal/config/commitment.go create mode 100644 internal/database/postgres/migrations/000112_plan_step_cap_auto_purchase_switch.down.sql create mode 100644 internal/database/postgres/migrations/000112_plan_step_cap_auto_purchase_switch.up.sql create mode 100644 internal/database/postgres/migrations/000112_plan_step_cap_auto_purchase_switch_test.go create mode 100644 internal/purchase/step_cap.go create mode 100644 internal/purchase/step_cap_helpers_test.go create mode 100644 internal/purchase/step_cap_test.go diff --git a/internal/api/handler_plans.go b/internal/api/handler_plans.go index 61a5ce2a..376f2701 100644 --- a/internal/api/handler_plans.go +++ b/internal/api/handler_plans.go @@ -339,6 +339,10 @@ func mergeUpdatedPlan(existingPlan *config.PurchasePlan, req *PlanRequest, planI return nil, err } + if req.MaxStepCommitment == nil { + plan.MaxStepCommitment = existingPlan.MaxStepCommitment + } + // If no services were created from request, preserve existing services if len(plan.Services) == 0 && len(existingPlan.Services) > 0 { plan.Services = existingPlan.Services diff --git a/internal/api/handler_plans_step_cap_test.go b/internal/api/handler_plans_step_cap_test.go new file mode 100644 index 00000000..a5b5a1d0 --- /dev/null +++ b/internal/api/handler_plans_step_cap_test.go @@ -0,0 +1,36 @@ +package api + +import ( + "math" + "testing" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestMergeUpdatedPlan_MaxStepCommitment(t *testing.T) { + cap100, cap5 := 100.0, 5.0 + existing := &config.PurchasePlan{Name: "p", MaxStepCommitment: &cap100, + RampSchedule: config.RampSchedule{Type: "immediate", PercentPerStep: 100, TotalSteps: 1}} + + t.Run("omitted on update keeps the current cap", func(t *testing.T) { + plan, err := mergeUpdatedPlan(existing, &PlanRequest{Name: "p", RampSchedule: "immediate"}, "id") + require.NoError(t, err) + require.NotNil(t, plan.MaxStepCommitment) + assert.Equal(t, 100.0, *plan.MaxStepCommitment) + }) + t.Run("provided on update replaces it", func(t *testing.T) { + plan, err := mergeUpdatedPlan(existing, &PlanRequest{Name: "p", RampSchedule: "immediate", MaxStepCommitment: &cap5}, "id") + require.NoError(t, err) + assert.Equal(t, 5.0, *plan.MaxStepCommitment) + }) + for name, v := range map[string]float64{"zero": 0, "negative": -1, "NaN": math.NaN(), "Inf": math.Inf(1)} { + t.Run("rejects "+name, func(t *testing.T) { + bad := v + _, err := mergeUpdatedPlan(existing, &PlanRequest{Name: "p", RampSchedule: "immediate", MaxStepCommitment: &bad}, "id") + require.Error(t, err) + assert.Contains(t, err.Error(), "max_step_commitment") + }) + } +} diff --git a/internal/api/handler_purchases.go b/internal/api/handler_purchases.go index bf1ff252..e65e8025 100644 --- a/internal/api/handler_purchases.go +++ b/internal/api/handler_purchases.go @@ -2586,11 +2586,7 @@ func purchaseConstraintSets(recs []config.RecommendationRecord) []auth.Permissio // execute path the two cost fields are the store-derived values written by // priceRecommendationsFromStore. func recTotalCommitment(rec *config.RecommendationRecord) float64 { - total := rec.UpfrontCost - if rec.Term > 0 && rec.MonthlyCost != nil { - total += *rec.MonthlyCost * float64(rec.Term*12) - } - return total + return rec.TotalCommitment() } // requireNonZeroCommitment guards against a purchase batch whose computed diff --git a/internal/api/handler_purchases_pause_claim_integration_test.go b/internal/api/handler_purchases_pause_claim_integration_test.go index 3a69ab83..c5d3ff00 100644 --- a/internal/api/handler_purchases_pause_claim_integration_test.go +++ b/internal/api/handler_purchases_pause_claim_integration_test.go @@ -67,6 +67,19 @@ type pauseClaimFixture struct { workers sync.WaitGroup } +// pauseClaimCap is a plan cap far above the fixture recommendations. +var pauseClaimCap = 1e9 + +// markNotified moves the row to "notified", the only state in which the +// scheduler and SQS paths will auto-execute a recommendation-bearing plan row +// (the notification email is the veto window). pause only applies to "pending", +// so tests call this after pausing and resuming and before the worker runs. +func (f *pauseClaimFixture) markNotified(t *testing.T) { + t.Helper() + _, err := f.pool.Exec(f.ctx, `UPDATE purchase_executions SET status = 'notified' WHERE execution_id = $1`, f.execution.ExecutionID) + require.NoError(t, err) +} + func newPauseClaimFixture(t *testing.T) *pauseClaimFixture { t.Helper() ctx, cancel := context.WithTimeout(context.Background(), 90*time.Second) @@ -75,7 +88,12 @@ func newPauseClaimFixture(t *testing.T) *pauseClaimFixture { plan, err := f.store.GetPurchasePlan(ctx, f.planID) require.NoError(t, err) plan.AutoPurchase = true + plan.MaxStepCommitment = &pauseClaimCap require.NoError(t, f.store.UpdatePurchasePlan(ctx, plan)) + globalCfg, err := f.store.GetGlobalConfig(ctx) + require.NoError(t, err) + globalCfg.AutoPurchaseEnabled = true + require.NoError(t, f.store.SaveGlobalConfig(ctx, globalCfg)) accounts, err := f.store.GetPlanAccounts(ctx, plan.ID) require.NoError(t, err) require.Len(t, accounts, 1) @@ -187,6 +205,7 @@ func assertPauseConflict(t *testing.T, err error) { func TestPauseClaim_RunningWorkerCannotBeReclaimed(t *testing.T) { f := newPauseClaimFixture(t) + f.markNotified(t) first := f.startWorker() select { case <-f.service.entered: @@ -222,6 +241,11 @@ func TestPauseClaim_PausedQueueResumesExactlyOnce(t *testing.T) { require.Equal(t, paused, f.state(t)) require.NoError(t, f.resume()) require.Equal(t, "pending", f.state(t).Status) + // A resumed row is pending again, so auto-execution still refuses it until + // a notification has gone out. + require.Error(t, f.manager.ProcessMessage(f.ctx, f.body)) + require.Zero(t, f.service.calls.Load()) + f.markNotified(t) f.service.unblock() require.NoError(t, f.manager.ProcessMessage(f.ctx, f.body)) require.NoError(t, f.manager.ProcessMessage(f.ctx, f.body)) @@ -253,22 +277,20 @@ func TestPauseClaim_ConcurrentPauseAndWorker(t *testing.T) { close(start) select { case err := <-paused: - if err == nil { - f.waitWorker(t, worker) - require.Zero(t, f.service.calls.Load()) - require.Equal(t, "paused", f.state(t).Status) - require.NoError(t, f.resume()) - f.service.unblock() - require.NoError(t, f.manager.ProcessMessage(f.ctx, f.body)) - } else { - assertPauseConflict(t, err) - require.Equal(t, "running", f.state(t).Status) - require.NoError(t, f.manager.ProcessMessage(f.ctx, f.body)) - f.service.unblock() - f.waitWorker(t, worker) - } + require.NoError(t, err, "a pending row is pausable: the worker can never claim it") case <-f.ctx.Done(): t.Fatal("pause did not finish") } + select { + case <-worker: // refused (row still pending) or lost the claim (row paused): both buy nothing + case <-f.ctx.Done(): + t.Fatal("purchase worker did not finish") + } + require.Zero(t, f.service.calls.Load()) + require.Equal(t, "paused", f.state(t).Status) + require.NoError(t, f.resume()) + f.markNotified(t) + f.service.unblock() + require.NoError(t, f.manager.ProcessMessage(f.ctx, f.body)) f.assertCompleted(t) } diff --git a/internal/api/openapi.yaml b/internal/api/openapi.yaml index c874d680..6a2ee39d 100644 --- a/internal/api/openapi.yaml +++ b/internal/api/openapi.yaml @@ -3170,6 +3170,14 @@ components: type: boolean notification_days_before: type: integer + max_step_commitment: + type: number + exclusiveMinimum: 0 + description: >- + Ceiling on the total commitment (upfront + monthly * term) one ramp + step may auto-purchase. Omit on create for no cap (auto-purchase of + steps with recommendations is then refused); omit on update to keep + the current cap. provider: type: string service: @@ -3212,6 +3220,10 @@ components: type: boolean notification_days_before: type: integer + max_step_commitment: + type: number + nullable: true + description: Cap on the total commitment one ramp step may auto-purchase; null means auto-purchase of steps with recommendations is refused. services: type: object additionalProperties: diff --git a/internal/api/types.go b/internal/api/types.go index 5f84701e..9570dca8 100644 --- a/internal/api/types.go +++ b/internal/api/types.go @@ -800,8 +800,12 @@ type PlanRequest struct { NotificationDaysBefore int `json:"notification_days_before"` CustomStepPercent int `json:"custom_step_percent,omitempty"` CustomIntervalDays int `json:"custom_interval_days,omitempty"` - AutoPurchase bool `json:"auto_purchase"` - Enabled bool `json:"enabled"` + // MaxStepCommitment caps the total commitment one ramp step may + // auto-purchase. Omitted on create = no cap (auto-purchase of steps with + // recommendations stays refused); omitted on update = keep the current cap. + MaxStepCommitment *float64 `json:"max_step_commitment,omitempty"` + AutoPurchase bool `json:"auto_purchase"` + Enabled bool `json:"enabled"` } // toPurchasePlan converts a PlanRequest to a config.PurchasePlan. @@ -812,6 +816,7 @@ func (r *PlanRequest) toPurchasePlan() *config.PurchasePlan { Enabled: r.Enabled, AutoPurchase: r.AutoPurchase, NotificationDaysBefore: r.NotificationDaysBefore, + MaxStepCommitment: r.MaxStepCommitment, CreatedAt: now, UpdatedAt: now, } diff --git a/internal/config/commitment.go b/internal/config/commitment.go new file mode 100644 index 00000000..2596562c --- /dev/null +++ b/internal/config/commitment.go @@ -0,0 +1,16 @@ +package config + +// TotalCommitment returns the recommendation's full committed spend: the +// upfront cost plus the recurring monthly cost over the term. UpfrontCost and +// MonthlyCost are totals for the rec's Count, so they sum with no per-instance +// scaling. Term is in years (converted with *12). A nil MonthlyCost (all-upfront +// commitments) or a non-positive Term contributes nothing to the recurring leg. +// Used for spend caps instead of UpfrontCost alone, which is $0 for no-upfront +// commitments (Azure monthly billing). +func (r *RecommendationRecord) TotalCommitment() float64 { + total := r.UpfrontCost + if r.Term > 0 && r.MonthlyCost != nil { + total += *r.MonthlyCost * float64(r.Term*12) + } + return total +} diff --git a/internal/config/store_postgres.go b/internal/config/store_postgres.go index 1fadf94b..d92acdad 100644 --- a/internal/config/store_postgres.go +++ b/internal/config/store_postgres.go @@ -92,7 +92,8 @@ func getGlobalConfigFrom(ctx context.Context, q globalConfigExecutor) (*GlobalCo COALESCE(laddering_enabled, false), COALESCE(ladder_execution_enabled, false), offering_class, - require_different_approver + require_different_approver, + auto_purchase_enabled FROM global_config WHERE id = 1 ` @@ -126,6 +127,7 @@ func getGlobalConfigFrom(ctx context.Context, q globalConfigExecutor) (*GlobalCo &config.LadderExecutionEnabled, &config.OfferingClass, &config.RequireDifferentApprover, + &config.AutoPurchaseEnabled, ) if err != nil { @@ -259,8 +261,8 @@ func saveGlobalConfigWith(ctx context.Context, q globalConfigExecutor, config *G grace_period_days, recommendations_cache_stale_hours, recommendations_lookback_days, purchase_delay_hours, laddering_enabled, ladder_execution_enabled, offering_class, - require_different_approver - ) VALUES (1, $1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17, $18, $19, $20, $21, $22, $23, $24) + require_different_approver, auto_purchase_enabled + ) VALUES (1, $1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17, $18, $19, $20, $21, $22, $23, $24, $25) ON CONFLICT (id) DO UPDATE SET enabled_providers = $1, notification_email = $2, @@ -286,6 +288,7 @@ func saveGlobalConfigWith(ctx context.Context, q globalConfigExecutor, config *G ladder_execution_enabled = $22, offering_class = $23, require_different_approver = $24, + auto_purchase_enabled = $25, updated_at = NOW() ` @@ -354,6 +357,7 @@ func saveGlobalConfigWith(ctx context.Context, q globalConfigExecutor, config *G config.LadderExecutionEnabled, offeringClass, config.RequireDifferentApprover, + config.AutoPurchaseEnabled, ) if err != nil { @@ -556,8 +560,9 @@ func (s *PostgresStore) CreatePurchasePlan(ctx context.Context, plan *PurchasePl INSERT INTO purchase_plans ( id, name, enabled, auto_purchase, notification_days_before, services, ramp_schedule, created_at, updated_at, - next_execution_date, last_execution_date, last_notification_sent - ) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12) + next_execution_date, last_execution_date, last_notification_sent, + max_step_commitment + ) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13) RETURNING updated_at ` @@ -575,6 +580,7 @@ func (s *PostgresStore) CreatePurchasePlan(ctx context.Context, plan *PurchasePl plan.NextExecutionDate, plan.LastExecutionDate, plan.LastNotificationSent, + plan.MaxStepCommitment, ).Scan(&updatedAt) if err != nil { @@ -588,7 +594,8 @@ func (s *PostgresStore) CreatePurchasePlan(ctx context.Context, plan *PurchasePl const purchasePlanSelectCols = ` SELECT id, name, enabled, auto_purchase, notification_days_before, services, ramp_schedule, created_at, updated_at, - next_execution_date, last_execution_date, last_notification_sent + next_execution_date, last_execution_date, last_notification_sent, + max_step_commitment FROM purchase_plans` // scanPurchasePlanRow deserialises one purchase_plans row returned by QueryRow @@ -612,6 +619,7 @@ func scanPurchasePlanRow(row pgx.Row) (*PurchasePlan, error) { &nextExecDate, &lastExecDate, &lastNotifSent, + &plan.MaxStepCommitment, ) if err != nil { return nil, err @@ -799,7 +807,8 @@ func (s *PostgresStore) UpdatePurchasePlanTx(ctx context.Context, tx pgx.Tx, pla ramp_schedule = $7, next_execution_date = $9, last_execution_date = $10, - last_notification_sent = $11 + last_notification_sent = $11, + max_step_commitment = $12 WHERE id = $1 AND updated_at = $8 RETURNING updated_at ` @@ -817,6 +826,7 @@ func (s *PostgresStore) UpdatePurchasePlanTx(ctx context.Context, tx pgx.Tx, pla plan.NextExecutionDate, plan.LastExecutionDate, plan.LastNotificationSent, + plan.MaxStepCommitment, ).Scan(&updatedAt) if errors.Is(err, pgx.ErrNoRows) { @@ -888,6 +898,7 @@ func buildListPlansQuery(accountIDs []string, includeUnassigned bool) (query str SELECT id, name, enabled, auto_purchase, notification_days_before, services, ramp_schedule, created_at, updated_at, next_execution_date, last_execution_date, last_notification_sent, + max_step_commitment, false AS unassigned FROM purchase_plans ORDER BY created_at DESC @@ -907,6 +918,7 @@ func buildListPlansQuery(accountIDs []string, includeUnassigned bool) (query str SELECT DISTINCT pp.id, pp.name, pp.enabled, pp.auto_purchase, pp.notification_days_before, pp.services, pp.ramp_schedule, pp.created_at, pp.updated_at, pp.next_execution_date, pp.last_execution_date, pp.last_notification_sent, + pp.max_step_commitment, (NOT EXISTS (SELECT 1 FROM plan_accounts WHERE plan_id = pp.id)) AS unassigned FROM purchase_plans pp LEFT JOIN plan_accounts pa ON pa.plan_id = pp.id @@ -948,6 +960,7 @@ func (s *PostgresStore) ListPurchasePlans(ctx context.Context, filter PurchasePl &nextExecDate, &lastExecDate, &lastNotifSent, + &plan.MaxStepCommitment, &plan.Unassigned, ) if err != nil { diff --git a/internal/config/store_postgres_complete_step_test.go b/internal/config/store_postgres_complete_step_test.go index d456ab59..4cac70b0 100644 --- a/internal/config/store_postgres_complete_step_test.go +++ b/internal/config/store_postgres_complete_step_test.go @@ -56,6 +56,7 @@ var purchasePlanRowCols = []string{ "id", "name", "enabled", "auto_purchase", "notification_days_before", "services", "ramp_schedule", "created_at", "updated_at", "next_execution_date", "last_execution_date", "last_notification_sent", + "max_step_commitment", } // rampStepArg is a pgxmock.Argument that unmarshals the ramp_schedule JSONB @@ -87,7 +88,7 @@ func (a nullTimeArg) Match(v interface{}) bool { return (tp == nil) == a.wantNil } -const purchasePlanUpdateArgs = 11 +const purchasePlanUpdateArgs = 12 // completeStepUpdateArgs pins the completed step, expected read version, and next-date presence. func completeStepUpdateArgs(wantStep int, staleUpdatedAt time.Time, wantNextNil bool) []interface{} { @@ -134,7 +135,7 @@ func rampPlanRows(t *testing.T, planID string, ramp RampSchedule, now, stale tim return pgxmock.NewRows(purchasePlanRowCols).AddRow( planID, "Ramp Plan", true, true, 3, svcJSON, rampJSON, now, stale, - nextExec, sql.NullTime{Valid: false}, sql.NullTime{Valid: false}, + nextExec, sql.NullTime{Valid: false}, sql.NullTime{Valid: false}, (*float64)(nil), ) } diff --git a/internal/config/store_postgres_coverage_test.go b/internal/config/store_postgres_coverage_test.go index b4a7a34e..c8c14ff0 100644 --- a/internal/config/store_postgres_coverage_test.go +++ b/internal/config/store_postgres_coverage_test.go @@ -569,7 +569,7 @@ func TestSaveGlobalConfig_OfferingClassBindsAt23(t *testing.T) { OfferingClass: "standard", } - // Expect exactly 24 args; pgxmock validates arg count and types. + // Expect exactly 25 args; pgxmock validates arg count and types. // The 21st arg is laddering_enabled; the 22nd is ladder_execution_enabled; // the 23rd arg must be "standard" (offering_class); the 24th is // require_different_approver (issue #1005). @@ -601,6 +601,7 @@ func TestSaveGlobalConfig_OfferingClassBindsAt23(t *testing.T) { pgxmock.AnyArg(), // $22 ladder_execution_enabled "standard", // $23 offering_class -- the field this test guards pgxmock.AnyArg(), // $24 require_different_approver + pgxmock.AnyArg(), // $25 auto_purchase_enabled ). WillReturnResult(pgxmock.NewResult("INSERT", 1)) diff --git a/internal/config/store_postgres_db_test.go b/internal/config/store_postgres_db_test.go index ede56a0a..79fab266 100644 --- a/internal/config/store_postgres_db_test.go +++ b/internal/config/store_postgres_db_test.go @@ -1722,3 +1722,50 @@ func TestPostgresStoreDB_RotatePendingApprovalToken(t *testing.T) { assert.Equal(t, HashApprovalToken("original-token"), reread.ApprovalToken) }) } + +// TestPostgresStoreDB_StepCapAndKillSwitchRoundTrip proves, against a real +// PostgreSQL, that max_step_commitment round-trips through every plan read and +// write path (NULL stays NULL) and that auto_purchase_enabled round-trips +// through the global config. +func TestPostgresStoreDB_StepCapAndKillSwitchRoundTrip(t *testing.T) { + conn := setupTestContainerDB(t) + cleanupTestData(t, conn) + store := NewPostgresStore(conn) + ctx := context.Background() + + plan := &PurchasePlan{ + Name: "capped", + Enabled: true, + Services: map[string]ServiceConfig{}, + RampSchedule: RampSchedule{Type: "immediate", PercentPerStep: 100, TotalSteps: 1}, + } + require.NoError(t, store.CreatePurchasePlan(ctx, plan)) + + got, err := store.GetPurchasePlan(ctx, plan.ID) + require.NoError(t, err) + assert.Nil(t, got.MaxStepCommitment, "a plan created without a cap has NULL") + + capValue := 2500.5 + got.MaxStepCommitment = &capValue + require.NoError(t, store.UpdatePurchasePlan(ctx, got)) + + got, err = store.GetPurchasePlan(ctx, plan.ID) + require.NoError(t, err) + require.NotNil(t, got.MaxStepCommitment) + assert.InDelta(t, 2500.5, *got.MaxStepCommitment, 1e-9) + + listed, err := store.ListPurchasePlans(ctx, PurchasePlanFilter{}) + require.NoError(t, err) + require.Len(t, listed, 1) + require.NotNil(t, listed[0].MaxStepCommitment) + assert.InDelta(t, 2500.5, *listed[0].MaxStepCommitment, 1e-9) + + cfg, err := store.GetGlobalConfig(ctx) + require.NoError(t, err) + assert.False(t, cfg.AutoPurchaseEnabled, "the kill switch defaults to off") + cfg.AutoPurchaseEnabled = true + require.NoError(t, store.SaveGlobalConfig(ctx, cfg)) + cfg, err = store.GetGlobalConfig(ctx) + require.NoError(t, err) + assert.True(t, cfg.AutoPurchaseEnabled) +} diff --git a/internal/config/store_postgres_pgxmock_test.go b/internal/config/store_postgres_pgxmock_test.go index 291b9625..f51017bc 100644 --- a/internal/config/store_postgres_pgxmock_test.go +++ b/internal/config/store_postgres_pgxmock_test.go @@ -64,6 +64,7 @@ func TestPGXMock_GetGlobalConfig_Success(t *testing.T) { "ladder_execution_enabled", "offering_class", "require_different_approver", + "auto_purchase_enabled", } rows := pgxmock.NewRows(cols).AddRow( []string{"aws"}, strPtr("ops@example.com"), true, @@ -78,6 +79,7 @@ func TestPGXMock_GetGlobalConfig_Success(t *testing.T) { false, "convertible", false, + false, ) mock.ExpectQuery("SELECT").WillReturnRows(rows) @@ -124,6 +126,7 @@ func TestPGXMock_GetGlobalConfig_GracePeriodDays(t *testing.T) { "ladder_execution_enabled", "offering_class", "require_different_approver", + "auto_purchase_enabled", } baseRow := func(graceJSON string) []any { return []any{ @@ -139,6 +142,7 @@ func TestPGXMock_GetGlobalConfig_GracePeriodDays(t *testing.T) { false, "convertible", false, + false, } } @@ -199,6 +203,7 @@ var globalConfigCols = []string{ "ladder_execution_enabled", "offering_class", "require_different_approver", + "auto_purchase_enabled", } // TestPGXMock_UpdateGlobalConfigAtomic_LockedReadModifyWrite proves the F2 @@ -230,6 +235,7 @@ func TestPGXMock_UpdateGlobalConfigAtomic_LockedReadModifyWrite(t *testing.T) { false, // ladder_execution_enabled = false "convertible", // offering_class false, // require_different_approver + false, // auto_purchase_enabled ) // Strict order: the SELECT and the UPSERT must sit between the same @@ -238,7 +244,7 @@ func TestPGXMock_UpdateGlobalConfigAtomic_LockedReadModifyWrite(t *testing.T) { mock.ExpectExec("pg_advisory_xact_lock").WithArgs(pgxmock.AnyArg()). WillReturnResult(pgxmock.NewResult("SELECT", 1)) mock.ExpectQuery("FROM global_config").WillReturnRows(seeded) - mock.ExpectExec("INSERT INTO global_config").WithArgs(anyArgsCfg(24)...). + mock.ExpectExec("INSERT INTO global_config").WithArgs(anyArgsCfg(25)...). WillReturnResult(pgxmock.NewResult("INSERT", 1)) mock.ExpectCommit() @@ -287,6 +293,7 @@ func TestPGXMock_UpdateGlobalConfigAtomic_ApplyErrorRollsBack(t *testing.T) { false, "convertible", false, + false, ) mock.ExpectBegin() @@ -323,6 +330,7 @@ func TestPGXMock_UpdateGlobalConfigAtomic_PropagatesOnlyChangedDefaults(t *testi false, "convertible", false, + false, ) } setCoverage := func(existing *GlobalConfig) error { @@ -337,7 +345,7 @@ func TestPGXMock_UpdateGlobalConfigAtomic_PropagatesOnlyChangedDefaults(t *testi mock.ExpectExec("pg_advisory_xact_lock").WithArgs(pgxmock.AnyArg()). WillReturnResult(pgxmock.NewResult("SELECT", 1)) mock.ExpectQuery("FROM global_config").WillReturnRows(seeded()) - mock.ExpectExec("INSERT INTO global_config").WithArgs(anyArgsCfg(24)...). + mock.ExpectExec("INSERT INTO global_config").WithArgs(anyArgsCfg(25)...). WillReturnResult(pgxmock.NewResult("INSERT", 1)) mock.ExpectExec("UPDATE service_configs"). WithArgs(false, 1, false, "no-upfront", true, 70.0, false, RampImmediate). @@ -356,7 +364,7 @@ func TestPGXMock_UpdateGlobalConfigAtomic_PropagatesOnlyChangedDefaults(t *testi mock.ExpectExec("pg_advisory_xact_lock").WithArgs(pgxmock.AnyArg()). WillReturnResult(pgxmock.NewResult("SELECT", 1)) mock.ExpectQuery("FROM global_config").WillReturnRows(seeded()) - mock.ExpectExec("INSERT INTO global_config").WithArgs(anyArgsCfg(24)...). + mock.ExpectExec("INSERT INTO global_config").WithArgs(anyArgsCfg(25)...). WillReturnResult(pgxmock.NewResult("INSERT", 1)) dbErr := errors.New("service_configs update failed") mock.ExpectExec("UPDATE service_configs").WithArgs(anyArgsCfg(8)...).WillReturnError(dbErr) @@ -447,11 +455,12 @@ func TestPGXMock_GetPurchasePlan_Success(t *testing.T) { "id", "name", "enabled", "auto_purchase", "notification_days_before", "services", "ramp_schedule", "created_at", "updated_at", "next_execution_date", "last_execution_date", "last_notification_sent", + "max_step_commitment", } rows := pgxmock.NewRows(cols).AddRow( "plan-id", "My Plan", true, false, 3, svcJSON, rampJSON, now, now, - sql.NullTime{Valid: false}, sql.NullTime{Valid: false}, sql.NullTime{Valid: false}, + sql.NullTime{Valid: false}, sql.NullTime{Valid: false}, sql.NullTime{Valid: false}, (*float64)(nil), ) mock.ExpectQuery("SELECT").WithArgs(pgxmock.AnyArg()).WillReturnRows(rows) @@ -474,6 +483,7 @@ func TestPGXMock_GetPurchasePlan_WithAllTimestamps(t *testing.T) { "id", "name", "enabled", "auto_purchase", "notification_days_before", "services", "ramp_schedule", "created_at", "updated_at", "next_execution_date", "last_execution_date", "last_notification_sent", + "max_step_commitment", } rows := pgxmock.NewRows(cols).AddRow( "plan-id", "My Plan", true, false, 3, @@ -481,6 +491,7 @@ func TestPGXMock_GetPurchasePlan_WithAllTimestamps(t *testing.T) { sql.NullTime{Valid: true, Time: now}, sql.NullTime{Valid: true, Time: now}, sql.NullTime{Valid: true, Time: now}, + (*float64)(nil), ) mock.ExpectQuery("SELECT").WithArgs(pgxmock.AnyArg()).WillReturnRows(rows) @@ -506,13 +517,14 @@ func TestPGXMock_ListPurchasePlans_Success(t *testing.T) { "id", "name", "enabled", "auto_purchase", "notification_days_before", "services", "ramp_schedule", "created_at", "updated_at", "next_execution_date", "last_execution_date", "last_notification_sent", + "max_step_commitment", "unassigned", } rows := pgxmock.NewRows(cols). AddRow("p1", "Plan 1", true, false, 3, svcJSON, rampJSON, now, now, - sql.NullTime{}, sql.NullTime{}, sql.NullTime{}, false). + sql.NullTime{}, sql.NullTime{}, sql.NullTime{}, (*float64)(nil), false). AddRow("p2", "Plan 2", false, true, 7, svcJSON, rampJSON, now, now, - sql.NullTime{}, sql.NullTime{}, sql.NullTime{}, false) + sql.NullTime{}, sql.NullTime{}, sql.NullTime{}, (*float64)(nil), false) mock.ExpectQuery("SELECT").WillReturnRows(rows) plans, err := store.ListPurchasePlans(ctx, PurchasePlanFilter{}) @@ -552,15 +564,16 @@ func TestPGXMock_ListPurchasePlans_UnassignedIncluded(t *testing.T) { "id", "name", "enabled", "auto_purchase", "notification_days_before", "services", "ramp_schedule", "created_at", "updated_at", "next_execution_date", "last_execution_date", "last_notification_sent", + "max_step_commitment", "unassigned", } // The query returns two rows: one assigned (unassigned=false) and one // legacy zero-account plan (unassigned=true). rows := pgxmock.NewRows(cols). AddRow("assigned-id", "Assigned Plan", true, false, 3, svcJSON, rampJSON, now, now, - sql.NullTime{}, sql.NullTime{}, sql.NullTime{}, false). + sql.NullTime{}, sql.NullTime{}, sql.NullTime{}, (*float64)(nil), false). AddRow("legacy-id", "Legacy Plan", true, false, 3, svcJSON, rampJSON, now, now, - sql.NullTime{}, sql.NullTime{}, sql.NullTime{}, true) + sql.NullTime{}, sql.NullTime{}, sql.NullTime{}, (*float64)(nil), true) mock.ExpectQuery("SELECT").WithArgs("acc-uuid").WillReturnRows(rows) plans, err := store.ListPurchasePlans(ctx, PurchasePlanFilter{AccountIDs: []string{"acc-uuid"}, IncludeUnassigned: true}) @@ -587,7 +600,7 @@ func TestPGXMock_UpdatePurchasePlan_NotFound(t *testing.T) { plan := &PurchasePlan{ID: "missing-id", Services: map[string]ServiceConfig{}, RampSchedule: RampSchedule{}} mock.ExpectBegin() - mock.ExpectQuery("UPDATE").WithArgs(anyArgsCfg(11)...). + mock.ExpectQuery("UPDATE").WithArgs(anyArgsCfg(12)...). WillReturnRows(pgxmock.NewRows([]string{"updated_at"})) mock.ExpectQuery("SELECT EXISTS").WithArgs("missing-id"). WillReturnRows(pgxmock.NewRows([]string{"exists"}).AddRow(false)) diff --git a/internal/config/store_postgres_plan_update_test.go b/internal/config/store_postgres_plan_update_test.go index 69d51d50..44bf9f09 100644 --- a/internal/config/store_postgres_plan_update_test.go +++ b/internal/config/store_postgres_plan_update_test.go @@ -19,7 +19,7 @@ func TestUpdatePurchasePlanPreservesTokenOnCommitFailure(t *testing.T) { commitErr := errors.New("commit failed") mock.ExpectBegin() mock.ExpectQuery(`UPDATE purchase_plans[\s\S]*WHERE id = \$1 AND updated_at = \$8[\s\S]*RETURNING updated_at`). - WithArgs(anyArgsCfg(11)...). + WithArgs(anyArgsCfg(12)...). WillReturnRows(pgxmock.NewRows([]string{"updated_at"}).AddRow(readVersion.Add(time.Second))) mock.ExpectCommit().WillReturnError(commitErr) mock.ExpectRollback().WillReturnError(pgx.ErrTxClosed) @@ -35,7 +35,7 @@ func TestUpdatePurchasePlanPropagatesConflictProbeFailure(t *testing.T) { readVersion := plan.UpdatedAt probeErr := errors.New("probe failed") mock.ExpectBegin() - mock.ExpectQuery("UPDATE purchase_plans").WithArgs(anyArgsCfg(11)...). + mock.ExpectQuery("UPDATE purchase_plans").WithArgs(anyArgsCfg(12)...). WillReturnRows(pgxmock.NewRows([]string{"updated_at"})) mock.ExpectQuery("SELECT EXISTS").WithArgs(plan.ID).WillReturnError(probeErr) mock.ExpectRollback() diff --git a/internal/config/types.go b/internal/config/types.go index 376ebb44..5b77b62a 100644 --- a/internal/config/types.go +++ b/internal/config/types.go @@ -79,6 +79,12 @@ type GlobalConfig struct { // a typed ErrLadderExecutionDisabled when this is false. LadderExecutionEnabled bool `json:"ladder_execution_enabled" db:"ladder_execution_enabled"` + // AutoPurchaseEnabled is the global kill switch for AutoPurchase plans + // (migration 000112; the scheduling.auto_purchase default). Default false: + // no AutoPurchase plan notifies or executes until an operator opts in. + // Manual purchases (Opportunities, token approval) are unaffected. + AutoPurchaseEnabled bool `json:"auto_purchase_enabled" db:"auto_purchase_enabled"` + // OfferingClass controls the EC2 Reserved Instance offering class used // during purchase. Accepted values: "convertible" (default) and // "standard". Convertible RIs can be exchanged for a different @@ -211,6 +217,10 @@ type PurchasePlan struct { NextExecutionDate *time.Time `json:"next_execution_date,omitempty" dynamodbav:"next_execution_date,omitempty"` LastExecutionDate *time.Time `json:"last_execution_date,omitempty" dynamodbav:"last_execution_date,omitempty"` LastNotificationSent *time.Time `json:"last_notification_sent,omitempty" dynamodbav:"last_notification_sent,omitempty"` + // MaxStepCommitment caps the TOTAL commitment (RecommendationRecord.TotalCommitment) + // one ramp step may auto-purchase. nil means no cap is set, and auto-purchase of + // recommendation-bearing steps is refused (fail closed). Always > 0 when set. + MaxStepCommitment *float64 `json:"max_step_commitment,omitempty" dynamodbav:"max_step_commitment,omitempty"` // Unassigned is true when the plan has zero rows in plan_accounts. // This can happen for legacy plans created before target_accounts was // required (issue #743). Such plans are invisible when an account filter diff --git a/internal/config/validation.go b/internal/config/validation.go index c519bf1e..1fe212ff 100644 --- a/internal/config/validation.go +++ b/internal/config/validation.go @@ -521,6 +521,19 @@ func (c *ServiceConfig) validateConfigCoverage() error { return nil } +// validateNotificationAndCap checks the notification window and requires a set +// cap to be a positive finite amount. A nil cap means no cap, which refuses +// auto-purchase of steps with recommendations. +func (p *PurchasePlan) validateNotificationAndCap() error { + if p.NotificationDaysBefore < 0 || p.NotificationDaysBefore > MaxNotificationDaysBefore { + return fmt.Errorf("notification days must be between 0 and %d, got: %d", MaxNotificationDaysBefore, p.NotificationDaysBefore) + } + if p.MaxStepCommitment != nil && (!(*p.MaxStepCommitment > 0) || math.IsInf(*p.MaxStepCommitment, 0)) { + return fmt.Errorf("max_step_commitment must be a positive finite number, got: %v", *p.MaxStepCommitment) + } + return nil +} + // Validate validates the PurchasePlan. func (p *PurchasePlan) Validate() error { // Name is required @@ -531,9 +544,8 @@ func (p *PurchasePlan) Validate() error { return fmt.Errorf("plan name is too long (max %d characters)", MaxPlanNameLength) } - // Validate notification days - if p.NotificationDaysBefore < 0 || p.NotificationDaysBefore > MaxNotificationDaysBefore { - return fmt.Errorf("notification days must be between 0 and %d, got: %d", MaxNotificationDaysBefore, p.NotificationDaysBefore) + if err := p.validateNotificationAndCap(); err != nil { + return err } // Validate ramp schedule diff --git a/internal/database/postgres/migrations/000112_plan_step_cap_auto_purchase_switch.down.sql b/internal/database/postgres/migrations/000112_plan_step_cap_auto_purchase_switch.down.sql new file mode 100644 index 00000000..fcb3a6b4 --- /dev/null +++ b/internal/database/postgres/migrations/000112_plan_step_cap_auto_purchase_switch.down.sql @@ -0,0 +1,5 @@ +ALTER TABLE global_config + DROP COLUMN IF EXISTS auto_purchase_enabled; + +ALTER TABLE purchase_plans + DROP COLUMN IF EXISTS max_step_commitment; diff --git a/internal/database/postgres/migrations/000112_plan_step_cap_auto_purchase_switch.up.sql b/internal/database/postgres/migrations/000112_plan_step_cap_auto_purchase_switch.up.sql new file mode 100644 index 00000000..2779b6d7 --- /dev/null +++ b/internal/database/postgres/migrations/000112_plan_step_cap_auto_purchase_switch.up.sql @@ -0,0 +1,18 @@ +-- Migration 000112: safety layer for plan-step auto-purchase (platform#631 PR2c). +-- +-- purchase_plans.max_step_commitment: per-plan ceiling on the TOTAL commitment +-- (upfront + monthly * term * 12) one ramp step may buy. Nullable and additive: +-- NULL means "refuse auto-purchase for recommendation-bearing steps" (fail +-- closed), so every existing plan stays unable to buy. +-- +-- global_config.auto_purchase_enabled: the global kill switch behind the +-- previously unread scheduling.auto_purchase default. Default FALSE (fail +-- closed): AutoPurchase plans never execute until an operator opts in. +-- +-- Both statements are idempotent (ADD COLUMN IF NOT EXISTS) and metadata-only. +ALTER TABLE purchase_plans + ADD COLUMN IF NOT EXISTS max_step_commitment NUMERIC(18,2) + CHECK (max_step_commitment IS NULL OR max_step_commitment > 0); + +ALTER TABLE global_config + ADD COLUMN IF NOT EXISTS auto_purchase_enabled BOOLEAN NOT NULL DEFAULT FALSE; diff --git a/internal/database/postgres/migrations/000112_plan_step_cap_auto_purchase_switch_test.go b/internal/database/postgres/migrations/000112_plan_step_cap_auto_purchase_switch_test.go new file mode 100644 index 00000000..c02277fc --- /dev/null +++ b/internal/database/postgres/migrations/000112_plan_step_cap_auto_purchase_switch_test.go @@ -0,0 +1,64 @@ +//go:build integration +// +build integration + +package migrations_test + +import ( + "context" + "testing" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/migrations" + "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestMigration_PlanStepCapAndAutoPurchaseSwitch pins migration 000112 against +// a real PostgreSQL: the new columns are additive, a pre-existing plan comes +// out with a NULL cap (fail closed: it cannot auto-purchase), the global kill +// switch defaults to FALSE, the CHECK rejects a non-positive cap, and the down +// migration removes both columns. +func TestMigration_PlanStepCapAndAutoPurchaseSwitch(t *testing.T) { + ctx := context.Background() + migrationsPath := getMigrationsPath() + + container, err := testhelpers.SetupPostgresContainer(ctx, t) + require.NoError(t, err) + defer container.Cleanup(ctx) + pool := container.DB.Pool() + + require.NoError(t, migrations.MigrateToVersion(ctx, pool, migrationsPath, previousMigrationVersion(t, 112))) + + var planID string + require.NoError(t, pool.QueryRow(ctx, ` + INSERT INTO purchase_plans (name, enabled, auto_purchase, notification_days_before, services, ramp_schedule) + VALUES ('pre-existing', true, true, 3, '{}'::jsonb, '{}'::jsonb) + RETURNING id`).Scan(&planID)) + _, err = pool.Exec(ctx, `INSERT INTO global_config (id) VALUES (1) ON CONFLICT (id) DO NOTHING`) + require.NoError(t, err) + + require.NoError(t, migrations.MigrateToVersion(ctx, pool, migrationsPath, 112)) + + var cap *float64 + require.NoError(t, pool.QueryRow(ctx, `SELECT max_step_commitment FROM purchase_plans WHERE id = $1`, planID).Scan(&cap)) + assert.Nil(t, cap, "an existing plan must get a NULL cap so it stays unable to auto-purchase") + + var enabled bool + require.NoError(t, pool.QueryRow(ctx, `SELECT auto_purchase_enabled FROM global_config WHERE id = 1`).Scan(&enabled)) + assert.False(t, enabled, "the kill switch must default to off") + + for _, bad := range []string{"0", "-5"} { + _, err = pool.Exec(ctx, `UPDATE purchase_plans SET max_step_commitment = `+bad+` WHERE id = $1`, planID) + require.Error(t, err, "a non-positive cap %s must be rejected by the CHECK", bad) + } + _, err = pool.Exec(ctx, `UPDATE purchase_plans SET max_step_commitment = 1234.56 WHERE id = $1`, planID) + require.NoError(t, err) + + require.NoError(t, migrations.MigrateToVersion(ctx, pool, migrationsPath, 111)) + var cols int + require.NoError(t, pool.QueryRow(ctx, ` + SELECT count(*) FROM information_schema.columns + WHERE (table_name = 'purchase_plans' AND column_name = 'max_step_commitment') + OR (table_name = 'global_config' AND column_name = 'auto_purchase_enabled')`).Scan(&cols)) + assert.Equal(t, 0, cols, "down migration must drop both columns") +} diff --git a/internal/purchase/coverage_extra_test.go b/internal/purchase/coverage_extra_test.go index a6710ad3..5e9e33b7 100644 --- a/internal/purchase/coverage_extra_test.go +++ b/internal/purchase/coverage_extra_test.go @@ -303,9 +303,10 @@ func TestHandleExecutePurchase_SaveError(t *testing.T) { mockSTS := new(MockSTSClient) plan := &config.PurchasePlan{ - ID: "plan-save-err", - Name: "Plan", - AutoPurchase: true, + ID: "plan-save-err", + Name: "Plan", + AutoPurchase: true, + MaxStepCommitment: capOf(1e9), } rec := config.RecommendationRecord{ @@ -314,16 +315,18 @@ func TestHandleExecutePurchase_SaveError(t *testing.T) { ResourceType: "m5.large", Region: "us-east-1", Count: 1, + UpfrontCost: 100, Selected: true, } exec := &config.PurchaseExecution{ ExecutionID: "exec-save-err", PlanID: "plan-save-err", - Status: "pending", + Status: "notified", Recommendations: []config.RecommendationRecord{rec}, } + mockStore.On("GetGlobalConfig", ctx).Return(autoPurchaseOnCfg(), nil) mockStore.On("GetExecutionByID", ctx, "exec-save-err").Return(exec, nil) // claimAndExecute CASes the row to "running" before executing (issue #1013). runningExec := *exec diff --git a/internal/purchase/manager.go b/internal/purchase/manager.go index 5fa97f43..e05719e5 100644 --- a/internal/purchase/manager.go +++ b/internal/purchase/manager.go @@ -702,6 +702,14 @@ func (m *Manager) RecoverStrandedApprovals(ctx context.Context) (int, error) { // - All other pending/notified rows require the owning plan to have // AutoPurchase=true. A plan-fetch error is propagated so the caller can // fail closed rather than defaulting to "execute". +// - The global kill switch (GlobalConfig.AutoPurchaseEnabled) must be on. A +// read error is propagated. Only AutoPurchase plans reach this check, so +// manual purchases are unaffected. +// - A recommendation-bearing row must be "notified" (the notification email is +// the human veto window; a "pending" row may never have been emailed) and +// must fit the plan's max_step_commitment (CheckStepCommitmentCap, whose +// refusal is returned as an error). Rows without recommendations are left +// to the no-recommendations refusal in executePurchase. // // "approved" rows are handled by the session/token approval paths and // RecoverStrandedApprovals; this helper is only called for pending/notified. @@ -717,7 +725,39 @@ func (m *Manager) executableByScheduler(ctx context.Context, exec *config.Purcha if err != nil { return false, fmt.Errorf("failed to fetch plan %s for AutoPurchase gate: %w", exec.PlanID, err) } - return plan.AutoPurchase, nil + if !plan.AutoPurchase { + return false, nil + } + enabled, err := m.autoPurchaseEnabled(ctx) + if err != nil { + return false, err + } + if !enabled { + return false, nil + } + if len(exec.Recommendations) == 0 { + return true, nil + } + if exec.Status != "notified" { + return false, nil + } + if err := CheckStepCommitmentCap(plan, exec.Recommendations); err != nil { + return false, err + } + return true, nil +} + +// autoPurchaseEnabled reads the global auto-purchase kill switch. It fails +// closed: a read error is returned, never read as "enabled". +func (m *Manager) autoPurchaseEnabled(ctx context.Context) (bool, error) { + cfg, err := m.config.GetGlobalConfig(ctx) + if err != nil { + return false, fmt.Errorf("failed to read global config for the auto-purchase kill switch: %w", err) + } + if cfg == nil { + return false, errors.New("global config unavailable for the auto-purchase kill switch") + } + return cfg.AutoPurchaseEnabled, nil } // processOneExecution runs the full gate+claim+execute pipeline for a single @@ -738,7 +778,7 @@ func (m *Manager) processOneExecution(ctx context.Context, exec config.PurchaseE return } if !eligible { - logging.Infof("Skipping execution %s (AutoPurchase=false or source=web; requires explicit approval)", exec.ExecutionID) + logging.Infof("Skipping execution %s (AutoPurchase=false, kill switch off, source=web, or recommendation-bearing row not yet notified; requires explicit approval)", exec.ExecutionID) return } diff --git a/internal/purchase/manager_test.go b/internal/purchase/manager_test.go index 1920bdb6..0e42fa3e 100644 --- a/internal/purchase/manager_test.go +++ b/internal/purchase/manager_test.go @@ -180,7 +180,7 @@ func TestManager_ProcessScheduledPurchases_DuePurchase(t *testing.T) { { ExecutionID: "exec-123", PlanID: "plan-456", - Status: "pending", + Status: "notified", StepNumber: 1, ScheduledDate: pastDate, Recommendations: []config.RecommendationRecord{ @@ -198,9 +198,10 @@ func TestManager_ProcessScheduledPurchases_DuePurchase(t *testing.T) { } plan := &config.PurchasePlan{ - ID: "plan-456", - Name: "Test Plan", - AutoPurchase: true, // required by executableByScheduler gate (F1 fix) + ID: "plan-456", + Name: "Test Plan", + AutoPurchase: true, // required by executableByScheduler gate (F1 fix) + MaxStepCommitment: capOf(1e9), RampSchedule: config.RampSchedule{ CurrentStep: 0, TotalSteps: 4, @@ -212,6 +213,7 @@ func TestManager_ProcessScheduledPurchases_DuePurchase(t *testing.T) { claimedExec.Status = "running" mockStore.On("GetStaleApprovedExecutions", ctx, mock.Anything).Return([]config.PurchaseExecution{}, nil) mockStore.On("GetPendingExecutions", ctx).Return(executions, nil) + mockStore.On("GetGlobalConfig", ctx).Return(autoPurchaseOnCfg(), nil) // executableByScheduler calls GetPurchasePlan first (AutoPurchase gate), then // executePurchase calls it again to build purchase options — two calls total. mockStore.On("GetPurchasePlan", ctx, "plan-456").Return(plan, nil) @@ -314,6 +316,7 @@ func TestManager_ProcessScheduledPurchases_ExecutionFails(t *testing.T) { claimedExec.Status = "running" mockStore.On("GetStaleApprovedExecutions", ctx, mock.Anything).Return([]config.PurchaseExecution{}, nil) mockStore.On("GetPendingExecutions", ctx).Return(executions, nil) + mockStore.On("GetGlobalConfig", ctx).Return(autoPurchaseOnCfg(), nil) // First call: executableByScheduler gate check (AutoPurchase=true -> proceed). // Second call: executePurchase fetches the plan config and returns error to // simulate a purchase failure — this is what drives the row to "failed". diff --git a/internal/purchase/messages.go b/internal/purchase/messages.go index bf3a3bf3..14d7585a 100644 --- a/internal/purchase/messages.go +++ b/internal/purchase/messages.go @@ -95,7 +95,7 @@ func (m *Manager) checkAutoExecuteGate(ctx context.Context, execution *config.Pu return fmt.Errorf("AutoPurchase gate check failed for execution %s: %w", execution.ExecutionID, err) } if !eligible { - return fmt.Errorf("execution %s (status=%s source=%q) is not eligible for auto-execution: AutoPurchase=false or source=web — approve via the approval link", + return fmt.Errorf("execution %s (status=%s source=%q) is not eligible for auto-execution: AutoPurchase=false, kill switch off, source=web, or recommendation-bearing row not yet notified - approve via the approval link", execution.ExecutionID, execution.Status, execution.Source) } return nil diff --git a/internal/purchase/money_path_regression_test.go b/internal/purchase/money_path_regression_test.go index 9f0a46a2..f58f35a1 100644 --- a/internal/purchase/money_path_regression_test.go +++ b/internal/purchase/money_path_regression_test.go @@ -304,7 +304,7 @@ func TestSQSRedeliveryDoesNotDoubleExecute(t *testing.T) { return &config.PurchaseExecution{ ExecutionID: "exec-dup", IdempotencyKey: "lineage-dup", - Status: "pending", + Status: "notified", Recommendations: []config.RecommendationRecord{ {Provider: "aws", Service: "ec2", ResourceType: "m5.large", Region: "us-east-1", Count: 1, UpfrontCost: 300, Selected: true}, }, @@ -315,6 +315,7 @@ func TestSQSRedeliveryDoesNotDoubleExecute(t *testing.T) { // pre-fix nothing CASes it to running before the cloud call, so a redelivery // re-reads a still-claimable row). Each GetExecutionByID returns a FRESH // pending copy, faithfully modeling the real double-delivery scenario. + mockStore.On("GetGlobalConfig", ctx).Return(autoPurchaseOnCfg(), nil) mockStore.On("GetExecutionByID", ctx, "exec-dup").Return(newPending(), nil).Once() mockStore.On("GetExecutionByID", ctx, "exec-dup").Return(newPending(), nil).Once() @@ -333,7 +334,7 @@ func TestSQSRedeliveryDoesNotDoubleExecute(t *testing.T) { mockStore.SavePurchaseExecutionFn = func(_ context.Context, _ *config.PurchaseExecution) error { return nil } mockStore.On("SavePurchaseHistory", ctx, mock.AnythingOfType("*config.PurchaseHistoryRecord")).Return(nil) mockEmail.On("SendPurchaseConfirmation", ctx, mock.AnythingOfType("email.NotificationData")).Return(nil) - mockStore.On("GetPurchasePlan", ctx, mock.Anything).Return(&config.PurchasePlan{Name: "p", AutoPurchase: true}, nil).Maybe() + mockStore.On("GetPurchasePlan", ctx, mock.Anything).Return(&config.PurchasePlan{Name: "p", AutoPurchase: true, MaxStepCommitment: capOf(1e9)}, nil).Maybe() mockFactory.On("CreateAndValidateProvider", mock.Anything, "aws", mock.Anything).Return(mockProviderInst, nil) mockProviderInst.On("GetServiceClient", mock.Anything, common.ServiceEC2, mock.Anything).Return(mockServiceClient, nil) @@ -383,15 +384,16 @@ func TestMultiAccountPartialSuccessIsAcked(t *testing.T) { exec := &config.PurchaseExecution{ ExecutionID: "root-partial", IdempotencyKey: "lineage-partial", - Status: "pending", + Status: "notified", PlanID: "plan-x", Recommendations: []config.RecommendationRecord{ scopedTestRec("acct-ok"), scopedTestRec("acct-bad"), }, } - plan := &config.PurchasePlan{ID: "plan-x", Name: "Plan X", AutoPurchase: true} + plan := &config.PurchasePlan{ID: "plan-x", Name: "Plan X", AutoPurchase: true, MaxStepCommitment: capOf(1e9)} + mockStore.On("GetGlobalConfig", ctx).Return(autoPurchaseOnCfg(), nil) mockStore.On("GetExecutionByID", ctx, "root-partial").Return(exec, nil) running := *exec running.Status = "running" @@ -483,15 +485,16 @@ func TestExecuteForAccount_CommittedButUnsavedIsAcked(t *testing.T) { exec := &config.PurchaseExecution{ ExecutionID: "root-unsaved", IdempotencyKey: "lineage-unsaved", - Status: "pending", + Status: "notified", PlanID: "plan-y", Recommendations: []config.RecommendationRecord{ scopedTestRec("acct-bad"), scopedTestRec("acct-unsaved"), }, } - plan := &config.PurchasePlan{ID: "plan-y", Name: "Plan Y", AutoPurchase: true} + plan := &config.PurchasePlan{ID: "plan-y", Name: "Plan Y", AutoPurchase: true, MaxStepCommitment: capOf(1e9)} + mockStore.On("GetGlobalConfig", ctx).Return(autoPurchaseOnCfg(), nil) mockStore.On("GetExecutionByID", ctx, "root-unsaved").Return(exec, nil) running := *exec running.Status = "running" diff --git a/internal/purchase/notifications.go b/internal/purchase/notifications.go index 1b4507d6..764a36d8 100644 --- a/internal/purchase/notifications.go +++ b/internal/purchase/notifications.go @@ -23,10 +23,17 @@ func (m *Manager) SendUpcomingPurchaseNotifications(ctx context.Context) (*Notif return nil, fmt.Errorf("failed to list purchase plans: %w", err) } + // Kill switch, read once per tick. A read error fails the tick rather than + // defaulting to enabled (fail closed on the money path). + autoPurchaseEnabled, err := m.autoPurchaseEnabled(ctx) + if err != nil { + return nil, err + } + notified := 0 for _rvc := range plans { plan := plans[_rvc] - if m.shouldNotifyPlan(plan) { + if m.shouldNotifyPlan(plan, autoPurchaseEnabled) { if m.sendPlanNotification(ctx, &plan) { notified++ } @@ -39,8 +46,9 @@ func (m *Manager) SendUpcomingPurchaseNotifications(ctx context.Context) (*Notif } // shouldNotifyPlan checks if a plan should trigger a notification. -func (m *Manager) shouldNotifyPlan(plan config.PurchasePlan) bool { - if !plan.Enabled || !plan.AutoPurchase { +// autoPurchaseEnabled is the global scheduling.auto_purchase kill switch. +func (m *Manager) shouldNotifyPlan(plan config.PurchasePlan, autoPurchaseEnabled bool) bool { + if !autoPurchaseEnabled || !plan.Enabled || !plan.AutoPurchase { return false } diff --git a/internal/purchase/notifications_test.go b/internal/purchase/notifications_test.go index 27037320..4c01c5eb 100644 --- a/internal/purchase/notifications_test.go +++ b/internal/purchase/notifications_test.go @@ -497,7 +497,7 @@ func TestManager_SendUpcomingPurchaseNotifications_WithNotification(t *testing.T } notifyEmailStr := "notify@example.com" - globalCfg := &config.GlobalConfig{NotificationEmail: ¬ifyEmailStr} + globalCfg := &config.GlobalConfig{NotificationEmail: ¬ifyEmailStr, AutoPurchaseEnabled: true} mockStore.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{}).Return(plans, nil) // An existing pending row that carries recommendations is the only kind // the reminder still emails (#609); it is rotated, not created. @@ -653,7 +653,7 @@ func TestManager_SendUpcomingPurchaseNotifications_EmailFails(t *testing.T) { } notifyEmailStr := "notify@example.com" - globalCfg := &config.GlobalConfig{NotificationEmail: ¬ifyEmailStr} + globalCfg := &config.GlobalConfig{NotificationEmail: ¬ifyEmailStr, AutoPurchaseEnabled: true} mockStore.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{}).Return(plans, nil) pendingExec := &config.PurchaseExecution{ ExecutionID: "pending-exec-id", diff --git a/internal/purchase/plan_step_no_recs_test.go b/internal/purchase/plan_step_no_recs_test.go index 856c96b4..d2978196 100644 --- a/internal/purchase/plan_step_no_recs_test.go +++ b/internal/purchase/plan_step_no_recs_test.go @@ -2,6 +2,7 @@ package purchase import ( "context" + "fmt" "sync" "testing" "time" @@ -64,6 +65,16 @@ func (r *saveRecorder) save(_ context.Context, exec *config.PurchaseExecution) e } func TestScheduledBarePlanStepFailsOnceAndDoesNotAdvanceRamp(t *testing.T) { + // 0 is the immediate-run setting: nothing is notified ahead of time, so + // the rec-less row reaches the sweep as "pending" with no notice sent. + for _, days := range []int{0, 7} { + t.Run(fmt.Sprintf("NotificationDaysBefore=%d", days), func(t *testing.T) { + scheduledBarePlanStepFails(t, days) + }) + } +} + +func scheduledBarePlanStepFails(t *testing.T, notificationDaysBefore int) { ctx := context.Background() mockStore := new(MockConfigStore) mockEmail := new(MockEmailSender) @@ -73,12 +84,14 @@ func TestScheduledBarePlanStepFailsOnceAndDoesNotAdvanceRamp(t *testing.T) { }) plan := bareStepPlan() + plan.NotificationDaysBefore = notificationDaysBefore exec := bareStepExecution() claimed := exec claimed.Status = "running" mockStore.On("GetStaleApprovedExecutions", ctx, mock.Anything).Return([]config.PurchaseExecution{}, nil) mockStore.On("GetPendingExecutions", ctx).Return([]config.PurchaseExecution{exec}, nil) + mockStore.On("GetGlobalConfig", ctx).Return(autoPurchaseOnCfg(), nil) mockStore.On("GetPurchasePlan", ctx, bareStepPlanID).Return(plan, nil) mockStore.On("TransitionExecutionStatus", ctx, bareStepExecID, []string{"approved", "pending", "notified"}, "running", (*string)(nil)).Return(&claimed, nil) @@ -124,7 +137,7 @@ func TestNotificationTickOnBarePlanStepCreatesOneFailedRowAndSendsNoEmail(t *tes notifyTo := "notify@example.com" mockStore.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{}).Return([]config.PurchasePlan{plan}, nil) - mockStore.On("GetGlobalConfig", ctx).Return(&config.GlobalConfig{NotificationEmail: ¬ifyTo}, nil) + mockStore.On("GetGlobalConfig", ctx).Return(&config.GlobalConfig{NotificationEmail: ¬ifyTo, AutoPurchaseEnabled: true}, nil) // Tick 1 finds no row for the date; tick 2 finds whatever tick 1 saved. var stored config.PurchaseExecution diff --git a/internal/purchase/ramp_step_progress_integration_test.go b/internal/purchase/ramp_step_progress_integration_test.go index 3a0882a7..6eec5f57 100644 --- a/internal/purchase/ramp_step_progress_integration_test.go +++ b/internal/purchase/ramp_step_progress_integration_test.go @@ -86,9 +86,10 @@ func newRampStepFixture(ctx context.Context, t *testing.T) *rampStepFixture { store, _ := rampStepStore(ctx, t) plan := &config.PurchasePlan{ - Name: "Ramp Step Plan", - AutoPurchase: true, - Services: map[string]config.ServiceConfig{}, + Name: "Ramp Step Plan", + AutoPurchase: true, + MaxStepCommitment: capOf(1e9), + Services: map[string]config.ServiceConfig{}, RampSchedule: config.RampSchedule{ Type: "weekly", PercentPerStep: 25, @@ -100,6 +101,12 @@ func newRampStepFixture(ctx context.Context, t *testing.T) *rampStepFixture { } require.NoError(t, store.CreatePurchasePlan(ctx, plan)) + // The auto-execute gate needs the global kill switch on. + globalCfg, err := store.GetGlobalConfig(ctx) + require.NoError(t, err) + globalCfg.AutoPurchaseEnabled = true + require.NoError(t, store.SaveGlobalConfig(ctx, globalCfg)) + authModes := []string{"access_keys", "role_arn", "role_arn"} accountIDs := make([]string, 0, len(authModes)) for i, mode := range authModes { @@ -220,7 +227,7 @@ func (f *rampStepFixture) saveRootExecution(ctx context.Context, t *testing.T, s ExecutionID: uuid.New().String(), IdempotencyKey: fmt.Sprintf("lineage-step-%d", stepNumber), PlanID: f.plan.ID, - Status: "pending", + Status: "notified", StepNumber: stepNumber, ScheduledDate: time.Now(), Recommendations: f.rampStepRecommendationsPerAccount(), diff --git a/internal/purchase/step_cap.go b/internal/purchase/step_cap.go new file mode 100644 index 00000000..3f0124e9 --- /dev/null +++ b/internal/purchase/step_cap.go @@ -0,0 +1,45 @@ +package purchase + +import ( + "errors" + "fmt" + "math" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" +) + +// ErrStepCommitmentCap marks a plan step refused by the per-plan spend cap. +var ErrStepCommitmentCap = errors.New("plan step refused by commitment cap") + +// CheckStepCommitmentCap refuses a plan step's recommendations unless the plan +// carries a cap and the step's TOTAL commitment (upfront + monthly * term * 12, +// see RecommendationRecord.TotalCommitment) fits under it. It fails closed: +// - a nil plan, a plan without MaxStepCommitment (nil or <= 0), or a +// non-finite cap refuses everything; +// - a recommendation with a zero, negative or non-finite total commitment is +// refused (Azure monthly commitments have upfront 0, so upfront alone +// would pass anything); +// - no recommendations is not a step that can buy, so it is not this +// check's concern and returns nil (the no-recommendations refusal owns it). +// +// It only ever refuses; nothing here buys. +func CheckStepCommitmentCap(plan *config.PurchasePlan, recs []config.RecommendationRecord) error { + if len(recs) == 0 { + return nil + } + if plan == nil || plan.MaxStepCommitment == nil || !(*plan.MaxStepCommitment > 0) || math.IsInf(*plan.MaxStepCommitment, 0) { + return fmt.Errorf("%w: plan has no max_step_commitment set (auto-purchase of steps with recommendations is refused until one is set)", ErrStepCommitmentCap) + } + var total float64 + for i := range recs { + c := recs[i].TotalCommitment() + if !(c > 0) || math.IsInf(c, 0) { + return fmt.Errorf("%w: recommendation %q has non-positive or non-finite total commitment %v", ErrStepCommitmentCap, recs[i].ID, c) + } + total += c + } + if total > *plan.MaxStepCommitment { + return fmt.Errorf("%w: step total commitment %.2f exceeds max_step_commitment %.2f", ErrStepCommitmentCap, total, *plan.MaxStepCommitment) + } + return nil +} diff --git a/internal/purchase/step_cap_helpers_test.go b/internal/purchase/step_cap_helpers_test.go new file mode 100644 index 00000000..84b83700 --- /dev/null +++ b/internal/purchase/step_cap_helpers_test.go @@ -0,0 +1,11 @@ +package purchase + +import "github.com/LeanerCloud/cloud-commitments-platform/internal/config" + +// capOf returns a pointer to a plan cap value (fixture helper). +func capOf(v float64) *float64 { return &v } + +// autoPurchaseOnCfg is a fixture global config with the kill switch on. +func autoPurchaseOnCfg() *config.GlobalConfig { + return &config.GlobalConfig{AutoPurchaseEnabled: true} +} diff --git a/internal/purchase/step_cap_test.go b/internal/purchase/step_cap_test.go new file mode 100644 index 00000000..0f8e58a5 --- /dev/null +++ b/internal/purchase/step_cap_test.go @@ -0,0 +1,232 @@ +package purchase + +import ( + "context" + "errors" + "math" + "testing" + "time" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +// All fixtures below are mocks (MockConfigStore) and hand-built records, not +// real cloud data. + +func f64(v float64) *float64 { return &v } + +func upfrontRec(upfront float64) config.RecommendationRecord { + return config.RecommendationRecord{ID: "r-upfront", Provider: "aws", Service: "ec2", Count: 1, Term: 1, UpfrontCost: upfront} +} + +// azureMonthlyRec has upfront 0 and only a monthly charge, the shape that makes +// an upfront-only cap blind. +func azureMonthlyRec(monthly float64, termYears int) config.RecommendationRecord { + return config.RecommendationRecord{ID: "r-monthly", Provider: "azure", Service: "compute", Count: 1, Term: termYears, UpfrontCost: 0, MonthlyCost: f64(monthly)} +} + +func TestCheckStepCommitmentCap(t *testing.T) { + cases := []struct { + name string + plan *config.PurchasePlan + recs []config.RecommendationRecord + wantErr bool + }{ + {"NULL cap refuses", &config.PurchasePlan{}, []config.RecommendationRecord{upfrontRec(10)}, true}, + {"nil plan refuses", nil, []config.RecommendationRecord{upfrontRec(10)}, true}, + {"zero cap refuses", &config.PurchasePlan{MaxStepCommitment: f64(0)}, []config.RecommendationRecord{upfrontRec(10)}, true}, + {"negative cap refuses", &config.PurchasePlan{MaxStepCommitment: f64(-1)}, []config.RecommendationRecord{upfrontRec(10)}, true}, + {"NaN cap refuses", &config.PurchasePlan{MaxStepCommitment: f64(math.NaN())}, []config.RecommendationRecord{upfrontRec(10)}, true}, + {"Inf cap refuses", &config.PurchasePlan{MaxStepCommitment: f64(math.Inf(1))}, []config.RecommendationRecord{upfrontRec(10)}, true}, + {"over cap refuses", &config.PurchasePlan{MaxStepCommitment: f64(99)}, []config.RecommendationRecord{upfrontRec(100)}, true}, + {"sum of recs over cap refuses", &config.PurchasePlan{MaxStepCommitment: f64(100)}, []config.RecommendationRecord{upfrontRec(60), upfrontRec(60)}, true}, + {"zero commitment refuses", &config.PurchasePlan{MaxStepCommitment: f64(1000)}, []config.RecommendationRecord{upfrontRec(0)}, true}, + {"zero rec among good refuses", &config.PurchasePlan{MaxStepCommitment: f64(1000)}, []config.RecommendationRecord{upfrontRec(10), upfrontRec(0)}, true}, + {"negative commitment refuses", &config.PurchasePlan{MaxStepCommitment: f64(1000)}, []config.RecommendationRecord{upfrontRec(-5)}, true}, + {"NaN commitment refuses", &config.PurchasePlan{MaxStepCommitment: f64(1000)}, []config.RecommendationRecord{upfrontRec(math.NaN())}, true}, + {"azure monthly counted on total not upfront", &config.PurchasePlan{MaxStepCommitment: f64(1000)}, []config.RecommendationRecord{azureMonthlyRec(100, 1)}, true}, // 100*12=1200 > 1000 + {"azure monthly under cap passes", &config.PurchasePlan{MaxStepCommitment: f64(1200)}, []config.RecommendationRecord{azureMonthlyRec(100, 1)}, false}, + {"exactly at cap passes", &config.PurchasePlan{MaxStepCommitment: f64(100)}, []config.RecommendationRecord{upfrontRec(100)}, false}, + {"no recs is not this check's concern", &config.PurchasePlan{}, nil, false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + err := CheckStepCommitmentCap(tc.plan, tc.recs) + if tc.wantErr { + require.Error(t, err) + assert.ErrorIs(t, err, ErrStepCommitmentCap) + return + } + assert.NoError(t, err) + }) + } +} + +// gateHarness drives one due plan row through ProcessScheduledPurchases with a +// mock store. TransitionExecutionStatus is deliberately NOT mocked: a claim +// attempt (i.e. anything that could go on to buy) panics the mock and fails +// the test. +func gateHarness(t *testing.T, plan *config.PurchasePlan, exec config.PurchaseExecution, cfg *config.GlobalConfig, cfgErr error) *ProcessResult { + t.Helper() + ctx := context.Background() + store := new(MockConfigStore) + store.On("GetStaleApprovedExecutions", ctx, mock.Anything).Return([]config.PurchaseExecution{}, nil) + store.On("GetPendingExecutions", ctx).Return([]config.PurchaseExecution{exec}, nil) + store.On("GetPurchasePlan", ctx, exec.PlanID).Return(plan, nil) + store.On("GetGlobalConfig", ctx).Return(cfg, cfgErr) + manager := &Manager{config: store, email: new(MockEmailSender)} + result, err := manager.ProcessScheduledPurchases(ctx) + require.NoError(t, err) + store.AssertNotCalled(t, "TransitionExecutionStatus", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything) + return result +} + +func dueRecExec(status string, recs ...config.RecommendationRecord) config.PurchaseExecution { + return config.PurchaseExecution{ + ExecutionID: "exec-gate", PlanID: "plan-gate", Status: status, StepNumber: 1, + ScheduledDate: time.Now().Add(-time.Hour), Recommendations: recs, + } +} + +func TestScheduler_RefusesBeforeClaim(t *testing.T) { + capped := func(c *float64) *config.PurchasePlan { + return &config.PurchasePlan{ID: "plan-gate", AutoPurchase: true, MaxStepCommitment: c} + } + cases := []struct { + name string + plan *config.PurchasePlan + exec config.PurchaseExecution + cfg *config.GlobalConfig + cfgErr error + wantFailed int // 0 = silently skipped, 1 = surfaced as a failure + }{ + {"kill switch off", capped(f64(1000)), dueRecExec("notified", upfrontRec(10)), &config.GlobalConfig{}, nil, 0}, + {"kill switch read error", capped(f64(1000)), dueRecExec("notified", upfrontRec(10)), nil, errors.New("db down"), 1}, + {"read error even with a populated config", capped(f64(1000)), dueRecExec("notified", upfrontRec(10)), autoPurchaseOnCfg(), errors.New("db down"), 1}, + {"nil global config", capped(f64(1000)), dueRecExec("notified", upfrontRec(10)), nil, nil, 1}, + {"pending rec-bearing row (no notice sent)", capped(f64(1000)), dueRecExec("pending", upfrontRec(10)), autoPurchaseOnCfg(), nil, 0}, + {"NULL cap", capped(nil), dueRecExec("notified", upfrontRec(10)), autoPurchaseOnCfg(), nil, 1}, + {"over cap", capped(f64(5)), dueRecExec("notified", upfrontRec(10)), autoPurchaseOnCfg(), nil, 1}, + {"zero commitment", capped(f64(1000)), dueRecExec("notified", upfrontRec(0)), autoPurchaseOnCfg(), nil, 1}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + result := gateHarness(t, tc.plan, tc.exec, tc.cfg, tc.cfgErr) + assert.Equal(t, 0, result.Processed) + assert.Equal(t, 0, result.Executed) + assert.Equal(t, tc.wantFailed, result.Failed) + }) + } +} + +// The SQS execute_purchase path enforces the same refusals before the claim. +func TestSQSExecutePath_RefusesBeforeClaim(t *testing.T) { + capped := func(c *float64) *config.PurchasePlan { + return &config.PurchasePlan{ID: "plan-gate", AutoPurchase: true, MaxStepCommitment: c} + } + cases := []struct { + name string + plan *config.PurchasePlan + exec config.PurchaseExecution + cfg *config.GlobalConfig + cfgErr error + }{ + {"kill switch off", capped(f64(1000)), dueRecExec("notified", upfrontRec(10)), &config.GlobalConfig{}, nil}, + {"kill switch read error", capped(f64(1000)), dueRecExec("notified", upfrontRec(10)), nil, errors.New("db down")}, + {"pending rec-bearing row", capped(f64(1000)), dueRecExec("pending", upfrontRec(10)), autoPurchaseOnCfg(), nil}, + {"NULL cap", capped(nil), dueRecExec("notified", upfrontRec(10)), autoPurchaseOnCfg(), nil}, + {"over cap", capped(f64(5)), dueRecExec("notified", upfrontRec(10)), autoPurchaseOnCfg(), nil}, + {"zero commitment", capped(f64(1000)), dueRecExec("notified", upfrontRec(0)), autoPurchaseOnCfg(), nil}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + ctx := context.Background() + store := new(MockConfigStore) + exec := tc.exec + store.On("GetExecutionByID", ctx, exec.ExecutionID).Return(&exec, nil) + store.On("GetPurchasePlan", ctx, exec.PlanID).Return(tc.plan, nil) + store.On("GetGlobalConfig", ctx).Return(tc.cfg, tc.cfgErr) + manager := &Manager{config: store, email: new(MockEmailSender)} + + err := manager.handleExecutePurchase(ctx, &AsyncMessage{Type: MessageTypeExecutePurchase, ExecutionID: exec.ExecutionID}) + + require.Error(t, err, "the message must be dead-lettered, not acked as done") + store.AssertNotCalled(t, "TransitionExecutionStatus", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything) + }) + } +} + +// Manual purchases are not touched by the kill switch or the cap: an approved +// row (human approval path) bypasses the auto-execute gate entirely, and the +// gate never reads the global config for it. +func TestAutoExecuteGate_ApprovedRowIgnoresKillSwitchAndCap(t *testing.T) { + ctx := context.Background() + store := new(MockConfigStore) // no GetGlobalConfig / GetPurchasePlan expectation: any read panics + manager := &Manager{config: store, email: new(MockEmailSender)} + exec := dueRecExec("approved", upfrontRec(1e9)) + require.NoError(t, manager.checkAutoExecuteGate(ctx, &exec)) + store.AssertNotCalled(t, "GetGlobalConfig", mock.Anything) + store.AssertNotCalled(t, "GetPurchasePlan", mock.Anything, mock.Anything) +} + +// A web-submitted direct purchase (no plan) never reaches the plan gate. +func TestExecutableByScheduler_WebRowNeverReadsKillSwitch(t *testing.T) { + store := new(MockConfigStore) + manager := &Manager{config: store} + exec := config.PurchaseExecution{ExecutionID: "w", Status: "pending", Source: "cudly-web", Recommendations: []config.RecommendationRecord{upfrontRec(1)}} + ok, err := manager.executableByScheduler(context.Background(), &exec) + require.NoError(t, err) + assert.False(t, ok) + store.AssertNotCalled(t, "GetGlobalConfig", mock.Anything) +} + +// An AutoPurchase=false plan is skipped without consulting the kill switch. +func TestExecutableByScheduler_NonAutoPlanNeverReadsKillSwitch(t *testing.T) { + ctx := context.Background() + store := new(MockConfigStore) + store.On("GetPurchasePlan", ctx, "plan-gate").Return(&config.PurchasePlan{ID: "plan-gate"}, nil) + manager := &Manager{config: store} + exec := dueRecExec("notified", upfrontRec(1)) + ok, err := manager.executableByScheduler(ctx, &exec) + require.NoError(t, err) + assert.False(t, ok) + store.AssertNotCalled(t, "GetGlobalConfig", mock.Anything) +} + +func TestShouldNotifyPlan_KillSwitch(t *testing.T) { + next := time.Now().Add(24 * time.Hour) + plan := config.PurchasePlan{Enabled: true, AutoPurchase: true, NotificationDaysBefore: 7, NextExecutionDate: &next} + m := &Manager{} + assert.True(t, m.shouldNotifyPlan(plan, true)) + assert.False(t, m.shouldNotifyPlan(plan, false), "kill switch off must suppress the notification") +} + +func TestSendUpcomingPurchaseNotifications_KillSwitch(t *testing.T) { + next := time.Now().Add(24 * time.Hour) + plan := config.PurchasePlan{ID: "p", Enabled: true, AutoPurchase: true, NotificationDaysBefore: 7, NextExecutionDate: &next} + + t.Run("off sends nothing and touches no plan", func(t *testing.T) { + ctx := context.Background() + store := new(MockConfigStore) + store.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{}).Return([]config.PurchasePlan{plan}, nil) + store.On("GetGlobalConfig", ctx).Return(&config.GlobalConfig{}, nil) + manager := &Manager{config: store, email: new(MockEmailSender)} + res, err := manager.SendUpcomingPurchaseNotifications(ctx) + require.NoError(t, err) + assert.Equal(t, 0, res.Notified) + store.AssertNotCalled(t, "LockPurchasePlanTx", mock.Anything, mock.Anything, mock.Anything) + }) + + t.Run("read error fails the tick", func(t *testing.T) { + ctx := context.Background() + store := new(MockConfigStore) + store.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{}).Return([]config.PurchasePlan{plan}, nil) + store.On("GetGlobalConfig", ctx).Return(nil, errors.New("db down")) + manager := &Manager{config: store, email: new(MockEmailSender)} + _, err := manager.SendUpcomingPurchaseNotifications(ctx) + require.Error(t, err) + }) +}