fix(aws/ladder): report an exchange with an unknown accept outcome as unknown, not failed

[cristim]

Summary

Part 2 of 2 for #443 (part 1: #497 added ExchangeOutcome.OutcomeUnknown, which stays in AutoExchangeResult.Failed).

1. build(aws): bump the providers/aws pkg pin 89772cf80f67 -> 36d8b1100c05 (origin/main). Besides feat(exchange): flag an exchange whose accept outcome is unknown #497 it pulls only the pkg/common token change (no providers/aws caller), a pkg/ladder doc comment and a pkg/recfilter test; go mod tidy changed nothing else.
2. fix(aws/ladder): summarizeExchangeResult (providers/aws/ladder/reshape.go) prints a flagged outcome as outcome unknown, reconcile before retrying: RI (type) -> target: err instead of failed: .... The returned error keeps its N of M exchange attempt(s) failed (first: ...) text, so existing callers and assertions are unaffected, and appends ; K with unknown outcome only when K > 0.

If the first failure is an unknown one the prefix still says "failed", but the quoted error, the suffix and the per-item line all mark it unknown.

Closes #443. The platform's manual exchange paths (which call ExecuteExchange directly) are tracked in LeanerCloud/cloud-commitments-platform#1050.

Tests

• New TestReshapeBuffer_UnknownOutcome_RenderedAsUnknownNotFailed: a mixed list of unknown and plain failures; each detail line has the right label, no cross-contamination, and the suffix reports K=1.
• The existing partial-failure test now also asserts the suffix is absent when K=0.
• Mutation probes (scratch copies): ignoring the flag fails the new test on both the label and the suffix; dropping the suffix fails it on the suffix.

Verification

make build, make vet, (cd providers/aws && GOWORK=off go build ./... && GOWORK=off go test -race -short ./ladder/...), pinned golangci-lint v2.10.1 (0 issues), gofmt, and the commit/pre-push hooks pass. Unit-level evidence only.

Local review: the same non-author reviewer that reviewed the plan and #497, no actionable findings. No other providers/aws code prints exchange outcomes (AutoExchangeResult is only passed through in ladder/interfaces.go and factory.go).

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Exchange errors now distinguish attempts with unknown outcomes from confirmed failures, making it clearer when an attempt needs reconciliation before retrying.
  • Aggregate error messages include the number of unknown outcomes when present, while omitting that count when there are none.

[cristim]

@coderabbitai review

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 2 billable files and costs up to $0.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 52 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 73 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: 09e22615-60b5-4eee-87d7-828ddd0ed7c8

📥 Commits

Reviewing files that changed from the base of the PR and between 36c2bbd and d995f10.

📒 Files selected for processing: 2

• providers/aws/ladder/reshape.go
• providers/aws/ladder/reshape_test.go

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: LeanerCloud/cloud-commitments-go/.coderabbit.yaml
• Review profile: CHILL
• Plan: Essentials
• Run ID: 89ce53df-ccb7-4187-94c4-3e3bdf2137ab

📥 Commits

Reviewing files that changed from the base of the PR and between 36d8b11 and 36c2bbd.

⛔ Files ignored due to path filters: 1

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

📒 Files selected for processing: 3

• providers/aws/go.mod
• providers/aws/ladder/reshape.go
• providers/aws/ladder/reshape_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

The AWS ladder exchange summary now distinguishes failed attempts with unknown outcomes from other failures. It labels unknown outcomes with a reconciliation warning and adds their count to the aggregate error when the count is nonzero. The AWS module dependency version also changes.

Changes: Exchange Outcome Reporting

Layer / File(s)	Summary
Distinguish unknown exchange outcomes providers/aws/ladder/reshape.go, providers/aws/ladder/reshape_test.go, providers/aws/go.mod	The summary labels unknown outcomes with a reconciliation warning and includes their count in the aggregate error when nonzero. Tests cover unknown and ordinary failures. The AWS module dependency version changes.

Priority: ⬇️ Low

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

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 36c2b

The AWS ladder summary identifies unknown outcomes for reconciliation while preserving ordinary failure labels; no actionable merge-blocking risk is established.

Pre-merge checks: 3 · 2

Failed checks: 2 warnings

Check name	Status	Explanation	Resolution
Linked Issues check		The PR satisfies the AWS ladder part of #443. summarizeExchangeResult labels OutcomeUnknown entries as outcome unknown, reconcile before retrying and reports the unknown count. The typed `Outcom…	Implement and test the required platform handling for direct ExecuteExchange paths, or update #443 to remove that requirement before closing it.
Docstring Coverage		Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. (1 skipped: 1…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 3 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: reporting AWS ladder exchange outcomes marked unknown as unknown instead of failed.
Out of Scope Changes check		The changed files stay connected to #443. The AWS package pin supplies the typed outcome signal used by the ladder change. The ladder test changes verify the required labels and count behavior. The de…

Full details: Out of Scope Changes check

Explanation

The changed files stay connected to #443. The AWS package pin supplies the typed outcome signal used by the ladder change. The ladder test changes verify the required labels and count behavior. The dependency pin's additional documented and test changes are incidental to the requested pin update and do not demonstrate unrelated PR scope.

Full details: Linked Issues check

Explanation

The PR satisfies the AWS ladder part of #443. summarizeExchangeResult labels OutcomeUnknown entries as outcome unknown, reconcile before retrying and reports the unknown count. The typed OutcomeUnknown signal is supplied by the updated package pin. However, #443 also requests platform handling so unknown outcomes are not treated as ordinary retryable failures. The PR explicitly leaves manual platform paths to external issue #1050, and no platform handling change is present here.

Resolution

Implement and test the required platform handling for direct ExecuteExchange paths, or update #443 to remove that requirement before closing it.

Full details: Docstring Coverage

Explanation

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

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

✨ 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]

Independent review: BLOCKING FINDING at 36c2bbd (one, narrow)

Reviewer: fresh Claude agent (Sonnet, high effort), not the author, cold brief. Coordinator: claude-cc-reviewer, who confirmed it by reading the code.

Blocking: providers/aws/ladder/reshape.go:203-209 builds the returned error with plain fmt.Errorf and no %w, so it does not wrap common.ErrOutcomeUnknown. errors.Is(err, common.ErrOutcomeUnknown) is false. The only signal is the text suffix "; N with unknown outcome". A caller (for example platform's ladder through internal/server/ladder_write.go) cannot tell "maybe executed, reconcile" from "definitely failed, retry is safe" without parsing strings. That is the double-exchange risk #443 is meant to close.

Requested fix:

• When unknown > 0, wrap the sentinel, e.g. fmt.Errorf("%w: ReshapeBuffer: ...", common.ErrOutcomeUnknown, ...) or errors.Join.
• In the test, assert ErrorIs(err, common.ErrOutcomeUnknown) for the unknown and mixed cases, and NotErrorIs for a definite-only failure.
• Optional: in the mixed case the "first:" field names Failed[0], which may be a definite failure. Consider naming the first unknown entry instead.

Verified OK:

• The pkg pin contains feat(exchange): flag an exchange whose accept outcome is unknown #497 (b5e755f).
• Unknown entries are rendered distinctly (:183-190, asserted in reshape_test.go:330-334).
• go vet is clean, and go test -race ./... in providers/aws passes 2274.
• With main's reshape.go restored, the new test fails, so it catches a regression.

A fleet implementer will push the fix as a new commit, and the same review will re-check the delta. (claude-cc-reviewer)

[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]

Delta verdict: CLEAN at d995f10

Same reviewer, scope git diff 36c2bbde d995f103. This resolves the blocking finding.

• The sentinel is wrapped: when unknown > 0, the ReshapeBuffer error wraps common.ErrOutcomeUnknown via %w, so errors.Is is true. With unknown == 0 it is not wrapped. The message is unchanged apart from a trailing sentinel text in the unknown case.
• "first:" names the right entry: it is the first OutcomeUnknown entry when unknown > 0, and Failed[0] otherwise. The mixed-case test puts the definite failure first, so it covers this.
• Tests: they assert ErrorIs for the unknown-only and mixed cases, and NotErrorIs for definite-only.
• Checks: go vet ./ladder/... is clean, and go test -race -count=1 ./ladder/... passes 268.
• Mutation probe: %w changed to %v fails both TestReshapeBuffer_UnknownOutcome_RenderedAsUnknownNotFailed and TestReshapeBuffer_UnknownOnly_WrapsErrOutcomeUnknown.

Gate: labels are mirrored from #443. Merge once CI is green at d995f10. (claude-cc-reviewer)