[cristim]

Summary

Part of #702. Restores Redshift reserved-node purchases with the library dedupe from LeanerCloud/cloud-commitments-go#515, while Redshift re-drive and retry stay refused. This is option A from the scope question on #702; the retry half stays open there.

• 08fe7baf chore(deps): bumps providers/aws to aeb8a6f7ff51 and pkg to 541e05e5fd29, which aws requires for PurchaseOptions.RequestedAt. azure and gcp stay at 3def3f898617.
• 6db77b04 fix(purchase): executeSinglePurchase sets opts.RequestedAt = time.Now().UTC() right before the single PurchaseCommitment call. Every dispatch path (scheduler, approve, execute-now, scheduled fire, fan-out per account) goes through it.
  • The new library refuses a tokenized Redshift purchase with a zero RequestedAt, and the platform always sends a token. The two commits must ship together.
• cba71250 docs(known-issues): known_issues/39_redshift_dedupe_window.md records the unbounded match window. A same account/region/offering/node-count purchase within about 5 minutes of a completed one adopts the earlier node without buying. An unknown outcome followed by a new same-tuple purchase outside the window can double-buy.

Why the go caller contract holds

go requires that no second same-tuple Redshift purchase is dispatched while an earlier lineage can still be re-driven. Redshift re-drive and retry remain refused on every path, and this PR does not touch them:

• awsPurchaseHasNoDuplicateGuard / recRedriveRefusalReason (RecoverStrandedApprovals);
• armedRedriveRefusal (pre-existing armed successors);
• the Retry handler's eligibility gate.

Behaviour changes that ride along with the library bump

• Redshift: pre-buy dedupe uses DescribeReservedNodes (already granted) instead of DescribeTags (never granted), and node tagging is dropped. Today every Redshift buy is refused before purchase; after this PR, first-dispatch buys go through again. Two concurrent same-tuple purchases are guarded only by go's check-then-act matcher, the per-row execution claim and suppressions.

• RDS: the offering lookup now refuses at the pagination cap even when a match was found (go#303), so an RDS buy can newly fail with "cannot be proven unique".

• RDS: isLicensedEngine now also matches the Cost Explorer spelling "sql server" (go#302), so those recommendations are refused like other licensed engines.

• Savings Plans: the plan type, term and payment are validated before a Cost Explorer OfferingID is trusted. This fails loud on a mismatch.

• Recommendations: Cost Explorer parsing now fails loud.

  • An unparsable EstimatedBreakEvenInMonths drops that recommendation detail instead of defaulting, and a parsed value is stored in BreakEvenMonths.
  • Missing or malformed utilization and usage-history values now return errors instead of reading as 0, so a refresh can newly fail on one bad CE row.

• Rate limiter: RateLimiter.Wait returns at once on an already-cancelled context.

• Adopted nodes look like purchases: when the library adopts an existing node (a guard match, or recovery after an unknown outcome), it sets PurchaseResult.ExistingCommitment. The platform does not read that field yet, so an adopted node is recorded as an ordinary successful purchase. known_issues/39 records this, together with the concurrent same-tuple case.

• No behaviour change: ElastiCache moves its guard into internal/purchaseguard with the same logic. EC2, MemoryDB and OpenSearch have no service code changes between the two versions. The pkg break-even and scorer changes have no consumer in this repo.

Verification

Results are posted as PR comments.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 5 billable files and costs up to $1.25.}

• 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 25 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-platform/.coderabbit.yaml
• Review profile: CHILL
• Plan: Essentials
• Run ID: c334fe64-de34-47ff-b4d9-82d03dcc7713

📥 Commits

Reviewing files that changed from the base of the PR and between e0215b0 and 86c4dcd.

⛔ Files ignored due to path filters: 1

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

📒 Files selected for processing: 5

• go.mod
• internal/purchase/execution.go
• internal/purchase/execution_test.go
• internal/purchase/requested_at_test.go
• known_issues/39_redshift_dedupe_window.md

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

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

[cristim]

Merge gate at 86c4dcd4bf498d11ad4a8e7dfdb1c9121b91773b

Independent review 1 (cold brief, non-author): APPROVE 86c4dcd4bf498d11ad4a8e7dfdb1c9121b91773b (pass 1 APPROVE at cba71250; pass 2 on the docs-only delta)

Attacked:

• Dispatch paths: the only non-test PurchaseCommitment( call is internal/purchase/execution.go:1185, stamped at :1184. recOpts := opts is a value copy, so fan-out goroutines share no mutable state. Every executor funnels through executeAndFinalize, then executePurchase, then executeSinglePurchase, with no internal retry loop.
• Re-drive still refused:
  • RecoverStrandedApprovals sends Redshift to safeFail via awsPurchaseHasNoDuplicateGuard.
  • armedRedriveRefusal runs in preExecutionRefusal.
  • checkRetryEligibilityGates cannot be overridden by force.
  • The claim CAS never re-drives a running row.
• Library dedupe: read dedupe.go in full. A zero RequestedAt refuses, as do a lookup error and more than one match; exactly one match is adopted. Unknown-outcome adoption needs exactly one match. findMatchingNodes fails closed and refuses at the page cap. The Redshift purchase makes a single attempt. redshift:DescribeReservedNodes is granted in the Terraform and CloudFormation targets.
• Module checks: go mod verify and go build ./... are clean. Only aws and pkg move; azure and gcp compile against the newer pkg.
• Mutation: deleting the stamp fails TestExecuteMultiAccount_StampsRequestedAtOnEveryDispatch.

Pass 1 findings, both resolved:

• The duplicate and concurrent same-tuple case was missing from known_issues/39; added in 86c4dcd4.
• Library changes were missing from the PR body (fail-loud Cost Explorer parsing, BreakEvenMonths, RateLimiter.Wait, the ElastiCache refactor); added.

Pass 2: the new entries match the code and the library. The RDS "sql server" line is confirmed in licensedEnginePrefixes.

Independent review 2 (cold brief, non-author): APPROVE 86c4dcd4bf498d11ad4a8e7dfdb1c9121b91773b (pass 1 APPROVE at cba71250; pass 2 on the docs-only delta)

Attacked:

• Concurrent same-tuple purchases: both guard lookups can miss, or the second adopts the first node with Cost nil. Documented.
• Unknown outcome: the library adopts only on exactly one match; otherwise the platform records a plain failure.
• Fan-out: per-account clients and tuples, with a value-copied opts.
• Other services: diff -rq on services/ between the old and new versions shows changes only in elasticache, rds, redshift and savingsplans. The newly refused cases are the RDS pagination cap (go#303), the RDS "sql server" engines (go#302) and Savings Plans validation; all fail loud and none changes what is bought.
• Stamp point: the stamp comes after client construction and before the library's offering lookup, guard and buy. The window has no upper bound, so an earlier stamp would only widen adoption of older nodes.
• Non-AWS providers: azure and gcp never read RequestedAt.
• Tests: go test -run 'RequestedAt|WebSourcePropagates|InvalidSourceFallsBack' ./internal/purchase/ passes.
• Mutations: a no-op stamp, a Local-time stamp and a stamp shifted an hour earlier each fail a test.
• PR body: closingIssuesReferences is [], and the body says only "Part of fix(iac): grant ec2:CreateTags to member-account purchase roles so the idempotency tag is written #702".

Pass 1 findings: the concurrent case, invisible adoption (the platform never reads ExistingCommitment) and missing body items. Pass 2 confirms all three are recorded in known_issues/39 (86c4dcd4) and the PR body.

Nit: the test pins UTC, although only the instant matters.

Local verification (macOS, exact head, worktree clean):

• go build ./..., go test -race -short -count=1 ./... and make lint pass.
• The new tests (TestExecuteMultiAccount_StampsRequestedAtOnEveryDispatch, TestExecuteMultiAccount_StampsRequestedAtOnRedshift, TestExecuteSinglePurchase_EmptyTokenOptsOtherwiseUntouched) fail with the stamp removed and pass with it.

Not verified against a real account:

• No real purchase was made: verification never authorizes real purchases.
• The Redshift path runs on fakes and the library's own tests.
• The first live Redshift reserved-node buy after deploy is the first real-account exercise of DescribeReservedNodes dedupe.

CI: all checks are green at this SHA.

Threads: none open.

Owner scope: option A was posted on #702 (2026-10-19) with a stated default and no objection. CodeRabbit is waived as a merge gate.