fix(cmd): repair red main after #2238 and drop the RI hint on SP-only runs

[IAmJSD]

Closes #2242. Follow-up to #2238 (issue #2157). There are two commits on main (4ed7248).

1. Fix red main (test(csv))

#2238 made a real run with any failed purchase return errPurchasesFailed, so the CLI exits non-zero. #2230 merged at about the same time, and its TestCSVRealPurchase_FailedPurchaseCarriesError still did require.NoError(runCSVPurchases(...)) while forcing a failed ElastiCache purchase. So go test ./cmd/ fails on main.

The shared csvPurchaseEnv gets an expectPurchaseFailure flag. When it's set, run requires errors.Is(err, errPurchasesFailed); otherwise it still requires no error. The failed-purchase test sets it. This is test-only, and the per-row report and audit assertions are unchanged.

2. CodeRabbit finding on #2238 (fix(cmd))

This commit is 9c24083, cherry-picked unchanged. It was pushed to the #2238 branch after that PR merged.

printFinalMessage(isDryRun, riSuccess, success, failed) prints "Allow up to 15 minutes for RIs to appear" only when an RI purchase succeeded. The completion line still prints for any success.

TestPrintMultiServiceSummary_RIHintOnlyAfterRISuccess covers five cases:

• Savings Plans only: no hint.
• A Savings Plan success plus a failed RI: no hint.
• RIs only: hint.
• RIs and Savings Plans: hint.
• A dry run: unchanged.

Before the fix, the two Savings Plan cases failed. Three mutation probes were all caught.

Verification

Tests ran on macOS with AWS config set to /dev/null, using mocks and the offline HTTP fake. Nothing called real AWS, and nothing ran with --yes.

• Before: on main (4ed7248), go test ./cmd/ -count=1 fails with TestCSVRealPurchase_FailedPurchaseCarriesError: "Received unexpected error".
• After: on this branch, go test ./cmd/ -count=1 passes.
• go build ./cmd passes, go vet ./... is clean, and golangci-lint v2.10.1 reports 0 issues.

Maintainer: please copy the labels from #2157.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Reserved Instance guidance now appears only after a successful Reserved Instance purchase. Savings Plan-only results and failed Reserved Instance purchases no longer display the hint.
  • Purchase summaries continue to reflect combined Reserved Instance and Savings Plan results. Dry-run summaries show purchase guidance instead of a purchase-completion message.
• Tests
  • Added coverage for purchase-failure handling and the conditions that control Reserved Instance guidance.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: LeanerCloud/cloud-commitments-cli/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: 733fe879-437b-410c-be4b-8940cff36669

📥 Commits

Reviewing files that changed from the base of the PR and between 4ed7248 and c36842a.

📒 Files selected for processing: 4

• cmd/csv_real_purchase_test.go
• cmd/multi_service_stats.go
• cmd/multi_service_stats_test.go
• cmd/purchase_failure_exit_2157_test.go

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

The changes update the CSV purchase test helper to expect errPurchasesFailed in the failed-purchase case. They also make the RI availability hint depend on successful RI purchases, with tests for summary output across purchase and dry-run cases.

Changes: CSV purchase failure test

Layer / File(s)	Summary
CSV failure expectation cmd/csv_real_purchase_test.go	The test helper can expect errPurchasesFailed. The failed-purchase test enables this expectation.

RI summary hint

Layer / File(s)	Summary
RI-specific summary output cmd/multi_service_stats.go, cmd/multi_service_stats_test.go, cmd/purchase_failure_exit_2157_test.go	printFinalMessage receives RI successes separately from combined purchase counts. It prints the RI hint only when an RI succeeds. Tests cover RI and Savings Plan results, failed RIs, and dry runs.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: cristim

Merge Risk: ⚪ Minimal · up to c3684

The CSV test expectation and RI hint change appear ready to merge after normal checks.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 4 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title accurately identifies both main changes: repairing the failing test on main after #2238 and removing the RI hint from Savings Plans-only runs.
Linked Issues check		Issue #2242 has two coding requirements. The test changes set expectPurchaseFailure for TestCSVRealPurchase_FailedPurchaseCarriesError and require errors.Is(err, errPurchasesFailed), which resol…
Out of Scope Changes check		The reviewed changes are limited to the two objectives in issue #2242. They update the CSV purchase test helper and its failed-purchase test, adjust RI hint logic, and add or update focused tests. The…

Full details: Linked Issues check

Explanation

Issue #2242 has two coding requirements. The test changes set expectPurchaseFailure for TestCSVRealPurchase_FailedPurchaseCarriesError and require errors.Is(err, errPurchasesFailed), which resolves the expected-error mismatch. The production change passes RI success separately to printFinalMessage and prints the propagation hint only when an RI succeeds. The added tests cover Savings Plans-only, failed RI, successful RI, combined purchases, and dry-run cases.

Full details: Out of Scope Changes check

Explanation

The reviewed changes are limited to the two objectives in issue #2242. They update the CSV purchase test helper and its failed-purchase test, adjust RI hint logic, and add or update focused tests. These changes support the linked issue. No unrelated implementation change is shown.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

[cristim]

Independent review verdict: CLEAN at c36842a

Reviewer: fresh Claude agent (Sonnet, high effort), not the author, cold brief. The PR body was treated as a claim. Coordinator: claude-cc-reviewer.

Blocking findings: none.

Main is red: on 4ed7248, go test -count=1 ./cmd/... fails exactly one test, TestCSVRealPurchase_FailedPurchaseCarriesError. On this head it passes, and nothing else in ./cmd fails.

Commit 1 (test-only):

• In cmd/csv_real_purchase_test.go:138-143, an unset expectPurchaseFailure still requires NoError. A set flag requires ErrorIs(err, errPurchasesFailed), which is specific, not just any error.
• Only the failed-purchase test sets it (:229).
• Its per-row report assertions are intact (:231-246): the failed row carries Success=false and the error text.

Commit 2:

• The RI hint prints only when riSuccess > 0 (cmd/multi_service_stats.go:223-225, where riSuccess is riAggregates.success, :106).
• The dry-run branch returns earlier, so nothing else in the output changes.
• TestPrintMultiServiceSummary_RIHintOnlyAfterRISuccess covers SP-only, SP success with RI failed, RI-only, mixed and dry-run.

What was attacked and how (AWS_CONFIG_FILE=/dev/null, no purchases):

• go vet ./cmd/... is clean. go test -race -count=1 ./cmd/... passes 1537.
• Mutation probes:
  • Changing the condition to success > 0 fails the SP-only cases.
  • Inverting it to riSuccess == 0 fails the new test.
  • Dropping the flag brings back the original red.

Gate:

• Labels are mirrored from test: main fails TestCSVRealPurchase_FailedPurchaseCarriesError after #2238 + #2230; and SP-only runs print the RI hint #2242, which I triaged as p1 because it fixes a red main.
• Merge once CI is green at this SHA.