[IAmJSD]

Root cause

The pre-commit go-vet hook ran go vet ./... and the pre-push go-test hook ran go test -short -race ./.... Go's ./... skips only vendor, testdata and ./_-prefixed directories, not node_modules. npm packages can ship Go source with no go.mod of their own, so those files are absorbed into this module. After npm ci, go list ./... includes github.com/LeanerCloud/cloud-commitments-platform/frontend/node_modules/flatted/golang/pkg/flatted. pre-commit.yml runs "Install frontend deps" (npm ci) before "Run pre-commit", so in CI the go-vet hook already vets third-party code. Locally the same is true for both hooks. It passes today only because the current flatted happens to vet clean. This is audit finding A15-003 on the issue.

Fix

Both hooks now build their package list with go list and drop node_modules:

set -euo pipefail; pkgs=$(go list -e ./... | grep -v "/node_modules/"); go vet $pkgs
set -euo pipefail; pkgs=$(go list -e ./... | grep -v "/node_modules/"); go test -short -race $pkgs

• -e: without it, a broken Go package inside node_modules (one with a syntax error, or an import of a module this repo does not require) makes go list ./... itself exit 1. That would fail the hook on third-party code again.
• pipefail with set -e: if go list fails for a first-party reason, the hook fails. It does not go on to run go vet with an empty argument list, which would vet only the root directory and pass.
• grep -v "/node_modules/" matches a whole path segment, the same idea as ci: keep gocyclo's filesystem walk out of node_modules and vendor #1286's (^|/)(node_modules|vendor)/. vendor/ needs no filter here, because ./... already excludes vendor directories in module mode.
• A comment above the hooks gives the reason.

This follows #1286, which fixed the gocyclo walk in ci.yml and the Makefile. The pre-commit gocyclo hook was already safe because it lists files with git ls-files.

Refs #177

How the issue's items stand:

• ci.yml gocyclo walk: ci: keep gocyclo's filesystem walk out of node_modules and vendor #1286.
• Makefile complexity / complexity-report gocyclo walks: ci: keep gocyclo's filesystem walk out of node_modules and vendor #1286.
• Pre-commit gocyclo hook: was never affected (git ls-files; confirmed in ci: keep gocyclo's filesystem walk out of node_modules and vendor #1286).
• A15-003, the pre-commit go vet ./... / go test ./... hooks: this PR.

It is Refs, not Closes, because the Makefile's own go vet ./... (make vet) and go test ... ./... targets (test-unit, test-integration, test-coverage, and the go vet -tags=integration ./... in test-compile-integration) still match node_modules packages in a local checkout after npm ci. That is the same "trap for local runs" the issue describes, although the issue does not name those targets. See "Out of scope" for a one-line fix that would cover them.

Verification

Hook-level fixture evidence on this branch (head f2a5fb87, main 3bfc2fe5). A real npm ci --ignore-scripts in the worktree populated frontend/node_modules, including flatted/golang/pkg/flatted/flatted.go. I also planted a package at frontend/node_modules/zz-planted-go/golang/bad.go containing fmt.Printf("%d\n", "not-an-int"), so vet has something to report. All fixtures were deleted afterwards.

• go list ./... | grep node_modules lists both .../frontend/node_modules/flatted/golang/pkg/flatted and the planted package.
• go-vet hook, before (main's config): pre-commit run go-vet --all-files FAILED with frontend/node_modules/zz-planted-go/golang/bad.go:7:26: fmt.Printf format %d has arg "not-an-int" of wrong type string.
• go-vet hook, after: pre-commit run go-vet --all-files passed with the same fixtures, plus two more planted node_modules packages, one with a syntax error and one importing a missing module.
• First-party code is still vetted: planting the same printf mismatch in an untracked internal/zzplanted/f.go makes the new hook fail with internal/zzplanted/f.go:5:24: fmt.Printf format %d has arg "x" of wrong type string.
• go list failure fails the hook: GOFLAGS=-mod=bogus pre-commit run go-vet --all-files gives Failed, not a silent pass on an empty list.
• Why -e is needed: with the missing-import fixture, go list ./... exits 1 (no required module provides package github.com/example/does-not-exist/pkg), and so does the syntax-error fixture. go list -e ./... exits 0 for both.
• go-test hook: run compile-only through pre-commit, to avoid a repo-wide -race test run: GOFLAGS='-run=^$' pre-commit run go-test --hook-stage pre-push --all-files. This still builds and vets every test binary with -race, but runs no tests.
  • Before: FAILED, with FAIL .../frontend/node_modules/zz-planted-go/golang [build failed] and the vet error; .../node_modules/flatted/golang/pkg/flatted [no test files] was also in the run.
  • After (--verbose): passed, and no node_modules package appears in the output.
  • Same package set: the 50 first-party packages in the before run and the after run are identical (sorted diff is empty), and that list matches go list ./... | grep -v node_modules.
• pre-commit validate-config .pre-commit-config.yaml: OK. pre-commit run check-yaml --files .pre-commit-config.yaml: Passed.
• shellcheck on the two extracted entry scripts reports only SC2086 (info) for the unquoted $pkgs. The word splitting is intended: Go import paths cannot contain spaces or glob characters.
• No Go code changed, so there are no Go unit, lint or gocyclo runs for this PR.

Combined-tree verification at local queue commit 2be43e1e = origin/main b9271e88 + fix/issue-363-next, fix/issue-177-next, fix/issue-919-next, fix/issue-313-next, fix/issue-532-next and fix/issue-316-next merged in that order. Passed: go build ./...; go vet with and without -tags integration; go test -race -short -count=1 ./...; golangci-lint v2.10.1 with and without the integration tag; gocyclo -over 10; go test -race -tags integration on internal/api, internal/auth, internal/config, internal/database/postgres/migrations, internal/purchase, internal/server and internal/testutil; the git-secrets scan and scripts/test-git-secrets-allowlist.sh.

Out of scope

• Makefile go vet ./... / go test ./... targets: these still match node_modules packages locally after npm ci. The CI jobs that run ./... (ci.yml lint, unit-tests and govulncheck) do not install npm deps, so CI is not affected.
• Alternative for the owner: since Go 1.25, go.mod supports an ignore directive. Adding ignore ./frontend/node_modules to go.mod makes every ./... skip it: these hooks, the Makefile, golangci-lint and govulncheck. I checked locally that go list ./... then drops a planted frontend/node_modules package and keeps all 50 first-party ones. I did not do it here: it is a module-level change beyond this item, and older tools that parse go.mod with an outdated golang.org/x/mod could reject the directive. That would need checking first. If that route is chosen, the hook changes here could be reverted to plain ./....

Maintainer: please mirror the issue's triage labels on this PR: type/chore, triaged, priority/p3, severity/low, urgency/eventually, impact/internal, effort/xs.

🤖 Generated with Claude Code

[coderabbitai]

Caution

Review failed

The pull request is closed.

Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• 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 f2a5fb87afd8bfe34fd3d61045f8ee8677590031

Blocking:

• none

Non-blocking:

• .pre-commit-config.yaml:28-30 (comment) and the PR body: "pipefail makes a failing go list fail the hook" holds for module-level failures such as a bad -mod or a broken go.mod. With -e, though, a broken first-party package does not make go list exit non-zero. That case is still caught, because go vet/go test then fails on the package (I checked: a first-party syntax error gives exit 1). So the behavior is fine and only the wording overstates what pipefail covers.
• Refs #177 rather than Closes is right: the Makefile vet/test-* targets still use ./.... The ignore ./frontend/node_modules go.mod directive mentioned under Out of scope (go.mod is go 1.26.10, so it is available) would be the one-place fix and is worth a follow-up decision.

Attacked:

• Planted three packages under frontend/node_modules in a scratch worktree: a printf vet error, a syntax error, and an import of a missing module. Main's entry bash -c 'go vet ./...' exits 1 on them. The new entry exits 0.
• Planted a printf mismatch, then a syntax error, in an untracked internal/zzrev/f.go. The new entry exits 1 on both, so first-party code is still vetted, including packages that go list -e lists with errors.
• GOFLAGS=-mod=bogus with the new entry exits 1, so it does not pass silently on an empty list.
• Ran shlex.split on both entries the way pre-commit does: each gives ['bash','-c','<script>'] with the inner double quotes intact. pre-commit validate-config is OK and pre-commit run go-vet --all-files passes on the branch. No Go code changed, so I ran no Go build or test.

Body-stale:

• none

_{Reviewer: rF5. 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 f2a5fb87afd8bfe34fd3d61045f8ee8677590031 merged onto main (29ed5ceb), together with #1512 in that order (combined tree 59762484, 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 1 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: 1 approving independent review(s) on f2a5fb87, green CI, and the combined-tree verification above. Squash-merged at exactly f2a5fb87afd8bfe34fd3d61045f8ee8677590031.