feat(purchase): atomic plan-step claim, shared suppressions and account checks

[cristim]

Refs #631 (PR2a of the split; PR2b adds the resolver). No resolver yet, so no user-visible change: bare steps still fail once.

What

Prerequisites that make attaching real recommendations to a plan step safe:

• Atomic get-or-create (A4/A8, E1/E2). getOrCreateExecution now runs under LockPurchasePlanTx and is keyed on (plan_id, step_number), not on plan.NextExecutionDate. ANY existing row for the step (failed, partially_completed, expired, canceled, completed, in flight) blocks creation; only a pending/notified ROOT row (no account, retry_attempt_n 0) is adopted. New store methods ListExecutionsForPlanStepTx, SetExecutionRecommendationsIfEmptyTx (CAS), ExpireExecutionAtomic.
• Shared suppressions (A5). buildSuppressions moved to config.BuildSuppressions. Manager.AttachResolvedRecommendations writes recs + suppressions in one tx (called by PR2b).
• E3. Suppressions are also deleted in the same tx on expiry (history sweep) and when a plan row ends failed having bought nothing.
• E5. executeSingleAccount refuses a plan row whose selected recs are not attributed to the row's own account (strict; direct execute unchanged).

Evidence (fixture/mock + real Postgres via testcontainers)

• Concurrent-tick integration test (8 ticks, exactly one row); settled-root (failed, partially_completed) blocking; concurrent attach (exactly one winner, one suppression set); expiry releases suppressions.
• Mutations proven: removing the plan lock fails the concurrent test; skipping suppressions fails the attach test; a live-status filter (E1) fails the unit and integration blocking tests.
• go test and go test -tags integration for purchase and api, go test for config/server/analytics, golangci-lint clean, under the repo test lock.
• Existing fixtures that relied on unattributed recs on account-bound rows now attribute them.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 26 billable files and costs up to $6.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 57 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 88 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: 7fc8b4cd-d276-40d0-88cf-7cf004e78dfc

---

📥 Commits

Reviewing files that changed from the base of the PR and between f7ffabc and 105501c.

---

📒 Files selected for processing (26)

• internal/analytics/collector_test.go
• internal/api/handler_history.go
• internal/api/handler_history_actionable_starvation_test.go
• internal/api/handler_history_test.go
• internal/api/handler_purchases.go
• internal/api/handler_purchases_pause_claim_integration_test.go
• internal/api/handler_purchases_retry_fanout_test.go
• internal/api/handler_purchases_run_now_integration_test.go
• internal/config/interfaces.go
• internal/config/store_postgres_ramp_step.go
• internal/config/suppressions_build.go
• internal/config/suppressions_build_test.go
• internal/mocks/stores.go
• internal/purchase/account_scope.go
• internal/purchase/attach_recommendations.go
• internal/purchase/execution.go
• internal/purchase/manager.go
• internal/purchase/money_path_regression_test.go
• internal/purchase/notifications.go
• internal/purchase/notifications_test.go
• internal/purchase/plan_step_atomic_integration_test.go
• internal/purchase/plan_step_atomic_test.go
• internal/purchase/plan_step_no_recs_test.go
• internal/purchase/ramp_step_progress_integration_test.go
• internal/purchase/step_rows_test.go
• internal/server/test_helpers_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]

Merge-gate review (independent, adversarial, money path): CHANGES REQUESTED

Reviewed SHA: 3448547c60084fddabf33bd34be0a5fd1a5cd0b4 (base main, up to date; CI 26/26 green, mergeStateStatus CLEAN). Not merged.

Blocking findings

1. E3 releases suppressions on an outcome-unknown failure - internal/purchase/manager.go saveTerminalExecution (the exec.Status != "failed" || anyRecPurchased(...) guard). "Bought nothing" is inferred from Purchased == false, but AWS EC2/Redshift report a lost response as purchase outcome unknown (see manager.go:405-407, :443-447): the rec stays Purchased=false, the row ends failed, and this PR now deletes its suppressions. The commitment may exist, yet the rec comes back on Opportunities and, once PR2b's resolver reads suppressions, into the next step's resolution, which is a second EC2/Redshift buy with no provider dedupe (fix(purchase): refuse retry and re-drive of AWS EC2 and Redshift purchases #778/fix(settings): a lingering inline range error stays visible after Reset settings or a reload because programmatic value sets do not re-run the validator #642 class). The same applies to an armed retry refused by armedRedriveRefusal, which is exactly the "may already have bought" case. E3 says a clean all-failed outcome; an unknown outcome is not clean. Suggested fix: keep suppressions when any selected rec's failure is outcome-unknown (or when the row is a refused armed re-drive), and add a test with that error shape that fails if the guard is removed.
2. E3 expiry path has no test that pins the suppression delete - internal/api/handler_history.go:293-300. Mutation: replace return h.config.DeleteSuppressionsByExecutionTx(...) with return nil -> go test ./internal/api/ passes (exit 0). handler_history_test.go:247-267 asserts only ExpireExecutionAtomic; TestExpireExecutionAtomic_ReleasesSuppressions re-implements the tx inline instead of driving the sweep, so it cannot catch it. E3 requires a test: assert DeleteSuppressionsByExecutionTx(staleID) is called when expire returns true and not called when it returns false.

Non-blocking

• CAS SetExecutionRecommendationsIfEmptyTx: dropping the '[]'::jsonb arm survives the suite (rows saved through SavePurchaseExecution store null, never []). Fails closed (attach returns false), but PR2b would silently never attach to a [] row; worth one integration case. Dropping IS NULL also survives, expected: the column is JSONB NOT NULL DEFAULT '[]' (migration 000001:84).

Verified (holds)

• E1/E2: any row for (plan_id, step_number) blocks; only pending/notified root (account NULL, retry_attempt_n 0) adopted. E5: plan rows only, strict equality, nil-account rec fails, direct execute unchanged. Fixture edits (retry fan-out, pause-claim, run-now) attribute recs to the row's account, matching what scopeExecutionsByAccount already guarantees for fan-out children; no hidden regression found. BuildSuppressions body is identical to the removed api.buildSuppressions (diff of bodies shows only package qualifiers). No migrations.
• Mutations, real failing output (Postgres via testcontainers): remove plan lock -> TestGetOrCreateExecution_ConcurrentTicksMintOneRow "should have 1 item(s), but has 2"; live-status filter -> concurrent test "has 8", SettledRootBlocksNewRoot "has 2" for failed and partially_completed, unit ExistingNonAdoptableRowBlocks/failed_root fails; skip suppressions -> ConcurrentAttachesOnce "[] should have 1 item(s)"; drop CAS predicate -> concurrent attach errors; drop root check -> pending_child/pending_retry fail; E5 nil-rec/PlanID/whole-check mutations each fail TestExecuteSingleAccount_PlanRowRecsMustMatchRowAccount; saveTerminal mutations fail TestSaveTerminalExecution_ReleasesSuppressionsOnCleanFailure.
• Local at exact SHA (git archive, GOTOOLCHAIN=go1.26.9, macOS): go build ./..., go vet, golangci-lint (0 issues) on purchase/config/api; go test purchase/config/api/server/analytics exit 0; go test -tags integration purchase (44.6s), api (200.3s), config (213.3s) exit 0 under the repo test lock. Evidence is mock + local Postgres integration; no real-account verification (none needed for PR2a, no resolver).
• Labels match feat(plans): plan steps resolve real recommendations and show real counts and prices (tier B of #609) #631; body is honest (Refs feat(plans): plan steps resolve real recommendations and show real counts and prices (tier B of #609) #631, no 409 claim).

[cristim]

Merge-gate re-review: CHANGES REQUESTED (one test gap; code is correct)

Head was 44c5c485, which was BEHIND main. I ran gh pr update-branch (merge, no force), giving the new head e12084784780d41bc7538036952e494712ad3443. The PR's own files are unchanged (git diff 44c5c485 e1208478 -- <PR files> is empty). Everything below was run at e120847. Not merged.

Code: holds

Every beforeSubmission site runs before any provider purchase call: plan load and plan nil and the no-recs guard (execution.go:62-72), scopeExecutionsByAccount (:202), requireRecsMatchRowAccount and resolveSingleAccountProvider (:99-104, both before processPurchaseRecommendations), and staleAzurePricingRefusal (manager.go:226, before executePurchase). No marker wraps an error from after submission. The multi-account path stays conservative: per-account errors reach errAllAccountsFailed only as joined strings, so a fan-out failure never carries the marker and keeps its suppressions. The armed re-drive refusal and outcome-unknown errors are unmarked, so their suppressions are kept. Blocker 1 from my earlier review is resolved in the code.

Blocking: the post-submission side of the guard is not pinned through the real path

• Mutation return procErr -> return beforeSubmission(procErr) (execution.go:126): go test ./internal/purchase/ exit 0.
• Mutation fmt.Errorf("some purchases failed: ...") -> wrapped in beforeSubmission (execution.go:132, the all-recs-failed / outcome-unknown path): exit 0.
• Mutation that marks every executePurchase error in executeAndFinalize: exit 0.

Only the direct saveTerminalExecution table test pins this, and it is handed a hand-built error. TestExecuteAndFinalize_SuppressionReleaseFollowsSubmission (plan_step_atomic_test.go:217) says "a provider failure keeps the suppressions" but has no subtest for that case. Please add a subtest that drives executeAndFinalize through a provider purchase that fails (for example a purchase outcome unknown error from the mock provider, all recs failed) and asserts DeleteSuppressionsByExecutionTx is NOT called. The three mutations above should then fail.

Verified at e120847 (git archive, GOTOOLCHAIN=go1.26.9, macOS, repo test lock)

• go build ./... and go vet are clean. golangci-lint reports 0 issues. go test for purchase, config, api, server and analytics exits 0. go test -tags integration passes for purchase (53.7s), api (166.5s) and config (177.5s), exit 0.
• Expiry-sweep mutation (DeleteSuppressionsByExecutionTx at handler_history.go:299 replaced by return nil) fails TestExpireStaleExecutionsSweep_ReleasesSuppressionsOnlyWhenExpired/flip_won.
• Removing the '[]' arm of the compare-and-set fails TestSetExecutionRecommendationsIfEmpty_MatchesEmptyArray ("Should be true"). Removing the 'null' arm fails ConcurrentAttachesOnce and ExpireExecutionAtomic_ReleasesSuppressions. Dropping the NULL arm is fine: the column is NOT NULL.
• Removing the marker check from saveTerminalExecution fails the outcome_unknown and refused_armed_re-drive cases.
• CI at e120847 was 24 passing and 1 pending when this was posted, and the merge state was BLOCKED. Evidence comes from mocks and local Postgres integration. There was no real-account verification.

[cristim]

Merge-gate re-review: APPROVED

Reviewed SHA: 105501c565b6563d92a6280d1cc8884fe5459640. The author's head was 21ecfef3, which was behind main by f7ffabc (insurance/Archera, internal/api). I ran gh pr update-branch (merge, no force). The PR's own files are unchanged (git diff 21ecfef3 105501c5 -- <PR files> is empty). The only change from the 21ecfef code is the main merge. Since e120847, the PR code changed only in plan_step_atomic_test.go (+56 lines).

Closes the earlier test gap

TestExecuteAndFinalize_PostSubmissionFailureKeepsSuppressions now pins the post-submission side of the guard. I reproduced these at 21ecfef:

• Wrapping "some purchases failed" (execution.go:132) in beforeSubmission fails provider_failure and outcome_unknown_(lost_response).
• Marking every executePurchase error in executeAndFinalize fails the same two subtests.
• Dropping the anyRecPurchased guard fails pre-submission_error_but_a_rec_bought.
• Wrapping procErr (execution.go:126) still passes, and that is correct. I read every return in processPurchaseRecommendations. The only non-nil error returns are the GlobalConfig load and the offering-class resolution, and both come before the dispatch log and the per-rec fan-out. The fan-out closure's errors (key parse, index range) end up in purchaseErrors, and the function's final return passes a nil error. So the mutation does not change behaviour.

Verified at 105501c (git archive, GOTOOLCHAIN=go1.26.9, macOS, repo test lock)

• go build ./... and go vet pass. golangci-lint reports 0 issues on purchase, config and api.
• go test passes for purchase, config, api, server and analytics (exit 0).
• go test -tags integration passes for purchase (31.7s), api (131.6s) and config (138.1s), exit 0, against Postgres in testcontainers.
• Earlier rounds, unchanged code: the lock, live-status, root-check, suppression, CAS and E5 mutations all fail against real Postgres. The expiry-sweep and '[]'/'null' CAS mutations fail their tests.
• Evidence comes from mocks plus local Postgres integration. There was no real-account verification. PR2a has no resolver and no user-visible change.
• CI at 105501c: 26/26 green, mergeStateStatus CLEAN, contains origin/main. Merging with --squash --match-head-commit 105501c565b6563d92a6280d1cc8884fe5459640.

[cristim]

Merged at reviewed SHA 105501c (squash, --match-head-commit, under the platform merge lock). Merge commit 0a76043.