script/lint does not pin the golangci-lint binary version, so local runs can false-green against a stale linter #58

Closed
opened 2026-08-09 03:00:19 +02:00 by clawbot · 3 comments
Collaborator

Discovered during PR #54 review (v2.12.2 canonical config rollout): script/lint runs golangci-lint run against whatever binary happens to be on PATH, with no version check. A local environment with a stale binary (v2.10.1 was observed) reports make check as green even when the pinned CI version (v2.12.2, matching the Dockerfile lint stage and script/bootstrap's intended install) would fail. This let a rework pass on PR #54 believe 15 new //nolint:gosec directives were needed (they were dead suppressions under the real pinned version) and only got caught because a reviewer ran the authoritative docker build --target lint ..

sneak/vaultik has the same gap (confirmed while cross-checking PR #54's pin conventions against it).

Definition of done

  • script/lint verifies the installed golangci-lint matches the version pinned in script/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 on PATH — pick whichever fits this repo's existing bootstrap/install layout).
  • make check/make lint run locally then reliably match docker build --target lint . and Gitea CI — no more silent false-greens on a stale toolchain.
  • make check green.
Discovered during PR #54 review (v2.12.2 canonical config rollout): `script/lint` runs `golangci-lint run` against whatever binary happens to be on `PATH`, with no version check. A local environment with a stale binary (v2.10.1 was observed) reports `make check` as green even when the pinned CI version (v2.12.2, matching the `Dockerfile` lint stage and `script/bootstrap`'s intended install) would fail. This let a rework pass on PR #54 believe 15 new `//nolint:gosec` directives were needed (they were dead suppressions under the real pinned version) and only got caught because a reviewer ran the authoritative `docker build --target lint .`. `sneak/vaultik` has the same gap (confirmed while cross-checking PR #54's pin conventions against it). ## Definition of done - `script/lint` verifies the installed `golangci-lint` matches the version pinned in `script/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 on `PATH` — pick whichever fits this repo's existing bootstrap/install layout). - `make check`/`make lint` run locally then reliably match `docker build --target lint .` and Gitea CI — no more silent false-greens on a stale toolchain. - `make check` green.
clawbot added this to the 1.0.0 milestone 2026-08-09 03:37:51 +02:00
Author
Collaborator

Extending this issue's scope with a second, worse script/lint failure mode observed during PR #54's round-4 rework.

The known problem is that script/lint runs whatever golangci-lint is on PATH, so a stale binary false-greens. The newly observed problem is that the bare golangci-lint run can escape the current git worktree.

During the rework, a host make lint run 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/lint passes to golangci-lint run resolving 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/lint constrains 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, run make lint from the first, and confirm the error is not reported.
  • Consider whether the golangci-lint result cache needs to be worktree-scoped, since a shared cache across worktrees is a plausible mechanism for cross-contamination.
  • If the cause turns out to be upstream golangci-lint behavior rather than this script, say so and pin the mitigation here anyway — the script is what we control.

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 check green" evidence is weaker than it looks. Sequencing captured in #103.

Extending this issue's scope with a second, worse `script/lint` failure mode observed during PR #54's round-4 rework. The known problem is that `script/lint` runs whatever `golangci-lint` is on `PATH`, so a stale binary false-greens. The newly observed problem is that the bare `golangci-lint run` **can escape the current git worktree**. During the rework, a host `make lint` run 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/lint` passes to `golangci-lint run` resolving 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/lint` constrains 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, run `make lint` from the first, and confirm the error is **not** reported. - Consider whether the golangci-lint result cache needs to be worktree-scoped, since a shared cache across worktrees is a plausible mechanism for cross-contamination. - If the cause turns out to be upstream golangci-lint behavior rather than this script, say so and pin the mitigation here anyway — the script is what we control. ## 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 check` green" evidence is weaker than it looks. Sequencing captured in #103.
Owner

fix it. stop waiting to be told

fix it. stop waiting to be told
Author
Collaborator

Superseded by #104 (owner ruling: lint runs only in Docker via script/lint, so there is no host binary left to pin). Closing.

Superseded by https://git.eeqj.de/sneak/pixa/issues/104 (owner ruling: lint runs only in Docker via `script/lint`, so there is no host binary left to pin). Closing.
Sign in to join this conversation.
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/pixa#58