Repository navigation
chore(api): ten functions in handler_purchases.go sit at the gocyclo ceiling, so any one-line change breaks pre-commit #163
Description
Eight functions in internal/api/handler_purchases.go sit at cyclomatic complexity exactly 10 on origin/main, which is the pre-commit gate's ceiling (gocyclo -over 10). Measured against git show origin/main:internal/api/handler_purchases.go:
10 (*Handler).validateExecutePurchaseRequest
10 (*Handler).sendPurchaseApprovalEmail
10 (*Handler).revokeViaEmailToken
10 (*Handler).loadAndValidateRetryRequest
10 (*Handler).cancelPurchaseViaSession
10 (*Handler).cancelPurchase
10 (*Handler).authorizeSessionExecuteDirect
10 (*Handler).authorizePlannedPurchaseCancel
Each passes today. Each breaks the pre-commit hook the moment anyone adds a single if, case, && or || to it — including a change that is otherwise a one-line bug fix. Whoever touches one of these next inherits a refactor they did not sign up for, in a file that carries approve, cancel, execute and retry authorization.
Note the class: adding a case to a dispatch switch adds 1, so a benign-looking addition breaches the gate.
Why this is filed rather than fixed opportunistically
Raised as a nit during LeanerCloud/cloud-commitments-cli#1713 and correctly declined there. That PR only renamed a callee and added no complexity, and loadAndValidateRetryRequest's own comment records that its gates stay "in the order documented on retryPurchase so the security boundary is the same" — order is the security boundary, so extracting a subset of gates is a change that wants its own review rather than being buried in a money-path diff.
Fixing one function in isolation also leaves the class untouched, which is the actual problem.
Suggested approach
Treat it as one deliberate hygiene pass over the file, not eight drive-bys:
- For the authorization functions (
authorizeSessionExecuteDirect,authorizePlannedPurchaseCancel,cancelPurchaseViaSession,revokeViaEmailToken,loadAndValidateRetryRequest), preserve gate ordering exactly and say so in each extracted helper's doc. Any reordering is a security change and must be called out explicitly, not absorbed into a refactor. - Extract only where the boundary is natural — a self-contained validation block, a lookup-and-check pair — never by splitting a function at an arbitrary line to get under the threshold.
- No behaviour change. Confirm by running the full
internal/apisuite before and after and diffing the results, not just checking it is green.
Verify with gocyclo -over 9 internal/api/handler_purchases.go so the remaining margin is visible rather than only the pass/fail at 10.
Found while verifying a declined nit on LeanerCloud/cloud-commitments-cli#1713.
Activity
[-]chore(api): eight functions in handler_purchases.go sit at the gocyclo ceiling, so any one-line change breaks pre-commit[/-][+]chore(api): ten functions in handler_purchases.go sit at the gocyclo ceiling, so any one-line change breaks pre-commit[/+]cristim commentedon Aug 3, 2026
Correcting the count in the issue body: it is ten, not eight. Verified with a predicate rather than a ranking, against
git show origin/main:internal/api/handler_purchases.go:Ten at exactly 10, and zero above 10 — so the file passes today with no margin anywhere.
Missing from the original list:
authorizeExecutionManagementandapprovePurchaseViaSession. Both are authorization functions, which puts seven of the ten in the authorize/approve/cancel/revoke family where gate ordering is the security boundary.How the count was wrong twice is worth recording, because it is a methodology error rather than a slip. The first report said six, from
gocyclo -top 6. The correction said eight, fromgocyclo -over 9 | head -8. In both cases the number reported was the value of the flag, not a property of the code — a ranking truncated to N always returns N rows, and both numbers looked plausible enough to pass. The reliable form is a predicate:gocyclo -over 9 <file> | awk '$1==10', which cannot return more than exists.The correction strengthens the case. Ten functions at the ceiling, with nothing above it, is unambiguously a file-wide property, so splitting whichever function a given PR happens to touch is clearly the wrong unit of work.
cristim commentedon Aug 8, 2026
Second file with the same problem:
cmd/multi_service.gohas three functions at exactly 10 —processPurchaseLoop,filterAndAdjustRecommendations,runToolFromCSV.Verified pre-existing rather than assumed: measured against
git show origin/main:cmd/multi_service.go, all three are at 10 on the base, unmoved by the PR that surfaced them (#1725).Why these three matter more than the count suggests: they are the CSV-path functions that #1609 (the
--input-csvpurchase path writes no audit record) and #1610 (CSV-mode confirmation is per-region and a decline does not abort) will have to modify. Whoever picks up either issue inherits a mandatory refactor before they can add a single branch — agocyclo -over 10pre-commit gate turns "add an audit write" into "restructure the CSV pipeline first".So the running tally for this issue is 13 functions at the ceiling across two files:
internal/api/handler_purchases.gocmd/multi_service.goThe same guidance applies to the new three: preserve behaviour exactly, extract only at natural boundaries, and run
gocyclo -over 9afterwards so the remaining margin is visible rather than just the pass/fail at 10.One addition to the original suggestion, based on how this keeps surfacing: it is worth checking whether any function in the repo sits at exactly 10, rather than discovering each file as a PR happens to touch it. Use a predicate (
gocyclo -over 9 . | awk '$1==10'), not a ranking flag — this issue was already filed twice with wrong counts derived from-top Nandhead -N, both of which return N by construction.Found while verifying the rebase of LeanerCloud/cloud-commitments-cli#1725.
fix(cli): enforce --min-count and savings-ordered capping on --input-csv