[cristim]

Summary

"Run now" on a planned purchase transitioned the row from pending/paused to running and returned success, but nothing executes a "running" row: the scheduler only picks up pending/notified, and purchase.Reaper flips anything stuck in running for 10 minutes to failed. The operator was told the purchase ran, nothing was bought, and the row read "failed" ten minutes later.

Root cause

internal/api/handler_purchases.go's runPlannedPurchase did a bare TransitionExecutionStatus(ctx, executionID, []string{"pending", "paused"}, "running", ...) and returned "status": "running" without ever calling an executor. claimAndExecute (the scheduler/SQS funnel) claims from approved/pending/notified, not from a row already sitting in running. ApproveAndExecute (the approve funnel) transitions pending/notified to approved and then runs the purchase synchronously, but "Run now" needs to also accept a paused source, which ApproveAndExecute's CAS does not.

Fix

• internal/purchase/approvals.go: extracted ApproveAndExecute's 4-eyes-gated, CAS-then-execute body into transitionApproveAndExecute(ctx, executionID, actor, transitionedBy, fromStatuses). ApproveAndExecute now calls it with ["pending", "notified"]; a new RunPlannedPurchaseNow calls it with ["pending", "paused"] (the same source set runPlannedPurchase used before), and both go through executeAndFinalize synchronously.
• internal/api/handler_purchases.go: runPlannedPurchase now delegates to h.purchase.RunPlannedPurchaseNow, mirroring how directExecutePurchase already delegates to ApproveAndExecute. On success the response reports "status": "completed" (the row's actual terminal status) instead of the transient "running" that nothing ever consumed.
• Interfaces updated to add RunPlannedPurchaseNow: internal/api/types.go (PurchaseManagerInterface), internal/server/interfaces.go (server's own copy of the same interface name), plus the two hand-written mocks (internal/api/mocks_test.go, internal/testutil/mocks.go).

Idempotency (issue's explicit ask)

• Double-click: the second click's CAS (pending/paused -> approved) affects zero rows once the first click's transaction commits, and returns a clean "cannot transition" error -- no second execution.
• Alongside the scheduler: a pending row is eligible to both claimAndExecute (approved/pending/notified -> running) and RunPlannedPurchaseNow (pending/paused -> approved). These are different target statuses on the same row, but Postgres row-level locking still serializes the two UPDATE ... WHERE status IN (...) statements: whichever commits first changes the status, and the second statement's WHERE re-evaluates against that committed value and matches zero rows. Only one caller ever executes the purchase.
• A side benefit: forcing an execution via "Run now" previously bypassed the 4-eyes dual-control check entirely (no gate at all), unlike every other execute path (approve, direct-execute). It now goes through the same enforceFourEyesPolicy check.

Regression test

• internal/purchase/approvals_test.go:
  • TestManager_RunPlannedPurchaseNow_ExecutesFromPaused: drives RunPlannedPurchaseNow from a paused source (a status ApproveAndExecute itself does not accept) and asserts the full executeAndFinalize chain runs (plan lookup, confirmation email, SavePurchaseExecution, CompletePlanStep) -- proving the purchase is actually executed, not left stuck in "running".
  • TestManager_RunPlannedPurchaseNow_LostCASReturnsError: a lost CAS surfaces as an error with no execution-chain calls, the idempotency half of the fix.
• internal/api/handler_purchases_test.go / handler_test.go: updated TestHandler_runPlannedPurchase, TestHandler_runPlannedPurchase_NilExecution, and the router-level TestHandler_HandleRequest_RunPlannedPurchase to assert the handler delegates to RunPlannedPurchaseNow instead of a bare status transition.

Confirmed the new purchase-package tests fail pre-fix: git stash on internal/purchase/approvals.go (and the interface/mock plumbing) reproduces a build failure, manager.RunPlannedPurchaseNow undefined.

Verification

• GOWORK=off go build ./... — pass
• GOWORK=off go vet ./... — pass
• GOWORK=off go test ./... -short — 6132 tests pass
• GOWORK=off go test ./internal/api/... ./internal/purchase/... -race -short — 2528 tests pass
• GOWORK=off go mod tidy -diff — empty
• gofmt -l on changed files — clean
• golangci-lint at the CI-pinned v2.10.1: reproduces the same pre-existing local toolchain issue noted on PR fix(frontend): RI-exchange execute never sends region, so the backend returns 400 #355/fix(api): PUT /api/plans/{id} resets the ramp schedule and re-buys completed steps #360 for this checkout, unrelated to this diff; relying on CI's clean go1.26.6 environment for the authoritative lint run.

Closes #218

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 9 billable files and costs up to $2.25.}

• 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 13 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used all 5 included reviews currently available. Your 20 included PR review attempts over the past 7 days set your current allowance at 5 reviews 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: 4578e1ab-6ed8-46d9-854f-652838341187

📥 Commits

Reviewing files that changed from the base of the PR and between e5ac86a and b00e31d.

📒 Files selected for processing (9)

• internal/api/handler_purchases.go
• internal/api/handler_purchases_test.go
• internal/api/handler_test.go
• internal/api/mocks_test.go
• internal/api/types.go
• internal/purchase/approvals.go
• internal/purchase/approvals_test.go
• internal/server/interfaces.go
• internal/testutil/mocks.go

---

_{Comment @coderabbitai help to get the list of available commands.}

[cristim]

Independent adversarial review + local verification: MERGE at b00e31d. Run now now CASes pending|paused -> approved and executes synchronously through the same funnel as approve (4-eyes, per-account idempotency keys, finalizeExecution, CompletePlanStep). Double-click and a concurrent scheduler tick are resolved by the single conditional UPDATE (loser gets 409); a crash leaves approved, which the reaper sweeps. 4-eyes now also applies to Run now (disclosed). Pre-fix tests fail (handler calls the wrong method; RunPlannedPurchaseNow undefined) and pass on head; -race OK for api/purchase/server; 24/24 checks. Follow-ups filed: error-to-status mapping, audit stamping.