fix(purchase): refuse retry and re-drive of AWS EC2 and Redshift purchases

[cristim]

Summary

Platform half of go#296. The Retry endpoint, the stranded-approval sweep, the reaper's retry advice and the executor backstop for retry successors now refuse to re-drive any AWS EC2 or Redshift purchase, so a lost response can no longer lead to a second commitment. Go half: merged as bcc0dfc (go#297), which sends these purchases once and reports a lost response as purchase outcome unknown.

Closes LeanerCloud/cloud-commitments-go#296

The refusal is by service, not by outcome

After ANY failure of an EC2 or Redshift execution, retry and re-drive are refused. That includes definite rejections: a 4xx, throttling, credentials errors, "no offerings found", and the new pin's rejection of an empty Tenancy or Scope. The platform cannot tell a definite failure from an unknown outcome because the go sentinel purchasecfg.ErrOutcomeUnknown is in an internal package and persisted errors are flattened to strings, and reaped or stranded rows carry no error at all. The operator checks the account and starts a new purchase. Exporting the sentinel so definite failures can be retried is go#301.

Problem

The EC2 and Redshift purchase APIs take no client token or caller ID. recRedriveRefusalReason (internal/purchase/manager.go) still said every AWS service honours a token, so four paths could buy a second commitment:

• the stranded-approval sweep (RecoverStrandedApprovals) re-drove the row in place;
• the reaper wrote "safe to retry" on reaped rows;
• the Retry endpoint created a successor (?force=true does not override a refusal);
• retry successors created earlier would execute (armedRedriveRefusal backstop).

Change

• recRedriveRefusalReason refuses AWS (and legacy empty provider) recs whose service dispatches to the EC2 or Redshift client: ec2, compute, redshift, data-warehouse. Matching goes through mapServiceSlug, the table the executor dispatches on, so guard and dispatch cannot drift. One predicate feeds all four paths.
• The refusal text tells the operator to check EC2 Reserved Instances or Redshift Reserved Nodes in the target account and region, and to start a new purchase if the commitment is missing. The stranded-row message no longer says "then Retry" for refused rows.
• go.mod: providers/aws bumped to bcc0dfc (only that module). Two comments that described the old EC2 Tenancy/Scope defaulting are corrected (comment-only commit e832349).
• A first purchase of these services (RetryAttemptN = 0) still executes. RDS, ElastiCache, MemoryDB, OpenSearch and Savings Plans stay retryable (negative controls in the tests).

Pin move heads-up (found in review)

The providers/aws pin moves from 9962786e to bcc0dfc, and that is more than the MON-02 commit: it pulls 17 providers/aws commits (42 files). Notable behaviour for platform consumers:

• EC2 now rejects empty or unknown Tenancy and Scope instead of defaulting (refactor(frontend): fourteen non-test files in frontend/src exceed the 500-line ceiling, up to 5481 lines #210). A recommendation with empty tenancy or scope fails its first purchase, loudly; no money is spent. Per the section above, such a row also cannot be retried.
• Typed InstanceTenancy/OfferingClass inputs (sec(db): pgx error logs carry bound query arguments including session tokens and hashes #231); expiry-adjustment layer A/B changes (test: assertions positioned before the call under test are a fourth vacuity mode the guard does not detect #176/chore(ci): gocyclo's filesystem walk in ci.yml scans frontend/node_modules when deps are installed #177/sec(api): approve/cancel discard an authorization denial and fall through — safe only by coincidence #173) and savings-plan coverage geography (ci: lint job is root-only — 1291 golangci-lint findings across five unlinted workspace modules #174) in recommendations.
• OpenSearch offering selection (chore(frontend): UI polish backlog from the 2026-08 UX review #191); fractional RI quantity warning (fix(auth): a consumed recovery code stays valid when the persist fails #228); Organizations later-page error propagation.
• ExistingCommitment adopted flag on EC2/RDS/ElastiCache/MemoryDB/OpenSearch re-drives (fix(api): openapi.yaml omits ~30 routed paths and mis-describes documented operations #262); bounded idempotency scans (fix(frontend): savings-analytics summary keys read by the client never exist on the wire #276); Go 1.26.9 bump.
  Platform consumers of the adopted flag are not in this PR.

Not in this PR

• Automatic reconcile against what AWS bought: go#298.
• Exporting ErrOutcomeUnknown so definite failures can be retried: go#301.
• An ApproveAndExecute EC2 retry-refusal test (the funnel is shared, covered today by structure and the Azure armed tests): platform#780.
• Untagged-commit gap (ec2:CreateTags in member-account roles): fix(iac): grant ec2:CreateTags to member-account purchase roles so the idempotency tag is written #702.
• purchase_history dedupe on re-drive: fix(purchase): skip duplicate purchase_history insert on re-drive and add a unique index #704.
• A brand-new execution from the same recommendation (second click on Purchase) is an explicit new buy and is not deduped here.
• If plan steps ever carry recommendations (platform#609), a failed EC2 step becomes un-retryable and needs its own recovery path. Plan steps have no recs today, so the Retry handler already returns 409 for them.

Tests

• internal/purchase/aws_no_token_redrive_test.go (new): guard reach equals dispatch reach for aws and "" over spellings and case variants; closed refusal set with negative controls; retry successor of ec2/compute/redshift/data-warehouse buys nothing and is persisted failed with the reason, first purchase buys once; stranded EC2 row makes zero provider calls.
• reaper_test.go: reaped EC2 row has no "safe to retry"; the RDS test is the positive control.
• handler_purchases_retry_redrive_test.go: Retry of failed aws/ec2, compute, redshift, data-warehouse and legacy "" rows returns 409 redrive_unsafe with and without ?force=true and fires no purchase; an RDS retry still buys once.
• Existing tests that used EC2 as the "safe AWS" example now use RDS.
• Mutation: with the guard disabled the new test groups fail (also run independently by the reviewer: 8 mutations, all caught).

Verification (local, macOS)

go build ./..., go vet and go test of internal/purchase and internal/api pass. Not run: real AWS (no live cloud calls or purchases).

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 15 billable files and costs up to $3.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 45 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 87 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: 8047db88-ef25-4383-9b99-6880f4a6eb28

---

📥 Commits

Reviewing files that changed from the base of the PR and between 1d933ad and bc5efe3.

---

⛔ Files ignored due to path filters (1)

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

---

📒 Files selected for processing (15)

• go.mod
• internal/api/execution_scope_test.go
• internal/api/handler_purchases.go
• internal/api/handler_purchases_retry_fanout_test.go
• internal/api/handler_purchases_retry_redrive_test.go
• internal/api/handler_purchases_test.go
• internal/api/purchase_pricing.go
• internal/api/retry_carveout_test.go
• internal/purchase/armed_redrive_test.go
• internal/purchase/aws_no_token_redrive_test.go
• internal/purchase/execution.go
• internal/purchase/manager.go
• internal/purchase/manager_test.go
• internal/purchase/ramp_step_progress_integration_test.go
• internal/purchase/reaper_test.go

---

---

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: LeanerCloud/cloud-commitments-platform/.coderabbit.yaml
• Review profile: CHILL
• Plan: Essentials
• Run ID: 56eaff4b-ed4e-4e91-a485-6e3a7e3f372c

---

📥 Commits

Reviewing files that changed from the base of the PR and between fe13430 and 1d933ad.

---

⛔ Files ignored due to path filters (1)

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

---

📒 Files selected for processing (12)

• go.mod
• internal/api/execution_scope_test.go
• internal/api/handler_purchases.go
• internal/api/handler_purchases_retry_fanout_test.go
• internal/api/handler_purchases_retry_redrive_test.go
• internal/api/handler_purchases_test.go
• internal/api/retry_carveout_test.go
• internal/purchase/armed_redrive_test.go
• internal/purchase/aws_no_token_redrive_test.go
• internal/purchase/manager.go
• internal/purchase/manager_test.go
• internal/purchase/reaper_test.go

---

Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

---

---

📝 Walkthrough

Walkthrough

The purchase retry flow now refuses re-drives for AWS EC2 and Redshift services without duplicate guards. Recovery messages distinguish refused re-drives from retryable failures. Tests cover refusal behavior, stranded executions, and allowed AWS RDS retries.

Changes

AWS purchase retry safety

Layer / File(s)	Summary
Refusal policy and recovery guidance go.mod, internal/purchase/manager.go, internal/api/handler_purchases.go	Retry assessment maps AWS service slugs and refuses EC2 and Redshift re-drives. Recovery guidance now depends on whether re-drive is refused. The AWS provider dependency revision and retry-safety comments are updated.
API retry behavior and fixtures internal/api/handler_purchases_retry_redrive_test.go, internal/api/*_test.go	API tests verify refusals for EC2 and Redshift service names, including forced retries, and verify that RDS retries still purchase. Related retry fixtures use RDS recommendations.
Purchase and stranded-execution coverage internal/purchase/*_test.go	Tests cover refusal reach, retry successors, first purchases, stranded EC2 approval recovery, and reaper messages. RDS fixtures retain allowed-retry coverage.

Priority: ⬆️ High

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix · Severity of issue fixed: High

---

Merge Risk: ⚪ Minimal · up to 1d933

The change is mergeable after normal checks. Operators must verify whether an EC2 or Redshift commitment exists before starting a new purchase when the outcome is unknown.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 74.36% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 39 functions across 11 files. (1 skipped:…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check		The PR addresses the coding requirements in #296. It bumps providers/aws to bcc0dfc, which supplies the single-send and outcome-unknown provider behavior described in the PR. `recRedriveRefusalRea…
Out of Scope Changes check		The production changes and dependency update remain within #296. Test fixture changes replace unsafe EC2 examples with RDS controls, and new tests verify the required refusal and retry behavior. The P…
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: refusing retries and re-drives for AWS EC2 and Redshift purchases.

---

Full details: Docstring Coverage

Explanation

Docstring coverage is 74.36% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 39 functions across 11 files. (1 skipped: 1 unsupported.)

---

---

✨ Finishing Touches 💡 1

📝 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

---

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

[cristim]

Subject: platform PR 778 (MON-02 platform half) @ 1d933ad - APPROVE conditional on green CI; one heads-up for the PR body

Reviewed 3a15d5a in full, then the merge head 1d933ad. Delta check: the PR's own changes are byte-identical (added/removed lines of 27c1a30..3a15d5a equal those of fe13430..1d933ad, md5 equal); the merge only brings main (incl. #772 gcp, #766, #773-adjacent). go.mod vs main: providers/aws 9962786e -> bcc0dfc49227 only (gcp 2c18f303 comes from main); go.sum hash for bcc0dfc matches go mod download (h1:S9pAwtTnwt02+Fo1uVWYgtmGyB1koQAHmBSficHOZ2E=).
Scratch copy of the head (git archive; no worktree): go build ./... ok; go test ./internal/purchase (463 at 3a15d5a, ok at head) and ./internal/api (2865, ok at head).
Paths that can re-run a purchase, traced:

• every executor funnels through executeAndFinalize (claimAndExecute for SQS/cron, ApproveAndExecute, fireOneDue, claimAndRedrive); armedRedriveRefusal there blocks retry successors (RetryAttemptN>0); fresh executions are not blocked (first buy still executes, tested).
• reaper: allRecsSafeToRedrive -> RedriveRefusalReason -> EC2/Redshift now go to safeFail with a message that says retry is refused.
• Retry endpoint: checkRetryEligibilityGates runs the predicate first; ?force=true does not override it (tested).
• SQS/cron redelivery: claimAndExecute CASes approved/pending/notified -> running; a failed row cannot be claimed again, so redelivery cannot re-buy.
• spelling: the predicate goes through mapServiceSlug, the same table the executor dispatches on; the pinned AWS GetServiceClient switch is exact-match on compute/ec2/data-warehouse/redshift (checked at bcc0dfc providers/aws/provider.go), so "EC2"/" ec2" fail as unsupported service with no purchase; unknown providers are refused (fail closed); a mixed execution is refused if any rec is EC2/Redshift.
  Mutations on 3a15d5a (go test purchase + api retry/redrive subset): predicate always false; drop ec2 / compute / redshift / data-warehouse slug (each); over-broad (add RDS); backstop disabled; safeFail message branch removed. All 8 fail tests (named tests: TestAWSRedriveRefusalSetIsExact, TestAWSRedriveGuardReachMatchesDispatchReach, TestAWSNoTokenRetrySuccessorIsRefusedAndFirstBuyStillExecutes, TestRecoverStrandedApprovals_EC2RowIsFailedNotRedriven, TestRetryOfFailedAWSEC2OrRedshiftRowIsRefused, TestSafeProviderRetrySuccessorStillExecutes). Not vacuous.
  Rewritten ec2->rds tests: diff is mechanical (Service/ResourceType/mock ServiceType swapped, comments corrected: RDS dedupes via its reservation ID, not ClientToken). They still exercise the same generic paths (retry fan-out, scope matrix, carve-out, safe-provider retry executes); the EC2-specific refusal is covered by the new aws_no_token_redrive_test.go.
  HEADS-UP (not blocking, please add to the PR body): the providers/aws pin move 9962786e -> bcc0dfc is NOT only the MON-02 commit. It pulls 17 providers/aws commits (42 files): EC2 now REJECTS empty/unknown Tenancy and Scope instead of defaulting (refactor(frontend): fourteen non-test files in frontend/src exceed the 500-line ceiling, up to 5481 lines #210; a rec with empty tenancy/scope now fails the first purchase, fail-loud, no money lost); typed InstanceTenancy/OfferingClass inputs (sec(db): pgx error logs carry bound query arguments including session tokens and hashes #231); expiry-adjustment layer A/B changes (test: assertions positioned before the call under test are a fourth vacuity mode the guard does not detect #176/chore(ci): gocyclo's filesystem walk in ci.yml scans frontend/node_modules when deps are installed #177/sec(api): approve/cancel discard an authorization denial and fall through — safe only by coincidence #173) and savings-plan coverage geography (ci: lint job is root-only — 1291 golangci-lint findings across five unlinted workspace modules #174) in recommendations; OpenSearch offering selection (chore(frontend): UI polish backlog from the 2026-08 UX review #191); fractional RI quantity warning (fix(auth): a consumed recovery code stays valid when the persist fails #228); Organizations later-page error propagation (#0f2c710); ExistingCommitment adopted-flag on EC2/RDS/ElastiCache/MemoryDB/OpenSearch re-drives (fix(api): openapi.yaml omits ~30 routed paths and mis-describes documented operations #262/#84c8ce2); bounded idempotency scans (fix(frontend): savings-analytics summary keys read by the client never exist on the wire #276); Go 1.26.9 bump. Platform consumers of the adopted flag are not in this PR.
  Accepted cost confirmed: after a clean EC2/Redshift failure the operator cannot Retry (409 redrive_unsafe, force does not override) and must start a new purchase; the new-purchase path has no dedupe against an outcome-unknown predecessor (follow-up: brand-new execution dedupe, as you listed).
  Not verified: nothing against live AWS; no -race; full-suite not run (targeted packages only, disk hold). CI at 1d933ad was queued when I looked (BLOCKED): approval is conditional on every check green and CLEAN at this exact SHA. Any new commit restarts this.

[cristim]

Gate check (gate-1) at head c5e1f00: not merged, head is behind main.

• Reviews: Opus APPROVE and kimi APPROVE at 1d933ad; Opus delta APPROVE at 8d00c5f (comments only).
• 8d00c5f..c5e1f00 checked here: one commit, test fixture only (ramp_step_progress_integration_test.go, mock GetServiceClient ServiceEC2 to ServiceRDS, rec ec2/m5.large to rds/db.r5.large). No assertion was changed or removed. Probe: with the fixture set back to EC2 at this head, both TestRampStep* tests fail (the retry successors are refused by this PR), so the switch to a retry-safe service is required and does not weaken the ramp-step assertions. The EC2/Redshift refusal itself stays covered by aws_no_token_redrive_test.go.
• CI: 25/25 pass, mergeStateStatus CLEAN.
• Local, git archive of c5e1f00, GOTOOLCHAIN=go1.26.9, macOS: go build ./... exit 0; go test ./internal/purchase/... ./internal/api/... exit 0; go test -race -tags=integration ./internal/purchase/... exit 0 (Docker Postgres).
• Blocker: the branch is 7 commits behind main (incl. fix(purchase): save purchase history once per commitment #776, which touches internal/purchase/execution_test.go). A trial merge with main 7e2a3d8 (git merge-tree, tree d18c9554) has no conflicts, and the same three commands exit 0 on it. Please merge main normally (no force push); the gate will then re-check the merge-only delta and CI at the new SHA.

[cristim]

Gate verdict (gate-1): PASS at bc5efe3, merging.

• bc5efe3 is a normal merge of main 7e2a3d8 into c5e1f00 (via update-branch). Its tree is d18c9554, identical to the trial merge verified earlier, so the PR-owned diff is unchanged and the merge has no conflicts. The only non-PR changes are main's commits, including fix(purchase): save purchase history once per commitment #776.
• Reviews: Opus and kimi APPROVE at 1d933ad; Opus delta APPROVE at 8d00c5f (comments only); c5e1f00 fixture-only delta verified here (see the earlier comment).
• CI at bc5efe3: 25/25 pass, mergeStateStatus CLEAN, not behind main.
• Local, git archive bc5efe3f, GOTOOLCHAIN=go1.26.9, macOS: go build ./... exit 0; go test ./internal/purchase/... ./internal/api/... exit 0; go test -race -tags=integration ./internal/purchase/... exit 0 (Docker Postgres).
• fix(purchase): save purchase history once per commitment #776 + fix(purchase): refuse retry and re-drive of AWS EC2 and Redshift purchases #778 together on this tree, integration tag: TestManager_ExecuteAndFinalize_EmptyCommitmentID_CompletesWithAuditGap, TestSavePurchaseHistory_* (incl. RejectsEmptyPurchaseID, RedriveDoesNotDuplicate, ConcurrentSaveWaitsForUncommittedTwin), TestMigration108_AppliesOverExistingDuplicates, TestAWSNoTokenRetrySuccessorIsRefusedAndFirstBuyStillExecutes, TestAWSRedriveRefusalSetIsExact, TestAWSRedriveGuardReachMatchesDispatchReach, TestRecoverStrandedApprovals_EC2RowIsFailedNotRedriven, TestSafeProviderRetrySuccessorStillExecutes, TestRampStep* (2), api TestRetryOfFailedAWSEC2OrRedshiftRowIsRefused: all PASS.
• Not verified: live AWS (mock/fixture and Postgres-container evidence only).