fix(aws/recs): honor canceled context on RateLimiter.Wait first attempt

[IAmJSD]

Root cause

RateLimiter.Wait (providers/aws/recommendations/ratelimiter.go) returned nil right away when retryCount == 0, without checking ctx.Err(). The context was only checked inside the backoff select, which runs on retries. So a context canceled before the first call went straight on to concurrency.Acquire and the Cost Explorer SDK call. When no shared semaphore is attached to ctx, concurrency.Acquire does nothing, and the SDK call ran on a dead context.

Fix

Wait now returns ctx.Err() before the first-attempt fast path. This is a 3-line change. All seven callers (fetchRIPageWithRetry, SP, coverage, utilization, on-demand series and the two SP coverage/utilization loops) already wrap and return the Wait error, so this needs no caller changes. Retry state is not touched.

Closes #16

Verification

All of this is unit/mock evidence. No real AWS calls were made.

New tests in providers/aws/recommendations/ratelimiter_test.go:

• TestWait_FirstAttemptHonorsCanceledContext: a fresh limiter (retryCount == 0) with a canceled context returns context.Canceled, and with an expired deadline returns context.DeadlineExceeded. The retry count stays at 0.
• TestFetchRIPageWithRetry_PreCanceledContextSkipsCostExplorer: caller-level check with no semaphore on ctx. A pre-canceled ctx returns context.Canceled, and the mock Cost Explorer records zero GetReservationPurchaseRecommendation calls.

Fail-before (test commit on top of unmodified ratelimiter.go):

$ cd providers/aws && go test ./recommendations/ -run 'TestWait_FirstAttemptHonorsCanceledContext|TestFetchRIPageWithRetry_PreCanceled' -count=1
--- FAIL: TestWait_FirstAttemptHonorsCanceledContext/canceled
        Error: Expected error with "context canceled" in chain but got nil.
--- FAIL: TestWait_FirstAttemptHonorsCanceledContext/deadline_exceeded
        Error: Expected error with "context deadline exceeded" in chain but got nil.
--- FAIL: TestFetchRIPageWithRetry_PreCanceledContextSkipsCostExplorer
        Error: Expected error with "context canceled" in chain but got nil.
FAIL

Pass-after:

--- PASS: TestWait_FirstAttemptHonorsCanceledContext/canceled
--- PASS: TestWait_FirstAttemptHonorsCanceledContext/deadline_exceeded
--- PASS: TestFetchRIPageWithRetry_PreCanceledContextSkipsCostExplorer
ok  github.com/LeanerCloud/cloud-commitments-go/providers/aws/recommendations

Other checks (macOS):

• cd providers/aws && go build ./... && go vet ./... && GOWORK=off go test -race -short -count=1 ./... passes for every package.
• golangci-lint run ./recommendations/... (v2.10.1): 0 issues.
• gocyclo -over 10 providers/aws/recommendations/ratelimiter.go: no output.
• gofmt -l: clean.
• gosec and pre-commit are not installed locally, so the gosec hook was left to CI.

All tests ran with AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null, because the default AWS profile on this machine is invalid. The tests need no credentials.

Real-cloud gap: this was not exercised against live Cost Explorer. The change only adds a local context check that runs before any SDK call.

Out of scope

• The existing tests that use a pre-canceled ctx (for example concurrency_cap_test.go and TestFetchCoveragePaged_CtxCancelReturnsError) still pass unchanged. Their error now comes from the rate limiter rather than from Acquire or the SDK mock. They still assert ErrorIs(context.Canceled), so they are not affected.

Maintainer: please mirror the issue's triage labels on this PR: triaged, impact/internal, priority/p3, severity/low, urgency/this-quarter, effort/s, type/bug.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 2 billable files and costs up to $0.50.}

Or wait 46 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Repository: LeanerCloud/cloud-commitments-go/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: 8e9cc5b0-4a8f-4f8a-bd29-48276f7103ad

📥 Commits

Reviewing files that changed from the base of the PR and between 740dfbd and 8ffe010.

📒 Files selected for processing: 2

• providers/aws/recommendations/ratelimiter.go
• providers/aws/recommendations/ratelimiter_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 adversarial review: CLEAN at 8ffe010fb2c804f6e105c3e5d13fd8a307e008c0 (claude-go-pr-merge-2, non-author reviewer). No blocking findings.

• Callers of Wait (client.go:232, collection_sp.go:129, coverage.go:393, sp_coverage.go:389, :478) wrap and return its error; none treats it as retryable or relied on Wait succeeding with a canceled ctx. retryCount untouched on the early return (asserted 0).
• Fail-before: with origin/main's ratelimiter.go, both TestWait_FirstAttemptHonorsCanceledContext subtests and TestFetchRIPageWithRetry_PreCanceledContextSkipsCostExplorer fail. Mutation: deleting the ctx.Err() guard fails the same tests.
• Non-blocking: only the RI path has a caller-level test; SP and coverage share the same Wait.
• Local verification (macOS, unit/mock, no AWS): (cd providers/aws && go test -race -short ./services/rds/... ./recommendations/...) pass.

Still needed before merge: owner approval of the fork CI runs and green CI on this SHA, and the author marking the PR ready. Labels mirrored from the linked issue. CodeRabbit skipped this draft; CR skipped: quota/draft, so the merge comment will carry CR waived: quota, adversarial review + local verification + green CI once CI is green.