fix(cli): remove --dry-run flag; --purchase alone executes real purchases by cristim · Pull Request #1485 · LeanerCloud/cloud-commitments-cli

cristim · 2026-07-22T20:58:08Z

Problem

The --purchase flag never executed real purchases when used alone. --dry-run defaulted to true, and the guard is:

func effectiveDryRun(cfg Config) bool {
	return !cfg.ActualPurchase || cfg.DryRun
}

So --purchase (ActualPurchase=true) with the default DryRun=true gave !true || true == true — a dry run. Users had to pass --purchase --dry-run=false to actually buy, which is unintuitive and defeats the flag. (Surfaced by CodeRabbit on #1364.)

Fix (updated: remove the flag entirely)

The original version of this PR flipped --dry-run's default to false. On review that was rejected as still confusing: a flag literally named --dry-run that defaults off is a footgun, and its only remaining use would be a redundant "force dry-run even with --purchase" override that muddied the contract.

Instead, --dry-run is removed entirely. --purchase is now the single purchase control:

func effectiveDryRun(cfg Config) bool {
	return !cfg.ActualPurchase
}

Flags	Result
(none)	✅ dry-run
`--purchase`	▶ real purchase (still gated by the `--yes` / interactive confirmation prompt)

A bare run is always a dry run; --purchase is the one and only opt-in that moves money. This also unifies the two code paths — cloud-fetch mode and --input-csv mode now behave identically (isDryRun = !ActualPurchase), which is exactly what the CSV path was already documented to do.

Docs

docs/cli/purchase-safety.md and docs/cli/README.md are rewritten for the single-flag model, with a History note explaining why --dry-run was removed. The stale --purchase --dry-run=false examples are corrected to --purchase.

Tests

cmd/effective_dry_run_test.go:

TestEffectiveDryRun — asserts the two-state contract (bare → dry-run, --purchase → real).
TestDryRunFlagRemoved — replaces the old default-value guard; fails if the --dry-run flag is ever reintroduced.

go build ./cmd/..., go vet ./cmd/... clean; gocyclo -over 10 clean; golangci-lint (CI-pinned v2.10.1) = 0 issues; go test ./cmd/ passing.

Summary by CodeRabbit

New Features
- Added a simplified purchase workflow: commands run in dry-run mode by default, while --purchase explicitly enables real purchases.
- Preserved confirmation safeguards for real purchases, including the --yes requirement.
Bug Fixes
- Prevented duplicate registration and conflicting behavior from the legacy --dry-run option.
Documentation
- Updated CLI usage, purchase safety guidance, examples, and dry-run review instructions to reflect the streamlined controls.

cristim · 2026-07-22T20:58:13Z

@coderabbitai review

coderabbitai · 2026-07-22T20:58:16Z

📝 Walkthrough

Walkthrough

The CLI purchase contract now treats dry-run as the default, with real purchases enabled through --purchase. Configuration, helper documentation, tests, and purchase-safety documentation are updated accordingly.

Changes

Single-flag purchase contract

Layer / File(s)	Summary
CLI dry-run behavior and validation `cmd/main.go`, `cmd/multi_service.go`, `cmd/effective_dry_run_test.go`	The configuration and dry-run handling are updated, helper documentation describes the purchase rules, and tests verify effective behavior and flag registration.
Purchase safety documentation `docs/cli/README.md`, `docs/cli/purchase-safety.md`	CLI guidance and examples describe `--purchase` as the purchase opt-in across supported modes, retain `--yes` confirmation, and update dry-run checklist instructions.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: removing --dry-run and making --purchase the sole real-purchase switch.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/purchase-dry-run-default

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai · 2026-07-22T20:58:19Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

…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.

cristim · 2026-07-22T22:53:12Z

@coderabbitai review

coderabbitai · 2026-07-22T22:53:18Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

…ases The `--purchase` flag never bought anything on its own because `--dry-run` defaulted to true and the guard was `!ActualPurchase || DryRun`, so `--purchase` gave `!true || true == true` (a dry run). Users had to pass `--purchase --dry-run=false` to actually buy. (Surfaced by CodeRabbit on #1364.) Rather than flip `--dry-run`'s default to false (which leaves a confusing flag named `--dry-run` that defaults off and only acts as a redundant "force dry-run even with --purchase" override), remove the flag entirely. `--purchase` is now the single purchase control: effectiveDryRun(cfg) = !cfg.ActualPurchase A bare run is always a dry run; `--purchase` is the one opt-in that moves money (still gated by the `--yes` / interactive confirmation prompt). This also unifies the two code paths: cloud-fetch and `--input-csv` mode now behave identically, matching what the CSV path already documented. Docs (purchase-safety.md, cli/README.md) are updated to the single-flag model with a History note explaining the removal. Tests replace the default-value guard with `TestDryRunFlagRemoved`, which fails if the flag is ever reintroduced.

cristim · 2026-07-23T15:35:26Z

@coderabbitai review

coderabbitai · 2026-07-23T15:35:35Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

cristim · 2026-07-23T15:42:50Z

@coderabbitai full review

coderabbitai · 2026-07-23T15:42:58Z

✅ Action performed

Full review finished.

coderabbitai

🧹 Nitpick comments (1)

cmd/effective_dry_run_test.go (1)
5-30: 🎯 Functional Correctness | 🔵 Trivial | 🏗️ Heavy lift

Test the CLI and CSV boundaries, not only the helper.

This test constructs Config directly, so it would pass even if --purchase stopped populating ActualPurchase or the CSV handler bypassed effectiveDryRun. Add focused command-binding and CSV-path tests with the purchase boundary mocked.

Based on learnings, dry-run behavior must be verified consistently across CSV and non-CSV execution paths.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cmd/effective_dry_run_test.go` around lines 5 - 30, Extend
TestEffectiveDryRun coverage beyond direct Config construction by testing CLI
argument binding for --purchase and both CSV and non-CSV execution paths. Mock
the purchase boundary, verify bare runs remain dry-run, and verify --purchase
reaches real purchases consistently without the CSV handler bypassing
effectiveDryRun.
Source: Learnings

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@cmd/effective_dry_run_test.go`:
- Around line 5-30: Extend TestEffectiveDryRun coverage beyond direct Config
construction by testing CLI argument binding for --purchase and both CSV and
non-CSV execution paths. Mock the purchase boundary, verify bare runs remain
dry-run, and verify --purchase reaches real purchases consistently without the
CSV handler bypassing effectiveDryRun.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b18467a0-d3ac-46cd-a606-5b2b2b069339

📥 Commits

Reviewing files that changed from the base of the PR and between 59db0f1 and 238812f.

📒 Files selected for processing (5)

cmd/effective_dry_run_test.go
cmd/main.go
cmd/multi_service.go
docs/cli/README.md
docs/cli/purchase-safety.md

💤 Files with no reviewable changes (1)

cmd/main.go

cristim · 2026-07-23T15:53:13Z

Ready for human merge

Commit 238812f is green and clean.

CI: all 19 checks passing (Unit / Integration / E2E Tests, Lint Code, gosec, Trivy, Snyk, Security Scanning, Build Docker Image, AWS+Azure sanity dry-runs, all 3 Terraform validations, IAM/RBAC parity). No failures.

CodeRabbit: clean. The earlier incremental pass was rate-limited (its walkthrough still described the prior default-flip approach), so a @coderabbitai full review was forced. The completed full review now reflects the actual change (single-flag purchase contract, --dry-run removed) with zero actionable findings and no inline comments.

Not self-merging or self-approving; leaving for a human to merge.

coderabbitai · 2026-07-23T15:53:23Z

Tip

For best results, initiate chat on the files or code changes.

@cristim Confirmed. The full review is clean, and the stated CI coverage supports the updated single-flag purchase contract. This is ready for human review and merge. 🐇

…ases (#1485) * 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. * fix(cli): remove --dry-run flag; --purchase alone executes real purchases The `--purchase` flag never bought anything on its own because `--dry-run` defaulted to true and the guard was `!ActualPurchase || DryRun`, so `--purchase` gave `!true || true == true` (a dry run). Users had to pass `--purchase --dry-run=false` to actually buy. (Surfaced by CodeRabbit on #1364.) Rather than flip `--dry-run`'s default to false (which leaves a confusing flag named `--dry-run` that defaults off and only acts as a redundant "force dry-run even with --purchase" override), remove the flag entirely. `--purchase` is now the single purchase control: effectiveDryRun(cfg) = !cfg.ActualPurchase A bare run is always a dry run; `--purchase` is the one opt-in that moves money (still gated by the `--yes` / interactive confirmation prompt). This also unifies the two code paths: cloud-fetch and `--input-csv` mode now behave identically, matching what the CSV path already documented. Docs (purchase-safety.md, cli/README.md) are updated to the single-flag model with a History note explaining the removal. Tests replace the default-value guard with `TestDryRunFlagRemoved`, which fails if the flag is ever reintroduced.

cristim added type/bug Defect priority/p1 Next up; this sprint severity/high Significant harm urgency/this-sprint Within the current sprint impact/all-users Affects every user effort/xs Trivial / one-liner triaged Item has been triaged labels Jul 22, 2026

cristim force-pushed the fix/purchase-dry-run-default branch from c9c49d6 to fcf50b2 Compare July 22, 2026 22:53

cristim changed the title ~~fix(cli): make --purchase execute real purchases (--dry-run defaults false)~~ fix(cli): remove --dry-run flag; --purchase alone executes real purchases Jul 23, 2026

coderabbitai Bot reviewed Jul 23, 2026

View reviewed changes

cristim merged commit 5f134a0 into main Jul 23, 2026
19 checks passed

cristim deleted the fix/purchase-dry-run-default branch July 27, 2026 10:00

Conversation

cristim commented Jul 22, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Fix (updated: remove the flag entirely)

Docs

Tests

Summary by CodeRabbit

Uh oh!

cristim commented Jul 22, 2026

Uh oh!

coderabbitai Bot commented Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

coderabbitai Bot commented Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cristim commented Jul 22, 2026

Uh oh!

coderabbitai Bot commented Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cristim commented Jul 23, 2026

Uh oh!

coderabbitai Bot commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cristim commented Jul 23, 2026

Uh oh!

coderabbitai Bot commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

cristim commented Jul 23, 2026

Ready for human merge

Uh oh!

coderabbitai Bot commented Jul 23, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

cristim commented Jul 22, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 22, 2026 •

edited

Loading

coderabbitai Bot commented Jul 22, 2026 •

edited

Loading

coderabbitai Bot commented Jul 22, 2026 •

edited

Loading

coderabbitai Bot commented Jul 23, 2026 •

edited

Loading

coderabbitai Bot commented Jul 23, 2026 •

edited

Loading