1 Commits

Author SHA1 Message Date
clawbot
0620416869 Give golangci-lint per-checkout cache and lock state (closes #30)
All checks were successful
check / check (push) Successful in 7s
A golangci-lint result on a host running many concurrent workers does not
reliably belong to the tree that asked for it. Two independent mechanisms,
which have repeatedly been mistaken for one:

The result cache is keyed on file content, not location, so two checkouts
of the same commit hold byte-identical files, share cache entries, and one
tree's findings are served for the other under the other tree's path. This
produced a confirmed false green as well as the loud false reds. Moving
workers from shared worktrees to their own clones does not address it —
two clones collide exactly as two worktrees did — and removes only the
foreign-path artefact that made the defect noticeable.

The concurrency lock is $TMPDIR/golangci-lint.lock (pkg/commands/run.go,
acquireFileLock), host-global and independent of GOLANGCI_LINT_CACHE, with
a five-second acquire timeout, so it fails when the host is busiest. A
private cache directory does not isolate it. Setting only the cache closes
the contamination half and leaves runs failing red on a condition that is
not a result at all.

REPO_POLICIES.md now carries the canonical Go script/lint: both variables
scoped into a .lint-cache/ directory inside the checkout, above any
container-versus-host branch so every path reaching the linter gets them;
--allow-serial-runners, which keeps the mutual-exclusion guard and queues
rather than aborting, for the same-checkout overlap TMPDIR scoping cannot
cover, with --allow-parallel-runners rejected because it deletes the
guard; and a bounded retry that treats the lock error as VOID rather than
as findings, exiting 75 on exhaustion so it is neither a pass nor a
failure. Detection is on the stderr stream and never on exit status:
findings go to stdout, so a finding quoting the lock message in source
cannot be retried away, and the exit status is not a stable discriminator
anyway. The interim void rule is recorded with the ../ clause that the
original filter missed, and with its limit stated — it catches
contamination that names foreign files, not contamination that suppresses
findings. Both checklists gained the corresponding items, since a half-fix
that sets only the cache reads as complete.

GOCACHE was measured rather than assumed and does not need isolating: with
the two variables scoped per checkout and GOCACHE shared at the host
default, each checkout reported its own paths.

Verified with the snippet extracted from the committed document and
executed as a consuming repo would adopt it, each control paired against
the pre-fix form: contamination reproduced on the pre-fix script and
absent on the adopted one; a stub linter colliding twice then clearing,
with the retry engaging and succeeding; exhaustion exiting 75 with a VOID
message; a genuine finding whose text quotes the lock message reported as
findings with no retry; and a real held lock failing the pre-fix script
with exit 3 while the adopted script, inheriting the same environment,
completed in one second.
2026-08-09 18:51:22 +00:00

View File

@@ -959,10 +959,13 @@ style conventions are in separate documents:
wait, where the process holding things up is the linter itself. Kill the
group (`kill -- -<pgid>`) or use Ctrl-C.
- **Known, accepted gap:** a signal arriving between the `mktemp` calls and
the `trap ... EXIT` line leaves the two capture files behind. Closing it
needs a trap installed before the files have names and rewritten after,
which is more moving parts than a couple of stray files in a gitignored
directory is worth. Stated rather than silently left.
the `trap ... EXIT` line leaves the two capture files behind. It is
closable, and cheaply — initialise both variables to the empty string and
move all four `trap` lines above the `mktemp` calls, with nothing
rewritten afterwards. It is accepted anyway because of what the gap costs,
not because of what closing it costs: two stray files in a gitignored
directory, never an incorrect result. Reconsider it on that trade-off if
the balance ever changes.
Adopting repos must add `.lint-cache/` to both `.gitignore` and
`.dockerignore`. The second matters as much as the first: the directory