feat(aws/recommendations): selectable account scope for Savings Plan recommendations (step 2 of 2)

[cristim]

Closes #356. Step 2 of 2 (step 1: #357, merged as cdcdc7f).

What

getSavingsPlansRecommendations (providers/aws/recommendations/collection_sp.go) requested AccountScope=LINKED unconditionally. It now reads RecommendationParams.SPAccountScope: empty or linked -> LINKED (unchanged default), payer -> PAYER (case-insensitive). Any other value is an error before any API call, never a silent LINKED fallback. providers/aws/go.mod bumps the pkg pin to cdcdc7f (GOWORK=off GOPROXY=direct go get ...pkg@cdcdc7f, then go mod tidy).

Evidence

parser_sp_account_scope_test.go: TestSPAccountScope (empty/linked/payer/PAYER -> LINKED/LINKED/PAYER/PAYER, recorded from the SDK request) and TestSPAccountScopeRejectsUnknown (org errors, zero requests sent). Without the collection_sp.go change: FAIL (expected "PAYER" actual "LINKED"; unknown value returned no error). With it: PASS. Standalone check: cd providers/aws && GOWORK=off go vet ./... && GOWORK=off go test -short ./... passes; GOWORK=off go build ./... passes in providers/azure, providers/gcp and ci_cd_sanity_tests.
Real account (read-only CE call, earlier): PAYER Compute SP 1yr no-upfront 30d returns one rec, $2.787/hr, $166.80/mo, 3.2%; CLI end-to-end is in cli#2185.

Next: cli#2185 bumps its pins to this merge commit.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 3 billable files and costs up to $0.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 38 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 61 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-go/.coderabbit.yaml
• Review profile: CHILL
• Plan: Essentials
• Run ID: 7c46355e-46fd-4733-9ea1-8334e5f1bbd1

📥 Commits

Reviewing files that changed from the base of the PR and between cdcdc7f and 80b9241.

⛔ Files ignored due to path filters (1)

• providers/aws/go.sum is excluded by !**/*.sum

📒 Files selected for processing (3)

• providers/aws/go.mod
• providers/aws/recommendations/collection_sp.go
• providers/aws/recommendations/parser_sp_account_scope_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 80b9241: APPROVE

Claims checked

• Pin: providers/aws/go.mod now requires pkg v0.0.0-20261013182117-cdcdc7fbcc32. cdcdc7f is the merge commit of feat(common): add RecommendationParams.SPAccountScope (step 1 of 2) #357 (gh pr view 357 mergeCommit), and its commit time 2026-10-13T18:21:17Z matches the pseudo-version timestamp. The branch contains current main.
• go.sum: only the two pkg lines changed. With GOWORK=off, go mod verify reports "all modules verified" and go mod tidy -diff is clean. The /go.mod hash is unchanged from the previous pin, as expected since pkg/go.mod did not change.
• Scope: the diff is go.mod, go.sum, collection_sp.go (+19/-1) and the new test file. Nothing else. The RI path (recommendations/client.go:153) still requests LINKED, which is correct for a field named SP*.
• Behaviour (collection_sp.go:36-39, 64-72): the scope is resolved once before the plan-type loop, so an invalid value errors before any Cost Explorer call. Empty and linked map to LINKED, payer maps to PAYER, case-insensitive, and anything else errors.

Non-blocking

• collection_sp.go:38 returns the error unwrapped, while the sibling converters wrap with invalid ... for Savings Plans recommendation: %w. The message is already self-describing, so this is cosmetic.
• No TrimSpace, so " payer" errors. That is the right fail-loud choice; the CLI should trim if it ever needs to.

Evidence (fresh clone, this SHA)

• cd providers/aws && GOWORK=off go vet ./...: clean. GOWORK=off go test -count=1 -short ./...: no failures. gofmt clean.
• GOWORK=off go build ./... passes in pkg, providers/azure, providers/gcp and ci_cd_sanity_tests.
• Fail-before (main's collection_sp.go, new tests kept): TestSPAccountScope/payer and /PAYER fail with expected "PAYER" actual "LINKED". TestSPAccountScopeRejectsUnknown fails with "An error is expected but got nil".
• Mutations, each FAILS:
  • ignore the param (hardcode LINKED): payer and PAYER fail;
  • accept an unknown value as LINKED: RejectsUnknown fails;
  • case-sensitive switch: /PAYER fails.
• Real-account check (cli#2185 with --sp-account-scope payer against the AWS account): not run by this reviewer. Library-level evidence only.
• CI at review time: lint (all modules), pre-commit, Snyk and CodeRabbit pass. Unit Tests, Integration Tests and Security Scanning were still running. Confirm they are green on this SHA before merging.