sneak/prompts
1
0
Fork 0
Code Issues 18 Pull Requests 4 Actions Packages Projects Releases Wiki Activity

Give golangci-lint per-checkout cache and lock state (closes #30)
All checks were successful
check / check (push) Successful in 7s
Details

Browse Source 
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.
This commit is contained in:
clawbot
2026-08-09 17:35:48 +00:00
parent 3a218497b8
commit 6ddf46e894
4 changed files with 278 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

25
TODO.md
View File

@@ -21,6 +21,31 @@ fmt-check, and commit.
# Completed Steps
- 2026-08-09: Made a golangci-lint result belong to the tree that asked for it.
REPO_POLICIES.md now carries the canonical Go `script/lint`, which gives the
linter per-checkout `GOLANGCI_LINT_CACHE` and per-checkout `TMPDIR`. The two
are separate defects and the second is the one that gets dropped: the result
cache is keyed on file content rather than location, so checkouts holding
identical files serve each other's findings under the other's path, while the
concurrency lock is `$TMPDIR/golangci-lint.lock` — host-global, independent of
the cache, and unaffected by isolating it. Moving workers from worktrees to
their own clones does not help either half; it only removes the foreign-path
artefact that made the defect visible. The lock error is retried rather than
surfaced, because it is not a result: it exits non-zero exactly as findings
do, and reporting it as findings sends a correct branch back for rework.
Detection is on the stderr stream and never on exit status, so a finding
quoting the lock message in source cannot be retried away, and exhaustion
exits 75 with a VOID message rather than passing or failing quietly.
`--allow-serial-runners` (which keeps the guard and queues) covers the
same-checkout overlap that `TMPDIR` scoping cannot; `--allow-parallel-runners`
is rejected outright. Both checklists gained the corresponding items, since a
half-fix that sets only the cache reads as complete. `GOCACHE` was measured
and does not need isolating. Verified with the snippet extracted from the
committed document and executed as a consuming repo would adopt it, against
paired controls: contamination reproduced on the pre-fix form and absent on
the adopted one, retry engaged, exhaustion loud, a genuine finding still
reported, and a held host lock failing the pre-fix script while leaving the
adopted one untouched.
- 2026-08-09: Kept in-repo agent scratch out of the Docker build context and out
of version control. `.claude/` holds one worktree — an entire additional
checkout of the repo — per in-flight agent, and under `COPY . .` all of it was

7
prompts/EXISTING_REPO_CHECKLIST.md
View File

@@ -100,6 +100,13 @@ with your task.
`script/install-precommit`, shimmed by `make hooks`) runs it
- [ ] README has an **Entrypoints** section documenting the `script/`
entrypoints and linking the standard
- [ ] Go: `script/lint` isolates golangci-lint per checkout —
`GOLANGCI_LINT_CACHE` and `TMPDIR` both exported into `.lint-cache/`
(which is in `.gitignore` and `.dockerignore`), `--allow-serial-runners`
passed, and the lock error retried rather than reported as findings. Copy
the canonical block from `REPO_POLICIES.md`. Setting only the cache is the
common half-fix and leaves `parallel golangci-lint is running` failing
runs red.
- [ ] `make check` does not modify any files in the repo
- [ ] `make test` has a 30-second timeout
- [ ] `make test` runs real tests, not a no-op (at minimum, import/compile

11
prompts/NEW_REPO_CHECKLIST.md
View File

@@ -109,6 +109,17 @@ are thin shims calling them. Model scripts:
- [ ] `script/test` / `make test` — runs real tests, not a no-op (30-second
timeout)
- [ ] `script/lint` / `make lint` — runs linter
- [ ] Go: exports `GOLANGCI_LINT_CACHE` **and** `TMPDIR` into a
`.lint-cache/` directory inside the checkout, above any
container-versus-host branch so every path that reaches the linter
gets them; passes `--allow-serial-runners` (never
`--allow-parallel-runners`); retries on
`parallel golangci-lint is running` detected on **stderr** and exits
75 with a VOID message on exhaustion. Copy the canonical block from
`REPO_POLICIES.md` rather than writing your own: a version that sets
only the cache leaves the false-red half live, and one that detects
the collision by exit status can retry a real finding away.
- [ ] Go: `.lint-cache/` is in both `.gitignore` and `.dockerignore`
- [ ] `script/fmt` / `make fmt` — formats code (writes)
- [ ] `script/fmt-check` / `make fmt-check` — checks formatting (read-only)
- [ ] `script/check` / `make check` — runs `test`, `lint`, `fmt-check`; must not

235
prompts/REPO_POLICIES.md
View File

@@ -691,6 +691,241 @@ style conventions are in separate documents:
missing call site passes every control while the adopted snippet does
nothing.
- **`script/lint` in Go repos must give golangci-lint per-checkout cache and
lock state, and must never report a lock collision as a lint result.**
golangci-lint shares two pieces of state across every process on the host, and
they are separate mechanisms with separate fixes. Isolating one and stopping
leaves the other fully live while reading as a fix. This is independent of the
pinned-install rule above and does not replace it: that one makes the host run
the right linter, this one makes the run's result belong to your own tree.
**Mechanism 1, the result cache — produces false greens as well as false
reds.** golangci-lint keys cached results 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 — reported at the
_other_ tree's path. Observed across the org: 399 issues attributed to a
`/tmp` worktree that no longer existed, returned from a clean clone that
genuinely lints 0 issues; ten findings against a deleted worktree; findings
reported against `../wt82-lint/...`; and, in the dangerous direction, an
implementer reporting "lint 0 issues" on a branch that was genuinely red.
Note what content-keying implies: **moving agents from worktrees to their
own clones does not help.** Two clones of a repo are byte-identical exactly
as two worktrees were. What own-clones removes is the deleted-worktree path
artefact — the loud, obviously wrong symptom — while leaving the mechanism
live, which makes the defect quieter rather than rarer.
**Mechanism 2, the concurrency lock — and it does not live in the cache
directory.** From `pkg/commands/run.go`, `acquireFileLock()`:
```go
lockFile := filepath.Join(os.TempDir(), "golangci-lint.lock")
```
That is `$TMPDIR/golangci-lint.lock` — host-global, keyed on the temp
directory, entirely independent of `GOLANGCI_LINT_CACHE`. It is an `flock`
retried every second under a **5-second total timeout**, so it fails
precisely when the host is busiest. On failure the run emits
`parallel golangci-lint is running` and analyzes nothing. **A private cache
directory does not prevent this**; that was established by controlled test,
with two concurrent runs under separate cache directories sharing no mounted
path, one of which still collided. Anyone who sets only
`GOLANGCI_LINT_CACHE` has closed the contamination half and left the
false-red half untouched.
The canonical form for a Go repo's `script/lint`:
```sh
#!/bin/sh
# script/lint: run the linter.
set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
# Per-checkout golangci-lint state. Both variables are required and they
# fix different defects; neither is redundant with the other.
#
# GOLANGCI_LINT_CACHE: the result cache is keyed on file content, not
# location, so checkouts holding identical files serve each other's
# findings. A per-REPO cache directory does NOT fix this — every checkout
# of that repo still collides — so the path must be inside the invoking
# checkout.
#
# TMPDIR: golangci-lint flocks $TMPDIR/golangci-lint.lock
# (pkg/commands/run.go, acquireFileLock: filepath.Join(os.TempDir(),
# "golangci-lint.lock")). That lock is host-global and independent of
# GOLANGCI_LINT_CACHE, with a 5s acquire timeout. Scoping TMPDIR into the
# checkout is the only thing here that isolates it. Do not delete this as
# redundant with the cache variable; it is not.
#
# The leading dot in .lint-cache is load-bearing: the go tool skips
# dot-prefixed directories when expanding ./..., so the linter never reads
# its own cache and temp files back as source. Do not rename it.
LINT_STATE="$ROOT/.lint-cache"
GOLANGCI_LINT_CACHE="$LINT_STATE/cache"
TMPDIR="$LINT_STATE/tmp"
export GOLANGCI_LINT_CACHE TMPDIR
mkdir -p "$GOLANGCI_LINT_CACHE" "$TMPDIR"
# Backstop for a caller that reached the linter without the environment
# above. With it set, this should never fire.
LINT_MAX_ATTEMPTS=5
# EX_TEMPFAIL. Distinct from 1 (findings) and 3 (linter error) so a void
# run is never counted as either.
LINT_VOID_EXIT=75
golangci_lint_run() {
lint_out="$LINT_STATE/run.stdout"
lint_err="$LINT_STATE/run.stderr"
attempt=1
delay=2
while :; do
rc=0
# --allow-serial-runners KEEPS the mutual-exclusion guard and makes
# an overlapping run queue on the lock instead of aborting after
# 5s. It is NOT --allow-parallel-runners, which removes the guard
# entirely; never use that one. This is what covers two runs inside
# the SAME checkout, which TMPDIR scoping cannot — script/precommit
# overlapping a make check is the realistic trigger.
golangci-lint run --allow-serial-runners "$@" \
>"$lint_out" 2>"$lint_err" || rc=$?
# Detect the lock collision on the STDERR STREAM, never on the exit
# status. Findings are written to stdout and golangci-lint reports
# this failure only on stderr, so a finding that quotes the string
# from source cannot be mistaken for a collision and retried away —
# that direction would be a false green. The exit status is not a
# usable discriminator: the collision exits 3 (exitcodes.Failure)
# while findings exit 1, and field reports of 2 mean the value is
# not stable across versions.
if ! grep -q 'parallel golangci-lint is running' "$lint_err"; then
cat "$lint_err" >&2
cat "$lint_out"
return "$rc"
fi
if [ "$attempt" -ge "$LINT_MAX_ATTEMPTS" ]; then
cat "$lint_err" >&2
echo "lint: VOID after $LINT_MAX_ATTEMPTS attempts:" \
"golangci-lint never acquired its lock, so nothing was" \
"analyzed. This is NOT a lint result and no verdict may" \
"be recorded from it. Re-run it." >&2
return "$LINT_VOID_EXIT"
fi
echo "lint: lock held by another golangci-lint; attempt" \
"$attempt of $LINT_MAX_ATTEMPTS, retrying in ${delay}s" >&2
sleep "$delay"
attempt=$((attempt + 1))
delay=$((delay * 2))
done
}
main() {
cd "$ROOT"
golangci_lint_run ./...
}
main "$@"
```
Load-bearing properties, each guarding a mode that otherwise reports a
verdict it did not earn:
- **Both variables, per checkout.** Cache alone leaves the false reds;
`TMPDIR` alone leaves the contamination that produced a confirmed false
green. A per-repo path for either is not isolation on a host where every
worker holds its own copy of the same repo.
- **Set them on every path that reaches the linter.** The export block goes
_above_ any container-versus-host branch, and a native escape hatch must
call `golangci_lint_run` rather than `exec golangci-lint` directly. One
repo in the org had exactly one such path with no cache environment at
all, inheriting the fleet-wide default, so the fix on the other path was
worth nothing there.
- **The lock collision is not a result, and must never be reported as one.**
Retry it, and on exhaustion exit a status that is neither the findings
status nor success, with a message that says VOID. Swallowing it into a
success is the worst available outcome; reporting it as findings sends a
correct branch back for rework against findings that do not exist.
- **Detect the collision by the message on stderr, not by exit status, and
never retry a genuine finding.** Distinguishing "exited non-zero because
of the lock" from "exited non-zero because of findings" is the whole crux,
and getting it wrong in the direction of treating findings as a lock error
retries a real failure into a void — or, if a later implementation decided
to treat exhaustion as success, into a green.
- **`--allow-serial-runners`, never `--allow-parallel-runners`.** The first
keeps the guard and queues; the second deletes it and lets two runs
corrupt shared state. With the flag set, an overlapping run inside the
same checkout waits rather than failing, which is what is actually wanted.
The honest cost is that it waits without bound, so a stale process holding
the lock hangs the run instead of failing it; the contending set is
bounded to the same checkout, and an eventual result is preferable to a
fabricated one.
Adopting repos must add `.lint-cache/` to both `.gitignore` and
`.dockerignore`. The second matters as much as the first: the directory
reaches tens of megabytes, and without the exclusion it enters the build
context and invalidates `COPY . .` for reasons unrelated to the repo's
content.
**`GOCACHE` does not need isolating, and this was measured rather than
assumed.** With `GOLANGCI_LINT_CACHE` and `TMPDIR` per checkout and
`GOCACHE` left at the host default and shared, two checkouts of identical
content each reported their own paths and neither reported the other's. The
Go build cache is content-addressed and its entries are compiled artifacts
rather than diagnostics carrying a foreign tree's paths, and it has no
equivalent global lock — the whole fleet compiles concurrently against one
`GOCACHE` all day without a contention error. Isolating it would cost a full
cold compile per checkout for no measured benefit. The earlier hypothesis
that Go build-cache contention might explain the lock error is superseded:
the lock is located in source at `$TMPDIR/golangci-lint.lock`.
**Verifying a change to this logic requires a negative control, and the
control must be built out of checkouts with identical content.** Create two
checkouts of the same tree containing a deliberate lint finding, run the
linter in the second so it populates the cache, then run it in the first and
confirm the finding is reported at the first checkout's own path and never
at the second's. Run the same control against the unisolated form and
confirm the contamination appears there — a control that passes against the
broken implementation proves nothing. Note specifically that a control built
from checkouts whose **content differs** passes against the unisolated form
too, because differing content does not collide in a content-keyed cache, so
it is not a test of anything. For the lock half, hold
`$TMPDIR/golangci-lint.lock` with `flock` and confirm the run queues rather
than aborting, that a caller never sees the collision as findings, and that
exhaustion fails loudly and distinguishably.
**Run those controls against the block as a consuming repo would adopt it**
— pasted into a `script/lint`-shaped file that is then executed, not sourced
with the functions driven by hand. The same warning as for the bootstrap
block above, for the same reason.
- **Interim rule for reading a golangci-lint result on a shared host, until
every repo has adopted the isolation above.** A lint run is **VOID** unless
both hold:
- the output contains no `parallel golangci-lint is running`, and
- no reported file path begins with `../`, and none is an absolute path
outside the tree the run was launched from.
Do not record a verdict from a void run, and do not "fix" findings in files
the change does not touch — chasing phantom findings across untouched files
puts unrelated edits into a reviewed diff, which is more expensive than the
wasted rework.
The `../` clause is the one that actually bites, and it is why a filter
keyed on `/tmp` or on absolute prefixes is not enough: golangci-lint reports
paths relative to its own resolved root rather than yours, and three of the
org's reported sightings had relative paths and would have passed such a
filter. Both clauses are needed and neither alone is sufficient — one
reproduction exited non-zero with the lock error and no foreign paths at
all, and another reported 34 well-formed findings, every one of them against
another checkout.
**State the limit of these tests rather than treating them as a guarantee.**
They catch contamination that **names** foreign files. They cannot catch
contamination that **suppresses** findings through a poisoned entry for
colliding content, which has no wall-clock tell either. They are a filter
for the loud mode, not a proof of soundness — which is the whole argument
for fixing this in the tooling instead of documenting a discipline that
depends on every agent remembering to apply it.
- When pinning images or packages by hash, add a comment above the reference
with the version and date (YYYY-MM-DD).