script/lint does not pin the golangci-lint binary version, so local runs can false-green against a stale linter #58
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Discovered during PR #54 review (v2.12.2 canonical config rollout):
script/lintrunsgolangci-lint runagainst whatever binary happens to be onPATH, with no version check. A local environment with a stale binary (v2.10.1 was observed) reportsmake checkas green even when the pinned CI version (v2.12.2, matching theDockerfilelint stage andscript/bootstrap's intended install) would fail. This let a rework pass on PR #54 believe 15 new//nolint:gosecdirectives were needed (they were dead suppressions under the real pinned version) and only got caught because a reviewer ran the authoritativedocker build --target lint ..sneak/vaultikhas the same gap (confirmed while cross-checking PR #54's pin conventions against it).Definition of done
script/lintverifies the installedgolangci-lintmatches the version pinned inscript/bootstrap(GOLANGCI_LINT_VERSION) before running, and fails loudly with a clear message if it doesn't match (or, alternatively, always invokes the pinned binary explicitly by a version-qualified path rather than relying onPATH— pick whichever fits this repo's existing bootstrap/install layout).make check/make lintrun locally then reliably matchdocker build --target lint .and Gitea CI — no more silent false-greens on a stale toolchain.make checkgreen.Extending this issue's scope with a second, worse
script/lintfailure mode observed during PR #54's round-4 rework.The known problem is that
script/lintruns whatevergolangci-lintis onPATH, so a stale binary false-greens. The newly observed problem is that the baregolangci-lint runcan escape the current git worktree.During the rework, a host
make lintrun reported findings whose paths pointed into a different, concurrent agent's worktree (../agent-<other-id>/...) rather than the working tree it was invoked from. The agent correctly discarded that run as void and relied solely on the Docker-pinned result.This is materially worse than the stale-binary case. A stale binary gives you the wrong answer about your code; this gives you an answer about someone else's code while looking entirely legitimate. In a repo where several agents may hold worktrees under
.claude/worktrees/simultaneously, a lint run can attribute another branch's findings to yours — or, worse, report clean because the findings it surfaced were filtered out as belonging elsewhere.Not yet root-caused. Plausible candidates worth checking before fixing: the package pattern
script/lintpasses togolangci-lint runresolving above the worktree root; golangci-lint's module/directory discovery walking up past the worktree into the parent repository; or its result cache being shared across worktrees and replaying another tree's findings.Added to the definition of done
script/lintconstrains golangci-lint to the invoking working tree, so a run inside a worktree can never report a finding located outside it. Verify explicitly: create a second worktree with a deliberate lint error, runmake lintfrom the first, and confirm the error is not reported.Note
This is the same class as #101 (a check that can report a result it did not earn) and both are
script/-level. Worth resolving in the same sweep, and both are high-priority for that reason: until they land, every other PR's "make checkgreen" evidence is weaker than it looks. Sequencing captured in #103.fix it. stop waiting to be told
Superseded by #104 (owner ruling: lint runs only in Docker via
script/lint, so there is no host binary left to pin). Closing.