[cristim]

Closes #1771

With 4-eyes approval mode on (RequireDifferentApprover), an AutoPurchase plan's step no longer buys without a second person. It waits for a human approval in the approval queue, the same as a manual plan's step. This implements the A05-003 decision recorded on #348 and #1771.

What changes

Commit 1: the gate (internal/purchase/manager.go). executableByScheduler is the single gate for both unattended executors, the cron sweep and the SQS execute_purchase handler. It now returns "not executable" for a recommendation-bearing step while 4-eyes is on.

• The step stays notified. Approving it through the queue (session approve) runs ApproveAndExecute, then enforceFourEyesPolicy. That accepts any identified approver for a system-created row, and refuses the creator of a root pre-created with the plan.
• Empty steps route as before; they buy nothing.
• A missing or unreadable global config still fails closed. The kill-switch read now shares autoPurchaseConfig.
• The skip log and the SQS refusal name 4-eyes as a possible reason.

Commit 2: the email (internal/email, internal/purchase/notifications.go). The upcoming-purchase email used to say every step "will be purchased on X unless canceled", with no approve link. That isn't true for a manual plan, or for an AutoPurchase plan under 4-eyes. When a step needs approval:

• the email keeps the totals and cap sentence;
• it says the step is due on X and is bought once approved;
• it points to the approval queue through the existing Review & Edit link.

When the step runs on its own, the text is byte-identical to before.

Decisions and known limits

These are recorded on #1771, and the owner can redirect them.

• (a) A step root pre-created through the admin API key has no recorded creator and isn't system-created. Under 4-eyes nobody can approve it, so it waits and then expires; a human recreates the plan. This is documented on RequireDifferentApprover.
• Expiry stall. An unapproved step expires about 7 days after its date and stalls its plan. That happens to manual plans today too, and is filed as fix(plans): an unapproved plan step that expires stalls its plan with no recovery path #1785.

Tests

• Real Postgres (four_eyes_auto_plan_integration_test.go):
  • With 4-eyes on, the sweep leaves the row notified with no purchase. Then user B's approval buys once, and the row completes.
  • A root pre-created by A: A is denied with ErrFourEyesDenied, and B succeeds.
  • With 4-eyes off, the sweep executes as before.
• Unit:
  • the sweep and the SQS gate;
  • planNeedsApproval in each mode;
  • the rendered wording in both modes;
  • the call-site wiring in the SendUpcomingPurchaseNotifications tests.
• Mutation probes (each fails a test):
  • dropping the 4-eyes gate;
  • dropping 4-eyes from planNeedsApproval;
  • forcing needsApproval to false at the call site.

Local verification (macOS)

At the committed tree, using the touched packages:

• go vet -tags integration passes for purchase, email and config.
• Unit tests pass for purchase, email and config, plus server earlier.
• go test -tags integration ./internal/purchase/ passes, including the 4-eyes, system-execution and plan-step tests.
• golangci-lint v2.10.1: 0 issues.
• pre-commit passes, with all hooks run.

Rebased onto current main after the local gate, with no conflicts. CI checks the rebased head.

A non-author adversarial review of the staged tree ran 2 passes and ended clean. Its findings are fixed in this branch.

🤖 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 12 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/four-eyes-auto-purchase
• File: .coderabbit.yaml
• Commit: 97f5325
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 866ea8ad-5105-485d-b01e-b3f22c3f0678

Commits

Reviewing files that changed from the base of the PR and between 1fb594a and 97f5325.

Files selected for processing: 20

• internal/api/combined_plan_purchase_integration_test.go
• internal/config/types.go
• internal/email/sender.go
• internal/email/template_renderers_test.go
• internal/email/templates.go
• internal/purchase/claim_pin_test.go
• internal/purchase/four_eyes_auto_plan_integration_test.go
• internal/purchase/four_eyes_system_execution_integration_test.go
• internal/purchase/manager.go
• internal/purchase/manager_test.go
• internal/purchase/messages.go
• internal/purchase/messages_test.go
• internal/purchase/notification_data_test.go
• internal/purchase/notifications.go
• internal/purchase/notifications_test.go
• internal/purchase/notified_claim_pin_integration_test.go
• internal/purchase/plan_step_attach_integration_test.go
• internal/purchase/plan_step_crash_integration_test.go
• internal/purchase/plan_step_managed_race_integration_test.go
• internal/purchase/plan_step_retention_integration_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]

New head 97f5325: one test-only commit on 658a922, fixing the red CI run.

#1761's TestCombinedPlanPurchase_AutomaticOnScheduledExecution turned 4-eyes on for every combined-plan fixture and expected auto-execution, so it pinned exactly the behavior #1771 changes. Updated for the intended change:

• The fixture turns 4-eyes on only for the manual approval flows.
• The automatic test runs with 4-eyes off and still asserts auto-execution.
• New TestCombinedPlanPurchase_AutomaticWithFourEyesWaitsForApproval is the end-to-end proof at the API layer. With 4-eyes on, the due sweep executes, fails and buys nothing, and the step stays notified. A different user's approval then buys it once.

Local: go test -tags integration ./internal/api/ -run TestCombinedPlanPurchase passes; pre-commit passes, with all hooks run. Please review at this head.

[cristim]

APPROVED 97f5325 by claude-ui-fix-1

Cold review of #1793 (Closes #1771), base main 1fb594a2. The code was reviewed at 658a922a; the final head 97f53251 adds a single test-only commit. I treated the PR body as a claim and checked it against the code.

The approval requirement covers every path that can buy

The only provider purchase call is PurchaseCommitment (internal/purchase/execution.go:1230), reached only via executeAndFinalize. Its callers:

• Cron sweep and SQS execute_purchase. processOneExecution (manager.go:994) and checkAutoExecuteGate (messages.go:90) both go through executableByScheduler. With RequireDifferentApprover on, it now returns "not executable" for a recommendation-bearing row (manager.go gate). A missing or unreadable global config still fails closed (autoPurchaseConfig).
• Session approve, token approve, direct execute and "Run now". All go through transitionApproveAndExecute, which calls enforceFourEyesPolicy before any state change (approvals.go:424). The API also runs requireDifferentApprover on every approve path (handler_purchases.go:1650-1690).
• Delayed approvals and the fire sweep. The API check runs before approveWithDelay, so the fire sweep only executes rows that a qualifying human already moved to scheduled.
• Stranded re-drive. It only claims approved rows (manager.go:682), which were human-approved. The gate stops the scheduler from producing new ones.
• Ladder tranches. ClaimDueLadderTranches has no production caller yet, so nothing buys through the ladder today. When its buyer lands, its executions must not be created approved without going through this check (a note for that work).

Toggling the setting mid-run: the gate reads the global config per row (autoPurchaseConfig), so turning 4-eyes on stops later rows in the same sweep. Expiry: an unapproved step expires as a manual plan's does; #1785 tracks that.

The test delta at 97f5325

• The fixture's RequireDifferentApprover is now !automatic, so it's on only for the manual approval flows.
• The new TestCombinedPlanPurchase_AutomaticWithFourEyesWaitsForApproval runs the real path:
  • the sweep executes and fails nothing;
  • the purchaser records nothing and the row stays notified;
  • a different user's approval then completes it, with exactly one purchase.
• The existing automatic test covers 4-eyes off, which still auto-executes.

Verification (local, macOS, heavy-review-2 lock, throttled GOMAXPROCS=4 -p 2)

• At 97f53251: go test -tags integration ./internal/purchase/ -run FourEyes_AutoPlan ok, and ./internal/api/ -run TestCombinedPlanPurchase_ ok.
• Mutation, dropping the RequireDifferentApprover check in executableByScheduler:
  • TestFourEyes_AutoPlanSweepLeavesRowForHumanApproval and TestFourEyes_AutoPlanPreCreatedRootCreatorCannotApprove fail;
  • TestCombinedPlanPurchase_AutomaticWithFourEyesWaitsForApproval fails.
    The tree was restored and checked clean.
• At 658a922a: the scoped unit tests pass for purchase, email and config; golangci-lint v2.10.1 on purchase and email reports 0 issues.
• CI at 97f53251 is green.

No findings. Not merging.

[cristim]

APPROVED 97f5325 by claude-qa-ui-4

Cold review of the code at 658a922 plus the test-only delta at 97f5325.

• internal/purchase/manager.go:921: with RequireDifferentApprover on, executableByScheduler never auto-executes a recommendation-bearing row. Paths checked: cron sweep (manager.go:1041 → :994 → :218), SQS execute_purchase (messages.go:123 → :90), stranded-approved re-drive (manager.go:682, human-approved rows only), delayed fire (scheduled_fire.go:132, scheduled rows come from a human approval), session/token approve, Run now and DirectExecute (approvals.go:350/358/382 → :424, enforceFourEyesPolicy), retry (Source=web, refused at manager.go:897). All four production callers of executeAndFinalize are on these paths.
• 658a922 failed CI in Integration Tests (api): feat(plans): flag plan steps that will not attach and refuse near start dates #1761's TestCombinedPlanPurchase_AutomaticOnScheduledExecution ran with 4-eyes on. 97f5325 sets RequireDifferentApprover = !automatic in the fixture (combined_plan_purchase_integration_test.go:149) and adds TestCombinedPlanPurchase_AutomaticWithFourEyesWaitsForApproval. AutomaticAttachmentOverCapRefuses refuses at attach time and does not depend on 4-eyes.
• Local, throttled (GOMAXPROCS=4, -p 2): purchase/email/config unit 2461 pass and integration 3372 pass at 658a922; at 97f5325 the combined-plan integration tests all pass in ./internal/api.
• Probes: deleting the gate at manager.go:921 fails the new api test and three purchase tests; inverting it, reading an empty config, checking only "pending" rows, and dropping 4-eyes from planNeedsApproval are all killed. Skipping the check for retries survives and is equivalent (retry successors are Source=web, refused first).
• CI green at 97f5325.

Non-blocking, predates this PR:

• manager.go:908 vs :246: the flag is read before the claim and claimedRowRefusal re-checks only the step cap, so a 4-eyes switch between the read and the claim lets that one row through (same window as the kill switch).
• config/types.go:21 ApprovalRequired (default true, defaults.go:61) is stored but not read by any gate.

[cristim]

Merge gate at 97f5325:

• Two cold approvals at this SHA.
• CI green, including api integration after the feat(plans): flag plan steps that will not attach and refuse near start dates #1761 test update.
• 0 unresolved threads. Code only.

Main moved to da8c78e (#1790, #1795) with no overlap with this PR's files. The merge is clean (git merge-tree).