[IAmJSD]

Closes #163

Root cause

gocyclo -over 9 internal/api/handler_purchases.go on main listed 15 functions at exactly 10, the pre-commit gate's ceiling: getPlannedPurchases, authorizeExecutionManagement, executeFailureError, cancelOrRecoverExecution, approveViaToken, approvePurchaseViaSession, authorizeSessionExecuteDirect, cancelPurchase, cancelPurchaseViaSession, revokeViaEmailToken, retryPurchase, getPurchaseDetails, validateExecutePurchaseRequest, executePurchase and sendPurchaseApprovalEmail. The issue counted 8 to 10; the file has grown since. Any added if, case, && or || in one of them failed pre-commit. (cmd/multi_service.go from the issue's second comment is in cloud-commitments-cli now, not here.)

Fix

One hygiene pass over the file. Each extraction is a natural unit, not a split at an arbitrary line:

• Authorization paths, with gate order unchanged. Each helper's doc says which gates ran before it.
  • authorizeExecuteOwnDirect: the execute-own branch, after admin, nil-auth and execute-any.
  • cancelViaToken and revokeViaToken: the token-authed tails. The contact_email gate still runs first, then the token, then the status.
  • executeTokenApprove and executeSessionApprove: the immediate-execute tails, which run after every approve gate.
  • authorizeExecutionManagement now uses the existing loadPlannedExecution, which has the same 404 and 500 mapping.
• Other paths:
  • cancelAndReleaseSuppressions: the cancel-plus-suppressions transaction.
  • releaseCanceledSuppressions: the success branch of cancelOrRecoverExecution. It sits above the hunk that fix(db): contract the canceled rename: normalize, narrow the status CHECKs, stop using cancelled_by #1253 rewrites.
  • unclaimedExecuteError: the final == nil cases of executeFailureError. ErrFourEyesDenied is still checked first.
  • parseExecutePurchaseBody: decode, the 1..1000 bound and capacity normalization, still after the permission check and before the scope check.
  • purchasePlanName, plannedPurchasesInScope and sendRetryApprovalAndFinalize.
  • Shared helpers: purchaseDelay (still fails closed on a config error), gracePeriodConfig, approvalRecommendationSummaries and approvalSendFailure.

No behaviour change. After this, gocyclo -over 9 internal/api/handler_purchases.go prints nothing. Functions now at 9 are authorizeApprovalAction, executePurchase, authorizeSessionRetry, writeRetrySuccessorTx, loadAndValidateRetryRequest, requireDifferentApprover, authorizeSessionApprove, approvePurchaseViaSession, cancelOrRecoverExecution (8 once #1253 lands), deletePlannedPurchase, runPlannedPurchase and authorizeExecutionManagement.

Overlap with open PRs: #1253, #1241 and #1239 each touch one hunk of this file. #1241 and #1239 change functions this PR doesn't touch. #1253 changes the lower half of cancelOrRecoverExecution, and this PR only touches its first branch.

Verification

• Behaviour check, as the issue asks: I ran the full internal/api suite on the merge base and on this branch and diffed the sorted --- PASS/FAIL/SKIP lines.
  • go test -race -short -v: 4590 lines, identical.
  • go test -race -tags integration -v (testcontainers Postgres): 4958 lines, identical, run on the final commit.
• gocyclo -over 10 and -over 9 on the file: none.
• golangci-lint v2.10.1 on internal/api, with and without integration: 0 issues.
• go build ./..., go vet ./... (plain, integration, e2e): pass.
• Secrets scan clean (planted key flagged). scripts/test-git-secrets-allowlist.sh: 183 PASS.

There is no new test. This is a pure refactor, so the evidence is the gocyclo output (15 functions before, 0 after) and the identical test results.

🤖 Generated with Claude Code

[IAmJSD]

Pushed 8e3c17d: one refactor commit taking the 15 functions at gocyclo 10 down to 9 or less, as described in the PR body.

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 1 billable file and costs up to $0.25.}

Or wait 59 minutes for your next included review.

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: main
• File: .coderabbit.yaml
• Commit: cdb4591
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 1dc6e634-409f-456c-94a8-01115d32ee49

Commits

Reviewing files that changed from the base of the PR and between cdb4591 and 8e3c17d.

Files selected for processing: 1

• internal/api/handler_purchases.go

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

[IAmJSD]

Independent review (IAmJSD's agent, fresh context, given only the diff, the issue and the PR text): VERDICT: APPROVE 8e3c17d01ec61fc9ffe959dfa06f17a94a28156b

Blocking:

• none
  Non-blocking:
• Test gap, present before this PR: no test pins the fail-closed config read on the approve paths. I mutated purchaseDelay to swallow the GetGlobalConfig error (return 0, nil), which would silently skip the free-cancel window and execute immediately, and go test -short ./internal/api/ stayed green. The extraction preserves the behaviour, but a follow-up test (GetGlobalConfig error makes token and session approve return 500 without reaching ApproveExecution) would protect the money-path invariant the comments describe. The error strings "failed to release suppressions", "cannot manage another user's scheduled purchase" and "a concurrent operation already transitioned" also appear in no test file, so those branches are covered by status code at best.
• Expect a textual rebase conflict with fix(db): contract the canceled rename: normalize, narrow the status CHECKs, stop using cancelled_by #1253 in cancelOrRecoverExecution, as the body says.
  Attacked: I read every hunk for gate order and semantics:
• authorizeExecutionManagement → loadPlannedExecution: same ErrNotFound→404 and wrapped-500 mapping, byte for byte.
• executeFailureError: FourEyes 403 is still first, then the final == nil 409/plain cases, then the rest.
• purchaseDelay: still fails closed. The delay value is read once, as before.
• executeTokenApprove and executeSessionApprove: entered only after every approve gate and the delay branch, with bodies unchanged.
• cancelViaToken and revokeViaToken: contact_email gate, then token, then status.
• authorizeExecuteOwnDirect: entered after the admin, nil-auth and execute-any gates, keeping verb → creator → Constraints.
• cancelAndReleaseSuppressions: same tx with the same !canceled skip.
• parseExecutePurchaseBody: same position after the permission check and before the scope checks.
• gracePeriodConfig: still nil on error.

Ran: go build ./..., go vet ./internal/api/ with and without -tags integration, gocyclo -over 9 (15 functions on the merge-base file, none at head), go test -race -short -count=1 ./internal/api/ (ok), go test -race -tags integration -count=1 ./internal/api/ (ok, 184s), and golangci-lint run ./internal/api/ (0 issues). This is a pure refactor with no fail-before, so I ran the mutation above in its place. GitHub reports MERGEABLE.
Body-stale: none

_{Reviewer: rE3. Bar while the maintainers are away (set by IAmJSD as acting owner): one approving independent review on the head, two for money, auth and security paths.}

[IAmJSD]

Independent review (IAmJSD's agent, fresh context, given only the diff, the issue and the PR text): VERDICT: APPROVE 8e3c17d01ec61fc9ffe959dfa06f17a94a28156b

Blocking:

• none

Non-blocking:

• The extracted tails executeTokenApprove, executeSessionApprove, cancelViaToken, revokeViaToken and authorizeExecuteOwnDirect assume their caller has already run the earlier gates. That is documented in each doc comment but not enforced. They are unexported and each has a single caller today. A future second caller reusing one of them without the preceding gates would bypass authorization, so a reviewer of any later change that calls them should check this.

Attacked: Proving no behaviour change was the main job. I sorted every whitespace-stripped line of the merge-base handler_purchases.go and of the head, then accounted for each line that exists only on the base. Each one is either a comment that moved into a helper's doc or a statement re-expressed one-for-one in a helper: loadPlannedExecution maps errors exactly like the inlined GetExecutionByID block (errNotFound, then "failed to get execution: %w"); purchaseDelay still fails closed with the same message; gracePeriodConfig still swallows the error to nil; executeFailureError keeps ErrFourEyesDenied ahead of the final == nil 409 and plain-error cases; approvalSendFailure keeps the same reasons and recipients; cancelAndReleaseSuppressions keeps the same tx body and error wrapping. Gate order in the cancel, revoke, approve and execute-own paths is unchanged: the contact_email gate comes before the token, and verb, then creator, then Constraints. gocyclo -over 9 internal/api/handler_purchases.go prints nothing, which meets #163's acceptance criteria (Closes is right). Ran go build ./..., go vet (plain and integration) on internal/api, go test -race -short -count=1 ./internal/api/ (ok), and go test -race -tags integration -count=1 -v ./internal/api/ (ok: 4957 PASS, 1 SKIP, 0 FAIL, i.e. the 4958 lines the body reports). There is no fail-before for a pure refactor with no new tests.

Body-stale:

• none

_{Reviewer: rE5. Bar while the maintainers are away (set by IAmJSD as acting owner): one approving independent review on the head, two for money, auth and security paths.}

[IAmJSD]

Local verification at 8e3c17d01ec61fc9ffe959dfa06f17a94a28156b merged onto main (08105c87), together with #1460, #1469 in that order (combined tree 45490995, Linux, Go 1.26.10): go build ./..., go vet with and without -tags integration, the full go test -race -short -count=1 ./..., and go test -race -tags integration -count=1 on every changed package, against Docker Postgres. All pass. CI Success and pre-commit are green on this head, with 2 approving independent review(s) on it. Nothing was run against any cloud.

[IAmJSD]

Merged by IAmJSD's agent at IAmJSD's request (IAmJSD is acting owner while the maintainers are away), under the bar IAmJSD set: 2 approving independent review(s) on 8e3c17d0 (two required: money, auth or security path), green CI, and the combined-tree verification above. Squash-merged at exactly 8e3c17d01ec61fc9ffe959dfa06f17a94a28156b.