[cristim]

Part of #682 (slice P0 of the reviewed plan). This slice doesn't close the issue.

Why

#682 P1 will refuse a GCP purchase per recommendation, before any purchase call, when a recent commitment for the same project, region and family already exists. For that refusal to mean "nothing was submitted", the execution has to record no_submission and release its suppressions. Today it can't:

• Per-rec purchase errors reach the execution only as strings ("some purchases failed: ...").
• So a rec-level error wrapped with beforeSubmission(...) never reaches saveTerminalExecution as that class.

Without this slice:

• a refused row would keep its suppressions;
• it would count as holding capacity in the ramp, which under-buys;
• a refused ladder tranche would stay at outcome unknown until a human acted.

What

failureTally (new internal/purchase/refusal_tally.go) counts, while the error values are still in hand, how many failed recs carried the beforeSubmission class without common.ErrOutcomeUnknown. A row counts as all refused only when:

• at least one rec failed;
• every failure was such a refusal;
• no rec was purchased.

In that case only:

• Single-account run: failedPurchasesError returns the aggregate error with the beforeSubmission class, so saveTerminalExecution records no_submission and releases suppressions.
• Fan-out: markRefusedAccountRow sets NoSubmission on the failed per-account row before saveAccountRow. Suppressions belong to the root execution, so there is nothing to release per account, the same as on the existing skipped-account path.

Any purchased rec, any unknown-outcome rec, or any other failure keeps today's behaviour.

Fan-out scope. On a multi-account run only the refused account's own row carries NoSubmission, which affects History, per-account Retry and first-dispatch minting. The root row still ends failed, or partially completed when another account bought, without the proof. So the ramp, suppression and ladder effects above apply to single-account runs only; for fan-out roots they stay as today, in the safe direction. Carrying the proof to an all-refused root is tracked in #1844.

Blast radius today: none. No rec-level beforeSubmission wrap exists on main. Every current use is at execution level and returns before the per-rec aggregate: execution.go plan, scope and shape checks, manager.go refusals, step_cap.go, and redshift_lineage.go, a gate on the whole execution that runs before the fan-out. #682 P1 adds the first rec-level refusal.

Tests (internal/purchase/refusal_tally_test.go)

• Single-account, all recs refused: the error has the beforeSubmission class; after saveTerminalExecution, NoSubmission is true and DeleteSuppressionsByExecutionTx was called.
• Fan-out: the refused account's saved row has NoSubmission, and the account that bought doesn't.
• Mixed outcomes, single-account and fan-out, each unchanged:
  • purchased plus refused;
  • unknown outcome plus refused;
  • plain failure plus refused;
  • a refusal that also wraps an unknown outcome plus refused. This one is not counted as refused, because it may have reached the cloud.

Verification

Local, macOS, at 95c7d79 (base 8bdc4d9; merges cleanly onto main, and main hasn't changed execution.go or manager.go since):

• Probes, each reverted. All three were killed:
  1. the predicate never matches: 3 tests fail;
  2. the purchased-rec check is dropped: 2 fail;
  3. the unknown-outcome check is dropped: 4 fail. This needed the refusal-with-unknown-outcome case added in 95c7d79.
• Tests: unit tests for internal/purchase, internal/api and internal/server pass, and so does go test -tags integration ./internal/purchase/.
• Lint: golangci-lint reports no new issues.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 15 minutes.

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

• Source mode: Auto
• Repository: LeanerCloud/cloud-commitments-platform
• Branch: fix/682-p0-before-submission-aggregate
• File: .coderabbit.yaml
• Commit: eb72026
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 03ce826f-abce-4357-8191-effed59d8352

Commits

Reviewing files that changed from the base of the PR and between 97f82af and eb72026.

Files selected for processing: 8

• internal/purchase/azure_redis_routing_test.go
• internal/purchase/azure_savings_plans_alias_proof_test.go
• internal/purchase/execution.go
• internal/purchase/execution_test.go
• internal/purchase/money_path_regression_test.go
• internal/purchase/plan_step_proofs_test.go
• internal/purchase/refusal_tally.go
• internal/purchase/refusal_tally_test.go

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

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

[cristim]

CHANGES REQUESTED 95c7d79 by claude-qa-ui-3

Cold review (money path). The aggregation logic is correct. One test gap: a branch that guards against a re-buy has no test, so a regression there would pass.

Finding (should-fix, test gap on a money path)

tally.add(r.Err) for a fan-out-level error is untested.

• aggregatePurchaseOutcomes (internal/purchase/execution.go, the r.Err != nil branch) counts a fan-out-level error as a failed, non-refused rec.
• That r.Err includes a panic recovered inside the per-rec closure (internal/execution/fanout.go:116-122). The closure runs executeSinglePurchase, so the panic can come after the provider call, with the outcome unknown.
• Probe P5: I deleted tally.add(r.Err), and the suite stays green.
• Without that line, a run with one rec refused before submission and one rec that panicked mid-purchase becomes failed=1, refused=1, so it's "all refused". saveTerminalExecution would then record no_submission and release the row's suppressions, and a purchase that may have been sent could be planned and bought again.

Fix: add a case, single-account and fan-out, where one rec is refused (beforeSubmission) and another produces a fan-out-level r.Err. A panicking fake provider is the realistic shape. Assert the row is not no_submission and suppressions are not released.

Verified

• Predicate (refusal_tally.go): allRefused needs ≥1 failure, every failure beforeSubmission without ErrOutcomeUnknown, and no Purchased rec.
  • An out-of-range index adds a non-refused failure (tally.add(nil)).
  • A rec that bought but couldn't record (PurchaseID set, not Purchased) carries ErrOutcomeUnknown (go R1-3), so it can never count as refused.
• Every caller of the changed aggregation:
  • processPurchaseRecommendations is called only at execution.go:131 (single account), which feeds failedPurchasesError at :138 only inside len(purchaseErrors) > 0 && !anyRecPurchased, and at :376 (fan-out), which feeds markRefusedAccountRow at :387 after applyAccountOutcome.
  • aggregatePurchaseOutcomes is called only at :845.
• Zero blast radius today: confirmed. Every non-test beforeSubmission( site is execution-level and returns before the per-rec aggregation:
  • execution.go:66,69,76,169,173,246 (plan, scope, account gates);
  • fan_out_shape.go:65,76,82,97,100;
  • manager.go:425,428 (preExecutionRefusal);
  • step_cap.go:82 (failClaimedBeforeSubmission);
  • redshift_lineage.go:97,134,139 (redshiftGate takes the whole exec, reached through stampRedshiftDispatch from manager.go:342).
  • refusal_tally.go:41 is the new aggregate wrap. No per-rec wrap exists yet.

Probes (re-run on a scratch copy at this SHA, scoped -run)

Probe	Mutation	Result
P1	predicate never matches	killed: TestSingleAccount_AllRefusedBeforeSubmission_EndsNoSubmissionAndReleasesSuppressions, TestMultiAccount_RefusedAccountRowIsNoSubmission
P2	purchased-rec check dropped	killed: TestSingleAccount_MixedOutcomesAreNotAllRefused
P3	unknown-outcome check dropped	killed: TestSingleAccount_MixedOutcomesAreNotAllRefused, TestMultiAccount_MixedOutcomesAreNotNoSubmission
P4	markRefusedAccountRow ignores Status == failed	survives. Equivalent in practice: allRefused implies a failed row from applyAccountOutcome.
P5	tally.add(r.Err) removed	survives: the finding above
P6	t.failed > 0 dropped	survives. Equivalent today: both callers only act with ≥1 error and a failed row.

CI: run 42336134755 at this SHA was still running when I posted (30 pass, 7 pending). The full suites are left to CI.

[cristim]

CHANGES REQUESTED 95c7d79 by claude-cc-platform

One blocking item (a missing money-path test, no code change needed). failureTally.add (refusal_tally.go:20-26) counts a failure as refused only when it carries beforeSubmissionError and no ErrOutcomeUnknown. The fan-out branch tally.add(r.Err) at execution.go:866 is the only thing that counts a closure-level failure (for example a recovered panic after the provider call) as a non-refused failure. Delete it and a run with that failure plus refused recs satisfies allRefused, gets NoSubmission, and can be retried into a second buy. Probe m5 below deletes it and the whole internal/purchase unit suite stays green. Please add a test with one refused rec and one rec whose closure returns an error or panics, asserting that NoSubmission is not set. With that test in place (m5 killed), this becomes an approval. Everything else below holds.

---

Cold adversarial review of PR #1842 (Part of #682, slice P0) at head 95c7d79, base 8bdc4d9. The head did not move during the review. The PR merges cleanly onto main d618d7c (git merge-tree clean); the only main drift in internal/purchase since the base is errnotes.go (an RI exchange allowlist pattern), which does not touch this path, and the stored error text of a refused run is unchanged because beforeSubmissionError keeps the wrapped message (manager.go:440).

Blocking findings

None.

Non-blocking findings

1. Fan-out runs do not get the ramp or ladder effect P0 promises. On a multi-account plan the flag lands only on the per-account row (execution.go:387). The root row still ends failed without NoSubmission when every account failed (summarizeFanOut returns errAllAccountsFailed as a plain string error, execution.go:290-293; finalizeExecution maps it to failed, manager.go:181-183; saveTerminalExecution does not see the beforeSubmission class), or partially_completed when another account bought. ListPlanCommittedTx counts every row that holds capacity and takes the max per identity per step across root and account rows (store_postgres_ramp_baseline.go:101-103, 131-139, 170-176), so the unflagged root keeps counting the refused account's recs and the ramp still under-buys. The ladder chain starts at the tranche's execution, the root (store_postgres_ladder_reconcile.go:104-113), so a refused fan-out tranche stays fired with "failed without a no_submission proof" (ladder_reconcile.go:79-80). This is the safe direction and the reviewed plan lists kept suppressions on multi-account runs as a known limit, but the plan's P0 text says the ramp will not count the run, and the PR body's "Why" repeats that without the fan-out caveat. Fix: say in the PR body and on fix(gcp): wire the recent-commitment duplicate checker into the platform #682 that P0 changes only per-account rows on fan-out runs (History, per-account Retry, first-dispatch minting), and file a follow-up if all-accounts-refused roots should carry the proof (it would need every account error to keep the class through summarizeFanOut, plus skipped and credential-failure accounts to be treated the same way).

2. Test gap, probe m5 SURVIVED the whole internal/purchase unit suite: deleting tally.add(r.Err) (execution.go:866) leaves every test green. That line is what keeps a recovered panic after a provider call (fanout.go recover, surfaced as r.Err) from being ignored; without it, one refused rec plus one rec that panicked after submitting would be reported as all refused, stamp NoSubmission and release suppressions while a commitment may exist. The code is right today; only the guard is untested. Fix: add a case where one rec is refused and the other panics in PurchaseCommitment (the mock can panic), and assert allRefused is false.

3. Probe m6 SURVIVED: deleting tally.add(nil) (execution.go:878). The out-of-range branch is defensive and unreachable today, so this is low value; mention only.

4. Redundant guard (deletion probe m11 equivalent): row.Status == config.StatusFailed in markRefusedAccountRow (refusal_tally.go:50) can never be false when allRefused is true, because allRefused implies a failure and no purchased rec, which applyAccountOutcome maps to failed (execution.go:530-533). Harmless defensive check; keep or drop.

5. Out of scope, noted for P1 planning: rejectUnsupportedPartition (partition_guard.go:22-31) is a genuine per-rec refusal before any AWS call but returns a plain error, so an execution whose recs are all aws-cn Savings Plans still ends with an unknown outcome. Wrapping it with beforeSubmission would now flow through this aggregate. Not a defect of this PR.

Caller table (git grep at the head)

Caller	What it does with the result	Changed?
execution.go:131 executeSingleAccount -> processPurchaseRecommendations	when errors exist and nothing purchased, returns failedPurchasesError (execution.go:136-139) to executePurchase -> executeAndFinalize -> finalizeExecution (failed) -> saveOrRecoverTerminal -> saveTerminalExecution (manager.go:453-476)	Yes, only when allRefused: the error gains the beforeSubmission class, so the row gets NoSubmission and, for a plan row, suppressions are deleted in the same tx. Otherwise identical text and class.
execution.go:376 executeForAccountLate -> processPurchaseRecommendations	applyAccountOutcome stamps status (execution.go:386), then saveAccountRow (execution.go:389), confirmation if committed, returns a plain "some purchases failed" error to summarizeFanOut	Yes, only when allRefused: per-account row saved with NoSubmission=true. Returned error, root aggregation, notification unchanged.
execution.go:845 processPurchaseRecommendations -> aggregatePurchaseOutcomes	only caller of the aggregator	Adds the bool; totals, purchaseErrors and rec writes unchanged.
Test callers: azure_savings_plans_alias_proof_test.go:111, execution_test.go:2251, money_path_regression_test.go:80,137,165, plan_step_proofs_test.go:108, refusal_tally_test.go	signature update only	No behaviour change.

No caller in internal/api or internal/server (unexported). No other production caller.

Checked and holding

• Predicate: refusal_tally.go:21-31 counts a rec as refused only with the beforeSubmission class and no ErrOutcomeUnknown; allRefused needs failed > 0, failed == refused and no purchased rec. Closure errors (ctx cancel, recovered panic) and the out-of-range branch count as non-refused failures (execution.go:866, 878), so they keep today's behaviour.
• All-refused, single account: status failed (manager.go:181-183), no completed or purchased claim, totals only accumulate on success (execution.go:897-898), failure email via notifyRootFailure -> notifyExecutionFailure (failure_notify.go:203-209), no confirmation (execution.go:145). saveTerminalExecution re-checks failed and no purchased rec (manager.go:455), so the flag cannot land on a partial row.
• Purchased rec: any Purchased=true rec in the slice, including one carried from a predecessor, blocks allRefused (refusal_tally.go:31, anyRecPurchased execution.go:197-204). Purchased recs never call tally.add, so they are neither double counted nor turned into failures.
• Refused plus unknown outcome: the unknown rec is a failure that is not refused, so failed != refused and no flag. A refusal that itself wraps ErrOutcomeUnknown is not counted as refused (refusal_tally.go:23). Covered by tests and by probes pr3 and m4.
• Nil and empty recs: no selected rec returns early with allRefused false (execution.go:784-788); nil error into tally.add counts as a non-refused failure.
• Persistence: the upsert ORs no_submission on failed rows (store_postgres.go upsert no_submission = CASE WHEN $3 = 'failed' THEN purchase_executions.no_submission OR $28), so the account row insert carries the flag and it is never cleared by a later failed write.
• Retry and Redshift: NeverSubmitted needs NoSubmission and attempt 0 (redshift_retry.go:26-28); StampFirstDispatchTx mints a new stamp only after a no_submission attempt 0 (store_postgres_first_dispatch.go:31-38). No rec-level beforeSubmission wrap exists at the head (git grep: every wrap is in execution-level gates: execution.go:66,69,76,169,173,246, fan_out_shape.go, manager.go:425,428, redshift_lineage.go:97,134,139, step_cap.go:82), so the PR's "no blast radius today" claim holds: allRefused is false for every current run.
• Concurrency and DB: the tally is local to the single-threaded aggregator; no new DB reads, no pool reads inside a caller's tx.
• Over-engineering: one small struct and two helpers, all with a real caller in this PR; P1 is the stated consumer of the class.

CI at the head (all completed, success)

• 42336134755 CI - Build & Test (unit, lint, integration suites for purchase, config, api, server, migrations, E2E, Playwright): success
• 42336134697 pre-commit: success
• 42336134666 Frontend build (PR): success
• CodeRabbit: skipping

Probes

Each probe was inserted in a scratch worktree, the insertion asserted with cmp and git diff --stat, run under lockf -k -t 3600 /tmp/claude/heavy-review-2.lock nice -n 10 env GOWORK=off GOMAXPROCS=4 go test -p 2 -count=1 on ./internal/purchase/, then reverted and the revert asserted. The baseline passed (scoped and full unit suite).

Probe	Change	Result
pr1 (PR body)	allRefused always false	killed: TestSingleAccount_AllRefusedBeforeSubmission..., TestMultiAccount_RefusedAccountRowIsNoSubmission
pr2 (PR body)	drop !anyRecPurchased	killed: TestSingleAccount_MixedOutcomesAreNotAllRefused
pr3 (PR body)	drop the ErrOutcomeUnknown exclusion	killed: TestSingleAccount_MixedOutcomesAreNotAllRefused, TestMultiAccount_MixedOutcomesAreNotNoSubmission
m4	unknown outcome counts as refused	killed: same two tests
m5	delete tally.add(r.Err)	SURVIVED (scoped and full unit suite)
m6	delete tally.add(nil)	SURVIVED (scoped and full unit suite); unreachable branch
m7	delete tally.add(v.err)	killed: both all-refused tests
m8	failedPurchasesError always wraps	killed: TestExecuteAndFinalize_PostSubmissionFailureKeepsSuppressions
m9	markRefusedAccountRow ignores allRefused	killed: TestMultiAccount_MixedOutcomesAreNotNoSubmission
m10	drop failed > 0	equivalent: callers only consult the flag when errors exist
m11	drop the status guard in markRefusedAccountRow	equivalent, see non-blocking 4
m12	single-account path ignores allRefused	killed: TestSingleAccount_AllRefusedBeforeSubmission...
m13	fan-out never marks the row	killed: TestMultiAccount_RefusedAccountRowIsNoSubmission
m14	mark before applyAccountOutcome	killed: TestMultiAccount_RefusedAccountRowIsNoSubmission

Not verified

• Integration suites were not run locally; CI run 42336134755 ran them at this SHA and passed.
• Tests were not rerun on the merge of this head with current main; the merge is textually clean and main's changes do not touch these files' code paths.
• No real-cloud verification (not authorized, and no rec-level refusal exists until P1).

[cristim]

The fix round for both reviews is 67ba0c6, test-only. A rec whose PurchaseCommitment panics (the fan-out recovers it into a closure-level r.Err, possibly after the provider call) sits next to a refused rec, in both the single-account and fan-out mixed-outcome tests. The assertions are that the row is not no_submission and its suppressions are kept.

Probe m5 (delete tally.add(r.Err), internal/purchase/execution.go:866) is now killed by 4 tests. The internal/purchase unit suite passes and golangci reports no new issues.

The fan-out scope (only a refused account's own row gets the proof, and the root keeps today's outcome) is now stated in the PR body and on #682. The follow-up for the root row is #1844.

Delta reviews at the new head, please: @claude-qa-ui-3 @claude-cc-platform.

[cristim]

APPROVED 67ba0c6 by claude-cc-platform

Full re-review after my CHANGES at 95c7d791.

My finding is fixed. 67ba0c6b is a single test-only commit. In internal/purchase/refusal_tally_test.go, a recPanics behaviour makes the mocked PurchaseCommitment panic after the call (so the fan-out recovers it into a closure-level error), and a "panic plus refused" case is added to refusalCases(). That table drives both TestSingleAccount_MixedOutcomesAreNotAllRefused and TestMultiAccount_MixedOutcomesAreNotNoSubmission, so the case is pinned on both the single-account and fan-out paths. Deleting tally.add(r.Err) (execution.go:866) now fails both tests (probe m5 killed).

Rest of the diff. The non-test code is byte-identical to 95c7d791 (git diff 95c7d791 67ba0c6b touches only the test file). My earlier analysis therefore stands:

• the two callers (execution.go:131 and :376) and the all-refused, purchased-rec and refused-with-unknown-outcome paths;
• the non-blocking notes: fan-out root rows don't get the ramp or ladder effect yet, the redundant status check, and the aws-cn partition refusal for P1.

Scoped probes at 67ba0c6b (-run 'AllRefused|MixedOutcomes|RefusedAccountRow|NoSubmission|Refus', heavy-review-2; each insertion asserted, then reverted):

• m5, tally.add(r.Err) deleted: killed by MixedOutcomesAreNotAllRefused and MixedOutcomesAreNotNoSubmission.
• An unknown outcome counted as refused: killed by both.
• Every failure counted as refused: killed by both.
• The purchased-rec check removed from allRefused: killed by MixedOutcomesAreNotAllRefused.
• t.failed > 0 removed: survived. The only extra case it admits is a run with zero failures and nothing purchased, which really did submit nothing, so it's an equivalent mutant.

CI at 67ba0c6b was still running when I checked (12 pass, 25 pending, including Unit Tests and Integration Tests (purchase) in run 42414371682). Merge once it's green.

There's no review thread to resolve: my finding was a PR comment, not an inline thread.

[cristim]

CHANGES REQUESTED 67ba0c6 by claude-qa-ui-3

Full re-review at 67ba0c6. My finding is fixed, but CI at this SHA is red because of a combination with main.

Blocking: the PR no longer compiles against current main

• main's chore(deps): pin cloud-commitments-go at 0b677e1 with its Azure Redis and ladder status changes #1837 (merged) added internal/purchase/azure_redis_routing_test.go.
• At line 35 it calls m.processPurchaseRecommendations(...) with 4 results.
• This PR changes that function to return 5 (allRefused).
• CI builds the PR merged with main, so internal/purchase fails to build. Run 42414371682 reports "assignment mismatch: 4 variables but m.processPurchaseRecommendations returns 5 values" at azure_redis_routing_test.go:35:21.
• As a result, Unit Tests, Integration Tests (purchase), Lint Code, pre-commit and CI Success all fail.
• Fix: merge main and add the fifth return to that call, e.g. _, _, errs, _, err := .... Then re-run.

My earlier finding: fixed

• 67ba0c6 adds the "panic plus refused" case to refusalCases (internal/purchase/refusal_tally_test.go). The mock provider panics inside PurchaseCommitment, after the call could have reached the provider, and the fan-out recovers that into r.Err. The case expects not all refused and no no_submission, on both the single-account and fan-out paths.
• P5 (tally.add(r.Err) removed) now fails TestSingleAccount_MixedOutcomesAreNotAllRefused and TestMultiAccount_MixedOutcomesAreNotNoSubmission.

Rest of the diff (unchanged since 95c7d79; only the test file changed)

• P1-P3 still fail named tests: P1 (predicate never matches), P2 (purchased check dropped), P3 (unknown-outcome check dropped).
• P4 and P6 are equivalent in practice, as noted before.
• My caller and blast-radius verification from 95c7d79 still holds. No non-test file changed.

Local probes ran on the PR head alone (scratch copy, scoped -run, private GOCACHE), which is why they passed. The break only appears when the PR is combined with main.

[cristim]

Main is merged in (cd157d5), and #1837's Azure Redis routing test now uses the four-value processPurchaseRecommendations result (eb72026). Local results at that head:

• go vet passes.
• Unit tests pass for purchase, api, server, scheduler and config.
• -tags integration passes for purchase.
• golangci reports no new issues.

Full re-reviews at eb72026, please: @claude-qa-ui-3 @claude-cc-platform.

[cristim]

APPROVED eb72026 by claude-cc-platform

Full re-review at eb720262, after my APPROVED at 67ba0c6b (which followed my CHANGES at 95c7d791).

• My CHANGES finding is still fixed. The "panic plus refused" case in internal/purchase/refusal_tally_test.go (refusalCases(), recPanics) is unchanged. Deleting tally.add(r.Err) at internal/purchase/execution.go:866 on this merged tree fails TestSingleAccount_MixedOutcomesAreNotAllRefused and TestMultiAccount_MixedOutcomesAreNotNoSubmission (probe m5 killed).
• Merge resolution. cd157d54 merges main 97f82afd (fix(ri-exchange): guard direct execute against an RI held by an in-flight exchange #1838, fix(riexchange): show the approve outcome instead of always reporting success #1828, chore(deps): pin cloud-commitments-go at 0b677e1 with its Azure Redis and ladder status changes #1837). git show --remerge-diff cd157d54 is empty, so there were no conflict edits. The PR's own diff against the new base (git diff 97f82afd cd157d54) is byte-identical to git diff 8bdc4d97 67ba0c6b.
• eb720262 changes one line in chore(deps): pin cloud-commitments-go at 0b677e1 with its Azure Redis and ladder status changes #1837's internal/purchase/azure_redis_routing_test.go: the processPurchaseRecommendations call takes the extra allRefused return as _. That's the only adaptation the widened signature needs. go vet ./internal/purchase/ is clean.
• Scoped runs at eb720262 (heavy-review-2, -run 'AllRefused|MixedOutcomes|RefusedAccountRow|NoSubmission|Refus|AzureRedis|Routing'): pass, including chore(deps): pin cloud-commitments-go at 0b677e1 with its Azure Redis and ladder status changes #1837's routing test.

My earlier analysis stands unchanged, because the non-test code is unchanged: the two callers, the all-refused, purchased-rec and refused-with-unknown-outcome paths, and the non-blocking notes.

CI at eb720262 was still running when I checked (22 pass, 15 pending). Merge once it's green.

There are no inline review threads to resolve: my finding was a PR comment.

[cristim]

APPROVED eb72026 by claude-qa-ui-3

Full re-review at eb72026. Both of my CHANGES REQUESTED are resolved.

1. Test gap, from 95c7d79: fixed in 67ba0c6.

• The "panic plus refused" case in refusalCases (internal/purchase/refusal_tally_test.go:110) panics inside PurchaseCommitment (:85-88). It runs on both the single-account and fan-out paths.
• P5 (tally.add(r.Err) removed) fails TestSingleAccount_MixedOutcomesAreNotAllRefused and TestMultiAccount_MixedOutcomesAreNotNoSubmission.

2. Compile break against main, from 67ba0c6: fixed in cd157d5 and eb72026.

• cd157d5 merges main (97f82af, chore(deps): pin cloud-commitments-go at 0b677e1 with its Azure Redis and ladder status changes #1837). --remerge-diff is empty, so there were no hand resolutions.
• eb72026 updates the chore(deps): pin cloud-commitments-go at 0b677e1 with its Azure Redis and ladder status changes #1837 call at internal/purchase/azure_redis_routing_test.go:35 to take all five results: _, _, errs, _, err := .... The commit title says "four-value", but the call is correct.
• go build ./... and go vet ./internal/purchase/ are OK at this SHA.

The diff, re-checked after the merge

• Every caller of processPurchaseRecommendations and aggregatePurchaseOutcomes at this SHA takes the new results. The non-test callers are execution.go:131 and :376, plus :845. The test callers are azure_redis_routing_test.go:35, azure_savings_plans_alias_proof_test.go:111, execution_test.go:2253, money_path_regression_test.go:80,137,165, plan_step_proofs_test.go:108, refusal_tally_test.go:148 and aggregate_unrecorded_commitment_test.go:20.
• The beforeSubmission( site set is unchanged by the merge. It's the same 18 call sites plus the definition, all execution-level, so zero blast radius today still holds.
• The aggregation and refusal_tally.go logic is unchanged since my review at 95c7d79.

Probes at this SHA (scratch copy, scoped -run, private GOCACHE)

• P1 (predicate never matches): killed by TestSingleAccount_AllRefusedBeforeSubmission_EndsNoSubmissionAndReleasesSuppressions and TestMultiAccount_RefusedAccountRowIsNoSubmission.
• P2 (purchased check dropped): killed by TestSingleAccount_MixedOutcomesAreNotAllRefused.
• P3 (unknown-outcome check dropped): killed by TestSingleAccount_MixedOutcomesAreNotAllRefused and TestMultiAccount_MixedOutcomesAreNotNoSubmission.
• P5: killed, as above.
• P4 and P6 remain equivalent in practice, as noted before.

CI: run 42417767811 at this SHA; 39 checks pass and 1 was pending when I posted.