fix(cmd): discover recommendation regions from Cost Explorer

[cristim]

Closes #2189

Stacked on #2192 (base branch fix/2190-fetch-fail-loud): both change the region/fetch path and the completeness harness. Retarget to main once #2192 merges; until then the diff against main also shows #2192's commit.

What

For non-Savings-Plan services, a run without --regions iterated ec2:DescribeRegions (the caller's opted-in regions). Cost Explorer also returns linked-account recommendations in regions the payer has not opted into, and the per-region filter never saw them.

discoverRegionsForService now derives the regions from the Cost Explorer recommendations for the run's term, payment option and lookback (one extra Cost Explorer call per service), reusing fetchRecommendationsForRegion so incomplete and error handling is the same as the per-region fetch. determineRegionsForService takes Config instead of aws.Config + regions, and handleRegionDiscoveryError (the DescribeRegions-failure fallback) is gone. --regions behaviour is unchanged.

Not touched: the existing-RI coverage fetch for --target-coverage still lists regions with getAllAWSRegions (fetchExistingCoverage). The per-region fetch repeating the account-wide Cost Explorer call (efficiency) is the architect's item.

Tests

• TestDetermineRegionsForService_DiscoversRegionsTheCallerHasNotOptedInto and TestFetchAllRecs_DefaultRunKeepsRecommendationsInRegionsOutsideTheCallersOptIns use aws.Config{} with no credentials, so any DescribeRegions call would fail; on the old code they hit the DescribeRegions path.
• TestDetermineRegionsForService_CanceledContextIsTerminal replaces the handleRegionDiscoveryError cancel test.
• The end-to-end completeness harness drops the fallback region mode (no DescribeRegions any more) and now expects 1 Cost Explorer call for discovery plus 1 per discovered region, and zero DescribeRegions calls.
• go vet ./cmd and go test ./cmd pass (skipping TestQueryMajorEngineVersions_ErrorHandling, which hangs locally this session independent of this change). golangci-lint panics locally on this toolchain; pre-commit hooks pass.

Live dry run (read-only), shift-prd (payer )

cudly --services ec2 --term 1 --payment no-upfront --rec-lookback-period 30d --coverage 100 (no --regions).
Raw API, aws ce get-reservation-purchase-recommendation ... ONE_YEAR NO_UPFRONT THIRTY_DAYS LINKED: 25 recommendations (London 11, N. Virginia 7, Frankfurt 4, Zurich 2, Cape Town 1... per CE region name).

before (main 8ddb891): Found N recommendations summed over 23 regions = 23, no eu-central-2.
after: 5 regions discovered, 25 found, including

eu-central-2  ec2  m5.xlarge  1yr  1  $68.18
eu-central-2  ec2  m5.large   1yr  1  $34.09

(= $102.27/mo, the missing amount from the audit).

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 7 billable files and costs up to $1.75.}

• Ask an admin to make reviews automatic

Open in CodeRabbit

Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing.

Or wait 22 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available. Your 62 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Repository: LeanerCloud/cloud-commitments-cli/.coderabbit.yaml
• Review profile: CHILL
• Plan: Essentials
• Run ID: c3eab468-5fe4-4e7a-bc5d-8c0536593c96

📥 Commits

Reviewing files that changed from the base of the PR and between f2275a7 and 52b3b13.

📒 Files selected for processing: 7

• cmd/cancellation_prepurchase_test.go
• cmd/multi_service.go
• cmd/multi_service_helpers.go
• cmd/multi_service_helpers_test.go
• cmd/recommendation_completeness_proxy_test.go
• cmd/recommendation_completeness_test.go
• cmd/savings_plan_purchase_test.go

• 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 (claude-opus-5-5) of e543c34: APPROVE

Reviewed against its base fix/2190-fetch-fail-loud (c969c5f, #2192). The diff is cmd/multi_service.go and cmd/multi_service_helpers.go (+18/-40) plus test updates.

What changed, checked against the code

• determineRegionsForService (cmd/multi_service_helpers.go:314) no longer calls getAllAWSRegions (ec2:DescribeRegions on the caller). When --regions is unset, it uses discoverRegionsForService (:117). That function calls fetchRecommendationsForRegion(ctx, client, service, "", cfg), so the region set is derived with the run's own term (cfg.TermYears), payment (cfg.PaymentOption) and lookback (cfg.RecLookbackPeriod). Before, it used GetRecommendationsForService across all term and payment variants.
• Discovery errors go through fix(cmd): fail the run when recommendation fetching fails #2192's errRecommendationFetch path. Incomplete and Azure-partial answers still warn and continue.
• Savings Plans are unaffected: they return spAPIRegion before discovery.

Verification

• Fresh clone at the head: gofmt and go vet ./... are clean, and go test -count=1 ./... passes.

• Head merged with current main de71edc: git merge-tree reports no conflicts, and the merge builds.

• Real dry run against main de71edc (no --purchase; creds for the management account): --services ec2 --term 1 --payment no-upfront --rec-lookback-period 30d --coverage 100, no --regions.

  	main	PR
  Recommendations	23	25
  Regions processed	23 (the caller's DescribeRegions)	5 (those with recommendations)

  • aws ce get-reservation-purchase-recommendation --service "Amazon Elastic Compute Cloud - Compute" --term-in-years ONE_YEAR --payment-option NO_UPFRONT --lookback-period-in-days THIRTY_DAYS --account-scope LINKED returns 25 details.
  • The two rows only the PR shows are eu-central-2 m5.xlarge ($68.18/mo savings) and m5.large ($34.09/mo), $102.27/mo together, matching CE.

Cost Explorer call cost

The library filters by region on the client side (providers/aws/service_client.go:105-110), so each per-region call fetches the full set. Main made one full fetch per DescribeRegions region (23 here). The PR makes 1 discovery fetch plus 1 per region with recommendations (6 here), which is fewer CE requests in this account. In general the count is 1 + the number of regions with recommendations, instead of the number of enabled regions.

Non-blocking findings

1. --target-coverage without --regions still builds its coverage map from getAllAWSRegions (cmd/multi_service.go:58). The newly discovered regions get no coverage data, so the library drops their recommendations (recfilter/sizing.go target-coverage-unknown). This fails safe: they are never sized as 0% covered. Real run with --target-coverage 80: the eu-central-2 m5.large and m5.xlarge rows are dropped with that warning. So A1 is fixed for --coverage but not yet for --target-coverage. A follow-up could fetch coverage for the discovered regions (CE coverage is global, so the caller's opt-in set is not needed).
2. Since each per-region call refetches everything, the discovery result could be grouped by region and reused, making the whole run one CE fetch per service. That is optional; the request cost is small either way.
3. On an incomplete answer, the "incomplete AWS recommendations" warning is now printed once for discovery and again per region.

CI on e543c34: only CodeRabbit has run (pass). The main workflows have not run on this stacked base, so CI needs to run after the PR is retargeted to main.

[cristim]

Independent review (claude-opus-5-5) of bde52f9: APPROVE

This re-verifies the PR after the retarget to main. My earlier approval was of e543c34.

Delta from e543c34

• bde52f99 is a merge commit with parents e543c34 (the approved head) and f2275a7 (main). The range e543c34..bde52f9 contains only main's de71edc (feat(sp): show recurring monthly cost in the table and audit log (CLI side) #2186), f2275a7 (fix(cmd): fail the run when recommendation fetching fails #2192) and the merge itself.
• I redid the same merge locally. The only conflict is in cmd/recommendation_completeness_test.go, in TestRecommendationCompletenessHelpers.
• The author resolved it to the PR side, discoverRegionsForService(ctx, client, common.ServiceRDS, Config{TermYears: 1}). That is the only correct choice, because the main side calls the old 3-argument signature, which no longer exists.
• Diffing my auto-merged tree against bde52f9 shows nothing beyond that one hunk.

Verification at bde52f9

• gofmt and go vet ./... are clean, and go test -count=1 ./... passes.
• Real dry run (no --purchase; --services ec2 --term 1 --payment no-upfront --rec-lookback-period 30d --coverage 100, no --regions): 25 recommendations across 5 discovered regions, including the two eu-central-2 m5 rows. This matches Cost Explorer's 25 and the result at e543c34.

CI

No GitHub Actions workflow has run on bde52f9: gh run list --commit bde52f99... is empty, and only CodeRabbit is reported. CI has never run for this PR's code. A run needs to be triggered on this head, and must be green, before merge.

The earlier non-blocking findings still apply: the --target-coverage region set at cmd/multi_service.go:58, the possible reuse of the discovery fetch, and the duplicated incomplete warning.

[cristim]

Independent review (claude-opus-5-5) of 52b3b13: APPROVE

This re-verifies the PR after the lint fixes. I last approved bde52f9.

• The delta from bde52f9 is one commit, 52b3b13 (5 insertions, 6 deletions, test files only), containing exactly the three lint fixes:
  • regionMode is removed from assertCompletenessDiagnostics.
  • call is removed from completenessProxy.operation.
  • regions is preallocated in cmd/cancellation_prepurchase_test.go.
• At bde52f9, both removed parameters appeared only in their function signatures, never in the bodies, so no assertion was dropped.
• At the head, gofmt and go vet ./... are clean, and go test -count=1 ./... passes.
• CI on 52b3b13 is green: Unit Tests, Integration Tests, Lint Code and CI Success all pass, and gosec and Trivy are skipped.