$TMPDIR/golangci-lint.lock, i.e. /tmp/golangci-lint.lock when TMPDIR is unset. Not in the cache directory, which is why an isolated GOLANGCI_LINT_CACHE did not stop the collision PR #122 hit. It is a flock with a 1s retry and a 5s total timeout, so any run whose peer holds it longer than 5s aborts.
Fix
script/lint sets both, derived from the existing $ROOT idiom:
GOLANGCI_LINT_CACHE="$ROOT/.lint-cache/cache" — cache is never shared, so no run can be served another checkout's findings (1a).
TMPDIR="$ROOT/.lint-cache/tmp" — moves the lock file per checkout (1b). This scopes the lock rather than removing it, so it still serialises runs that genuinely share one cache. --allow-parallel-runners was the alternative and was rejected: it deletes the guard instead of scoping it.
.lint-cache/ added to .gitignore and .dockerignore. .golangci.yml untouched (sha256 still 021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb), linter pin untouched, no check suppressed, script/lint still POSIX sh and sh -n clean.
Verification
Reproduction used 12 (later 20) copies of this tree under /tmp, each a full checkout, running make lint concurrently.
Unfixed script:
Lock: 10 of 12 concurrent runs aborted with parallel golangci-lint is running.
Contamination: with an identical deliberately-lint-failing file added to every copy, and the runs made sequential to remove lock noise, 11 of 12 reported their findings at ../w1/internal/lintprobe/probe.go — another checkout's path, for a file they did not lint. Only w1, which populated the shared cache, reported its own path.
Fixed script: 20 concurrent runs, cold caches — 0 void, 0 foreign paths. Repeating with the lint-failing file present, all 12 copies reported exactly their own internal/lintprobe/probe.go and no ../ path.
Docker: real build, docker build --no-cache-filter=builder ., chosen over script/cibuild because that takes no arguments and can report a false green on a byte-identical tree (#115). The builder stage executed rather than being reused (go install golangci-lint 53.8s, RUN make check 51.8s, not CACHED); lint reported 0 issues inside the container with no lock error. HOME and TMPDIR differ there and the fix does not depend on either — the path comes from the checkout root.
Timing: warm run ~1.7–2.2s, against ~0.8s before; the gap is host scheduling noise, not the change — user CPU time went down (2.0s vs 2.6s). The real cost is that the first run in a fresh checkout is now cold, ~17s, because the cache is no longer shared across checkouts. That is inherent to the fix. Cache is ~80MB per checkout.
make check green (GOFLAGS=-count=1), tree unmodified afterwards, TODO.md updated in the same commit.
Anomalies
The gomodguard deprecation warning still appears on every lint run. Pre-existing and already recorded as accepted in TODO.md.
The watcher tests log failed to save state error="writing temp state file: open /state.json.tmp: permission denied" repeatedly while passing. Pre-existing, unrelated to this change, and out of scope here — flagging it rather than filing, since I cannot tell whether it is deliberate error-path coverage or tests falling back to the default data directory.
## Where the lock lives
golangci-lint v2.12.2 (`pkg/commands/run.go`, `acquireFileLock`):
```go
lockFile := filepath.Join(os.TempDir(), "golangci-lint.lock")
```
`$TMPDIR/golangci-lint.lock`, i.e. `/tmp/golangci-lint.lock` when `TMPDIR` is unset. Not in the cache directory, which is why an isolated `GOLANGCI_LINT_CACHE` did not stop the collision PR #122 hit. It is a `flock` with a 1s retry and a 5s total timeout, so any run whose peer holds it longer than 5s aborts.
## Fix
`script/lint` sets both, derived from the existing `$ROOT` idiom:
- `GOLANGCI_LINT_CACHE="$ROOT/.lint-cache/cache"` — cache is never shared, so no run can be served another checkout's findings (1a).
- `TMPDIR="$ROOT/.lint-cache/tmp"` — moves the lock file per checkout (1b). This scopes the lock rather than removing it, so it still serialises runs that genuinely share one cache. `--allow-parallel-runners` was the alternative and was rejected: it deletes the guard instead of scoping it.
`.lint-cache/` added to `.gitignore` and `.dockerignore`. `.golangci.yml` untouched (sha256 still `021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb`), linter pin untouched, no check suppressed, `script/lint` still POSIX `sh` and `sh -n` clean.
## Verification
Reproduction used 12 (later 20) copies of this tree under `/tmp`, each a full checkout, running `make lint` concurrently.
**Unfixed script:**
- Lock: 10 of 12 concurrent runs aborted with `parallel golangci-lint is running`.
- Contamination: with an identical deliberately-lint-failing file added to every copy, and the runs made sequential to remove lock noise, 11 of 12 reported their findings at `../w1/internal/lintprobe/probe.go` — another checkout's path, for a file they did not lint. Only `w1`, which populated the shared cache, reported its own path.
**Fixed script:** 20 concurrent runs, cold caches — 0 void, 0 foreign paths. Repeating with the lint-failing file present, all 12 copies reported exactly their own `internal/lintprobe/probe.go` and no `../` path.
**Docker:** real build, `docker build --no-cache-filter=builder .`, chosen over `script/cibuild` because that takes no arguments and can report a false green on a byte-identical tree (#115). The builder stage executed rather than being reused (`go install golangci-lint` 53.8s, `RUN make check` 51.8s, not `CACHED`); lint reported `0 issues` inside the container with no lock error. `HOME` and `TMPDIR` differ there and the fix does not depend on either — the path comes from the checkout root.
**Timing:** warm run ~1.7–2.2s, against ~0.8s before; the gap is host scheduling noise, not the change — user CPU time went down (2.0s vs 2.6s). The real cost is that the first run in a fresh checkout is now cold, ~17s, because the cache is no longer shared across checkouts. That is inherent to the fix. Cache is ~80MB per checkout.
`make check` green (`GOFLAGS=-count=1`), tree unmodified afterwards, `TODO.md` updated in the same commit.
## Anomalies
- The `gomodguard` deprecation warning still appears on every lint run. Pre-existing and already recorded as accepted in `TODO.md`.
- The `watcher` tests log `failed to save state error="writing temp state file: open /state.json.tmp: permission denied"` repeatedly while passing. Pre-existing, unrelated to this change, and out of scope here — flagging it rather than filing, since I cannot tell whether it is deliberate error-path coverage or tests falling back to the default data directory.
Built: script/lint now exports GOLANGCI_LINT_CACHE and TMPDIR under a git-ignored, Docker-ignored .lint-cache/ in the checkout root. The cache var closes the cross-contamination half; TMPDIR is the half that actually matters for the lock, because golangci-lint locks os.TempDir()/golangci-lint.lock, not anything under the cache directory.
Verified: reproduced both failure modes on the unfixed script (10 of 12 concurrent runs void with parallel golangci-lint is running; 11 of 12 runs reporting ../w1/... paths for a file they never linted), then 0 of 20 void and 0 foreign paths after. Real Docker build with the builder stage forced to execute, lint 0 issues inside the container. make check green with GOFLAGS=-count=1, tree unmodified after. Full detail and the timing numbers are in the PR description.
Two pre-existing anomalies are called out at the bottom of the PR description; neither is caused by this change.
Built: `script/lint` now exports `GOLANGCI_LINT_CACHE` and `TMPDIR` under a git-ignored, Docker-ignored `.lint-cache/` in the checkout root. The cache var closes the cross-contamination half; `TMPDIR` is the half that actually matters for the lock, because golangci-lint locks `os.TempDir()/golangci-lint.lock`, not anything under the cache directory.
Verified: reproduced both failure modes on the unfixed script (10 of 12 concurrent runs void with `parallel golangci-lint is running`; 11 of 12 runs reporting `../w1/...` paths for a file they never linted), then 0 of 20 void and 0 foreign paths after. Real Docker build with the builder stage forced to execute, lint `0 issues` inside the container. `make check` green with `GOFLAGS=-count=1`, tree unmodified after. Full detail and the timing numbers are in the PR description.
Two pre-existing anomalies are called out at the bottom of the PR description; neither is caused by this change.
Independent review of 520ce79. No blocking findings.
Lock claim verified against the pinned source
The pin c0d3ddc9cf3faa61a4e378e879ece580256d76e5 resolves to v2.12.2 (@v/v2.12.2.info, Origin.Hash), and that source says exactly what the PR says: pkg/commands/run.go:492lockFile := filepath.Join(os.TempDir(), "golangci-lint.lock"), flock, retryDelay = time.Second (:495), totalTimeout = 5 * time.Second (:499). Path, mechanism and both timings are correct, and the lock is demonstrably not under GOLANGCI_LINT_CACHE.
Concurrency test (my own, 12 fresh copies of this tree)
Condition
Result
Unfixed script, 12-way concurrent
11/12 void with parallel golangci-lint is running
Unfixed script, 12 sequential, identical planted lll violation in each copy
11/12 reported ../u4/internal/lintprobe/probe.go — another copy's path
12/12 reported only their own internal/lintprobe/probe.go; 0 lock errors, 0 foreign paths
Fixed 12 plus unfixed 12 running simultaneously (24-way)
all 12 fixed runs rc=0, 0 issues., no lock error, no foreign path, while 9/12 unfixed peers went void
The reproduction lands before the fix and disappears after it, including with the shared /tmp lock actively thrashed by peers. Both halves of the amended definition of done are met.
Timings here: cold 8.1s, warm 1.3s, cache 80MB — consistent with the PR's numbers.
Everything else checked
make checkrc=0 and git status --porcelain empty afterwards; .lint-cache/ git-ignored and Docker-ignored; docker build --progress=plain --no-cache-filter=builder --target builder . (direct docker, stated per policy) ran the builder for real — RUN make checkDONE 38.6s, not CACHED, lint 0 issues inside the container with TMPDIR=/src/.lint-cache/tmp; .golangci.yml sha256 matches; pin unchanged in Dockerfile and script/bootstrap; no linter disabled or narrowed; no test file touched; POSIX sh, sh -n clean, no bashisms, $ROOT idiom kept; CI green on head (54s, real); base is current origin/main; commit title ends (closes #121) with TODO.md in the same commit; no Claude/Anthropic references or attribution trailers anywhere in 9347a28..520ce79.
Non-blocking findings
1. The lock still aborts rather than serialises, within a single checkout — and the script comment says otherwise. Two concurrent make lint in the same checkout with a cold cache: one exits rc=0, the other rc=2 with parallel golangci-lint is running (measured). Realistic trigger: script/precommit firing while a make check is already linting the same worktree — a cold run is 8-17s, well past the 5s timeout. This is much narrower than the pre-fix behaviour and is not a regression, but script/lint's comment ("keeps the lock doing its actual job (serialising runs that share one cache)") overstates it: after 5s the loser is aborted, not queued.
The lever that would make that sentence true is --allow-serial-runners, not --allow-parallel-runners: pkg/commands/flagsets.go:59 binds run.allow-serial-runners, and run.go:498 uses it to skip the 5s context.WithTimeout, so the second run blocks on the flock until the first releases instead of aborting. It keeps the guard, unlike --allow-parallel-runners. The PR description and issue comment 49933 both state that --allow-parallel-runners "is the only other escape" — that is incorrect and worth correcting in the record even if the flag is not adopted.
2. make clean does not remove .lint-cache/.Makefile:43 is rm -rf bin/. 80MB per checkout, no automatic trimming, on a host that spawns many throwaway worktrees. rm -rf bin/ .lint-cache/ would close it.
3. Naming nit:.lint-cache/cache stutters, and the directory holds tmp as well as a cache. Something like .lint-state/{cache,tmp} reads better. Not worth a respin on its own.
Anomaly that passes anyway
TMPDIR now points inside the tree being linted. That is safe only because the Go tool skips directories whose names begin with . when expanding ./... — confirmed: no run reported anything under .lint-cache/. It is a load-bearing property of the leading dot, so renaming the directory to lint-cache/ later would silently start feeding temp files to the linter. Worth a word in the comment if anyone touches the name.
The two pre-existing anomalies the author flagged (the gomodguard deprecation warning, and the watcher tests logging permission denied on /state.json.tmp while passing) both reproduce for me on unmodified main and inside the container; neither is caused by this change.
## Review: PASS
Independent review of `520ce79`. No blocking findings.
### Lock claim verified against the pinned source
The pin `c0d3ddc9cf3faa61a4e378e879ece580256d76e5` resolves to `v2.12.2` (`@v/v2.12.2.info`, `Origin.Hash`), and that source says exactly what the PR says: `pkg/commands/run.go:492` `lockFile := filepath.Join(os.TempDir(), "golangci-lint.lock")`, `flock`, `retryDelay = time.Second` (:495), `totalTimeout = 5 * time.Second` (:499). Path, mechanism and both timings are correct, and the lock is demonstrably not under `GOLANGCI_LINT_CACHE`.
### Concurrency test (my own, 12 fresh copies of this tree)
| Condition | Result |
|---|---|
| Unfixed script, 12-way concurrent | 11/12 void with `parallel golangci-lint is running` |
| Unfixed script, 12 sequential, identical planted `lll` violation in each copy | 11/12 reported `../u4/internal/lintprobe/probe.go` — another copy's path |
| Fixed script, 12-way concurrent, planted violation | 12/12 reported only their own `internal/lintprobe/probe.go`; 0 lock errors, 0 foreign paths |
| Fixed 12 **plus** unfixed 12 running simultaneously (24-way) | all 12 fixed runs `rc=0`, `0 issues.`, no lock error, no foreign path, while 9/12 unfixed peers went void |
The reproduction lands before the fix and disappears after it, including with the shared `/tmp` lock actively thrashed by peers. Both halves of the amended definition of done are met.
Timings here: cold 8.1s, warm 1.3s, cache 80MB — consistent with the PR's numbers.
### Everything else checked
`make check` `rc=0` and `git status --porcelain` empty afterwards; `.lint-cache/` git-ignored and Docker-ignored; `docker build --progress=plain --no-cache-filter=builder --target builder .` (direct docker, stated per policy) ran the builder for real — `RUN make check` `DONE 38.6s`, not `CACHED`, lint `0 issues` inside the container with `TMPDIR=/src/.lint-cache/tmp`; `.golangci.yml` sha256 matches; pin unchanged in `Dockerfile` and `script/bootstrap`; no linter disabled or narrowed; no test file touched; POSIX `sh`, `sh -n` clean, no bashisms, `$ROOT` idiom kept; CI green on head (54s, real); base is current `origin/main`; commit title ends ` (closes #121)` with `TODO.md` in the same commit; no Claude/Anthropic references or attribution trailers anywhere in `9347a28..520ce79`.
### Non-blocking findings
**1. The lock still aborts rather than serialises, within a single checkout — and the script comment says otherwise.** Two concurrent `make lint` in the *same* checkout with a cold cache: one exits `rc=0`, the other `rc=2` with `parallel golangci-lint is running` (measured). Realistic trigger: `script/precommit` firing while a `make check` is already linting the same worktree — a cold run is 8-17s, well past the 5s timeout. This is much narrower than the pre-fix behaviour and is not a regression, but `script/lint`'s comment ("keeps the lock doing its actual job (serialising runs that share one cache)") overstates it: after 5s the loser is aborted, not queued.
The lever that would make that sentence true is `--allow-serial-runners`, not `--allow-parallel-runners`: `pkg/commands/flagsets.go:59` binds `run.allow-serial-runners`, and `run.go:498` uses it to skip the 5s `context.WithTimeout`, so the second run blocks on the flock until the first releases instead of aborting. It keeps the guard, unlike `--allow-parallel-runners`. The PR description and issue comment 49933 both state that `--allow-parallel-runners` "is the only other escape" — that is incorrect and worth correcting in the record even if the flag is not adopted.
**2. `make clean` does not remove `.lint-cache/`.** `Makefile:43` is `rm -rf bin/`. 80MB per checkout, no automatic trimming, on a host that spawns many throwaway worktrees. `rm -rf bin/ .lint-cache/` would close it.
**3. Naming nit:** `.lint-cache/cache` stutters, and the directory holds `tmp` as well as a cache. Something like `.lint-state/{cache,tmp}` reads better. Not worth a respin on its own.
### Anomaly that passes anyway
`TMPDIR` now points *inside* the tree being linted. That is safe only because the Go tool skips directories whose names begin with `.` when expanding `./...` — confirmed: no run reported anything under `.lint-cache/`. It is a load-bearing property of the leading dot, so renaming the directory to `lint-cache/` later would silently start feeding temp files to the linter. Worth a word in the comment if anyone touches the name.
The two pre-existing anomalies the author flagged (the `gomodguard` deprecation warning, and the `watcher` tests logging `permission denied` on `/state.json.tmp` while passing) both reproduce for me on unmodified `main` and inside the container; neither is caused by this change.
[manager] Independent review PASS, no blocking findings. merge-ready, assigned to @sneak.
Lock path independently confirmed against the pinned source (run.go:492, flock, 1s retry, 5s timeout). Reviewer's own 24-way probe — 12 fixed and 12 unfixed runs launched together — had all 12 fixed runs clean while 9/12 unfixed peers went void, so the fix is immune to peers thrashing the shared /tmp lock. Constraints all verified clean.
One correction to record, because I propagated the error upstream. The PR body and my comment on prompts #30 both say --allow-parallel-runners was "the alternative". That is wrong: --allow-serial-runners also exists (flagsets.go:59, consumed at run.go:498 to skip the timeout), and it is the better lever for the residual case — it keeps the guard and makes overlapping runs queue rather than abort. I have corrected the upstream comment, since that repo is actively writing the canonical fix and would otherwise inherit the mistake.
Follow-ups filed as #130, neither blocking: two concurrent make lintwithin one checkout still abort after 5s rather than queueing (trigger: script/precommit overlapping a make check), and make clean does not remove .lint-cache/, which accumulates ~80MB per throwaway worktree unbounded.
Anomaly worth carrying forward:TMPDIR now lives inside the linted tree, and that is safe only because the Go tool skips dot-prefixed directories when expanding ./.... The leading dot is load-bearing — renaming to lint-cache/ would silently start feeding temp files to the linter.
**[manager]** Independent review **PASS**, no blocking findings. `merge-ready`, assigned to @sneak.
Lock path independently confirmed against the pinned source (`run.go:492`, flock, 1s retry, 5s timeout). Reviewer's own 24-way probe — 12 fixed and 12 unfixed runs launched together — had all 12 fixed runs clean while 9/12 unfixed peers went void, so the fix is immune to peers thrashing the shared `/tmp` lock. Constraints all verified clean.
**One correction to record, because I propagated the error upstream.** The PR body and my comment on [prompts #30](https://git.eeqj.de/sneak/prompts/issues/30) both say `--allow-parallel-runners` was "the alternative". That is wrong: **`--allow-serial-runners`** also exists (`flagsets.go:59`, consumed at `run.go:498` to skip the timeout), and it is the better lever for the residual case — it keeps the guard and makes overlapping runs *queue* rather than abort. I have corrected the upstream comment, since that repo is actively writing the canonical fix and would otherwise inherit the mistake.
**Follow-ups filed as [#130](https://git.eeqj.de/sneak/dnswatcher/issues/130)**, neither blocking: two concurrent `make lint` *within one checkout* still abort after 5s rather than queueing (trigger: `script/precommit` overlapping a `make check`), and `make clean` does not remove `.lint-cache/`, which accumulates ~80MB per throwaway worktree unbounded.
**Anomaly worth carrying forward:** `TMPDIR` now lives inside the linted tree, and that is safe *only* because the Go tool skips dot-prefixed directories when expanding `./...`. The leading dot is load-bearing — renaming to `lint-cache/` would silently start feeding temp files to the linter.
script/lint used golangci-lint's per-user global state, which breaks
when several checkouts on one host lint concurrently. Two independent
failure modes, two causes:
- Cross-contamination. The analysis cache (GOLANGCI_LINT_CACHE,
default ~/.cache/golangci-lint) is keyed by content, not by
checkout, so a hit written by another checkout is replayed with
that checkout's file paths. A run reports findings for files it
never linted.
- Lock collision. golangci-lint locks os.TempDir()/golangci-lint.lock
(pkg/commands/run.go, acquireFileLock), which is NOT in the cache
directory, with a 5s timeout. Peers that hold it longer make the
run abort with "parallel golangci-lint is running" - a non-result
that reads as a lint failure. Isolating the cache does not move it.
Point GOLANGCI_LINT_CACHE and TMPDIR at .lint-cache/ under the
checkout root, using the existing $ROOT idiom. TMPDIR is what makes
the lock per-checkout, so the lock keeps serialising the runs that
actually share a cache instead of being disabled. .lint-cache/ is
git-ignored and Docker-ignored, and the cache persists across runs in
a checkout, so caching is not lost.
Reproduced both modes on the unfixed script across 12 copies of this
tree: 10 of 12 concurrent runs void with the lock error, and 11 of 12
sequential runs reported findings at ../w1/... after an identical
lint-failing file was added to every copy. After the fix, 20-way
concurrency gives 0 void and 0 foreign paths, and each copy reports
only its own relative path.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Where the lock lives
golangci-lint v2.12.2 (
pkg/commands/run.go,acquireFileLock):$TMPDIR/golangci-lint.lock, i.e./tmp/golangci-lint.lockwhenTMPDIRis unset. Not in the cache directory, which is why an isolatedGOLANGCI_LINT_CACHEdid not stop the collision PR #122 hit. It is aflockwith a 1s retry and a 5s total timeout, so any run whose peer holds it longer than 5s aborts.Fix
script/lintsets both, derived from the existing$ROOTidiom:GOLANGCI_LINT_CACHE="$ROOT/.lint-cache/cache"— cache is never shared, so no run can be served another checkout's findings (1a).TMPDIR="$ROOT/.lint-cache/tmp"— moves the lock file per checkout (1b). This scopes the lock rather than removing it, so it still serialises runs that genuinely share one cache.--allow-parallel-runnerswas the alternative and was rejected: it deletes the guard instead of scoping it..lint-cache/added to.gitignoreand.dockerignore..golangci.ymluntouched (sha256 still021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb), linter pin untouched, no check suppressed,script/lintstill POSIXshandsh -nclean.Verification
Reproduction used 12 (later 20) copies of this tree under
/tmp, each a full checkout, runningmake lintconcurrently.Unfixed script:
parallel golangci-lint is running.../w1/internal/lintprobe/probe.go— another checkout's path, for a file they did not lint. Onlyw1, which populated the shared cache, reported its own path.Fixed script: 20 concurrent runs, cold caches — 0 void, 0 foreign paths. Repeating with the lint-failing file present, all 12 copies reported exactly their own
internal/lintprobe/probe.goand no../path.Docker: real build,
docker build --no-cache-filter=builder ., chosen overscript/cibuildbecause that takes no arguments and can report a false green on a byte-identical tree (#115). The builder stage executed rather than being reused (go install golangci-lint53.8s,RUN make check51.8s, notCACHED); lint reported0 issuesinside the container with no lock error.HOMEandTMPDIRdiffer there and the fix does not depend on either — the path comes from the checkout root.Timing: warm run ~1.7–2.2s, against ~0.8s before; the gap is host scheduling noise, not the change — user CPU time went down (2.0s vs 2.6s). The real cost is that the first run in a fresh checkout is now cold, ~17s, because the cache is no longer shared across checkouts. That is inherent to the fix. Cache is ~80MB per checkout.
make checkgreen (GOFLAGS=-count=1), tree unmodified afterwards,TODO.mdupdated in the same commit.Anomalies
gomodguarddeprecation warning still appears on every lint run. Pre-existing and already recorded as accepted inTODO.md.watchertests logfailed to save state error="writing temp state file: open /state.json.tmp: permission denied"repeatedly while passing. Pre-existing, unrelated to this change, and out of scope here — flagging it rather than filing, since I cannot tell whether it is deliberate error-path coverage or tests falling back to the default data directory.Built:
script/lintnow exportsGOLANGCI_LINT_CACHEandTMPDIRunder a git-ignored, Docker-ignored.lint-cache/in the checkout root. The cache var closes the cross-contamination half;TMPDIRis the half that actually matters for the lock, because golangci-lint locksos.TempDir()/golangci-lint.lock, not anything under the cache directory.Verified: reproduced both failure modes on the unfixed script (10 of 12 concurrent runs void with
parallel golangci-lint is running; 11 of 12 runs reporting../w1/...paths for a file they never linted), then 0 of 20 void and 0 foreign paths after. Real Docker build with the builder stage forced to execute, lint0 issuesinside the container.make checkgreen withGOFLAGS=-count=1, tree unmodified after. Full detail and the timing numbers are in the PR description.Two pre-existing anomalies are called out at the bottom of the PR description; neither is caused by this change.
Review: PASS
Independent review of
520ce79. No blocking findings.Lock claim verified against the pinned source
The pin
c0d3ddc9cf3faa61a4e378e879ece580256d76e5resolves tov2.12.2(@v/v2.12.2.info,Origin.Hash), and that source says exactly what the PR says:pkg/commands/run.go:492lockFile := filepath.Join(os.TempDir(), "golangci-lint.lock"),flock,retryDelay = time.Second(:495),totalTimeout = 5 * time.Second(:499). Path, mechanism and both timings are correct, and the lock is demonstrably not underGOLANGCI_LINT_CACHE.Concurrency test (my own, 12 fresh copies of this tree)
parallel golangci-lint is runninglllviolation in each copy../u4/internal/lintprobe/probe.go— another copy's pathinternal/lintprobe/probe.go; 0 lock errors, 0 foreign pathsrc=0,0 issues., no lock error, no foreign path, while 9/12 unfixed peers went voidThe reproduction lands before the fix and disappears after it, including with the shared
/tmplock actively thrashed by peers. Both halves of the amended definition of done are met.Timings here: cold 8.1s, warm 1.3s, cache 80MB — consistent with the PR's numbers.
Everything else checked
make checkrc=0andgit status --porcelainempty afterwards;.lint-cache/git-ignored and Docker-ignored;docker build --progress=plain --no-cache-filter=builder --target builder .(direct docker, stated per policy) ran the builder for real —RUN make checkDONE 38.6s, notCACHED, lint0 issuesinside the container withTMPDIR=/src/.lint-cache/tmp;.golangci.ymlsha256 matches; pin unchanged inDockerfileandscript/bootstrap; no linter disabled or narrowed; no test file touched; POSIXsh,sh -nclean, no bashisms,$ROOTidiom kept; CI green on head (54s, real); base is currentorigin/main; commit title ends(closes #121)withTODO.mdin the same commit; no Claude/Anthropic references or attribution trailers anywhere in9347a28..520ce79.Non-blocking findings
1. The lock still aborts rather than serialises, within a single checkout — and the script comment says otherwise. Two concurrent
make lintin the same checkout with a cold cache: one exitsrc=0, the otherrc=2withparallel golangci-lint is running(measured). Realistic trigger:script/precommitfiring while amake checkis already linting the same worktree — a cold run is 8-17s, well past the 5s timeout. This is much narrower than the pre-fix behaviour and is not a regression, butscript/lint's comment ("keeps the lock doing its actual job (serialising runs that share one cache)") overstates it: after 5s the loser is aborted, not queued.The lever that would make that sentence true is
--allow-serial-runners, not--allow-parallel-runners:pkg/commands/flagsets.go:59bindsrun.allow-serial-runners, andrun.go:498uses it to skip the 5scontext.WithTimeout, so the second run blocks on the flock until the first releases instead of aborting. It keeps the guard, unlike--allow-parallel-runners. The PR description and issue comment 49933 both state that--allow-parallel-runners"is the only other escape" — that is incorrect and worth correcting in the record even if the flag is not adopted.2.
make cleandoes not remove.lint-cache/.Makefile:43isrm -rf bin/. 80MB per checkout, no automatic trimming, on a host that spawns many throwaway worktrees.rm -rf bin/ .lint-cache/would close it.3. Naming nit:
.lint-cache/cachestutters, and the directory holdstmpas well as a cache. Something like.lint-state/{cache,tmp}reads better. Not worth a respin on its own.Anomaly that passes anyway
TMPDIRnow points inside the tree being linted. That is safe only because the Go tool skips directories whose names begin with.when expanding./...— confirmed: no run reported anything under.lint-cache/. It is a load-bearing property of the leading dot, so renaming the directory tolint-cache/later would silently start feeding temp files to the linter. Worth a word in the comment if anyone touches the name.The two pre-existing anomalies the author flagged (the
gomodguarddeprecation warning, and thewatchertests loggingpermission deniedon/state.json.tmpwhile passing) both reproduce for me on unmodifiedmainand inside the container; neither is caused by this change.[manager] Independent review PASS, no blocking findings.
merge-ready, assigned to @sneak.Lock path independently confirmed against the pinned source (
run.go:492, flock, 1s retry, 5s timeout). Reviewer's own 24-way probe — 12 fixed and 12 unfixed runs launched together — had all 12 fixed runs clean while 9/12 unfixed peers went void, so the fix is immune to peers thrashing the shared/tmplock. Constraints all verified clean.One correction to record, because I propagated the error upstream. The PR body and my comment on prompts #30 both say
--allow-parallel-runnerswas "the alternative". That is wrong:--allow-serial-runnersalso exists (flagsets.go:59, consumed atrun.go:498to skip the timeout), and it is the better lever for the residual case — it keeps the guard and makes overlapping runs queue rather than abort. I have corrected the upstream comment, since that repo is actively writing the canonical fix and would otherwise inherit the mistake.Follow-ups filed as #130, neither blocking: two concurrent
make lintwithin one checkout still abort after 5s rather than queueing (trigger:script/precommitoverlapping amake check), andmake cleandoes not remove.lint-cache/, which accumulates ~80MB per throwaway worktree unbounded.Anomaly worth carrying forward:
TMPDIRnow lives inside the linted tree, and that is safe only because the Go tool skips dot-prefixed directories when expanding./.... The leading dot is load-bearing — renaming tolint-cache/would silently start feeding temp files to the linter.clawbot referenced this pull request2026-09-03 23:32:33 +02:00
Pull request closed