[IAmJSD]

Summary

Test-only change. Adds TestApproveAndExecuteRefusesAWSNoTokenRetrySuccessor to internal/purchase/aws_no_token_redrive_test.go. Before this, the EC2/Redshift retry-successor refusal from #778 (MON-02) was tested only through SQS ProcessMessage. The approval entry points reach the same executeAndFinalize -> preExecutionRefusal -> armedRedriveRefusal funnel, but no EC2 test called them.

Closes #780

What the test covers

For each AWS slug that dispatches to the EC2 or Redshift client (ec2, compute, redshift, data-warehouse), a retry successor (RetryAttemptN = 1, status pending) goes through both ways an operator approves a row:

• token path: ApproveExecution with a valid approval token. It calls ApproveAndExecute(..., transitionedBy=nil).
• session path: ApproveAndExecute with the session user's UUID as transitionedBy. This is what approvePurchaseViaSession in internal/api does.

The test asserts:

• zero provider PurchaseCommitment calls
• the error is returned to the approver (refusing to execute retry attempt 1)
• the last persisted row is failed, and its error carries the operator action (Check EC2 Reserved Instances or Redshift Reserved Nodes)
• the CAS claim is made with the expected transitionedBy (store.AssertExpectations)

Positive control: an aws/rds retry successor goes through the same harness and both paths and must buy exactly once. Without it, a zero count could mean the harness never reached the provider, and a guard that refused every AWS service would also pass.

Does the refusal actually work?

Yes. The new test passes on current main without any code change. No fix was needed.

Verification

Unit tests with mocks only (testify mocks for the store, provider factory and service client). No real AWS calls.

AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null go test ./internal/purchase/ -run TestApproveAndExecuteRefusesAWSNoTokenRetrySuccessor -count=1 -v   # 10/10 subtests PASS
AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null go test ./internal/purchase/ -count=1   # ok
go build ./...
go vet ./internal/purchase/
gofmt -l internal/purchase/          # empty
golangci-lint v2.10.1 run --allow-parallel-runners ./internal/purchase/...   # 0 issues
git diff --check                     # clean

gocyclo: no non-test Go files changed. AWS_CONFIG_FILE/AWS_SHARED_CREDENTIALS_FILE are set to /dev/null only because the local machine's default AWS profile is invalid.

Mutation evidence (this test is the guard, so "fail before" means "fails when the guard is broken")

Each mutation was applied to internal/purchase/manager.go, the new test was run, and then the file was restored with git checkout:

Mutation	Result
M1: delete the armedRedriveRefusal(exec) call in preExecutionRefusal	FAIL: all 8 refused subtests (token+session x ec2/compute/redshift/data-warehouse); rds controls pass
M2: armedRedriveRefusal always returns nil	FAIL (all refused subtests)
M3: AWS branch of recRedriveRefusalReason never refuses (if false && awsPurchaseHasNoDuplicateGuard(...))	FAIL (all refused subtests)
M4: AWS branch refuses every service (if true || ...)	FAIL: token/rds and session/rds positive controls; refused subtests pass

Coverage gaps

• No real-cloud check. The test proves the platform makes no provider call, not what AWS would do.
• fireOneDue (scheduled delayed-approval path) for EC2 is not added here. It goes through the same executeAndFinalize funnel and is covered for Azure by TestFireScheduledDelayedPurchasesRefusesArmedAzureSavingsPlanRetry. The issue's expected behavior and proposed fix name only ApproveAndExecute. Happy to add it if wanted.
• DirectExecute and RunPlannedPurchaseNow share transitionApproveAndExecute with ApproveAndExecute but are not driven separately.

Labels

Could a maintainer please copy the issue's triage labels onto this PR: triaged, priority/p3, severity/low, urgency/eventually, impact/few, effort/xs, type/bug.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 1 billable file and costs up to $0.25.}

Or wait 59 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-platform/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: 3f0c479d-bacb-45a0-89a5-90096fe6c085

📥 Commits

Reviewing files that changed from the base of the PR and between 33cd663 and 4625bfe.

📒 Files selected for processing: 1

• internal/purchase/aws_no_token_redrive_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 review of 4625bfe: draft: blocks ready

Findings

1. NIT (non-blocking): internal/purchase/aws_no_token_redrive_test.go:194-197. #780 asks for one ApproveAndExecute case; the PR adds token (ApproveExecution) and session (ApproveAndExecute) paths x 4 EC2/Redshift slugs plus an rds positive control. That is a superset of the acceptance criteria and is fine. The PR title says only ApproveAndExecute while the test also covers ApproveExecution; wording only.
2. NIT / follow-up: DirectExecute, RunPlannedPurchaseNow and fireOneDue (EC2 case) still have no EC2 retry-successor test. All reach the same executeAndFinalize funnel (internal/purchase/approvals.go:425, manager.go:255), so this is structural coverage, and #780 does not ask for them.

What I attacked

• Real path: the test goes ApproveExecution/ApproveAndExecute -> enforceFourEyesPolicy -> claimRunning (mocked store CAS, asserted with .Once() and the expected transitionedBy) -> executeAndFinalize -> preExecutionRefusal -> armedRedriveRefusal. Only the store, email and provider are mocked; no code between the entry point and preExecutionRefusal is stubbed.
• No-purchase assertion: yes, rec.count()==0 on the PurchaseCommitment mock for every refused row; the rds control buys exactly once through the same harness, so the zero is not a harness artifact. It also asserts the error text, final saved status "failed" and the operator reason.
• Mutation probe 1: disabled armedRedriveRefusal in preExecutionRefusal (manager.go:293). All 8 refused subtests FAIL; both rds controls still pass. Reverted.
• Approval-path-only bypass: not possible without editing the shared funnel, since the refusal exists only in executeAndFinalize and both approve paths call it at approvals.go:425. Probe 1 covers it.
• go test ./internal/purchase/...: 706 passed.
• #780 acceptance: pending EC2 retry successor approved via ApproveAndExecute buys nothing and persists failed with the refusal reason. Met.

Gate status

• Independent review: this comment, against the SHA above.
• Findings resolved: none actionable.
• CI on SHA: pending at review time (checks queued/in progress); must be green.
• Labels: PR carries triaged, priority/p3, severity/low, urgency/eventually, impact/few, effort/xs, type/bug, which mirror #780.
• Local real-path evidence: above (macOS, mocked store/provider, fail-before via mutation). Not on the PR body yet.
• No CodeRabbit threads.

Blocks ready

• Take the PR out of draft (contributor-owned).
• CI green on this SHA.
• Record this verdict and local verification evidence on the PR (done by this comment); any new commit restarts the gate.

[cristim]

Gate record for 4625bfe (origin/main CLAUDE.md "Review gate"):

• Independent review of 4625bfe: no actionable findings.
  • The test drives the real ApproveExecution and ApproveAndExecute through the 4-eyes check, the claim, executeAndFinalize, preExecutionRefusal and armedRedriveRefusal. Only the store, email and provider are mocked.
  • It asserts zero PurchaseCommitment calls for ec2, compute, redshift and data-warehouse, and an rds control buys exactly once.
• CI on 4625bfe: CI Success and Run pre-commit hooks pass.
• Local verification (macOS):
  • go test ./internal/purchase/... passes 706 tests.
  • Disabling armedRedriveRefusal (manager.go:293) fails all 8 refused subtests while the rds controls still pass.
  • Test-only change; no purchase.
• CR skipped: draft and quota-limited; covered by the independent review plus local verification.
• Labels mirrored from test(purchase): cover ApproveAndExecute refusal of an EC2 retry successor #780.

Marking ready (owner-authorized) and merging at 4625bfe with --match-head-commit.