golangci-lint cache is shared across worktrees, so lint results can come from another tree #88

Open
opened 2026-08-09 08:04:05 +02:00 by clawbot · 3 comments
Collaborator

golangci-lint keys its cache by a directory that vaultik does not
isolate per worktree, and it takes a lock on that cache. With multiple
agents linting concurrently from separate throwaway worktrees, two things
can happen:

  • Unearned red: cached findings from a different worktree are served
    as this tree's results — reported elsewhere in the fleet as 399 issues
    attributed to files under a /tmp worktree that no longer existed, on a
    tree that genuinely lints clean.
  • Void run: Error: parallel golangci-lint is running, a non-zero
    exit that is not a lint result at all. An agent treating non-zero as
    "lint failed" sends a correct PR back for rework.

This is the mirror image of #85 and #78. Those manufacture unearned
greens; this manufactures unearned greens and unearned reds — and
the red is arguably worse, because it fails a correct change against
findings from a different codebase, and the "evidence" looks concrete.

vaultik's actual exposure

Better than baseline, but not closed. script/lint (as landed in #78)
has two paths:

1. Docker path — partially isolated. cache_root() is
${XDG_CACHE_HOME:-~/.cache}/vaultik-lint and the container gets
GOLANGCI_LINT_CACHE=/cache/golangci-lint. That isolates vaultik from
other repos, which is why we have not seen cross-repo contamination
here. But it is one cache directory shared by every vaultik worktree.
Concurrent agents in /tmp/impl-84, /tmp/review-83, etc. all write the
same cache and contend for the same lock. Cross-worktree contamination
and the parallel-run error are both live.

2. Native escape hatch — not isolated at all. script/lint:119-121:

if [ "$(installed_version)" = "$(pinned_version)" ]; then
    exec golangci-lint run "$@" ./...
fi

No cache environment is set, so this uses the default
~/.cache/golangci-lint — shared with every other session and repo on
the host. This path exists for the in-container run (correct and isolated
there), but it also triggers on any host with a matching golangci-lint
on PATH. That is precisely the fleet-wide exposure.

Note this is the same defect shape as #80: the hatch is gated on version
equality rather than execution context. Cache isolation is part of that
context. #80 and this issue should probably be fixed together — a
context gate closes both.

Definition of done

  1. GOLANGCI_LINT_CACHE is per worktree, not per repo, on the Docker
    path. Derive it from something that distinguishes worktrees (the
    absolute worktree path hashed, or the git common-dir plus checkout
    path) so two concurrent worktrees cannot collide.
  2. The native path sets the same isolated cache rather than inheriting
    the default — or is removed in favour of a context gate per #80, which
    would make the question moot. State which was chosen.
  3. Caches remain persistent per worktree so repeat runs stay fast.
    Do not fix this by disabling the cache; report warm-run wall time
    before and after to show the inner loop is not destroyed.
  4. Stale worktree caches do not accumulate without bound. Either place
    them where normal cleanup reaches them, or document the growth and how
    to clear it.
  5. Demonstrate the fix against the failure mode: run two lint
    invocations concurrently from two different worktrees of this repo and
    confirm neither reports paths from the other and neither dies with
    parallel golangci-lint is running. Paste the evidence in the PR.
  6. Match the canonical fix landing upstream (sneak/prompts #30, a
    per-worktree GOLANGCI_LINT_CACHE in canonical script/lint) rather
    than inventing a local variant.
  7. .golangci.yml unchanged (sha256
    021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb);
    the Dockerfile FROM line stays the single source of truth for the
    linter version (#78).

Interim rule — applies to every lint run in this repo now

Treat a lint run as VOID, not as a result, unless both hold:

  1. the output does not contain parallel golangci-lint is running;
    and
  2. it mentions no file paths outside the worktree it was launched
    from
    .

Retry until both hold. Never record a verdict — pass or fail — from a
void run. A red that cites paths from another tree is not a finding.

`golangci-lint` keys its cache by a directory that vaultik does not isolate per worktree, and it takes a lock on that cache. With multiple agents linting concurrently from separate throwaway worktrees, two things can happen: - **Unearned red**: cached findings from a *different* worktree are served as this tree's results — reported elsewhere in the fleet as 399 issues attributed to files under a `/tmp` worktree that no longer existed, on a tree that genuinely lints clean. - **Void run**: `Error: parallel golangci-lint is running`, a non-zero exit that is not a lint result at all. An agent treating non-zero as "lint failed" sends a correct PR back for rework. This is the mirror image of #85 and #78. Those manufacture unearned *greens*; this manufactures unearned greens **and** unearned reds — and the red is arguably worse, because it fails a correct change against findings from a different codebase, and the "evidence" looks concrete. ## vaultik's actual exposure Better than baseline, but not closed. `script/lint` (as landed in #78) has two paths: **1. Docker path — partially isolated.** `cache_root()` is `${XDG_CACHE_HOME:-~/.cache}/vaultik-lint` and the container gets `GOLANGCI_LINT_CACHE=/cache/golangci-lint`. That isolates vaultik from *other repos*, which is why we have not seen cross-repo contamination here. But it is **one cache directory shared by every vaultik worktree**. Concurrent agents in `/tmp/impl-84`, `/tmp/review-83`, etc. all write the same cache and contend for the same lock. Cross-*worktree* contamination and the parallel-run error are both live. **2. Native escape hatch — not isolated at all.** `script/lint:119-121`: ```sh if [ "$(installed_version)" = "$(pinned_version)" ]; then exec golangci-lint run "$@" ./... fi ``` No cache environment is set, so this uses the default `~/.cache/golangci-lint` — shared with every other session and repo on the host. This path exists for the in-container run (correct and isolated there), but it also triggers on any host with a matching `golangci-lint` on `PATH`. That is precisely the fleet-wide exposure. Note this is the same defect shape as #80: the hatch is gated on version equality rather than execution context. Cache isolation is part of that context. **#80 and this issue should probably be fixed together** — a context gate closes both. ## Definition of done 1. `GOLANGCI_LINT_CACHE` is **per worktree**, not per repo, on the Docker path. Derive it from something that distinguishes worktrees (the absolute worktree path hashed, or the git common-dir plus checkout path) so two concurrent worktrees cannot collide. 2. The native path sets the same isolated cache rather than inheriting the default — or is removed in favour of a context gate per #80, which would make the question moot. State which was chosen. 3. Caches remain **persistent per worktree** so repeat runs stay fast. Do not fix this by disabling the cache; report warm-run wall time before and after to show the inner loop is not destroyed. 4. Stale worktree caches do not accumulate without bound. Either place them where normal cleanup reaches them, or document the growth and how to clear it. 5. **Demonstrate the fix against the failure mode**: run two lint invocations concurrently from two different worktrees of this repo and confirm neither reports paths from the other and neither dies with `parallel golangci-lint is running`. Paste the evidence in the PR. 6. Match the canonical fix landing upstream (`sneak/prompts` #30, a per-worktree `GOLANGCI_LINT_CACHE` in canonical `script/lint`) rather than inventing a local variant. 7. `.golangci.yml` unchanged (sha256 `021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb`); the `Dockerfile` `FROM` line stays the single source of truth for the linter version (#78). ## Interim rule — applies to every lint run in this repo now Treat a lint run as **VOID**, not as a result, unless both hold: 1. the output does **not** contain `parallel golangci-lint is running`; and 2. it mentions **no file paths outside the worktree it was launched from**. Retry until both hold. Never record a verdict — pass or fail — from a void run. A red that cites paths from another tree is not a finding.
clawbot added this to the 1.0.0 milestone 2026-08-09 08:04:05 +02:00
Author
Collaborator

Reproduced in this repo, and the void rule then resolved it. Recording
both halves as concrete evidence.

The void run. make lint on main at 3bcdbcf — a tree with four
merged PRs that I had already verified clean — while an implementation
agent was linting concurrently from /tmp/impl-85:

EXIT=2
"parallel golangci-lint is running": 2 occurrences
foreign /tmp paths: none
result line: none — no findings emitted at all

Exit 2, no findings, no result line. An agent applying a naive "non-zero
means lint failed" rule would have reported a regression on a tree that
is clean. Note this run produced no findings, so it is the lock half of
the defect rather than the contamination half; the contamination half
(another worktree's cached findings served as your own) has been seen
elsewhere in the fleet but not yet here, most likely because the
vaultik-lint cache directory isolates this repo from other repos.

The retry. Applying the interim rule — retry until the output has no
parallel-run error and cites no paths outside the launching worktree:

attempt 1: VOID (parallel lock)
attempt 2: VALID exit=0
EXIT=0
foreign /tmp paths: (none)
0 issues.

Second attempt satisfied both conditions. main genuinely lints clean.
The rule works and is cheap — one retry — so there is no excuse for
recording a verdict from a void run.

Practical consequence for this repo right now: collisions are not
theoretical here, they are routine. This session runs implementation and
review agents concurrently, each in its own /tmp worktree, and each runs
lint. The very first make lint I attempted collided. Until this is
fixed, every lint result in this repo — mine, an implementer's, or a
reviewer's — must be checked against the two conditions before it is
believed in either direction.

Bearing on already-merged work. PRs #77, #79, #83, and #86 were each
merged on a recorded 0 issues. A contaminated red would have shown
findings, so it cannot explain a 0 issues. result. A contaminated
green would require another vaultik worktree's clean cache being served
for a tree that was actually dirty — possible in principle. Against that:
each of those merges also passed ok:14 cached:0 on the test side, which
the lint cache cannot fake, and #83 additionally had a reviewer who
reverted each fix and confirmed the corresponding test failed. The
re-verification above closes it out: main, which is the merged product
of all four, lints clean in a run that satisfies both conditions. No
re-review is warranted.

Reproduced in this repo, and the void rule then resolved it. Recording both halves as concrete evidence. **The void run.** `make lint` on `main` at `3bcdbcf` — a tree with four merged PRs that I had already verified clean — while an implementation agent was linting concurrently from `/tmp/impl-85`: ``` EXIT=2 "parallel golangci-lint is running": 2 occurrences foreign /tmp paths: none result line: none — no findings emitted at all ``` Exit 2, no findings, no result line. An agent applying a naive "non-zero means lint failed" rule would have reported a regression on a tree that is clean. Note this run produced *no* findings, so it is the lock half of the defect rather than the contamination half; the contamination half (another worktree's cached findings served as your own) has been seen elsewhere in the fleet but not yet here, most likely because the `vaultik-lint` cache directory isolates this repo from other repos. **The retry.** Applying the interim rule — retry until the output has no parallel-run error and cites no paths outside the launching worktree: ``` attempt 1: VOID (parallel lock) attempt 2: VALID exit=0 EXIT=0 foreign /tmp paths: (none) 0 issues. ``` Second attempt satisfied both conditions. `main` genuinely lints clean. The rule works and is cheap — one retry — so there is no excuse for recording a verdict from a void run. **Practical consequence for this repo right now:** collisions are not theoretical here, they are routine. This session runs implementation and review agents concurrently, each in its own `/tmp` worktree, and each runs lint. The very first `make lint` I attempted collided. Until this is fixed, every lint result in this repo — mine, an implementer's, or a reviewer's — must be checked against the two conditions before it is believed in either direction. **Bearing on already-merged work.** PRs #77, #79, #83, and #86 were each merged on a recorded `0 issues.` A contaminated *red* would have shown findings, so it cannot explain a `0 issues.` result. A contaminated *green* would require another vaultik worktree's clean cache being served for a tree that was actually dirty — possible in principle. Against that: each of those merges also passed `ok:14 cached:0` on the test side, which the lint cache cannot fake, and #83 additionally had a reviewer who reverted each fix and confirmed the corresponding test failed. The re-verification above closes it out: `main`, which is the merged product of all four, lints clean in a run that satisfies both conditions. No re-review is warranted.
Author
Collaborator

Scope refinement, from evidence gathered while working #85.

The containerised lint path is structurally immune to this defect,
not merely isolated by convention. script/cibuild runs make lint
inside the pinned image during the Docker build, so the linter's cache
is built from the pinned base within that build's own filesystem. It
never touches the host cache directory and never contends for the host
lock. Seven script/cibuild runs during #85 were audited against both
void conditions and all were clean, while a concurrent host-side
make lint on main collided in the same window.

So the exposure is narrower than this issue's original framing:

path exposed? why
script/cibuild (containerised lint) no cache lives inside the build, from the pinned image
host make lint → Docker path yes ~/.cache/vaultik-lint shared by every vaultik worktree
host make lint → native escape hatch yes no cache env set; inherits the fully-shared ~/.cache/golangci-lint

Two consequences worth acting on:

  1. The fix is narrower than feared. Only the host-side paths need
    per-worktree keying. The container path should be left alone — adding
    a host cache mount to it would introduce the exposure it currently
    does not have.
  2. There is a reliable escape valve today. A verdict backed by
    script/cibuild is not subject to this defect at all. Where a
    verdict matters and host make lint keeps coming back void, running
    the containerised gate is a sound alternative rather than merely a
    slower one. Note this only became true once #85's fix lands — before
    it, script/cibuild had its own false-green mode, so the two defects
    were covering for each other in opposite directions.

The interim VOID rule stands unchanged for every host-side lint run.

Scope refinement, from evidence gathered while working #85. The containerised lint path is **structurally immune** to this defect, not merely isolated by convention. `script/cibuild` runs `make lint` *inside* the pinned image during the Docker build, so the linter's cache is built from the pinned base within that build's own filesystem. It never touches the host cache directory and never contends for the host lock. Seven `script/cibuild` runs during #85 were audited against both void conditions and all were clean, while a concurrent host-side `make lint` on `main` collided in the same window. So the exposure is narrower than this issue's original framing: | path | exposed? | why | | --- | --- | --- | | `script/cibuild` (containerised lint) | **no** | cache lives inside the build, from the pinned image | | host `make lint` → Docker path | **yes** | `~/.cache/vaultik-lint` shared by every vaultik worktree | | host `make lint` → native escape hatch | **yes** | no cache env set; inherits the fully-shared `~/.cache/golangci-lint` | Two consequences worth acting on: 1. **The fix is narrower than feared.** Only the host-side paths need per-worktree keying. The container path should be left alone — adding a host cache mount to it would *introduce* the exposure it currently does not have. 2. **There is a reliable escape valve today.** A verdict backed by `script/cibuild` is not subject to this defect at all. Where a verdict matters and host `make lint` keeps coming back void, running the containerised gate is a sound alternative rather than merely a slower one. Note this only became true once #85's fix lands — before it, `script/cibuild` had its own false-green mode, so the two defects were covering for each other in opposite directions. The interim VOID rule stands unchanged for every host-side lint run.
Author
Collaborator

Experimental data on where the lock actually lives — this bears on the
proposed fix, and it contradicts one of the two accounts circulating.

The experiment

Two make lint runs launched concurrently, from two different
worktrees
of this repo, each with a completely separate cache
directory
:

  • A: cwd=/srv/code/vaultik, XDG_CACHE_HOME=/tmp/cacheA
  • B: cwd=/tmp/lockexp (detached origin/main),
    XDG_CACHE_HOME=/tmp/cacheB

Both take the Docker path (host has 2.10.1, pin is 2.12.2), so each
container mounts only its own source root and its own cache dir. The
two containers share no mounted path.

Result

A_EXIT=2   "parallel golangci-lint is running": 2 occurrences
B_EXIT=0   "parallel golangci-lint is running": 0    → 0 issues.

A's tail:

Error: parallel golangci-lint is running
The command is terminated due to an error: parallel golangci-lint is running
make: *** [Makefile:44: lint] Error 3

Separate GOLANGCI_LINT_CACHE directories did not prevent the
collision.
A per-worktree cache key alone therefore does not fix the
lock-contention half of this issue — it addresses the contamination
half (one tree's cached findings served as another's) but not the
parallel golangci-lint is running half.

Caveat, stated because it matters

I cannot prove A collided with B specifically. A third host-side lint
was plausibly running concurrently (a reviewer working PR #89 also runs
GOFLAGS=-count=1 make check, which invokes host make lint under the
default XDG_CACHE_HOME). So the honest claim is the weaker one, which is
still sufficient:

With a private cache directory, a lint run still collided with some
other concurrent lint on this host.

If the lock were scoped to the cache directory, a private cache would
have made A immune to every other run regardless of which one it hit.
It did not. That is the load-bearing conclusion.

Consequence for the fix

Definition-of-done item 1 as written — per-worktree GOLANGCI_LINT_CACHE
— is necessary but not sufficient. It should be extended to either:

  1. serialise host-side lint runs explicitly (a repo-level lock, so
    collisions queue instead of erroring), or
  2. make script/lint retry on the parallel-run error rather than
    surfacing it as a lint failure — since that error is not a result, and
    an exit code of 2 from it is indistinguishable, to a caller, from real
    findings.

Option 2 is the smaller change and directly removes the false-red, which
is the damaging direction. It also encodes the interim VOID rule in the
tooling instead of relying on every agent brief to restate it — the
current defence is that every reviewer and implementer remembers to check
two conditions by hand, which will not hold.

Reported upstream to sneak/prompts #30, where the two accounts
disagreed.

Experimental data on where the lock actually lives — this bears on the proposed fix, and it contradicts one of the two accounts circulating. ## The experiment Two `make lint` runs launched concurrently, from **two different worktrees** of this repo, each with a **completely separate cache directory**: - A: `cwd=/srv/code/vaultik`, `XDG_CACHE_HOME=/tmp/cacheA` - B: `cwd=/tmp/lockexp` (detached `origin/main`), `XDG_CACHE_HOME=/tmp/cacheB` Both take the Docker path (host has 2.10.1, pin is 2.12.2), so each container mounts only its own source root and its own cache dir. **The two containers share no mounted path.** ## Result ``` A_EXIT=2 "parallel golangci-lint is running": 2 occurrences B_EXIT=0 "parallel golangci-lint is running": 0 → 0 issues. ``` A's tail: ``` Error: parallel golangci-lint is running The command is terminated due to an error: parallel golangci-lint is running make: *** [Makefile:44: lint] Error 3 ``` **Separate `GOLANGCI_LINT_CACHE` directories did not prevent the collision.** A per-worktree cache key alone therefore does **not** fix the lock-contention half of this issue — it addresses the *contamination* half (one tree's cached findings served as another's) but not the `parallel golangci-lint is running` half. ## Caveat, stated because it matters I cannot prove A collided with **B specifically**. A third host-side lint was plausibly running concurrently (a reviewer working PR #89 also runs `GOFLAGS=-count=1 make check`, which invokes host `make lint` under the default `XDG_CACHE_HOME`). So the honest claim is the weaker one, which is still sufficient: > With a private cache directory, a lint run still collided with some > other concurrent lint on this host. If the lock were scoped to the cache directory, a private cache would have made A immune to *every* other run regardless of which one it hit. It did not. That is the load-bearing conclusion. ## Consequence for the fix Definition-of-done item 1 as written — per-worktree `GOLANGCI_LINT_CACHE` — is **necessary but not sufficient**. It should be extended to either: 1. serialise host-side lint runs explicitly (a repo-level lock, so collisions queue instead of erroring), or 2. make `script/lint` **retry on the parallel-run error** rather than surfacing it as a lint failure — since that error is not a result, and an exit code of 2 from it is indistinguishable, to a caller, from real findings. Option 2 is the smaller change and directly removes the false-red, which is the damaging direction. It also encodes the interim VOID rule in the tooling instead of relying on every agent brief to restate it — the current defence is that every reviewer and implementer remembers to check two conditions by hand, which will not hold. Reported upstream to `sneak/prompts` #30, where the two accounts disagreed.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/vaultik#88