[cristim]

Closes #2235
Closes #2213 (the aligned Savings Plans precheck rejects NaN and ±Inf commitments, which TestValidatePurchasePreconditions_CommitmentBounds covers)

What

A --target-coverage run could recommend re-buying replacement RIs that were bought inside the Cost Explorer coverage lookback, which spends money twice. go#449 (go commit 512e42fe) added AdjustExistingCoverageForExpiringCommitmentsWithCoverageOptions, which credits those replacements. This PR adopts it.

• cmd/multi_service_helpers.go calls the new function with PoolExpiryOptions{WindowDays: cfg.RebuyWindowDays, CoverageLookbackDays: coverageLookbackDays(cfg)}. coverageLookbackDays (cmd/multi_service.go:1086) is the same value that already feeds the coverage fetch (cmd/multi_service.go:56), so the credit window matches the data window.
• The providers/aws pin moves from 9b35837e to 512e42fe37c6. pkg stays at main's 570d168c6bc7; providers/aws@512e42fe needs only pkg 498b74f3, so pkg is not downgraded.

Other providers/aws changes pulled in by the bump (9b35837e..512e42fe)

These go commits are already merged in cloud-commitments-go: 5ee1f6c (SP commitment rates), 29adce1 (SP precision floor), 9511dcf (scoped EC2 rates), 8b27158 (ladder baselines), ca29272 (test isolation), 740dfbd (payment currency), 25bffb5 (exchange accept), 0e95e9f (RI exchange AZ filter), 53dfba9 (empty-token purchase) and 512e42f (the credit).

One existing CLI test asserted the old error wording, "AvailabilityZone"; the new error says "availability zone". The assertion is now case-insensitive. The fix is in the bump commit so that commit builds and passes on its own.

User-visible change from the bump: Savings Plans commitment amount

go 29adce1 changes how CreateSavingsPlan sends the hourly commitment. It used to be rounded to cents (%.2f); it is now truncated to at most 5 decimals, and the allowed range is 0.001 to 1,000,000. For example, 1.239 is now sent as 1.239, not 1.24, and 0.005 as 0.005, not 0.01. Truncation never rounds up, so there is no overspend. The CLI's own precheck runs before dry-run and before the confirmation prompt, without an AWS call. Commit 3 aligns it with the library's spcommitment.Normalize: it rejects values below 0.001 or above 1e6, NaN and Inf, and no longer rejects 0.001-0.004 with a stale message. The CHANGELOG records this. The purchase-results CSV's HourlyCommitment column (cmd/multi_service_csv_details.go:25) is filled from the recommendation after Normalize, so it now shows the truncated amount that was actually sent, for example 1.23456 instead of 1.2345678.

Commits (each <= 400 additions+deletions)

1. build(deps): bump providers/aws (9)
2. fix(aws): credit replacement RIs bought inside the coverage lookback (74)
3. fix(aws): align the Savings Plans commitment precheck with the library rule (43)

Local verification (macOS, this HEAD)

• go build -o cudly ./cmd, go vet ./..., go test ./... and golangci-lint (0 issues) all pass.
• TestReservationExpiryCommand/replaced-inside-lookback and TestReservationExpiryCreditsReplacementsInsideLookback fail with main's old helper and pass after the change.
• Mutation probe: with CoverageLookbackDays: 0, the command test and the replaced-inside-lookback and default-lookback subtests fail.
• Mocks only; no AWS calls.

Independent reviews follow as PR comments.

🤖 Generated with Claude Code

[cristim]

@coderabbitai review

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 8 billable files and costs up to $2.00.}

• 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 36 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 88 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: d0f0c72f-0b45-4204-9aec-61f742e9cd91

📥 Commits

Reviewing files that changed from the base of the PR and between 83a7193 and 59b42a9.

⛔ Files ignored due to path filters: 1

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

📒 Files selected for processing: 8

• CHANGELOG.md
• cmd/ec2_zonal_az_test.go
• cmd/multi_service_helpers.go
• cmd/reservation_expiry_command_test.go
• cmd/reservation_expiry_test.go
• cmd/savings_plan_purchase.go
• cmd/savings_plan_purchase_test.go
• go.mod

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

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

[coderabbitai]

⚠️ Action not completed

Review rate limited.

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]

Independent review A (claude-cc-libs-reviewA-2273, Sonnet, cold brief, not the author): money path

Reviewed SHA: 3082f64
Verdict: APPROVE. No blockers.

Local verification (scratch worktree at this SHA, macOS): go build ./cmd, go vet ./..., go test ./... and golangci-lint v2.10.1 (0 issues) all pass.

What was attacked:

1. Lookback consistency.
   • The only coverage fetch is cmd/multi_service.go:56,69, GetRICoverageMap(ctx, coverageLookbackDays(cfg), regions). The new call at multi_service_helpers.go:596-598 passes the same coverageLookbackDays(cfg) (multi_service.go:1086, which maps <=0 to 30).
   • The flag defaults to 30 (main.go:154), and validators.go:109 rejects values below 1. No per-service or config override exists.
   • Both consumers, the pool path and the existing RDS family path at :619-620, use the same helper.
2. Semantics against go@512e42fe (providers/aws/recommendations/expiry.go): both fields are int days and are converted from a single time.Now(). The boundary is exclusive (StartDate.After(lagStart)), and the CLI does no date arithmetic of its own. go.mod bumps only providers/aws.
3. Other callers: no non-test caller of the old function remains in the CLI.
4. Tests: TestReservationExpiryCommand/replaced-inside-lookback runs the real CLI subprocess, so it goes through root dispatch. The unit table covers inside the window, unset (default 30), outside the window, and a lookback narrower than the purchase age.

Fail-before evidence: with main's multi_service_helpers.go and the new tests, TestReservationExpiryCommand/replaced-inside-lookback, TestReservationExpiryCreditsReplacementsInsideLookback/replaced-inside-lookback and .../default-lookback-when-unset fail.

Mutation probes (all caught):

• Hardcoded 30 days.
• CoverageLookbackDays: 0.
• Raw cfg.CoverageLookbackDays instead of the helper.

Nits (non-blocking):

• Add a command-level case with --coverage-lookback-days set. The unit test already catches a hardcoded window.
• The ec2_zonal_az_test.go assertion change is already explained in the PR body.

[cristim]

Independent review B (claude-cc-libs-reviewB-2273, Sonnet, cold brief, not the author, independent of review A): blast radius of the bump

Reviewed SHA: 3082f64
Verdict: APPROVE, with finding 1 to be addressed on this PR.

Local verification (scratch worktree at this SHA): go mod tidy -diff is clean, and go build ./cmd, go vet ./..., go test ./... and golangci-lint (0 issues) all pass. go list -m confirms pkg 570d168c is selected, so pkg is not downgraded (providers/aws@512e42f needs only 498b74f).

Mutation probes:

• CoverageLookbackDays: 0 fails the command and unit cases.
• Using cfg.RebuyWindowDays as the lookback fails lookback-narrower-than-purchase.
• The old API's only call site (:596) is replaced.

Pulled-in go commits, and whether each affects the CLI:

Commit	Change	Affects CLI?
512e42f	expiry credit	yes, intended and tested
53dfba9	single attempt plus ErrOutcomeUnknown on empty token (RDS/ElastiCache/MemoryDB/OpenSearch clients)	safe direction; the CLI never matches on ErrOutcomeUnknown
25bffb5	comment only	no
740dfbd	CommitmentCurrency field	no; the CLI does not read it
0e95e9f	lowercase "availability zone" error text	no; this is why ec2_zonal_az_test.go:94 changed, and assert.Empty(recorder.actions) still proves no AWS call happens before the refusal
29adce1	SP precision floor	yes, see finding 1
5ee1f6c, 9511dcf, 8b27158, ca29272	ladder and rate additions	no; the CLI does not import them

Findings:

1. cmd/savings_plan_purchase.go:20-24,152-153 and cmd/savings_plan_purchase_test.go:303-316 | low-medium
   • Issue: 29adce1 changes CreateSavingsPlan from %.2f rounding to floor truncation, up to 5 decimals, with a 0.001 minimum. The CLI's mirrored precheck and its message ("would be sent as 0.00; it must be at least 0.005") are now stale: it rejects 0.001-0.004, which the library accepts. The amount sent also changes for users (0.005 is sent as "0.005" instead of "0.01", and 1.239 as "1.239" instead of "1.24"). There is no overspend, because the new rule floors.
   • Fix: align the precheck, its message, comment and test with the new rule, and disclose the change in the CHANGELOG and PR body.

[cristim]

Review B re-review (claude-cc-libs-reviewB-2273): delta 3082f64..59b42a9

Reviewed SHA: 59b42a9
Verdict: APPROVE

Local verification at this SHA: go test ./... passes and golangci-lint reports 0 issues.

Evidence:

• The bounds at cmd/savings_plan_purchase.go:27-28 and :154-158 match spcommitment.Normalize at go 512e42fe: 0.001..1e6, with NaN, Inf and values <= 0 rejected. spCommitmentFormat has no remaining references.
• The test covers both sides of each bound, plus NaN, Inf, 0 and negatives. Finding 1 (stale precheck) is resolved.

Low, docs only (handled): the purchase-results CSV HourlyCommitment column (multi_service_csv.go:341,373 via detailCells, multi_service_csv_details.go:25) shows the post-Normalize amount, which is the truncated value actually sent. This is safe because the amount is floored, and arguably more accurate. The PR body now discloses it, and the CHANGELOG line follows in the reviewers' follow-up PR.

[cristim]

Review A re-review (claude-cc-libs-reviewA-2273): delta 3082f64..59b42a9, money path

Reviewed SHA: 59b42a9
Verdict: APPROVE

Local verification at this SHA: go test ./... passes and golangci-lint reports 0 issues.

Bounds against spcommitment.Normalize at go 512e42fe: Normalize rejects NaN, Inf, values <= 0, values < 0.001 and values > 1e6, so both ends of the range are inclusive. cmd/savings_plan_purchase.go:154-159 uses the same constants and operators. No amount is accepted by one side and rejected by the other, and truncation cannot push an accepted amount below 0.001.

Call sites: validatePurchasePreconditions runs per recommendation, before the dry-run branch, on both purchase paths (cmd/multi_service.go:620, :1010).

Boundary mutation: changing </> to <=/>= makes TestValidatePurchasePreconditions_CommitmentBounds fail.

Nit, existing and not introduced here: the precheck runs after confirmPurchaseRun (multi_service.go:247), not before it. An invalid amount is still refused before any purchase call.

[cristim]

@coderabbitai review

[coderabbitai]

⚠️ Action not completed

Review rate limited.

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]

@coderabbitai review

[coderabbitai]

⚠️ Action not completed

Review rate limited.

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]

Merge gate verdict (claude-cc-libs)

Every gate holds at head 59b42a9:

• Two independent reviews approve the reviewed head 59b42a9. The branch was then updated from main, and the PR diff is byte-identical to the reviewed diff (compared mechanically).
• CI is green on 59b42a9, which is the reviewed change on top of current main.
• 0 unresolved threads; MERGEABLE/CLEAN.

CodeRabbit is waived as a merge gate under the 2026-10-19 decision. Squash-merging.