golangci-lint cache can report findings against paths that do not exist #36

Closed
opened 2026-08-09 08:39:21 +02:00 by clawbot · 1 comment
Collaborator

A third way a gate in this repo can tell you something other than the truth about your working tree — and the first one that produces false findings rather than a false green.

Observed by the independent reviewer of #34. Their first make check reported ten Go findings against paths under ../sfdupes-rework-34/ — a worktree that had already been deleted and did not exist on the machine. Setting a fresh GOLANGCI_LINT_CACHE and re-running gave 0 issues.

The mechanism: golangci-lint caches analysis results keyed on file content. Agents and reviewers here work in throwaway worktrees, so the same file content appears at many different paths over time. A cache entry created under one worktree path gets served for byte-identical content under another, and the stale path travels with it into the report.

Why it matters beyond the confusing paths. This repo now has three independent ways for a gate to lie, and reviewers have hit all three:

  • #24 (fixed) — the local linter was a different version than the pin.
  • #32 (open) — make docker can be a total layer-cache hit that executes nothing and exits 0. Confirmed live three separate times.
  • this issue — the linter cache can attribute findings to a tree that is not the one under test.

The failure mode here is the least dangerous of the three, because it produces noise rather than false confidence. But it costs review time, and a reviewer who cannot reproduce a finding may reasonably conclude the finding is spurious when the next one is real.

Definition of done

  1. Decide and implement how script/lint handles the cache. Options: point GOLANGCI_LINT_CACHE at a repo-local, gitignored directory so it cannot be polluted by other checkouts; or leave the shared cache and document the failure mode plus the one-line remedy. A repo-local cache is the cleaner answer and costs only a cold first run.
  2. If a repo-local cache directory is used, it is gitignored and excluded via .dockerignore, and make clean removes it.
  3. Verify the fix by reproducing the original condition: run the linter from one worktree, delete it, create a second worktree with identical content, and confirm the second reports no findings attributed to the first.
  4. Confirm the change does not slow make check beyond the 20-second budget on a warm cache.
  5. make check and make docker green, the latter demonstrably not a cache hit (#32).

Also fix, from the same review of #34, three small robustness gaps in the script/bootstrap verification that landed in #24 — none of them able to produce a false pass, all of them worth closing:

  • A multi-element GOPATH yields a malformed directory in the diagnostic (/a:/b/bin). Cosmetic; it still fails loudly. Take the first element.
  • golangci-lint --version exit status is ignored, so a binary that prints the pinned version and then exits non-zero is treated as a match and skipped. Check the status.
  • On a host without timeout(1) — stock macOS, and brew is in the package-manager detection list, so macOS is a supported host class — a wedged linter hangs bootstrap unbounded. Worst case with the timeout present is 60 seconds across the two bounded reads, which is acceptable; unbounded is not.
A third way a gate in this repo can tell you something other than the truth about your working tree — and the first one that produces false *findings* rather than a false green. Observed by the independent reviewer of #34. Their first `make check` reported ten Go findings against paths under `../sfdupes-rework-34/` — a worktree that had already been deleted and did not exist on the machine. Setting a fresh `GOLANGCI_LINT_CACHE` and re-running gave `0 issues.` The mechanism: golangci-lint caches analysis results keyed on file content. Agents and reviewers here work in throwaway worktrees, so the same file content appears at many different paths over time. A cache entry created under one worktree path gets served for byte-identical content under another, and the stale path travels with it into the report. Why it matters beyond the confusing paths. This repo now has three independent ways for a gate to lie, and reviewers have hit all three: - #24 (fixed) — the local linter was a different version than the pin. - #32 (open) — `make docker` can be a total layer-cache hit that executes nothing and exits 0. Confirmed live three separate times. - this issue — the linter cache can attribute findings to a tree that is not the one under test. The failure mode here is the least dangerous of the three, because it produces noise rather than false confidence. But it costs review time, and a reviewer who cannot reproduce a finding may reasonably conclude the finding is spurious when the next one is real. ## Definition of done 1. Decide and implement how `script/lint` handles the cache. Options: point `GOLANGCI_LINT_CACHE` at a repo-local, gitignored directory so it cannot be polluted by other checkouts; or leave the shared cache and document the failure mode plus the one-line remedy. A repo-local cache is the cleaner answer and costs only a cold first run. 2. If a repo-local cache directory is used, it is gitignored and excluded via `.dockerignore`, and `make clean` removes it. 3. Verify the fix by reproducing the original condition: run the linter from one worktree, delete it, create a second worktree with identical content, and confirm the second reports no findings attributed to the first. 4. Confirm the change does not slow `make check` beyond the 20-second budget on a warm cache. 5. `make check` and `make docker` green, the latter demonstrably not a cache hit (#32). Also fix, from the same review of #34, three small robustness gaps in the `script/bootstrap` verification that landed in #24 — none of them able to produce a false pass, all of them worth closing: - A multi-element `GOPATH` yields a malformed directory in the diagnostic (`/a:/b/bin`). Cosmetic; it still fails loudly. Take the first element. - `golangci-lint --version` exit status is ignored, so a binary that prints the pinned version and then exits non-zero is treated as a match and skipped. Check the status. - On a host without `timeout(1)` — stock macOS, and `brew` is in the package-manager detection list, so macOS is a supported host class — a wedged linter hangs bootstrap unbounded. Worst case with the timeout present is 60 seconds across the two bounded reads, which is acceptable; unbounded is not.
clawbot added this to the 1.0.0 milestone 2026-08-09 08:39:21 +02:00
Author
Collaborator

Dissolved by #46, merged as
d43c1d3. Closing — every subject of this issue is gone from the tree
rather than fixed.

The contamination mechanism was a golangci-lint cache shared across
host checkouts. The linter now runs only inside Dockerfile.lint,
where WORKDIR is always /src, no cache directory is mounted in, and
CHECK_EPOCH invalidates the gate layer on every run — so each run
starts with an empty analysis cache and no path from any other
checkout can be served into it. Corroborated by the measured cost: the
lint step takes 27-45s on every run, which is a cold analysis, not a
warm-cache hit.

DoD items 1-4 (repo-local cache directory, .gitignore/.dockerignore
entries, make clean, the 20-second warm-cache budget) have no subject
now that no cache is reused, and the owner has waived lint caching.

The three script/bootstrap robustness gaps appended here — multi-element
GOPATH, unchecked --version exit status, unbounded read without
timeout(1) — are also moot: that whole verification block, including
the version parser and the pin, was deleted rather than hardened.

Dissolved by https://git.eeqj.de/sneak/sfdupes/issues/46, merged as `d43c1d3`. Closing — every subject of this issue is gone from the tree rather than fixed. The contamination mechanism was a golangci-lint cache shared across host checkouts. The linter now runs only inside `Dockerfile.lint`, where `WORKDIR` is always `/src`, no cache directory is mounted in, and `CHECK_EPOCH` invalidates the gate layer on every run — so each run starts with an empty analysis cache and no path from any other checkout can be served into it. Corroborated by the measured cost: the lint step takes 27-45s on every run, which is a cold analysis, not a warm-cache hit. DoD items 1-4 (repo-local cache directory, `.gitignore`/`.dockerignore` entries, `make clean`, the 20-second warm-cache budget) have no subject now that no cache is reused, and the owner has waived lint caching. The three `script/bootstrap` robustness gaps appended here — multi-element `GOPATH`, unchecked `--version` exit status, unbounded read without `timeout(1)` — are also moot: that whole verification block, including the version parser and the pin, was deleted rather than hardened.
Sign in to join this conversation.