Skip to content

Commit fcf50b2

Browse files
committed
fix(cli): make --purchase execute real purchases (--dry-run defaults false)
The --dry-run flag defaulted to true, so effectiveDryRun (!ActualPurchase || DryRun) stayed true even when --purchase was given -- meaning --purchase alone never executed real purchases, defeating the flag. Default --dry-run to false; the safe default is preserved because ActualPurchase defaults to false (a bare invocation is still dry-run). Passing --dry-run alongside --purchase still forces dry-run as an explicit safety override. Adds a regression test that fails on the prior true default and covers all four flag combinations.
1 parent 59db0f1 commit fcf50b2

3 files changed

Lines changed: 56 additions & 4 deletions

File tree

‎cmd/effective_dry_run_test.go‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
package main
2+
3+
import "testing"
4+
5+
// TestEffectiveDryRun documents the flag contract for real purchases and guards
6+
// the regression where `--purchase` alone silently stayed in dry-run because
7+
// `--dry-run` defaulted to true (effectiveDryRun = !ActualPurchase || DryRun,
8+
// so !true || true == true). The default for --dry-run is now false; safety is
9+
// preserved by ActualPurchase defaulting to false (a bare run is still dry-run).
10+
func TestEffectiveDryRun(t *testing.T) {
11+
tests := []struct {
12+
name string
13+
actualPurchase bool // --purchase
14+
dryRun bool // --dry-run (default false)
15+
want bool
16+
}{
17+
{name: "bare invocation is dry-run", actualPurchase: false, dryRun: false, want: true},
18+
{name: "--purchase executes real purchases", actualPurchase: true, dryRun: false, want: false},
19+
{name: "--purchase --dry-run forces dry-run (explicit safety override)", actualPurchase: true, dryRun: true, want: true},
20+
{name: "--dry-run without --purchase stays dry-run", actualPurchase: false, dryRun: true, want: true},
21+
}
22+
23+
for _, tt := range tests {
24+
t.Run(tt.name, func(t *testing.T) {
25+
got := effectiveDryRun(Config{ActualPurchase: tt.actualPurchase, DryRun: tt.dryRun})
26+
if got != tt.want {
27+
t.Errorf("effectiveDryRun(ActualPurchase=%v, DryRun=%v) = %v, want %v",
28+
tt.actualPurchase, tt.dryRun, got, tt.want)
29+
}
30+
})
31+
}
32+
}
33+
34+
// TestDryRunFlagDefaultIsFalse guards the actual fix: the --dry-run flag must
35+
// default to false. When it defaulted to true, `--purchase` alone resolved to
36+
// effectiveDryRun == true (!ActualPurchase || DryRun => !true || true), so real
37+
// purchases never executed. A bare run stays dry-run via ActualPurchase's own
38+
// false default, so flipping this default is safe.
39+
func TestDryRunFlagDefaultIsFalse(t *testing.T) {
40+
f := rootCmd.Flags().Lookup("dry-run")
41+
if f == nil {
42+
t.Fatal("--dry-run flag not registered")
43+
}
44+
if f.DefValue != "false" {
45+
t.Errorf("--dry-run default = %q, want \"false\" (so --purchase alone executes real purchases)", f.DefValue)
46+
}
47+
}

‎cmd/main.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ func init() {
134134

135135
// Purchase pipeline flags
136136
rootCmd.Flags().StringVar(&toolCfg.AuditLog, "audit-log", "./cudly-audit.jsonl", "Path to JSONL audit log file")
137-
rootCmd.Flags().BoolVar(&toolCfg.DryRun, "dry-run", true, "Dry-run mode: show what would be purchased without actually buying")
137+
rootCmd.Flags().BoolVar(&toolCfg.DryRun, "dry-run", false, "Force dry-run even when --purchase is set (runs are already dry-run unless --purchase is given)")
138138
rootCmd.Flags().StringVar(&toolCfg.IdempotencyWindow, "idempotency-window", "24h", "Lookback window for duplicate purchase detection")
139139
rootCmd.Flags().Float64Var(&toolCfg.MinSavingsPct, "min-savings-pct", 0, "Minimum savings percentage to include a recommendation (0 = no filter)")
140140
rootCmd.Flags().IntVar(&toolCfg.MaxBreakEvenMonths, "max-break-even-months", 0, "Maximum break-even period in months (0 = no filter)")

‎cmd/multi_service.go‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,9 +71,14 @@ func fetchExistingCoverage(ctx context.Context, awsCfg aws.Config, recClient pro
7171
// shutdownRequested is set to true when SIGINT is received during a purchase run.
7272
var shutdownRequested atomic.Bool
7373

74-
// effectiveDryRun returns true when either DryRun is explicitly set or
75-
// ActualPurchase is not enabled. Both the non-CSV and CSV code paths use
76-
// this helper so the guard is consistent and defined in one place.
74+
// effectiveDryRun reports whether the run must stay in dry-run mode. A run is
75+
// dry-run unless the user opts into real purchases with --purchase; passing
76+
// --dry-run additionally forces dry-run even alongside --purchase (an explicit
77+
// safety override). --dry-run defaults to false so that --purchase alone
78+
// actually executes purchases; the safe default is preserved because
79+
// ActualPurchase defaults to false, so a bare invocation is still dry-run.
80+
// Both the non-CSV and CSV code paths use this helper so the guard is
81+
// consistent and defined in one place.
7782
func effectiveDryRun(cfg Config) bool {
7883
return !cfg.ActualPurchase || cfg.DryRun
7984
}

0 commit comments

Comments
 (0)