[cristim]

What

A re-driven EC2 purchase adopts the RI tagged with its idempotency token. findRIByIdempotencyToken returned the first match, so if two active or payment-pending RIs carried the same token (a retry whose tag lookup missed the earlier RI, so it bought and tagged a second one) the re-drive reported the first and nothing told anyone a second commitment existed.

It now fails closed when more than one RI with an ID carries the token: no adoption, no purchase, and an error that wraps common.ErrRefusedBeforePurchase, names the count and the masked token, and says the commitment state is ambiguous and needs a human to reconcile. A single match still adopts, zero matches still buys, entries without an ID are ignored, retired RIs stay excluded by the state filter.

Decisions

• Option A of the decision on ec2: adopting a re-drive does not check for several RIs with the same idempotency token #724 (fail closed only on more than one match). Validating a single match against the recommendation is left out.
• ErrRefusedBeforePurchase, not ErrOutcomeUnknown: this call sent no purchase request. The platform never reads the former, so the error surfaces as an ordinary failed recommendation with the "needs a human" text in History and no automatic retry loop.
• DescribeReservedInstances does not paginate, so a second RI cannot hide on another page. The reconcile path already returns Unknown with ambiguous candidates for two matches.

Verification

• go test -race ./services/ec2/ (providers/aws), golangci-lint --new-from-rev=origin/main 0 issues.
• Table test: 0, 1, 2, 3 matches, ID-less mixes and an ID-less-only case; the failed-result shape, count, masked token (never the raw token) and no purchase call asserted.
• Two cold staged-diff reviews with mutation probes (return first match, count ID-less entries, drop the pass-through, fall through to a purchase, treat 0 as an error, raw token in the error, adopt on 2): each killed by a named subtest.
• Not verified against a real AWS account.

Closes #724

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • EC2 purchases are now refused when an idempotency token matches multiple active or payment-pending Reserved Instances. The error reports the match count and masked token, and no purchase is sent.
  • Existing refusal errors from the lookup are preserved; other lookup failures retain their context.
• Documentation
  • Added an Unreleased notice describing the ambiguous-match refusal and the need for human reconciliation.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

EC2 re-drive idempotency lookup now distinguishes zero, one, and multiple ID-bearing Reserved Instance matches. Multiple matches produce a refusal before purchase, and lookup errors preserve the existing refusal classification.

Changes: EC2 re-drive idempotency matching

Layer / File(s)	Summary
Match RIs and refuse ambiguity providers/aws/services/ec2/client.go, providers/aws/services/ec2/client_test.go, CHANGELOG.md	The lookup ignores results without IDs, returns no match or the sole match, and refuses multiple matches with the match count and masked token. Existing ErrRefusedBeforePurchase errors pass through unchanged. Tests verify the lookup outcomes and confirm ambiguous matches do not trigger an offering lookup or purchase. The changelog documents the refusal.

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

Change: Bug fix · Severity of issue fixed: Medium

Possibly related PRs

• LeanerCloud/cloud-commitments-go#721: This PR changes the same EC2 idempotency lookup and adoption flow by refusing multiple ID-bearing matches.
• LeanerCloud/cloud-commitments-go#672: This PR applies the same common.ErrRefusedBeforePurchase classification to EC2 idempotency lookup failures and ambiguous matches.

Suggested labels: type/bug, severity/medium, impact/few, effort/s

Merge Risk: Minimal · up to 51b73

No concrete issue remains that should block merging after normal checks.

Pre-merge checks: 5

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly describes the main change: refusing an EC2 re-drive when multiple Reserved Instances share the idempotency token.
Linked Issues check		#724 asks whether the EC2 re-drive should fail closed when multiple RIs share the token. findRIByIdempotencyToken counts only matching RIs with IDs, returns the sole match, and wraps `common.ErrRefu…
Out of Scope Changes check		The reported changes are limited to the EC2 idempotency lookup and its refusal handling, focused tests, and a changelog notice. These changes support #724. No unrelated changes are shown in the suppli…
Docstring Coverage		Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…

Full details: Linked Issues check

Explanation

#724 asks whether the EC2 re-drive should fail closed when multiple RIs share the token. findRIByIdempotencyToken counts only matching RIs with IDs, returns the sole match, and wraps common.ErrRefusedBeforePurchase with the count and masked token when there are multiple matches. The purchase path returns that error before offering lookup or purchase. Its state filter excludes retired RIs. The supplied test summary reports coverage for zero, one, multiple, and ID-less matches, including no purchase on ambiguity. Validating a single match against the recommendation is not part of the selected scope.

Full details: Out of Scope Changes check

Explanation

The reported changes are limited to the EC2 idempotency lookup and its refusal handling, focused tests, and a changelog notice. These changes support #724. No unrelated changes are shown in the supplied change summary.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 2 functions across 2 files. (1 skipped: 1 unsupported.)

Finishing Touches

Generate docstrings

• Commit to this branch
• Create a new PR

Generate unit tests (beta)

• Commit to this branch
• Create a new PR

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

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

Recent review info

Run configuration

• Source mode: Auto
• Repository: LeanerCloud/cloud-commitments-go
• Branch: fix/724-ec2-multi-token
• File: .coderabbit.yaml
• Commit: 51b7347
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: b13b6e15-952d-4272-b0b8-58b70608ea65

Commits

Reviewing files that changed from the base of the PR and between 5b19c8a and 51b7347.

Files selected for processing: 3

• CHANGELOG.md
• providers/aws/services/ec2/client.go
• providers/aws/services/ec2/client_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.