[cristim]

What it fixes

Refs #631 (item 48b, task D1). A plan-step row has no CreatedByUserID, so an approve-own-only user who is also the account contact_email hit the "cannot approve another user's pending purchase" 403, fell through to the token path, and approveConstraintsForSession returned nil: their capped approve-own was never applied. A contact_email with no approve verb was bound by no cap at all.

Now, for a row with PlanID set approved via the email token:

• approve-own Constraints apply whenever the verb is held, regardless of creator;
• an approver with no verb, or no session, is held to CheckStepCommitmentCap (auto-purchase plans, or manual plans that set a cap; a manual plan with no cap approves as before);
• both run before the session-nil early return. The admin API key stays exempt.

User-created rows (no PlanID) behave as before.

Verification

Fixture/mock evidence (in-memory stores and purchase manager; the rec is the real planstep.Resolve output and permission Constraints go through the real auth matcher). Docker was unresponsive here, so no integration run; no purchase package is touched.

• On the parent (74c3234) the new tests fail: ApproveOwnCappedContactEmail, ZeroTotalIs400, NoVerbHonorsPlanCap, ManualPlan, SessionlessAndAdminKey.
• After: go test ./internal/api passes; go build ./..., go vet, gofmt, pre-commit clean.
• Mutation probes (8, all killed): ignore PlanID in the own branch; drop the plan cap call; manual-skip always; auto-skip never; drop approve-any enforce; drop approve-own enforce; admin key not exempt; swallow plan read error.

🤖 Generated with Claude Code

[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 4 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 62 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: dd5dbc27-efaf-4d98-b632-8d53be0a25d0

📥 Commits

Reviewing files that changed from the base of the PR and between 74c3234 and c7a65ec.

📒 Files selected for processing: 2

• internal/api/handler_purchases.go
• internal/api/handler_purchases_plan_step_approve_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]

Gate verdict: CHANGES REQUESTED at 3fb9d60

Blocker (CI red, reproduced locally): TestNoUnfailableMockAssertions (internal/mocks) fails on the new test file:

• internal/api/handler_purchases_plan_step_approve_test.go:122 f.purchase.AssertCalled(...)
• internal/api/handler_purchases_plan_step_approve_test.go:128 f.purchase.AssertNotCalled(...)

The guard rejects mock assertions whose receiver is not a plain identifier. Fix: bind the mock to a local (mp := f.purchase; mp.AssertCalled(...)) or pass *MockPurchaseManager into the helpers. Both "Unit Tests" and "Integration Tests" jobs fail on this alone (run 38479054052). Local: go test ./internal/mocks/ -run TestNoUnfailableMockAssertions -> FAIL at the head.

Production change review (no defects found):

• Spec D1 (631 amendments): plan-step rows approved by token now apply approve-own Constraints regardless of creator, and an approver with no verb or no session is held to CheckStepCommitmentCap; the check runs before the old session-nil early return; admin API key stays exempt. Scope = AutoPurchase plans or manual plans that set a cap; manual plan with no cap approves as before. This matches the spec's parenthetical and its required test list (manual WITH cap over cap -> 403).
• Fails-before (fixture/mock evidence, git archive of parent 74c3234 + the new test file): 6 of 8 new tests FAIL (ApproveOwnCappedContactEmail, ZeroTotalIs400, NoVerbHonorsPlanCap, ManualPlan, SessionlessAndAdminKey, PlanReadErrorFailsClosed); ApproveAnyCap and UserCreatedRowUnchanged pass on the parent as expected (pre-existing behavior pinned).
• Head: go test ./internal/api/ ./internal/purchase/ 3724 passed; go vet clean.
• Mutation (18 mutants on the changed money/auth lines, one run): 17 killed incl. admin-key condition, session/auth gate, PlanID approve-own clause, both enforce calls, PlanID guard, all three AutoPurchase/cap condition variants, 403 return, plan-read error, the call site, and >->>= in matchPurchaseAmountConstraint. Survivor (non-blocking): total >= MaxStepCommitment in step_cap.go is not caught by the api tests (no at-cap plan-cap case).
• Bypasses tried by reading: admin API key (exempt by spec default), contact_email no-verb (capped), approve-own holder (bound), session-less (capped), manual plan no cap (unchanged), non-plan row (unchanged, PlanID == ""). Note (non-blocking, pre-existing): purchase_executions.plan_id is ON DELETE SET NULL, so deleting a plan leaves its pending step rows uncapped for token approval.
• Integration tests (-tags integration): not run locally, Docker not available on the gate host.
• Labels mirror feat(plans): plan steps resolve real recommendations and show real counts and prices (tier B of #609) #631.

Re-gate needed after the test fix (new head restarts the gate).

[cristim]

Gate verdict: APPROVED at c7a65ec

• Delta since the gated 3fb9d60: test-only (handler_purchases_plan_step_approve_test.go, +27/-14). The mock-assert guard fix counts ApproveExecution calls on a plain identifier, and there is a new plan-cap at-equality case. The production diff is unchanged from the reviewed 3fb9d60 (see the previous gate comment for the prod review, bypass attempts and the mutation run).
• Local at the exact head (git archive, GOTOOLCHAIN=go1.26.9): go vet ./internal/api ./internal/mocks clean. go test ./internal/api ./internal/purchase ./internal/mocks passed (3745), including TestNoUnfailableMockAssertions.
• Fails-before (fixture/mock evidence): new test file on parent 74c3234 -> 6 FAIL (ApproveOwnCappedContactEmail, ZeroTotalIs400, NoVerbHonorsPlanCap, ManualPlan, SessionlessAndAdminKey, PlanReadErrorFailsClosed).
• The earlier survivor is now killed: total >= MaxStepCommitment in step_cap.go fails NoVerbHonorsPlanCap.
• PR applied onto current main (bdf1735): vet clean; api/mocks/purchase/config tests pass.
• CI: green at this SHA, mergeStateStatus CLEAN. The first Security Scanning attempt was SIGTERM'd (gosec exit 143, a runner kill, not a finding); the rerun passed.
• -tags integration: not run locally, Docker not available on the gate host; CI Integration Tests are green.
• Follow-up (non-blocking): fix(api): deleting a plan leaves its pending step rows uncapped on token approval #862 (plan delete SETs plan_id NULL, leaving pending step rows uncapped on token approval).
• Labels mirror feat(plans): plan steps resolve real recommendations and show real counts and prices (tier B of #609) #631.