diff --git a/internal/api/combined_plan_purchase_integration_test.go b/internal/api/combined_plan_purchase_integration_test.go index 84d08d9c4..70cb6768b 100644 --- a/internal/api/combined_plan_purchase_integration_test.go +++ b/internal/api/combined_plan_purchase_integration_test.go @@ -143,7 +143,10 @@ func newCombinedPlanFixtureIn(t *testing.T, automatic bool, capAmount *float64, global, err := f.store.GetGlobalConfig(ctx) require.NoError(t, err) recipient := "fixture@example.invalid" - global.NotificationEmail, global.AutoPurchaseEnabled, global.RequireDifferentApprover = &recipient, automatic, true + // 4-eyes is on for the manual (approval) flows only. With 4-eyes on, an + // AutoPurchase step waits for a human approval instead of auto-executing + // (#1771); internal/purchase/four_eyes_auto_plan_integration_test.go covers that. + global.NotificationEmail, global.AutoPurchaseEnabled, global.RequireDifferentApprover = &recipient, automatic, !automatic global.PurchaseDelayHours = 0 require.NoError(t, f.store.SaveGlobalConfig(ctx, global)) response := combinedRoute(t, f.handler, "POST", "/api/plans", PlanRequest{ @@ -384,6 +387,40 @@ func TestCombinedPlanPurchase_NonUSDApprovalRefusedUnderCap(t *testing.T) { f.assertCompleted(t) } +// With 4-eyes on, an AutoPurchase plan's due step is not auto-executed: it +// waits for a human approval, and a different user's approval then buys it +// (#1771). +func TestCombinedPlanPurchase_AutomaticWithFourEyesWaitsForApproval(t *testing.T) { + capAmount := 2400.0 + f := newCombinedPlanFixture(t, true, &capAmount) + f.assertAttached(t) + ctx := context.Background() + global, err := f.store.GetGlobalConfig(ctx) + require.NoError(t, err) + global.RequireDifferentApprover = true + require.NoError(t, f.store.SaveGlobalConfig(ctx, global)) + _, err = f.pool.Exec(ctx, `UPDATE purchase_executions SET scheduled_date=now()-interval '1 minute' WHERE execution_id=$1`, f.root.ExecutionID) + require.NoError(t, err) + + result, err := f.manager.ProcessScheduledPurchases(ctx) + require.NoError(t, err) + assert.Zero(t, result.Executed, "%+v", result) + assert.Zero(t, result.Failed, "%+v", result) + assert.Empty(t, f.service.recorded(), "4-eyes on: the due step must not be bought automatically") + row, err := f.store.GetExecutionByID(ctx, f.root.ExecutionID) + require.NoError(t, err) + assert.Equal(t, "notified", row.Status, "the step waits in the approval queue") + + approver := uuid.NewString() + _, err = f.pool.Exec(ctx, `INSERT INTO users (id,email,password_hash,salt,active,group_ids,created_at,updated_at) SELECT $1,'approver@example.invalid','x','y',true,ARRAY[g.id],now(),now() FROM groups g ORDER BY g.name LIMIT 1`, approver) + require.NoError(t, err) + response := f.approveAs(t, approver, 0) + require.Equal(t, 200, response.StatusCode, response.Body) + f.assertCompleted(t) + assert.Len(t, f.service.recorded(), 1) +} + +// With 4-eyes off (the fixture's automatic default), the due step auto-executes. func TestCombinedPlanPurchase_AutomaticOnScheduledExecution(t *testing.T) { capAmount := 2400.0 f := newCombinedPlanFixture(t, true, &capAmount) diff --git a/internal/config/types.go b/internal/config/types.go index a9c901535..c3f3e63d0 100644 --- a/internal/config/types.go +++ b/internal/config/types.go @@ -117,7 +117,13 @@ type GlobalConfig struct { // mode first (the admin wildcard is NOT exempt from the restriction). // While it is on, unattended RI exchange runs (RIExchangeMode "auto" and // ladder reshapes) run as "manual" and only raise pending records - // (issue #679). + // (issue #679), and recommendation-bearing AutoPurchase steps are not + // auto-executed: they wait for a human approval in the approval queue + // (issue #1771); empty steps still route as before. A step root + // pre-created with the plan keeps its human creator, who then cannot + // approve it. A step root pre-created through the admin API key (no + // creator and not system-created) cannot be approved by anyone while this + // is on: it waits, then expires, and a human recreates the plan. RequireDifferentApprover bool `json:"require_different_approver" dynamodbav:"require_different_approver"` } diff --git a/internal/email/sender.go b/internal/email/sender.go index a09753cec..04ad347fc 100644 --- a/internal/email/sender.go +++ b/internal/email/sender.go @@ -587,6 +587,9 @@ type NotificationData struct { TotalRecurring float64 TotalUnits int StepCap float64 + // NeedsApproval is set when the step does not run on its date but waits + // for a human approval in the approval queue (manual plan, or 4-eyes on). + NeedsApproval bool // RevocationToken is the one-time token embedded in the revocation link // of a post-execution notification email. When non-empty, the template // renders a "Revoke this purchase" CTA that hits diff --git a/internal/email/template_renderers_test.go b/internal/email/template_renderers_test.go index 5f6f66e5a..b98e7b9ae 100644 --- a/internal/email/template_renderers_test.go +++ b/internal/email/template_renderers_test.go @@ -797,3 +797,21 @@ func TestRenderRIExchangePendingApprovalEmail_PlainTextVerbatim(t *testing.T) { assert.NotContains(t, body, "'", "plain-text must not HTML-encode apostrophe") assert.NotContains(t, body, "&", "plain-text must not HTML-encode ampersand") } + +func TestRenderScheduledPurchaseEmail_NeedsApprovalWording(t *testing.T) { + data := NotificationData{ + PlanName: "p", PurchaseDate: "November 3, 2026", DaysUntilPurchase: 3, DashboardURL: "https://d.example", ExecutionID: "e1", + TotalCommitment: 100, StepCap: 500, TotalUnits: 1, Recommendations: []RecommendationSummary{{Count: 1, ResourceType: "m5.large", Region: "r"}}, + } + body, err := RenderScheduledPurchaseEmail(data) + require.NoError(t, err) + assert.Contains(t, body, ", cap $500.00, on November 3, 2026 unless canceled.") + assert.Contains(t, body, "You have 3 days to modify or cancel before automatic execution.") + + data.NeedsApproval = true + body, err = RenderScheduledPurchaseEmail(data) + require.NoError(t, err) + assert.Contains(t, body, ", cap $500.00, due on November 3, 2026. It is bought once approved. Review & Edit below and approve it in the approval queue.") + assert.NotContains(t, body, "unless canceled") + assert.NotContains(t, body, "before automatic execution") +} diff --git a/internal/email/templates.go b/internal/email/templates.go index 27086badd..c5a642dcc 100644 --- a/internal/email/templates.go +++ b/internal/email/templates.go @@ -41,9 +41,9 @@ This is an automated message from CUDly. const scheduledPurchaseTemplate = `CUDly - Scheduled Purchase in {{.DaysUntilPurchase}} Days ========================================= -Based on your Purchase Plan "{{.PlanName}}", the following commitments will be purchased on {{.PurchaseDate}}: +Based on your Purchase Plan "{{.PlanName}}", the following commitments {{if .NeedsApproval}}are due on{{else}}will be purchased on{{end}} {{.PurchaseDate}}: {{if gt .TotalCommitment 0.0}} -Will buy {{len .Recommendations}} recommendation(s) ({{.TotalUnits}} units) for ${{printf "%.2f" .TotalCommitment}} total commitment (upfront ${{printf "%.2f" .TotalUpfrontCost}} + ${{printf "%.2f" .TotalRecurring}} recurring over the terms){{if gt .StepCap 0.0}}, cap ${{printf "%.2f" .StepCap}}{{end}}, on {{.PurchaseDate}} unless canceled. +Will buy {{len .Recommendations}} recommendation(s) ({{.TotalUnits}} units) for ${{printf "%.2f" .TotalCommitment}} total commitment (upfront ${{printf "%.2f" .TotalUpfrontCost}} + ${{printf "%.2f" .TotalRecurring}} recurring over the terms){{if gt .StepCap 0.0}}, cap ${{printf "%.2f" .StepCap}}{{end}}, {{if .NeedsApproval}}due on {{.PurchaseDate}}. It is bought once approved. Review & Edit below and approve it in the approval queue.{{else}}on {{.PurchaseDate}} unless canceled.{{end}} {{end}} {{range .Recommendations}} - {{.Count}}x {{.ResourceType}}{{if .Engine}} ({{.Engine}}){{end}} in {{.Region}}{{if .AvailabilityZone}} ({{.AvailabilityZone}}){{end}}{{if .Platform}} | {{.Platform}}{{end}}{{if .Tenancy}} | {{.Tenancy}}{{end}} @@ -64,7 +64,7 @@ Actions: [Cancel This Purchase] {{.DashboardURL}}/purchases/cancel/{{.ExecutionID}}?token={{urlquery .ApprovalToken}} -You have {{.DaysUntilPurchase}} days to modify or cancel before automatic execution. +{{if .NeedsApproval}}This purchase is not bought automatically. It needs a human approval in the approval queue; you have {{.DaysUntilPurchase}} days to review, modify or cancel it.{{else}}You have {{.DaysUntilPurchase}} days to modify or cancel before automatic execution.{{end}} This is an automated message from CUDly. ` diff --git a/internal/purchase/claim_pin_test.go b/internal/purchase/claim_pin_test.go index 5175a619f..8a6fe18cd 100644 --- a/internal/purchase/claim_pin_test.go +++ b/internal/purchase/claim_pin_test.go @@ -201,7 +201,7 @@ func TestNotification_EmailFailureLeavesRowPendingAndSchedulerRefusesIt(t *testi sender.On("SendScheduledPurchaseNotification", ctx, mock.Anything).Return(errors.New("ses down")) manager := &Manager{config: store, email: sender} - require.False(t, manager.sendPlanNotification(ctx, plan)) + require.False(t, manager.sendPlanNotification(ctx, plan, false)) store.AssertNotCalled(t, "RotatePendingApprovalToken", mock.Anything, mock.Anything, mock.Anything, mock.Anything) // The row as the DB still holds it: pending. The scheduler leaves it alone. diff --git a/internal/purchase/four_eyes_auto_plan_integration_test.go b/internal/purchase/four_eyes_auto_plan_integration_test.go new file mode 100644 index 000000000..41a16d581 --- /dev/null +++ b/internal/purchase/four_eyes_auto_plan_integration_test.go @@ -0,0 +1,105 @@ +//go:build integration + +package purchase + +import ( + "context" + "testing" + "time" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" + "github.com/google/uuid" + "github.com/jackc/pgx/v5" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Issue #1771: with 4-eyes on, an armed auto plan's due step waits for a human +// approval instead of being bought by the sweep. Real PostgreSQL; the cloud is +// the labeled fake provider of the ramp-step fixture. + +func fourEyesUser(ctx context.Context, t *testing.T, f *rampStepFixture, name string) string { + t.Helper() + id := uuid.New().String() + require.NoError(t, f.store.WithTx(ctx, func(tx pgx.Tx) error { + _, err := tx.Exec(ctx, `INSERT INTO users (id, email, password_hash, group_ids) VALUES ($1, $2, 'x', ARRAY[$1::uuid])`, id, name+"@example.invalid") + return err + })) + return id +} + +func fourEyesAutoRow(ctx context.Context, t *testing.T, f *rampStepFixture, creator *string) *config.PurchaseExecution { + t.Helper() + row := &config.PurchaseExecution{ + PlanID: f.plan.ID, ExecutionID: uuid.New().String(), Status: "notified", StepNumber: 3, + ScheduledDate: time.Now().Add(-time.Hour), CloudAccountID: &f.accounts[0].ID, + Recommendations: []config.RecommendationRecord{withAccount(rampStepRecommendation()[0], f.accounts[0].ID)}, + CreatedByUserID: creator, CreatedBySystem: creator == nil, + } + require.NoError(t, f.store.SavePurchaseExecution(ctx, row)) + return row +} + +func setFourEyes(ctx context.Context, t *testing.T, f *rampStepFixture, on bool) { + t.Helper() + global, err := f.store.GetGlobalConfig(ctx) + require.NoError(t, err) + global.RequireDifferentApprover = on + require.NoError(t, f.store.SaveGlobalConfig(ctx, global)) +} + +func TestFourEyes_AutoPlanSweepLeavesRowForHumanApproval(t *testing.T) { + ctx := context.Background() + f := newRampStepFixture(ctx, t) + setFourEyes(ctx, t, f, true) + row := fourEyesAutoRow(ctx, t, f, nil) + + result, err := f.manager.ProcessScheduledPurchases(ctx) + require.NoError(t, err) + assert.Equal(t, 0, result.Processed) + assert.Equal(t, 0, f.purchaseCount()) + assert.Equal(t, "notified", rowStatus(ctx, t, f, row.ExecutionID).Status) + + userB := fourEyesUser(ctx, t, f, "b") + _, _, err = f.manager.ApproveAndExecute(ctx, row.ExecutionID, "b@example.invalid", &userB, nil) + require.NoError(t, err) + assert.Equal(t, 1, f.purchaseCount()) + assert.Equal(t, "completed", rowStatus(ctx, t, f, row.ExecutionID).Status) +} + +func TestFourEyes_AutoPlanPreCreatedRootCreatorCannotApprove(t *testing.T) { + ctx := context.Background() + f := newRampStepFixture(ctx, t) + setFourEyes(ctx, t, f, true) + userA := fourEyesUser(ctx, t, f, "a") + userB := fourEyesUser(ctx, t, f, "b") + row := fourEyesAutoRow(ctx, t, f, &userA) + + result, err := f.manager.ProcessScheduledPurchases(ctx) + require.NoError(t, err) + assert.Equal(t, 0, result.Processed) + assert.Equal(t, 0, f.purchaseCount()) + + _, _, err = f.manager.ApproveAndExecute(ctx, row.ExecutionID, "a@example.invalid", &userA, nil) + require.ErrorIs(t, err, ErrFourEyesDenied) + assert.Equal(t, 0, f.purchaseCount()) + assert.Equal(t, "notified", rowStatus(ctx, t, f, row.ExecutionID).Status) + + _, _, err = f.manager.ApproveAndExecute(ctx, row.ExecutionID, "b@example.invalid", &userB, nil) + require.NoError(t, err) + assert.Equal(t, 1, f.purchaseCount()) + assert.Equal(t, "completed", rowStatus(ctx, t, f, row.ExecutionID).Status) +} + +func TestFourEyes_AutoPlanSweepExecutesWhenFlagOff(t *testing.T) { + ctx := context.Background() + f := newRampStepFixture(ctx, t) + setFourEyes(ctx, t, f, false) + row := fourEyesAutoRow(ctx, t, f, nil) + + result, err := f.manager.ProcessScheduledPurchases(ctx) + require.NoError(t, err) + assert.Equal(t, 1, result.Executed) + assert.Equal(t, 1, f.purchaseCount()) + assert.Equal(t, "completed", rowStatus(ctx, t, f, row.ExecutionID).Status) +} diff --git a/internal/purchase/four_eyes_system_execution_integration_test.go b/internal/purchase/four_eyes_system_execution_integration_test.go index 5fead30ff..d1a844095 100644 --- a/internal/purchase/four_eyes_system_execution_integration_test.go +++ b/internal/purchase/four_eyes_system_execution_integration_test.go @@ -20,7 +20,11 @@ func TestFourEyes_SweepMintedRowIsApprovableByIdentifiedApprover(t *testing.T) { plan := attachPlanInDB(ctx, t, store, "four-eyes-sweep", time.Now().Add(24*time.Hour)) m := &Manager{config: store, email: newCountingSender(ctx), resolver: &fakeResolver{recs: []config.RecommendationRecord{attachRec("rec-a", 2)}}} - require.True(t, m.sendPlanNotification(ctx, plan)) + global, err := store.GetGlobalConfig(ctx) + require.NoError(t, err) + global.RequireDifferentApprover = true + require.NoError(t, store.SaveGlobalConfig(ctx, global)) + require.True(t, m.sendPlanNotification(ctx, plan, planNeedsApproval(*plan, global))) rows := stepRowsInDB(ctx, t, store, plan.ID) require.Len(t, rows, 1) @@ -28,11 +32,6 @@ func TestFourEyes_SweepMintedRowIsApprovableByIdentifiedApprover(t *testing.T) { assert.True(t, rows[0].CreatedBySystem, "sweep-minted row must be marked system-created") assert.Nil(t, rows[0].CreatedByUserID) - global, err := store.GetGlobalConfig(ctx) - require.NoError(t, err) - global.RequireDifferentApprover = true - require.NoError(t, store.SaveGlobalConfig(ctx, global)) - require.NoError(t, m.enforceFourEyesPolicy(ctx, rows[0].ExecutionID, "approver@example.com", nil)) require.ErrorIs(t, m.enforceFourEyesPolicy(ctx, rows[0].ExecutionID, "", nil), ErrFourEyesDenied) } diff --git a/internal/purchase/manager.go b/internal/purchase/manager.go index e0c9b1122..b719366bc 100644 --- a/internal/purchase/manager.go +++ b/internal/purchase/manager.go @@ -879,6 +879,9 @@ func (m *Manager) RecoverStrandedApprovals(ctx context.Context) (int, error) { // - 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. +// - With 4-eyes approval mode on (GlobalConfig.RequireDifferentApprover), a +// recommendation-bearing row is never auto-executed: it waits for a human +// approval in the approval queue (#1771). // - 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 @@ -902,16 +905,22 @@ func (m *Manager) executableByScheduler(ctx context.Context, exec *config.Purcha if plan == nil { return false, nil } - enabled, err := m.autoPurchaseEnabled(ctx) + cfg, err := m.autoPurchaseConfig(ctx) if err != nil { return false, err } - if !enabled { + if !cfg.AutoPurchaseEnabled { return false, nil } if len(exec.Recommendations) == 0 { return true, nil } + // With 4-eyes on, no step buys without a second person: an auto plan's + // step waits in the approval queue for a human approval (session approve), + // like a manual plan's (#1771). + if cfg.RequireDifferentApprover { + return false, nil + } if exec.Status != "notified" { return false, nil } @@ -951,14 +960,24 @@ func (m *Manager) autoPurchasePlan(ctx context.Context, exec *config.PurchaseExe // 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.autoPurchaseConfig(ctx) + if err != nil { + return false, err + } + return cfg.AutoPurchaseEnabled, nil +} + +// autoPurchaseConfig reads the global config the auto-purchase gates need. A +// read error or a missing config is returned, so no gate reads it as "on". +func (m *Manager) autoPurchaseConfig(ctx context.Context) (*config.GlobalConfig, 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) + return nil, 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 nil, errors.New("global config unavailable for the auto-purchase kill switch") } - return cfg.AutoPurchaseEnabled, nil + return cfg, nil } // processOneExecution runs the full gate+claim+execute pipeline for a single @@ -979,7 +998,7 @@ func (m *Manager) processOneExecution(ctx context.Context, exec config.PurchaseE return } if !eligible { - logging.Infof("Skipping execution %s (AutoPurchase=false, kill switch off, source=web, or recommendation-bearing row not yet notified; requires explicit approval)", exec.ExecutionID) + logging.Infof("Skipping execution %s (AutoPurchase=false, kill switch off, source=web, 4-eyes on so it needs a human approval in the approval queue, 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 ae9f95d9b..299d4ef78 100644 --- a/internal/purchase/manager_test.go +++ b/internal/purchase/manager_test.go @@ -1275,3 +1275,46 @@ func TestManager_RecoverStrandedApprovals_AWSClaimLost_NoRedrive(t *testing.T) { mockFactory.AssertNotCalled(t, "CreateAndValidateProvider", mock.Anything, mock.Anything, mock.Anything) mockStore.AssertNotCalled(t, "SavePurchaseExecution", mock.Anything, mock.Anything) } + +// #1771: with 4-eyes on, an armed auto plan's rec-bearing notified row is not +// auto-executed; it waits for a human approval in the approval queue. +func TestManager_ProcessScheduledPurchases_FourEyesOnSkipsAutoRow(t *testing.T) { + cfg := autoPurchaseOnCfg() + cfg.RequireDifferentApprover = true + plan := &config.PurchasePlan{ID: "plan-gate", AutoPurchase: true, MaxStepCommitment: f64(1000)} + result := gateHarness(t, plan, dueRecExec("notified", upfrontRec(10)), cfg, nil) + assert.Equal(t, 0, result.Processed) + assert.Equal(t, 0, result.Executed) + assert.Equal(t, 0, result.Failed, "waiting for approval is a skip, not a failure") +} + +// With 4-eyes off the gate is unchanged. +func TestExecutableByScheduler_FourEyesOffUnchanged(t *testing.T) { + ctx := context.Background() + store := new(MockConfigStore) + assertMocksOnCleanup(t, store) + plan := &config.PurchasePlan{ID: "plan-gate", AutoPurchase: true, MaxStepCommitment: f64(1000)} + store.On("GetPurchasePlan", ctx, "plan-gate").Return(plan, nil) + store.On("GetGlobalConfig", ctx).Return(autoPurchaseOnCfg(), nil) + manager := &Manager{config: store} + exec := dueRecExec("notified", upfrontRec(10)) + ok, err := manager.executableByScheduler(ctx, &exec) + require.NoError(t, err) + assert.True(t, ok) +} + +// An empty step buys nothing, so 4-eyes does not park it in limbo. +func TestExecutableByScheduler_FourEyesOnEmptyStepStillRoutes(t *testing.T) { + ctx := context.Background() + store := new(MockConfigStore) + assertMocksOnCleanup(t, store) + cfg := autoPurchaseOnCfg() + cfg.RequireDifferentApprover = true + store.On("GetPurchasePlan", ctx, "plan-gate").Return(&config.PurchasePlan{ID: "plan-gate", AutoPurchase: true}, nil) + store.On("GetGlobalConfig", ctx).Return(cfg, nil) + manager := &Manager{config: store} + exec := dueRecExec("notified") + ok, err := manager.executableByScheduler(ctx, &exec) + require.NoError(t, err) + assert.True(t, ok) +} diff --git a/internal/purchase/messages.go b/internal/purchase/messages.go index fa1a5f1bc..587137910 100644 --- a/internal/purchase/messages.go +++ b/internal/purchase/messages.go @@ -96,7 +96,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, kill switch off, source=web, or recommendation-bearing row not yet notified - 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, 4-eyes on (it needs a human approval in the approval queue), or recommendation-bearing row not yet notified - approve it in the approval queue", execution.ExecutionID, execution.Status, execution.Source) } return nil diff --git a/internal/purchase/messages_test.go b/internal/purchase/messages_test.go index 362b8a8d9..149064f84 100644 --- a/internal/purchase/messages_test.go +++ b/internal/purchase/messages_test.go @@ -9,6 +9,7 @@ import ( "github.com/LeanerCloud/cloud-commitments-platform/internal/config" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" ) func TestManager_ProcessMessage(t *testing.T) { @@ -190,3 +191,23 @@ func TestManager_ProcessMessage(t *testing.T) { assert.NoError(t, err) }) } + +// #1771: an SQS execute_purchase for an auto row is refused while 4-eyes is +// on, with an error that names the reason, and nothing is claimed or bought. +func TestCheckAutoExecuteGate_FourEyesOnRefusesNotifiedRow(t *testing.T) { + ctx := context.Background() + store := new(MockConfigStore) // TransitionExecutionStatus is not mocked: a claim would panic + assertMocksOnCleanup(t, store) + cfg := autoPurchaseOnCfg() + cfg.RequireDifferentApprover = true + store.On("GetPurchasePlan", ctx, "plan-gate").Return(&config.PurchasePlan{ID: "plan-gate", AutoPurchase: true, MaxStepCommitment: f64(1000)}, nil) + store.On("GetGlobalConfig", ctx).Return(cfg, nil) + manager := &Manager{config: store} + exec := dueRecExec("notified", upfrontRec(10)) + + err := manager.checkAutoExecuteGate(ctx, &exec) + + require.Error(t, err) + assert.Contains(t, err.Error(), "4-eyes") + store.AssertNotCalled(t, "TransitionExecutionStatus", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything) +} diff --git a/internal/purchase/notification_data_test.go b/internal/purchase/notification_data_test.go index 5f3493b7a..9aa26a8dc 100644 --- a/internal/purchase/notification_data_test.go +++ b/internal/purchase/notification_data_test.go @@ -28,7 +28,7 @@ func TestBuildNotificationData_ShowsTotalCommitmentAndCap(t *testing.T) { } m := &Manager{} - data := m.buildNotificationData(plan, exec, "tok", 3, "ops@example.invalid") + data := m.buildNotificationData(plan, exec, "tok", 3, "ops@example.invalid", true) assert.InDelta(t, 1500.0, data.TotalCommitment, 1e-9) assert.InDelta(t, 1200.0, data.TotalRecurring, 1e-9) @@ -42,7 +42,74 @@ func TestBuildNotificationData_ShowsTotalCommitmentAndCap(t *testing.T) { body, err := email.RenderScheduledPurchaseEmail(data) require.NoError(t, err) - assert.Contains(t, body, "Will buy 2 recommendation(s) (5 units) for $1500.00 total commitment (upfront $300.00 + $1200.00 recurring over the terms), cap $5000.00, on November 3, 2026 unless canceled.") + assert.Contains(t, body, "Will buy 2 recommendation(s) (5 units) for $1500.00 total commitment (upfront $300.00 + $1200.00 recurring over the terms), cap $5000.00, due on November 3, 2026. It is bought once approved. Review & Edit below and approve it in the approval queue.") + assert.NotContains(t, body, "unless canceled") + + // Auto plan with 4-eyes off: the original wording is unchanged. + autoData := m.buildNotificationData(plan, exec, "tok", 3, "ops@example.invalid", false) + autoBody, err := email.RenderScheduledPurchaseEmail(autoData) + require.NoError(t, err) + assert.Contains(t, autoBody, "cap $5000.00, on November 3, 2026 unless canceled.") assert.Contains(t, body, "Term: 1 yr monthly | Upfront $0.00 + $100.00/month | Commitment $1200.00") assert.Contains(t, body, "in us-east-1 (us-east-1a) | Linux/UNIX | default") } + +// #1771: a step that waits for a human approval (manual plan, or an auto plan +// while 4-eyes is on) must not tell the approver it buys unless canceled. +func TestPlanNeedsApproval_Modes(t *testing.T) { + on := &config.GlobalConfig{RequireDifferentApprover: true} + off := &config.GlobalConfig{} + cases := []struct { + name string + plan config.PurchasePlan + global *config.GlobalConfig + want bool + }{ + {"manual plan", config.PurchasePlan{}, off, true}, + {"auto plan, 4-eyes on", config.PurchasePlan{AutoPurchase: true}, on, true}, + {"auto plan, 4-eyes off", config.PurchasePlan{AutoPurchase: true}, off, false}, + {"auto plan, nil config fails closed", config.PurchasePlan{AutoPurchase: true}, nil, true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.want, planNeedsApproval(tc.plan, tc.global)) + }) + } +} + +func TestBuildNotificationData_ApprovalWordingByMode(t *testing.T) { + capAmount := 5000.0 + plan := config.PurchasePlan{ID: "plan-1", Name: "ramp", MaxStepCommitment: &capAmount} + exec := &config.PurchaseExecution{ + ExecutionID: "exec-1", PlanID: "plan-1", ScheduledDate: time.Date(2026, 11, 3, 0, 0, 0, 0, time.UTC), + Recommendations: []config.RecommendationRecord{ + {Service: "ec2", ResourceType: "m5.large", Region: "us-east-1", Count: 1, Term: 1, Payment: "all-upfront", UpfrontCost: 100}, + }, + } + m := &Manager{} + const totals = "cap $5000.00, " + for _, tc := range []struct { + name string + needsApproval bool + }{ + {"manual plan or auto with 4-eyes on", true}, + {"auto plan with 4-eyes off", false}, + } { + t.Run(tc.name, func(t *testing.T) { + data := m.buildNotificationData(plan, exec, "tok", 3, "ops@example.invalid", tc.needsApproval) + assert.Equal(t, tc.needsApproval, data.NeedsApproval) + body, err := email.RenderScheduledPurchaseEmail(data) + require.NoError(t, err) + assert.Contains(t, body, totals, "the totals and cap sentence stays in both modes") + if tc.needsApproval { + assert.Contains(t, body, totals+"due on November 3, 2026. It is bought once approved. Review & Edit below and approve it in the approval queue.") + assert.NotContains(t, body, "unless canceled") + assert.NotContains(t, body, "before automatic execution") + } else { + assert.Contains(t, body, totals+"on November 3, 2026 unless canceled.") + assert.Contains(t, body, "You have 3 days to modify or cancel before automatic execution.") + assert.NotContains(t, body, "approval queue") + } + }) + } +} diff --git a/internal/purchase/notifications.go b/internal/purchase/notifications.go index 042f82686..52d080f70 100644 --- a/internal/purchase/notifications.go +++ b/internal/purchase/notifications.go @@ -42,7 +42,7 @@ func (m *Manager) SendUpcomingPurchaseNotifications(ctx context.Context) (*Notif continue } if m.shouldNotifyPlan(plan, autoPurchaseEnabled) || shouldRetryOverduePlan(plan, autoPurchaseEnabled, global, time.Now()) { - if m.sendPlanNotification(ctx, &plan) { + if m.sendPlanNotification(ctx, &plan, planNeedsApproval(plan, global)) { notified++ } } @@ -152,8 +152,17 @@ func (m *Manager) dueManualRootTx(ctx context.Context, tx pgx.Tx, planID string) // plan's NextExecutionDate advances hits it, so it is skipped quietly. var errExecutionNotNotifiable = errors.New("execution is no longer awaiting approval") -// sendPlanNotification sends a notification for a plan and returns true if successful. -func (m *Manager) sendPlanNotification(ctx context.Context, plan *config.PurchasePlan) bool { +// planNeedsApproval reports whether the plan's recommendation-bearing step +// waits for a human approval instead of executing on its date: manual plans +// always do, and auto plans do while 4-eyes mode is on (#1771). A missing +// config fails closed. +func planNeedsApproval(plan config.PurchasePlan, global *config.GlobalConfig) bool { + return !plan.AutoPurchase || global == nil || global.RequireDifferentApprover +} + +// sendPlanNotification sends a notification for a plan and returns true if +// successful. needsApproval selects the approval wording of the email. +func (m *Manager) sendPlanNotification(ctx context.Context, plan *config.PurchasePlan, needsApproval bool) bool { daysUntil := int(time.Until(*plan.NextExecutionDate).Hours() / config.HoursPerDay) logging.Infof("Sending notification for plan %s (purchase in %d days)", plan.Name, daysUntil) @@ -180,7 +189,7 @@ func (m *Manager) sendPlanNotification(ctx context.Context, plan *config.Purchas // The row's own date, not the plan's: an attach for an overdue plan is // scheduled NotificationDaysBefore days out. daysUntil = int(time.Until(execution.ScheduledDate).Hours() / config.HoursPerDay) - data := m.buildNotificationData(*plan, execution, rawApprovalToken, daysUntil, notifyEmail) + data := m.buildNotificationData(*plan, execution, rawApprovalToken, daysUntil, notifyEmail, needsApproval) if err := m.email.SendScheduledPurchaseNotification(ctx, data); err != nil { logging.Errorf("Failed to send notification: %v", err) return false @@ -388,7 +397,9 @@ func bareStepFailure(planID string, step int, reason string) error { // as RecipientEmail so the token-bearing body routes through targeted SES. // rawApprovalToken is the RAW token from getOrCreateExecution -- never read // from exec.ApprovalToken, which holds only the hash (issue #103). -func (m *Manager) buildNotificationData(plan config.PurchasePlan, exec *config.PurchaseExecution, rawApprovalToken string, daysUntil int, notifyEmail string) email.NotificationData { +// needsApproval is decided by the caller from the tick's global config (see +// planNeedsApproval); this function reads no config. +func (m *Manager) buildNotificationData(plan config.PurchasePlan, exec *config.PurchaseExecution, rawApprovalToken string, daysUntil int, notifyEmail string, needsApproval bool) email.NotificationData { data := email.NotificationData{ DashboardURL: m.dashboardURL, ApprovalToken: rawApprovalToken, @@ -400,6 +411,7 @@ func (m *Manager) buildNotificationData(plan config.PurchasePlan, exec *config.P DaysUntilPurchase: daysUntil, PlanName: plan.Name, RecipientEmail: notifyEmail, + NeedsApproval: needsApproval, } if plan.MaxStepCommitment != nil { diff --git a/internal/purchase/notifications_test.go b/internal/purchase/notifications_test.go index c8a39fc26..e8d533418 100644 --- a/internal/purchase/notifications_test.go +++ b/internal/purchase/notifications_test.go @@ -155,7 +155,7 @@ func TestManager_BuildNotificationData(t *testing.T) { }, } - data := manager.buildNotificationData(plan, execution, "token-abc", 5, "notify@example.com") + data := manager.buildNotificationData(plan, execution, "token-abc", 5, "notify@example.com", false) assert.Equal(t, "https://dashboard.example.com", data.DashboardURL) assert.Equal(t, "token-abc", data.ApprovalToken) @@ -521,7 +521,9 @@ func TestManager_SendUpcomingPurchaseNotifications_WithNotification(t *testing.T expectStepRows(mockStore, &plans[0], *pendingExec) mockStore.On("RotatePendingApprovalToken", ctx, "pending-exec-id", mock.Anything, mock.Anything).Return(true, nil) mockStore.On("GetGlobalConfig", ctx).Return(globalCfg, nil) - mockEmail.On("SendScheduledPurchaseNotification", ctx, mock.AnythingOfType("email.NotificationData")).Return(nil) + mockEmail.On("SendScheduledPurchaseNotification", ctx, mock.MatchedBy(func(data email.NotificationData) bool { + return !data.NeedsApproval // auto plan, 4-eyes off: the original wording + })).Return(nil) mockStore.On("StampPlanNotificationSent", ctx, "plan-123", mock.AnythingOfType("time.Time")).Return(nil) manager := &Manager{ @@ -551,7 +553,7 @@ func TestManager_SendUpcomingPurchaseNotifications_ManualCleanupErrorDoesNotBloc } address := "fixture@example.invalid" store.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{}).Return(plans, nil) - store.On("GetGlobalConfig", ctx).Return(&config.GlobalConfig{NotificationEmail: &address, AutoPurchaseEnabled: true}, nil) + store.On("GetGlobalConfig", ctx).Return(&config.GlobalConfig{NotificationEmail: &address, AutoPurchaseEnabled: true, RequireDifferentApprover: true}, nil) store.On("WithTx", ctx, mock.Anything).Return(errors.New("manual cleanup transaction failed")).Once() store.On("WithTx", ctx, mock.Anything).Return(nil).Run(func(args mock.Arguments) { require.NoError(t, args.Get(1).(func(pgx.Tx) error)(nil)) @@ -563,7 +565,7 @@ func TestManager_SendUpcomingPurchaseNotifications_ManualCleanupErrorDoesNotBloc store.On("RotatePendingApprovalToken", ctx, "automatic-root", mock.Anything, mock.Anything).Return(true, nil).Once() store.On("StampPlanNotificationSent", ctx, plans[1].ID, mock.AnythingOfType("time.Time")).Return(nil).Once() sender.On("SendScheduledPurchaseNotification", ctx, mock.MatchedBy(func(data email.NotificationData) bool { - return data.PlanID == plans[1].ID + return data.PlanID == plans[1].ID && data.NeedsApproval // auto plan, 4-eyes on })).Return(nil).Once() manager := &Manager{config: store, email: sender, dashboardURL: "https://dashboard.example.invalid"} @@ -760,7 +762,7 @@ func TestManager_SendPlanNotification_NoRecipientLeavesLiveTokenAlone(t *testing mockStore.On("GetGlobalConfig", ctx).Return(&config.GlobalConfig{}, nil) manager := &Manager{config: mockStore, email: mockEmail, dashboardURL: "https://dashboard.example.com"} - assert.False(t, manager.sendPlanNotification(ctx, plan)) + assert.False(t, manager.sendPlanNotification(ctx, plan, false)) assert.Equal(t, config.HashApprovalToken("previously-emailed-token"), existing.ApprovalToken) mockStore.AssertExpectations(t) mockEmail.AssertExpectations(t) @@ -779,7 +781,7 @@ func TestManager_SendPlanNotification_SendFailureLeavesLiveTokenAlone(t *testing mockEmail.On("SendScheduledPurchaseNotification", ctx, mock.AnythingOfType("email.NotificationData")).Return(errors.New("ses down")) manager := &Manager{config: mockStore, email: mockEmail, dashboardURL: "https://dashboard.example.com"} - assert.False(t, manager.sendPlanNotification(ctx, plan)) + assert.False(t, manager.sendPlanNotification(ctx, plan, false)) mockStore.AssertNotCalled(t, "RotatePendingApprovalToken", mock.Anything, mock.Anything, mock.Anything, mock.Anything) mockStore.AssertNotCalled(t, "StampPlanNotificationSent", mock.Anything, mock.Anything, mock.Anything) mockStore.AssertExpectations(t) @@ -812,7 +814,7 @@ func TestManager_SendPlanNotification_PersistsEmailedTokenHashAfterSend(t *testi mockStore.On("StampPlanNotificationSent", ctx, plan.ID, mock.AnythingOfType("time.Time")).Return(nil) manager := &Manager{config: mockStore, email: mockEmail, dashboardURL: "https://dashboard.example.com"} - assert.True(t, manager.sendPlanNotification(ctx, plan)) + assert.True(t, manager.sendPlanNotification(ctx, plan, false)) require.NotEmpty(t, emailedToken) assert.Equal(t, config.HashApprovalToken(emailedToken), persistedHash) assert.NotEqual(t, emailedToken, persistedHash) @@ -831,7 +833,7 @@ func TestManager_SendPlanNotification_StampFailure(t *testing.T) { store.On("RotatePendingApprovalToken", ctx, "exec-rot", mock.Anything, mock.Anything).Return(true, nil) store.On("StampPlanNotificationSent", ctx, plan.ID, mock.Anything).Return(errors.New("stamp failed")) manager := &Manager{config: store, email: sender} - require.False(t, manager.sendPlanNotification(ctx, plan)) + require.False(t, manager.sendPlanNotification(ctx, plan, false)) require.Nil(t, plan.LastNotificationSent) store.AssertExpectations(t) sender.AssertExpectations(t) diff --git a/internal/purchase/notified_claim_pin_integration_test.go b/internal/purchase/notified_claim_pin_integration_test.go index db3d2a9d9..7c67f967f 100644 --- a/internal/purchase/notified_claim_pin_integration_test.go +++ b/internal/purchase/notified_claim_pin_integration_test.go @@ -64,7 +64,7 @@ func TestNotifiedTransition_OnlyAfterEmailSucceeds(t *testing.T) { assertMocksOnCleanup(t, sender) sender.On("SendScheduledPurchaseNotification", ctx, mock.Anything).Return(errors.New("ses down")).Once() manager := &Manager{config: store, email: sender, providerFactory: factory} - require.False(t, manager.sendPlanNotification(ctx, plan)) + require.False(t, manager.sendPlanNotification(ctx, plan, false)) assert.Equal(t, "pending", statusInDB(ctx, t, store, row.ExecutionID).Status) result, err := manager.ProcessScheduledPurchases(ctx) @@ -73,7 +73,7 @@ func TestNotifiedTransition_OnlyAfterEmailSucceeds(t *testing.T) { assert.Equal(t, "pending", statusInDB(ctx, t, store, row.ExecutionID).Status) sender.On("SendScheduledPurchaseNotification", ctx, mock.Anything).Return(nil).Once() - require.True(t, manager.sendPlanNotification(ctx, plan)) + require.True(t, manager.sendPlanNotification(ctx, plan, false)) notified := statusInDB(ctx, t, store, row.ExecutionID) assert.Equal(t, "notified", notified.Status) assert.NotEmpty(t, notified.ApprovalToken) diff --git a/internal/purchase/plan_step_attach_integration_test.go b/internal/purchase/plan_step_attach_integration_test.go index 8909241f5..a10e63b65 100644 --- a/internal/purchase/plan_step_attach_integration_test.go +++ b/internal/purchase/plan_step_attach_integration_test.go @@ -83,7 +83,7 @@ func TestAttachIntegration_NewRowAttachesSuppressesAndBecomesNotified(t *testing sender := newCountingSender(ctx) m := &Manager{config: store, email: sender, resolver: &fakeResolver{recs: []config.RecommendationRecord{attachRec("rec-b", 2), attachRec("rec-a", 3)}}} - require.True(t, m.sendPlanNotification(ctx, plan)) + require.True(t, m.sendPlanNotification(ctx, plan, false)) rows := stepRowsInDB(ctx, t, store, plan.ID) require.Len(t, rows, 1) @@ -112,12 +112,12 @@ func TestAttachIntegration_EmailFailureLeavesPendingAndNextTickDoesNotResolveAga sender.On("SendScheduledPurchaseNotification", ctx, mock.Anything).Return(nil).Once() m := &Manager{config: store, email: sender, resolver: res} - require.False(t, m.sendPlanNotification(ctx, plan)) + require.False(t, m.sendPlanNotification(ctx, plan, false)) rows := stepRowsInDB(ctx, t, store, plan.ID) require.Len(t, rows, 1) assert.Equal(t, "pending", rows[0].Status) - require.True(t, m.sendPlanNotification(ctx, plan)) + require.True(t, m.sendPlanNotification(ctx, plan, false)) assert.Equal(t, "notified", stepRowsInDB(ctx, t, store, plan.ID)[0].Status) assert.EqualValues(t, 1, res.calls.Load(), "the second tick adopts the committed row") } @@ -130,7 +130,7 @@ func TestAttachIntegration_NoMatchAndCap(t *testing.T) { t.Run("no match inside the grace window writes no row", func(t *testing.T) { plan := attachPlanInDB(ctx, t, store, "attach-nomatch-early", time.Now().Add(24*time.Hour)) m := &Manager{config: store, email: newCountingSender(ctx), resolver: &fakeResolver{err: planstep.ErrNoMatch}} - require.False(t, m.sendPlanNotification(ctx, plan)) + require.False(t, m.sendPlanNotification(ctx, plan, false)) assert.Empty(t, stepRowsInDB(ctx, t, store, plan.ID)) }) t.Run("cap lowered between list and lock fails the step on the locked plan", func(t *testing.T) { @@ -142,7 +142,7 @@ func TestAttachIntegration_NoMatchAndCap(t *testing.T) { sender := newCountingSender(ctx) m := &Manager{config: store, email: sender, resolver: &fakeResolver{recs: []config.RecommendationRecord{attachRec("rec-a", 5)}}} - require.False(t, m.sendPlanNotification(ctx, &snapshot)) + require.False(t, m.sendPlanNotification(ctx, &snapshot, false)) assert.Equal(t, 1, sender.failedSends, "the failed step is reported once (issue #1195)") rows := stepRowsInDB(ctx, t, store, plan.ID) @@ -158,7 +158,7 @@ func TestAttachIntegration_NoMatchAndCap(t *testing.T) { require.NoError(t, store.SavePurchaseExecution(ctx, root)) m := &Manager{config: store, email: newCountingSender(ctx), resolver: &fakeResolver{recs: []config.RecommendationRecord{attachRec("rec-a", 2)}}} - require.True(t, m.sendPlanNotification(ctx, plan)) + require.True(t, m.sendPlanNotification(ctx, plan, false)) rows := stepRowsInDB(ctx, t, store, plan.ID) require.Len(t, rows, 1, "no second row") @@ -216,7 +216,7 @@ func TestAttachIntegration_PlansOnOneAccountNeverOverAttach(t *testing.T) { } m := &Manager{config: store, email: sender, resolver: res} wg.Add(1) - go func() { defer wg.Done(); m.sendPlanNotification(ctx, plan) }() + go func() { defer wg.Done(); m.sendPlanNotification(ctx, plan, false) }() } wg.Wait() diff --git a/internal/purchase/plan_step_crash_integration_test.go b/internal/purchase/plan_step_crash_integration_test.go index d90e1e9cd..b4ad68f05 100644 --- a/internal/purchase/plan_step_crash_integration_test.go +++ b/internal/purchase/plan_step_crash_integration_test.go @@ -49,7 +49,7 @@ func TestCrash_BeforeCommitPersistsNothing(t *testing.T) { sender := newCountingSender(ctx) m := &Manager{config: suppressionFailingStore{store}, email: sender, resolver: &fakeResolver{recs: []config.RecommendationRecord{attachRec("rec-a", 3)}}} - require.False(t, m.sendPlanNotification(ctx, plan)) + require.False(t, m.sendPlanNotification(ctx, plan, false)) assert.Empty(t, stepRowsInDB(ctx, t, store, plan.ID), "the row insert rolled back with the failed suppressions") assert.Zero(t, suppressedTotal(ctx, t, pool)) @@ -65,13 +65,13 @@ func TestCrash_AfterEmailBeforeNotifiedResendsAndPromotes(t *testing.T) { res := &fakeResolver{recs: []config.RecommendationRecord{attachRec("rec-a", 3)}} m := &Manager{config: &rotateLosingStore{PostgresStore: store}, email: sender, resolver: res} - require.False(t, m.sendPlanNotification(ctx, plan)) + require.False(t, m.sendPlanNotification(ctx, plan, false)) rows := stepRowsInDB(ctx, t, store, plan.ID) require.Len(t, rows, 1) assert.Equal(t, "pending", rows[0].Status, "the scheduler refuses a pending row") assert.Equal(t, 1, sender.sends) - require.True(t, m.sendPlanNotification(ctx, plan), "the next tick re-sends and promotes") + require.True(t, m.sendPlanNotification(ctx, plan, false), "the next tick re-sends and promotes") rows = stepRowsInDB(ctx, t, store, plan.ID) require.Len(t, rows, 1) assert.Equal(t, "notified", rows[0].Status) diff --git a/internal/purchase/plan_step_managed_race_integration_test.go b/internal/purchase/plan_step_managed_race_integration_test.go index 4c6a128d7..0293d547a 100644 --- a/internal/purchase/plan_step_managed_race_integration_test.go +++ b/internal/purchase/plan_step_managed_race_integration_test.go @@ -39,7 +39,7 @@ func TestClaimSkipsPlanTheCommandConvertedWhileItWaited(t *testing.T) { r := newClaimRace(ctx, t) command := testhelpers.HoldCommandFootprint(ctx, t, r.pool, r.plan.ID) done := make(chan bool, 1) - go func() { done <- r.m.sendPlanNotification(ctx, r.plan) }() // r.plan is the stale, unmanaged copy + go func() { done <- r.m.sendPlanNotification(ctx, r.plan, false) }() // r.plan is the stale, unmanaged copy testhelpers.WaitForLockWait(ctx, t, r.pool, "%FOR UPDATE%") command.SetManaged(ctx, t, r.plan.ID) require.NoError(t, command.Commit(ctx)) @@ -52,7 +52,7 @@ func TestClaimSkipsPlanTheCommandConvertedWhileItWaited(t *testing.T) { func TestCommandCaptureSeesRootTheClaimCommittedFirst(t *testing.T) { ctx := context.Background() r := newClaimRace(ctx, t) - require.True(t, r.m.sendPlanNotification(ctx, r.plan)) + require.True(t, r.m.sendPlanNotification(ctx, r.plan, false)) rows := stepRowsInDB(ctx, t, r.store, r.plan.ID) require.Len(t, rows, 1) diff --git a/internal/purchase/plan_step_retention_integration_test.go b/internal/purchase/plan_step_retention_integration_test.go index f1ea65cd4..30b531495 100644 --- a/internal/purchase/plan_step_retention_integration_test.go +++ b/internal/purchase/plan_step_retention_integration_test.go @@ -167,7 +167,7 @@ func TestCumulativeAttach_UntrustedHistoryCannotSizeOrNotify(t *testing.T) { require.NoError(t, f.pool.QueryRow(f.ctx, `SELECT COUNT(*) FROM purchase_suppressions WHERE account_id = 'other-acct'`).Scan(&others)) require.Equal(t, 1, others, "another execution's suppressions are untouched") } - require.False(t, f.m.sendPlanNotification(f.ctx, f.plan())) + require.False(t, f.m.sendPlanNotification(f.ctx, f.plan(), false)) require.Zero(t, calls, "no new sizing or resizing") require.Zero(t, f.m.email.(*countingSender).sends) require.Zero(t, suppressedTotal(f.ctx, t, f.pool))