fix(exchange): send the RI exchange accept once and halt on an unknown outcome

[cristim]

Summary

Part 2 of #299: the RI exchange accept in pkg/exchange.

AcceptReservedInstancesExchangeQuote (exchange.go) used the default SDK retryer and carries no client token. A retry after a lost response does not double-exchange, because attempt 1 consumed the source RIs. But the committed exchange came back as a plain error, and RunAutoExchange then called FailRIExchange, recording a real exchange as failed.

• The accept now makes one attempt (per-call RetryMaxAttempts = 1; EC2ExchangeAPI already takes optFns).
• Its error goes through the new unexported classifyAcceptError (pkg/exchange/outcome.go). 4xx and throttle codes stay definite. Transport errors, timeouts, 5xx and undeserializable responses wrap common.ErrOutcomeUnknown. A 200 without an ExchangeId is also unknown.
• classifyAcceptError mirrors purchasecfg.ClassifyPurchaseError. pkg cannot import providers/aws/internal, so each file has a comment pointing at the other. Extracting one shared helper needs a pin bump and is left for later.
• On an unknown outcome, RunAutoExchange leaves the record processing and stops the run, for both standalone and ladder runs. It no longer calls FailRIExchange. Plain errors still fail the record, as before.
• pkg/go.mod: smithy-go becomes a direct dependency (go mod tidy; same version providers use, no go.sum change).

Consumer notes:

• The platform sees a wrapped error containing "purchase outcome unknown".
• A processing record keeps its reserved daily-spend slot until reconciled. GetStaleProcessingExchanges already surfaces such records. Picking up this change needs a pkg pin bump.

Closes #299. Merge after #429 (part 1, independent code) so the issue doesn't close before both parts land.

Verification (macOS, Go 1.26.9)

• (cd pkg && GOWORK=off go test -race -short ./...): pass.
• make build, make vet: pass.
• golangci-lint v2.10.1 (CI pin): 0 issues.
• GOWORK=off go build ./... in all five modules: pass.
• gocyclo -over 10 on the touched files: clean.
• New tests:
  • TestExecute_AcceptSendsOnceAndClassifies covers success, transport error (unknown), 500 (unknown), 400 (definite), 503 Throttling (definite) and empty ExchangeId (unknown). Each case records RetryMaxAttempts == 1 through a fake that applies optFns to a real ec2.Options. API errors carry a real HTTP status.
  • TestClassifyAcceptError checks nil input and that it never double-wraps.
  • TestRunAutoExchange_UnknownAcceptLeavesRecordProcessingAndHalts uses two RIs: one execute, zero FailRIExchange calls, and the record is not failed.
• Fail-before: with the original exchange.go and auto.go, all six accept subtests and the auto-exchange test fail.
• Mutation probes, run by the independent reviewer in a scratch copy, all fail tests:
  • classifying a 4xx as unknown;
  • dropping the empty-ExchangeId check;
  • calling FailRIExchange on an unknown outcome;
  • emptying the option.
• Review: the plan got two passes and the diff one pass from the same non-author reviewer. No actionable findings on tree 7d37dacc.

All evidence uses SDK-shaped fakes. No AWS call or exchange was made.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 6 billable files and costs up to $1.50.}

• 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 51 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 64 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-go/.coderabbit.yaml
• Review profile: CHILL
• Plan: Essentials
• Run ID: 458e4b0e-1c8d-49bd-911c-389d94df47fb

📥 Commits

Reviewing files that changed from the base of the PR and between 11200ee and 91adf00.

📒 Files selected for processing: 6

• pkg/exchange/auto.go
• pkg/exchange/exchange.go
• pkg/exchange/outcome.go
• pkg/exchange/outcome_test.go
• pkg/go.mod
• providers/aws/internal/purchasecfg/outcome.go

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

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

[cristim]

@coderabbitai review

[coderabbitai]

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[cristim]

CI is green on head 91adf000: pre-commit 38914497813 and Build & Test 38914497631. CodeRabbit reached its free review limit on this PR and only offers a paid on-demand review, which I'm not using. Per the review gate, the head is instead covered by the independent non-author review recorded in the body (no actionable findings on tree 7d37dacc, mutation probes caught) and by the local verification listed there.

[cristim]

Independent review verdict: CLEAN at 91adf00

Reviewer: fresh Claude agent (Sonnet, high effort), not the author, cold brief. The PR body was treated as a claim. Coordinator: claude-cc-reviewer.

Blocking findings: none. No double-send path was found.

Confirmed:

• Single send: exchange.go:447-450 sets a per-call o.RetryMaxAttempts = 1. This is the only accept call site in the repo; providers/aws never calls accept. It was checked against the real SDK client with a scratch httptest server:
  • a 500/503 gives 1 call with the option, and 3 without it;
  • a 400 gives 1 call either way and stays definite.
• Classification: transport errors, timeouts, 5xx and an empty ExchangeId wrap common.ErrOutcomeUnknown. 4xx and throttle codes stay definite, and success is unchanged.
• Halt: auto.go:555-559 returns halt=true and leaves the record processing without calling FailRIExchange. The loop (auto.go:214-221) breaks on halt, and ladder runs go through the same RunAutoExchange.
• pkg/go.mod: smithy-go v1.24.2 moves from indirect to direct, because outcome.go imports it. The version and go.sum are unchanged.
• API: no signature changes; Execute only returns a wrapped error. Consumers pick this up when they bump the pkg pin.

Non-blocking (follow-ups):

1. An unknown outcome lands in result.Failed, and RunAutoExchange returns nil. The only "unknown" signals are the error text and the processing record, so providers/aws/ladder/reshape.go:96-110 likely counts it as a plain failure. Filed as fix(exchange): surface an unknown accept outcome as a typed result, not a plain failure #443, for a typed unknown result.
2. The classifier is duplicated in pkg/exchange/outcome.go:20-35 and providers/aws/internal/purchasecfg/outcome.go. That is justified, because pkg can't import providers/aws/internal, and both now cross-reference each other. A shared helper is tracked in fix(exchange): surface an unknown accept outcome as a typed result, not a plain failure #443.
3. Pre-send failures (STS, credentials, DNS) are treated as unknown. This is conservative, the same as fix(aws): send empty-token reservation purchases once and report lost responses as unknown #429, and the window is small.

What was attacked and how (GOTOOLCHAIN=go1.26.9, AWS config set to /dev/null, no real exchange):

• In pkg, go vet ./exchange/... is clean and go test -race -count=1 ./exchange/... passes 163.
• In providers/aws, ./internal/purchasecfg/... passes 39.
• Fail-before: with the base exchange.go/auto.go restored, TestExecute_AcceptSendsOnceAndClassifies (6 subtests) and TestRunAutoExchange_UnknownAcceptLeavesRecordProcessingAndHalts fail.
• Mutation probes:
  • Killed: "continue the loop after unknown" and "re-enable retry".
  • Two more probes failed, but the run may have been a compile error, so they are inconclusive.
  • One probe was invalid (a build error). Its case is covered by TestClassifyAcceptError and the transport_error subtests.
  • No survivor revealed a double-send path.

Gate:

• CI is green at this SHA, and mergeability is CLEAN.
• Labels are mirrored from fix(aws): AlreadyExists after a committed buy reports failure when the token is empty; exchange accept has no retry guard #299.
• CR waived: quota, adversarial review + local verification + green CI. A retrospective review is owed.