feat(purchase): make notified real and pin the scheduler claim to the gated snapshot

[cristim]

Refs #631 (PR2b-2a of the plan-step attach work; PR2b-2b does the wiring and must not merge before this).

What this does

• S1, notified becomes real. RotatePendingApprovalToken now sets status = 'notified' in the same UPDATE that stores the token hash, and it still runs only after the email succeeds. A failed send leaves the row pending, and the feat(purchase): per-plan step commitment cap, auto-purchase kill switch and notified veto #825 gate refuses a rec-bearing pending row.
• S2, claim pinned to the gated snapshot. New TransitionExecutionStatusPinned adds AND (recommendations IN ('[]','null')) = $gateSawEmpty to the claim CAS. claimAndExecute (cron processOneExecution and the SQS handleExecutePurchase) passes what the gate saw; a lost pin is a benign skip and the next tick gates again. For pending/notified rows it then re-runs CheckStepCommitmentCap against the freshly read plan and fails the claimed row with beforeSubmission, so suppressions release. Approved, web-token and direct purchases are unchanged (tested).
• S6, fail-closed precheck. planstep.Precheck refuses any Include*/Exclude*/MinCount on a ServiceConfig and an immediate ramp with percent_per_step != 100; checkStoredSPCount refuses an SP stored row with Count != 1. Resolve calls both first. PurchasePlan.Validate is deliberately NOT changed: rejecting those plans at save time would break saving existing plans that never attach anything, and the attach path is the only caller that needs the refusal (PR2b-2b). One existing SP fixture used a stored count of 3 and was changed to 1.

What is NOT wired

Nothing attaches recommendations to a plan step. planstep.Resolve still has no scheduler caller; getOrCreateExecution still fails rec-less steps; no schedule, email content or UI message changes. The only behavior change on today's data: a notified plan row is now notified instead of staying pending after its email.

Current state

Kill switch auto_purchase_enabled stays default OFF and max_step_commitment stays NULL, so the scheduler still refuses every rec-bearing row. Nothing to flip.

Evidence

• Unit (MockConfigStore / MockProviderFactory fixtures, labeled): attach between gate and claim refused on cron and SQS with no provider call; pin sent as true/false; post-claim cap lowered after the gate fails the row before submission and releases suppressions; plan read error after claim fails closed; approved row skips the re-check; email failure leaves pending and the scheduler refuses it; rec-less row still fails.
• Real PostgreSQL 17 (throwaway local server, -tags integration, Docker helper temporarily overridden and reverted before commit): pinned CAS and the notified transition at the store level; manager-level notified transition; a real attach injected between gate and claim leaves the row notified with its recommendations.
• Capped mutation pass, 17 mutants on the pin SQL, the claim arguments, the re-check conditions, the beforeSubmission class, the notified promotion and the precheck: 15 killed at first run; the 2 survivors (post-claim plan read error text, MinCount boundary) got assertions and now fail.
• Review size: ~170 lines of production code, the rest tests.

Not run: macOS real-account verification (no attach exists to exercise); no real DB or cloud was touched.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Purchase processing now skips executions when recommendations change between eligibility checks and claiming, and rechecks commitment limits before submission.
  • Executions are marked as notified only after notification email succeeds, preventing failed notifications from making them eligible for scheduled processing.
  • Plans with unsupported filters, ramp settings, or Savings Plan quantities are rejected before resolution or purchase processing.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: LeanerCloud/cloud-commitments-platform/.coderabbit.yaml
• Review profile: CHILL
• Plan: Essentials
• Run ID: f9f7859c-b565-4f3f-b000-18f420dbde4a

📥 Commits

Reviewing files that changed from the base of the PR and between 93eb871 and 494ae7c.

📒 Files selected for processing (15)

• internal/analytics/collector_test.go
• internal/config/interfaces.go
• internal/config/store_postgres.go
• internal/config/store_postgres_db_test.go
• internal/config/store_postgres_pgxmock_test.go
• internal/mocks/stores.go
• internal/planstep/precheck.go
• internal/planstep/precheck_test.go
• internal/planstep/resolve.go
• internal/planstep/resolve_test.go
• internal/purchase/claim_pin_test.go
• internal/purchase/manager.go
• internal/purchase/notified_claim_pin_integration_test.go
• internal/purchase/step_cap.go
• internal/server/test_helpers_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 2 reviews per hour.

📝 Walkthrough

Walkthrough

The change adds plan prechecks for unsupported filters, immediate-ramp percentages, and Savings Plans counts. It also makes approval status updates atomic and pins execution claims to recommendation state, with commitment-cap checks after eligible claims.

Changes

Purchase notification and claim flow

Layer / File(s)	Summary
Notification status and pinned store transitions internal/config/interfaces.go, internal/config/store_postgres.go, internal/config/store_postgres_db_test.go, internal/config/store_postgres_pgxmock_test.go, internal/mocks/stores.go, internal/analytics/collector_test.go, internal/server/test_helpers_test.go	The store adds a transition that checks whether recommendations remain empty or non-empty. Token rotation sets eligible rows to notified. Store tests and mocks cover the new behavior.
Pinned claims and post-claim checks internal/purchase/manager.go, internal/purchase/step_cap.go, internal/purchase/claim_pin_test.go, internal/purchase/notified_claim_pin_integration_test.go	Purchase execution pins claims to the recommendation state seen at the gate. It skips claims that lose the pin and rechecks the commitment cap after eligible claims. Tests cover claim races, cap refusals, and notification outcomes.

Purchase plan validation

Layer / File(s)	Summary
Plan and recommendation precheck rules internal/planstep/precheck.go, internal/planstep/precheck_test.go	Precheck rejects unsupported service filters and immediate ramps below 100%. Stored Savings Plans rows must have a count of one.
Validation in plan resolution internal/planstep/resolve.go, internal/planstep/precheck_test.go, internal/planstep/resolve_test.go	Resolve runs prechecks before further reads. Sizing also rejects stored Savings Plans counts other than one.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant Scheduler
  participant Manager
  participant PostgresStore
  participant ProviderFactory
  Scheduler->>Manager: process eligible execution
  Manager->>PostgresStore: claim with recommendation-state pin
  PostgresStore-->>Manager: return claim result
  Manager->>PostgresStore: reload plan for commitment-cap check
  Manager->>ProviderFactory: create provider after checks pass

Loading

Merge Risk: ⚪ Minimal · up to ac4b3

No confirmed issue remains that should block merging after normal checks.

Pre-merge checks · 4 · 1

Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 64.52% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 31 functions across 15 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks (4 passed)

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly identifies the main changes: making the notified state real and pinning scheduler claims to the gated recommendation snapshot.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

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

Gate verdict: APPROVE (money path, scheduler auto-purchase)

Reviewed SHA: ac4b3bd2d5845519e7c945039626d5bcdff376b9. It is 494ae7c2 with main (#826) merged in by the gate through gh pr update-branch. git diff 494ae7c2 ac4b3bd2 -- <the PR's 15 files> is empty, and the diff against main is still +682/-10. Spec: #631 PR2b-2 S1, S2, S6. The PR body was treated as a claim to test, not as evidence.

Checks

• S1: RotatePendingApprovalToken sets status='notified' in the same UPDATE as the token hash. The only caller is notifications.go:115, which runs after SendScheduledPurchaseNotification succeeds. A failed send leaves the row pending, and executableByScheduler refuses a rec-bearing pending row. A row that has left pending/notified is not revived.
• S2: The claim CAS adds AND ($6 IS NULL OR (recommendations IN ('[]','null')) = $6). It is pinned on both callers of claimAndExecute, cron (manager.go:813) and SQS (messages.go:134). The empty test matches SetExecutionRecommendationsIfEmptyTx (store_postgres_ramp_step.go:359) and the Go-side len(...)==0. The column is JSONB NOT NULL DEFAULT '[]' (migration 000001), so a SQL NULL cannot strand a row. For pending/notified rows, the post-claim CheckStepCommitmentCap re-reads the plan and fails the row with beforeSubmission, which releases the suppressions. A plan read error also fails closed.
• Approved rows skip the cap re-check, and that is safe. They are human-approved. The pin still applies to them. The only attach writer is restricted to status IN ('pending','notified'), so an approved row cannot gain recommendations. ApproveAndExecute, DirectExecute, RunPlannedPurchaseNow and the web-token paths use transitionApproveAndExecute and are unchanged.
• S6: Precheck covers all six Include/Exclude fields plus MinCount != 0, and refuses an immediate ramp whose percent is not 100. It runs first in Resolve, and checkStoredSPCount runs in size. The AWS SP provider emits Count: 1 (providers/aws/services/savingsplans/client.go:194), so the check does not refuse real SP rows.
• Nothing attaches yet: Resolve has no scheduler caller, the kill switch defaults off, and caps are NULL. Rec-less rows still fail (TestClaimPin_RecLessRowStillFailsInsteadOfBuying).

Local evidence

Run from git archive of the exact SHA with GOTOOLCHAIN=go1.26.9 under the suite lock. Integration ran on a throwaway local PostgreSQL 17.11 because Docker hangs on this host. The testcontainers helper was replaced in the scratch copy only and never committed. No real DB or cloud was touched.

• At ac4b3bd2: build ok, vet 0. Unit config purchase planstep mocks server analytics api exit 0. -tags integration for config purchase planstep exit 0.
• Same result at 494ae7c2. The new integration tests ran (verbose PASS), not skipped.

Fails-before (fix reverted, tests kept):

• Pin SQL and the notified SET removed: TestPostgresStoreDB_TransitionExecutionStatusPinned, ..._RotatePendingApprovalToken, TestNotifiedTransition_OnlyAfterEmailSucceeds and TestClaimPin_RealAttachBetweenGateAndClaim FAIL. The last one fails by panicking on an unexpected provider call at execution.go:1001: on real PG, an attach between gate and claim reaches the provider.
• Manager claim reverted to the plain TransitionExecutionStatus: TestClaimPin_AttachBetweenGateAndClaimIsRefused FAILS.
• Post-claim re-check disabled: TestClaimPin_PostClaimCapRecheckFailsBeforeSubmission FAILS.
• Precheck and checkStoredSPCount removed from Resolve: TestResolveRefusesUnsupportedPlanBeforeReading and TestResolveRefusesStoredSPWithCountNotOne FAIL.

The gate ran no mutation sweep. The author's capped run (17 mutants) stands per the owner policy.

Non-blocking follow-ups (must be closed before PR2b-2b turns attach on)

Rows now become notified. Today this cannot happen, because nothing attaches. Once rows can be notified, these Plans-page controls stop working on them:

• pausePlannedPurchase (handler_purchases.go:208, from pending only)
• RunPlannedPurchaseNow (approvals.go:355, from pending/paused)
• planned-purchase DELETE (cancelOrRecoverExecution, handler_purchases.go:463, from pending/paused)
• frontend/src/plans.ts:688 (isPending = status === 'pending'), which hides the buttons

The email Cancel link and session cancel still accept notified, so a veto path remains.

Size: about 190 production lines including the mock. Everything above 400 lines is tests.

CI: 26/26 checks pass at ac4b3bd2, mergeStateStatus CLEAN.