2 Commits

Author SHA1 Message Date
cc6a5a00e7 Run every lint in a container via Dockerfile.lint (closes #40)
All checks were successful
check / check (push) Successful in 9s
script/lint runs the linter directly when it is already inside a
container and otherwise builds Dockerfile.lint, so the linter never runs
on a developer host. That closes three host-only mechanisms: the result
cache golangci-lint keys on file content rather than location, which
produced a confirmed false green and findings reported against other
checkouts; the host-global $TMPDIR/golangci-lint.lock, which fails a run
in a way no caller can distinguish from findings; and host/container
version skew, which hid thirteen findings on one repo.

Detection is on LINT_IN_CONTAINER=1, set by every Dockerfile, and on
nothing else. The two directions are not symmetric: a false negative
inside a container attempts a nested docker build, finds no daemon and
fails loudly, while a false positive on a host silently lints there,
which is the defect this issue exists to kill. /.dockerenv is therefore
rejected even as a fallback -- measured absent inside BuildKit RUN steps
and present on any host that is itself a container, so it fails in both
directions and one of them is the dangerous one.

Nothing else changes shape. The Dockerfile still runs make check,
script/check still runs test, lint and fmt-check, script/cibuild is
still a single docker build with CHECK_EPOCH and VERSION, and the Go
multistage lint stage and its COPY --from=lint ordering dependency
survive with ENV LINT_IN_CONTAINER=1 added. Dockerfile.lint is the
standalone developer-host path and carries the same CHECK_EPOCH guard,
with the ARG below the dependency layer so only the lint re-runs.

The script/bootstrap golangci-lint install and the per-checkout
GOLANGCI_LINT_CACHE/TMPDIR wrapper are deleted as superseded. Neither
has a caller left. A JS repo's yarn install stays: the rule is that no
lint verdict may come from a host invocation, not that no linter binary
may exist there, and in a repo whose formatter is its linter the
formatter necessarily runs on the host.

golangci-lint config verify is kept, on measurement. Under the pinned
v2.12.2 a bogus top-level key and a bogus key under linters.settings.lll
both pass `golangci-lint run` with exit 0 and `0 issues` while config
verify exits 3 and names them; an unknown linter name fails run and
passes config verify. It needs no network: every case reproduced
byte-identically under `docker run --network none`, in a container where
`getent hosts golangci-lint.run` exits 2.

Comment blocks were cut hard across every file this unit touches.
.dockerignore drops from 67 comment lines to 28, script/cibuild from 17
to 12, script/docker from 18 to 12, and prompts/REPO_POLICIES.md from
1182 lines to 907. What remains says why a line is load-bearing; the
discovery narratives are gone.

config verify lives in script/lint's native branch rather than in a
Dockerfile, so every path that lints inherits it: the lint stage of the
main image, which is what CI runs, as well as Dockerfile.lint. Putting
it in one Dockerfile is how the other path silently loses it.
2026-08-10 14:05:54 +00:00
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
12 changed files with 509 additions and 703 deletions

View File

@@ -1,75 +1,37 @@
# Docker matches this file with moby/patternmatcher: Go filepath.Match # .dockerignore does NOT use .gitignore semantics. Docker matches with
# semantics plus a `**` extension, compiled to a regexp. Plain # moby/patternmatcher: filepath.Match plus `**`, so `*` does not cross
# filepath.Match has no `**` at all. What follows from that: `*` does not # `/` and an unprefixed pattern is anchored at the context root. Every
# cross `/`, and a pattern without a leading `**/` is anchored at the # depth-independent pattern therefore needs `**/`, or `config/.env` and
# build-context root. Every depth-independent pattern therefore needs the # `certs/server.key` still ship while the file reads as solved. Only
# `**/` prefix — without it `config/.env` and `certs/server.key` still # genuinely root-anchored entries go unprefixed. Never transplant these
# ship while the file reads as solved. # into .gitignore, where `**/` is wrong.
# #
# Root-anchored entries are for paths that occur exactly once, at the # Matching is case-sensitive, so secrets use character ranges rather
# context root. A host-built binary is the usual case, and it must be # than an ALL-CAPS twin, which would still miss `Server.Key`.
# written anchored: `/myapp`, never `**/myapp`. The prefixed form also
# matches `cmd/myapp/`, which deletes the package directory from the
# context. In-repo agent scratch is the other case, for the same reason
# — with the caveat recorded at that entry: anchoring is exact only
# where agents run at the repo root, and a repo where they do not must
# add its own entries.
# #
# Matching is case-sensitive, so `**/*.key` does not match # Extend with this repo's own host-built artifacts, written anchored:
# `certs/SERVER.KEY`, which is reachable on the case-insensitive # `/myapp`, never `**/myapp`, which also matches `cmd/myapp/` and
# filesystems most laptops use. Adding an ALL-CAPS twin per pattern is # deletes the package directory from the context.
# not the fix: it still misses `Server.Key` while reading as though case
# were handled. Character ranges cover every spelling in one line, so
# every secret name below is written that way — including the
# extensionless SSH keys and `.envrc`, because on those same
# case-insensitive filesystems direnv reads `.ENVRC` and ssh reads
# `ID_RSA`.
#
# `**/*.[eE][nN][vV]` also excludes a committed env template such as
# `example.env`. If the build genuinely needs one, re-include it with a
# negation after the pattern: `!docs/example.env`.
#
# Extend this file with the repo's own host-built artifacts (compiled
# binaries, test binaries, coverage output); those are per-repo and
# belong here because a host build otherwise drops them into the
# context.
# Repository metadata: exactly one, at the context root. Excluding it # Excluding .git means `git describe` cannot run in any build stage and
# means `git describe` cannot run in any build stage, and it fails # fails quietly there; pass the version in with --build-arg VERSION.
# quietly there rather than erroring, so a version embedded that way
# comes out empty. Compute the version on the host and pass it in with
# `--build-arg VERSION=...`; see the version rule in REPO_POLICIES.md.
.git .git
# In-repo agent scratch: a directory holding a full additional checkout # Agent scratch: one full checkout of the repo per in-flight agent.
# of the repo for each in-flight agent. Anchored because it occurs # Anchored because it occurs once where agents run at the repo root.
# exactly once *where agents run at the repo root*, which is the # KNOWN GAP: a repo running agents in subdirectories still ships
# convention this file assumes; the `**/` form would also match any # `services/api/.claude/` and must add its own anchored entry.
# nested directory of that name and delete it from the build.
#
# KNOWN GAP, and it is not hypothetical: the directory is created in the
# agent's working directory. If agents in this repo run in
# subdirectories — a monorepo with a per-service agent, say — then
# `services/api/.claude/` is NOT excluded by the line below and still
# reaches the build context and the image, which is the exposure this
# entry exists to close. A repo in that shape adds its own anchored
# entries (`/services/api/.claude`), or `**/.claude` after confirming no
# legitimately named nested directory would be caught.
#
# Not case-folded, unlike the secret patterns below: tooling creates
# this directory in exactly one spelling, so a folded pattern would add
# no coverage.
.claude .claude
# Environment files. `*.env` covers both the bare `.env` name (`*` matches # Environment files. `*.env` covers bare `.env` and the `prod.env`
# the empty string) and the `prod.env` convention. # convention. Re-include a committed template with a negation if the
# build needs one: `!docs/example.env`.
**/*.[eE][nN][vV] **/*.[eE][nN][vV]
**/.[eE][nN][vV].* **/.[eE][nN][vV].*
**/.[eE][nN][vV][rR][cC] **/.[eE][nN][vV][rR][cC]
# Private keys and the bundles that carry them. Public certificates # Private keys and the bundles carrying them. Public certificates
# (*.crt, *.cer) are deliberately absent: they are not secrets and are # (*.crt, *.cer) are deliberately absent: they are legitimate inputs.
# sometimes a legitimate build input.
**/*.[pP][eE][mM] **/*.[pP][eE][mM]
**/*.[kK][eE][yY] **/*.[kK][eE][yY]
**/*.[pP]12 **/*.[pP]12
@@ -86,8 +48,7 @@
**/.DS_Store **/.DS_Store
**/Thumbs.db **/Thumbs.db
# Editor state. Never a build input, and it churns under a developer's # Editor state: never a build input, and it churns COPY.
# hands, so it invalidates COPY for reasons unrelated to the source.
**/*.swp **/*.swp
**/*.swo **/*.swo
**/*~ **/*~

View File

@@ -3,26 +3,26 @@ FROM node@sha256:e4bf2a82ad0a4037d28035ae71529873c069b13eb0455466ae0bc13363826e3
WORKDIR /app WORKDIR /app
# script/bootstrap installs all prerequisites (make via apk here; node # Makes script/lint run the linter directly rather than building
# and yarn are already in the base image, so those steps are skipped). # Dockerfile.lint, which would need a docker daemon here.
# Dependency manifests are copied first so the bootstrap layer is ENV LINT_IN_CONTAINER=1
# cached until they change.
# script/bootstrap installs all prerequisites. Manifests are copied
# first so that layer stays cached until dependencies change.
COPY script/ script/ COPY script/ script/
COPY package.json yarn.lock ./ COPY package.json yarn.lock ./
RUN script/bootstrap RUN script/bootstrap
COPY . . COPY . .
# CHECK_EPOCH is a per-invocation nonce supplied by script/cibuild and # CHECK_EPOCH is a per-invocation nonce from script/cibuild and
# script/docker. Without it an unchanged tree serves this layer from # script/docker; without it an unchanged tree serves this layer from
# cache and the build reports a green it never ran. ARG is stage-scoped, # cache and the build reports a green it never ran. ARG is stage-scoped,
# so it must be redeclared in every stage that runs checks. The guard # so declare it in every stage that runs checks. The guard fails a bare
# makes a bare `docker build .` fail loudly instead of silently reusing # `docker build .`, which would otherwise reuse the empty (and therefore
# the empty (and therefore stable) cache key. Expand the value into the # stable) cache key. The value is also expanded into the check command,
# command so the cache miss does not depend on BuildKit's handling of an # so the cache miss does not depend on BuildKit's handling of an
# unreferenced ARG. Both the guard and the check RUN reference the value, # unreferenced ARG; keep both references.
# so both are value-keyed: there are two independent invalidation points
# here, not one. Keep both.
ARG CHECK_EPOCH ARG CHECK_EPOCH
RUN [ -n "$CHECK_EPOCH" ] || exit 1 RUN [ -n "$CHECK_EPOCH" ] || exit 1
RUN echo "check epoch: ${CHECK_EPOCH}" && make check RUN echo "check epoch: ${CHECK_EPOCH}" && make check

27
Dockerfile.lint Normal file
View File

@@ -0,0 +1,27 @@
# Lint-only image, built by script/lint when it is not already inside a
# container. Linting is a build step, so a successful build is a clean
# lint, and nothing is bind-mounted, which matters when the daemon is
# remote.
#
# node 22-alpine, 2026-02-22
FROM node@sha256:e4bf2a82ad0a4037d28035ae71529873c069b13eb0455466ae0bc13363826e34
WORKDIR /app
# Makes script/lint run the linter directly instead of recursing into
# another docker build, which has no daemon here.
ENV LINT_IN_CONTAINER=1
COPY script/ script/
COPY package.json yarn.lock ./
RUN script/bootstrap
COPY . .
# ARG sits after the dependency layer so that layer stays cached and
# only the lint re-runs. The guard fails a bare `docker build
# -f Dockerfile.lint .`, which would otherwise reuse the empty (stable)
# cache key and report a lint it never ran.
ARG CHECK_EPOCH
RUN [ -n "$CHECK_EPOCH" ] || exit 1
RUN echo "lint epoch: ${CHECK_EPOCH}" && make lint

View File

@@ -117,7 +117,9 @@ alpine. We provide:
- `script/projectname` — output the project name (our own extension); used by - `script/projectname` — output the project name (our own extension); used by
`script/docker` for the image tag `script/docker` for the image tag
- `script/test` — run the test suite (no tests defined here) - `script/test` — run the test suite (no tests defined here)
- `script/lint` — lint the markdown files with prettier - `script/lint` — lint the markdown files with prettier. Inside a container
(`LINT_IN_CONTAINER=1`, set by both Dockerfiles) it runs prettier directly; on
a host it builds `Dockerfile.lint` so the linter still runs in a container
- `script/fmt` — format all markdown files with prettier (writes) - `script/fmt` — format all markdown files with prettier (writes)
- `script/fmt-check` — check formatting (read-only) - `script/fmt-check` — check formatting (read-only)
- `script/check` — run all checks: `test`, `lint`, `fmt-check` (our own - `script/check` — run all checks: `test`, `lint`, `fmt-check` (our own

21
TODO.md
View File

@@ -21,6 +21,27 @@ fmt-check, and commit.
# Completed Steps # Completed Steps
- 2026-08-10: Moved every lint run into a container. `script/lint` now runs the
linter directly when `LINT_IN_CONTAINER=1` and otherwise builds
`Dockerfile.lint`, so the linter never runs on a developer host — closing the
content-keyed result cache that produced a confirmed false green, the
host-global `$TMPDIR/golangci-lint.lock`, and host/container version skew.
Detection is on that marker alone: a false negative inside a container fails
loudly on the missing daemon, while a false positive on a host would silently
restore host linting, so `/.dockerenv` is rejected outright — measured absent
inside BuildKit `RUN` steps and present on hosts that are themselves
containers. Everything else keeps its existing shape: `make check` still runs
in the image, `script/cibuild` is still one build, and the Go multistage lint
stage survives with `ENV LINT_IN_CONTAINER=1`. `Dockerfile.lint` carries the
same `CHECK_EPOCH` guard, with the `ARG` below the dependency layer so only
the lint re-runs. The `script/bootstrap` golangci-lint install and the
per-checkout cache/lock/`.lint-cache` wrapper are deleted as superseded; a JS
repo's `yarn install` stays, since the rule is about where a verdict comes
from, not about which binaries exist. `golangci-lint config verify` was kept
on measurement: a bogus config key passes `golangci-lint run` with `0 issues`
and fails `config verify`, and every case reproduced byte-identically under
`--network none`, so the schema is embedded and the line costs no network.
Comment blocks across the touched files were cut hard in the same pass.
- 2026-08-09: Made a golangci-lint result belong to the tree that asked for it. - 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 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 linter per-checkout `GOLANGCI_LINT_CACHE` and per-checkout `TMPDIR`. The two

View File

@@ -1,6 +1,6 @@
--- ---
title: Code Styleguide — Go title: Code Styleguide — Go
last_modified: 2026-08-09 last_modified: 2026-08-10
--- ---
1. Try to hard wrap long lines at 77 characters or less. 1. Try to hard wrap long lines at 77 characters or less.
@@ -111,7 +111,11 @@ last_modified: 2026-08-09
1. For anything beyond a simple script or tool, or anything that is going to 1. For anything beyond a simple script or tool, or anything that is going to
run in any sort of "production" anywhere, make sure it passes run in any sort of "production" anywhere, make sure it passes
`golangci-lint`. `golangci-lint`. Run it with `make lint`, never by invoking the binary: the
linter always runs in a container, and `golangci-lint` is not installed on
the host by any repo. Invoked directly on a shared host it reads a result
cache keyed on file content rather than location, and a host-global lock, so
its answer may belong to another checkout entirely.
1. Write a `Dockerfile` for every repo, even if it only runs the tests and 1. Write a `Dockerfile` for every repo, even if it only runs the tests and
linting. `script/cibuild` and `script/docker` should always make sure that linting. `script/cibuild` and `script/docker` should always make sure that

View File

@@ -1,6 +1,6 @@
--- ---
title: Existing Repo Checklist title: Existing Repo Checklist
last_modified: 2026-08-09 last_modified: 2026-08-10
--- ---
Use this checklist when beginning work in a repo that may not yet conform to our Use this checklist when beginning work in a repo that may not yet conform to our
@@ -42,6 +42,15 @@ with your task.
`CHECK_EPOCH` rule in `REPO_POLICIES.md`. Without them the check layer is `CHECK_EPOCH` rule in `REPO_POLICIES.md`. Without them the check layer is
served from cache on an unchanged tree and the build reports a green it served from cache on an unchanged tree and the build reports a green it
never ran. never ran.
- [ ] **Every stage that runs checks sets `ENV LINT_IN_CONTAINER=1`** — the lint
stage and the build stage both. This is the item an existing repo most
often fails after adopting the containerised lint: without it
`script/lint` tries to build `Dockerfile.lint` from inside a build step,
where there is no daemon.
- [ ] `Dockerfile.lint` exists and `script/lint` builds it when not already in a
container — see the containerised-lint rule in `REPO_POLICIES.md`. Base
image pinned by sha256 with a version/date comment, `ARG CHECK_EPOCH`
**after** the dependency layer with the guard below it.
- [ ] `.dockerignore` excludes the repo's own host-built artifacts (compiled - [ ] `.dockerignore` excludes the repo's own host-built artifacts (compiled
binaries, test binaries, coverage output), written root-anchored — binaries, test binaries, coverage output), written root-anchored —
`/myapp`, never `**/myapp`, which would also match `cmd/myapp/`. An `/myapp`, never `**/myapp`, which would also match `cmd/myapp/`. An
@@ -100,13 +109,24 @@ with your task.
`script/install-precommit`, shimmed by `make hooks`) runs it `script/install-precommit`, shimmed by `make hooks`) runs it
- [ ] README has an **Entrypoints** section documenting the `script/` - [ ] README has an **Entrypoints** section documenting the `script/`
entrypoints and linking the standard entrypoints and linking the standard
- [ ] Go: `script/lint` isolates golangci-lint per checkout — - [ ] `script/lint` is the canonical detect-and-branch form, and no host
`GOLANGCI_LINT_CACHE` and `TMPDIR` both exported into `.lint-cache/` invocation anywhere in the repo can produce a lint **verdict** — grep for
(which is in `.gitignore` and `.dockerignore`), `--allow-serial-runners` the linter's own name across `script/`, the `Makefile` and CI config, not
passed, and the lock error retried rather than reported as findings. Copy just `script/lint`. A second path is likeliest here: a `make lint-fast`,
the canonical block from `REPO_POLICIES.md`. Setting only the cache is the an older container-versus-host branch, or a CI step calling the binary
common half-fix and leaves `parallel golangci-lint is running` failing directly. **Expected hits that are not the defect**: `script/fmt`, and in
runs red. a repo whose formatter is also its linter, `script/fmt-check`. Everything
else the grep finds is a real second path and goes.
- [ ] Detection is on `LINT_IN_CONTAINER` alone. Reject any `/.dockerenv` or
cgroup heuristic: absent in BuildKit `RUN` steps, present on hosts that
are themselves containers, and a false positive lints on the host.
- [ ] `script/bootstrap` installs no golangci-lint. Delete the block, its
version and ref variables, and its call site. A JS repo's `yarn install`
stays — it brings a linter along with every other dependency, which is
fine as long as no verdict is taken from it.
- [ ] The per-checkout lint state is gone: no `GOLANGCI_LINT_CACHE` or `TMPDIR`
exports, no `--allow-serial-runners`, and `.lint-cache/` removed from
`.gitignore` and `.dockerignore`.
- [ ] `make check` does not modify any files in the repo - [ ] `make check` does not modify any files in the repo
- [ ] `make test` has a 30-second timeout - [ ] `make test` has a 30-second timeout
- [ ] `make test` runs real tests, not a no-op (at minimum, import/compile - [ ] `make test` runs real tests, not a no-op (at minimum, import/compile
@@ -153,6 +173,9 @@ with your task.
# Final # Final
- [ ] `make check` passes - [ ] `make check` passes
- [ ] `script/cibuild` succeeds (a bare `docker build .` fails closed by design, - [ ] `make lint` runs twice on an unchanged tree with the lint layer `DONE`
on the `CHECK_EPOCH` guard) both times, never `CACHED` and never sub-second
- [ ] `script/cibuild` succeeds (a bare `docker build .` or
`docker build -f Dockerfile.lint .` fails closed by design, on the
`CHECK_EPOCH` guard)
- [ ] Commit and merge fixes before starting your actual task - [ ] Commit and merge fixes before starting your actual task

View File

@@ -1,6 +1,6 @@
--- ---
title: New Repo Checklist title: New Repo Checklist
last_modified: 2026-08-09 last_modified: 2026-08-10
--- ---
Use this checklist when creating a new repository from scratch. Follow the steps Use this checklist when creating a new repository from scratch. Follow the steps
@@ -79,9 +79,21 @@ Template files can be fetched from:
`CHECK_EPOCH` rule in `REPO_POLICIES.md`. Without them the check layer is `CHECK_EPOCH` rule in `REPO_POLICIES.md`. Without them the check layer is
served from cache on an unchanged tree and the build reports a green it served from cache on an unchanged tree and the build reports a green it
never ran. never ran.
- Every stage that runs checks sets `ENV LINT_IN_CONTAINER=1`, so
`script/lint` runs the linter natively instead of trying to build
`Dockerfile.lint` where there is no daemon.
- Go repos: separate `lint` stage on the `golangci/golangci-lint` image,
with `COPY --from=lint /src/go.sum /dev/null` in the build stage to force
the ordering. Re-prove that ordering warm after adopting `CHECK_EPOCH`.
- Server: also builds and runs the application - Server: also builds and runs the application
- Non-server: brings up dev environment and runs `make check` - Non-server: brings up dev environment and runs `make check`
- Image pinned by sha256 hash with version/date comment - Image pinned by sha256 hash with version/date comment
- [ ] `Dockerfile.lint` — the lint-only image `script/lint` builds when it is
not already inside a container. Sets `ENV LINT_IN_CONTAINER=1`; same
`ARG CHECK_EPOCH` + guard + expanded-value discipline as above, with the
`ARG` **after** the dependency layer so only the lint re-runs. Base image
pinned by sha256 with a version/date comment. Copy from
`REPO_POLICIES.md`.
- [ ] Gitea Actions workflow at `.gitea/workflows/check.yml` that runs - [ ] Gitea Actions workflow at `.gitea/workflows/check.yml` that runs
`script/cibuild` on push — reference `script/cibuild` on push — reference
`https://git.eeqj.de/sneak/prompts/raw/branch/main/.gitea/workflows/check.yml` `https://git.eeqj.de/sneak/prompts/raw/branch/main/.gitea/workflows/check.yml`
@@ -108,18 +120,15 @@ are thin shims calling them. Model scripts:
then `install-precommit`, plus repo-specific init then `install-precommit`, plus repo-specific init
- [ ] `script/test` / `make test` — runs real tests, not a no-op (30-second - [ ] `script/test` / `make test` — runs real tests, not a no-op (30-second
timeout) timeout)
- [ ] `script/lint` / `make lint` — runs linter - [ ] `script/lint` / `make lint` — runs the linter directly when
- [ ] Go: exports `GOLANGCI_LINT_CACHE` **and** `TMPDIR` into a `LINT_IN_CONTAINER=1`, otherwise `epoch="$(date +%s%N)$$"` on its own line
`.lint-cache/` directory inside the checkout, above any then `docker build --build-arg CHECK_EPOCH="$epoch" -f Dockerfile.lint .`.
container-versus-host branch so every path that reaches the linter No lint verdict may come from a host invocation. Copy from
gets them; passes `--allow-serial-runners` (never `REPO_POLICIES.md`. Detect on `LINT_IN_CONTAINER` only — never
`--allow-parallel-runners`); retries on `/.dockerenv`, which is absent in BuildKit `RUN` steps and present on
`parallel golangci-lint is running` detected on **stderr** and exits hosts that are themselves containers. Without the nonce this exits 0 on an
75 with a VOID message on exhaustion. Copy the canonical block from unchanged tree having linted nothing; without `-f Dockerfile.lint` it
`REPO_POLICIES.md` rather than writing your own: a version that sets builds the main image and lints nothing at all.
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` / `make fmt` — formats code (writes)
- [ ] `script/fmt-check` / `make fmt-check` — checks formatting (read-only) - [ ] `script/fmt-check` / `make fmt-check` — checks formatting (read-only)
- [ ] `script/check` / `make check` — runs `test`, `lint`, `fmt-check`; must not - [ ] `script/check` / `make check` — runs `test`, `lint`, `fmt-check`; must not
@@ -149,7 +158,10 @@ are thin shims calling them. Model scripts:
export with no `.git` and on a repo with no commits — keep it, and do not export with no `.git` and on a repo with no commits — keep it, and do not
collapse it into `|| echo unknown`, which makes it unreachable. See the collapse it into `|| echo unknown`, which makes it unreachable. See the
`CHECK_EPOCH` and git-describe rules in `REPO_POLICIES.md` for why each `CHECK_EPOCH` and git-describe rules in `REPO_POLICIES.md` for why each
element is load-bearing. A bare `docker build .` fails closed by design. element is load-bearing. A bare `docker build .` fails closed by design, and
so does a bare `docker build -f Dockerfile.lint .`. The image runs
`make check`, which includes lint, so `script/cibuild` needs no separate
lint step.
- [ ] `script/precommit` — called by the pre-commit hook; runs `script/check` - [ ] `script/precommit` — called by the pre-commit hook; runs `script/check`
- [ ] `script/install-precommit` — installs the pre-commit hook that runs - [ ] `script/install-precommit` — installs the pre-commit hook that runs
@@ -161,7 +173,11 @@ are thin shims calling them. Model scripts:
# 4. Verify # 4. Verify
- [ ] `make check` passes - [ ] `make check` passes
- [ ] `make lint` demonstrably runs the linter rather than returning a cached
build: run it twice on an unchanged tree and confirm the lint layer says
`DONE`, never `CACHED`, both times
- [ ] `make docker` succeeds - [ ] `make docker` succeeds
- [ ] `script/cibuild` succeeds and demonstrably executes
- [ ] No secrets in repo - [ ] No secrets in repo
- [ ] No mutable image/package references - [ ] No mutable image/package references
- [ ] No unnecessary files in repo root - [ ] No unnecessary files in repo root

View File

@@ -1,6 +1,6 @@
--- ---
title: Repository Policies title: Repository Policies
last_modified: 2026-08-09 last_modified: 2026-08-10
--- ---
This document covers repository structure, tooling, and workflow standards. Code This document covers repository structure, tooling, and workflow standards. Code
@@ -94,32 +94,49 @@ style conventions are in separate documents:
reading the Makefile. reading the Makefile.
- Every repo should have a `Dockerfile`. All Dockerfiles must run `make check` - Every repo should have a `Dockerfile`. All Dockerfiles must run `make check`
as a build step so the build fails if the branch is not green — which requires as a build step so the build fails if the branch is not green — the one
`ARG CHECK_EPOCH` and its guard in every stage containing a check-running exception being `Dockerfile.lint`, which runs `make lint` alone because that
`RUN`, per the `CHECK_EPOCH` rule below. Without them a Dockerfile satisfies is its entire purpose — which requires `ARG CHECK_EPOCH` and its guard in
this criterion while its check layers are served from cache, so the build every stage containing a check-running `RUN`, per the `CHECK_EPOCH` rule
cannot fail on a branch that is not green. For non-server repos, the below. Without them a Dockerfile satisfies this criterion while its check
Dockerfile should bring up a development environment and run `make check`. For layers are served from cache, so the build cannot fail on a branch that is not
server repos, `make check` should run as an early build stage before the final green.
image is assembled. Dockerfiles install development prerequisites by running
`script/bootstrap` rather than duplicating installs inline; COPY `script/` and **Every Dockerfile must also set `ENV LINT_IN_CONTAINER=1`**, above the
the dependency manifests (`package.json` + `yarn.lock`, `go.mod` + `go.sum`, checks. `script/lint` builds `Dockerfile.lint` when it is not already in a
etc.) before running it so the bootstrap layer stays cached until dependencies container; without the marker it would try that from inside a build step,
change. where there is no daemon. See the containerised-lint rule below.
For non-server repos, the Dockerfile should bring up a development
environment and run `make check`. For server repos, `make check` should run
as an early build stage before the final image is assembled. Dockerfiles
install development prerequisites by running `script/bootstrap` rather than
duplicating installs inline; COPY `script/` and the dependency manifests
(`package.json` + `yarn.lock`, `go.mod` + `go.sum`, etc.) before running it
so the bootstrap layer stays cached until dependencies change.
- **Every check-running `RUN` must be cache-busted with `CHECK_EPOCH`.** Docker - **Every check-running `RUN` must be cache-busted with `CHECK_EPOCH`.** Docker
invalidates a `COPY` layer only when the copied content changes, so on an invalidates a `COPY` layer only when the copied content changes, so on an
unchanged tree the `RUN make check` layer is served from cache, the suite unchanged tree the check layer is served from cache, the suite never runs, and
never runs, and the build still exits 0. A sub-second `docker build` reporting the build still exits 0. A sub-second `docker build` reporting success is a
success is a cache hit, not a result. The canonical form, in **every** stage cache hit, not a result. This applies to **every** file that runs checks in a
containing a check-running `RUN`: build step — `Dockerfile` and `Dockerfile.lint` alike; a `Dockerfile.lint`
without the cache-bust is a lint that never ran, reported as a pass. The
canonical form, in **every** stage containing a check-running `RUN`, placed
**after** the dependency-install layer so that layer stays cached:
```dockerfile ```dockerfile
ENV LINT_IN_CONTAINER=1
ARG CHECK_EPOCH ARG CHECK_EPOCH
RUN [ -n "$CHECK_EPOCH" ] || exit 1 RUN [ -n "$CHECK_EPOCH" ] || exit 1
RUN echo "check epoch: ${CHECK_EPOCH}" && make check RUN echo "check epoch: ${CHECK_EPOCH}" && make check
``` ```
`ENV LINT_IN_CONTAINER=1` belongs in every such stage too, and is the line
most often missed: without it `make check` reaches `script/lint`, which
tries to build `Dockerfile.lint` from inside a build step where there is no
daemon. See the containerised-lint rule below.
and in both `script/cibuild` and `script/docker`: and in both `script/cibuild` and `script/docker`:
```sh ```sh
@@ -132,6 +149,11 @@ style conventions are in separate documents:
. .
``` ```
`script/lint` needs the same nonce but is **not** this command: it builds a
different file with `-f Dockerfile.lint` and passes no version. Copy its
form from the containerised-lint rule below, not this block — a
`docker build` with no `-f` builds the main image and lints nothing.
The `VERSION` lines are there for a different reason, covered by the The `VERSION` lines are there for a different reason, covered by the
git-describe rule below; they are shown here so the two rules do not each git-describe rule below; they are shown here so the two rules do not each
document half a command. All four `CHECK_EPOCH` elements are load-bearing; document half a command. All four `CHECK_EPOCH` elements are load-bearing;
@@ -162,27 +184,189 @@ style conventions are in separate documents:
concurrent invocations would collide. concurrent invocations would collide.
This invalidates the check layers and everything after them while leaving This invalidates the check layers and everything after them while leaving
`go mod download`, `script/bootstrap`, and the pinned toolchain install `go mod download` and `script/bootstrap` cached, so it does not push against
cached, so it does not push against the five-minute Docker build ceiling. the five-minute Docker build ceiling. Blanket `--no-cache` is not an
Blanket `--no-cache` also works but is wasteful and can blow that ceiling. acceptable substitute: it also busts the dependency layer, so every run
reinstalls dependencies over the network instead of only the first and those
after a manifest change. Never reach for `docker builder prune` — the build
cache is shared with every other build on the host.
- **Every lint run happens in a container.** `script/lint` runs the linter
directly when it is already inside one, and otherwise builds `Dockerfile.lint`
so that it is. Either way the linter never runs on a developer host, where its
answer is not trustworthy:
- **Confirmed false green.** golangci-lint keys cached results on file
**content, not location**, so a second checkout of the same commit serves
its findings. One implementer reported `0 issues` on a branch genuinely
red with a `goconst` finding. Own-clones-instead-of-worktrees does not
help; two clones are byte-identical exactly as two worktrees were.
- **False reds**: findings reported against other checkouts and against
worktrees already deleted; in one case 399 issues returned to a clean
clone that genuinely lints 0.
- **Lock contention indistinguishable from findings.** golangci-lint flocks
`$TMPDIR/golangci-lint.lock` (`pkg/commands/run.go`, `acquireFileLock()`),
host-global and independent of `GOLANGCI_LINT_CACHE`, 5-second timeout. It
prints `parallel golangci-lint is running`, analyzes nothing, exits
non-zero. Not fixed by per-cache isolation — measured.
- **Version skew**: a host linter differing from the pinned one, with the
container surfacing thirteen findings the host missed.
A container has its own cache, its own `TMPDIR` and a binary pinned by
digest, so none of it is reachable. This supersedes the per-checkout
`GOLANGCI_LINT_CACHE`/`TMPDIR` wrapper, which existed only to make a host
run trustworthy; delete it on adoption.
The canonical `script/lint`, whose executable lines are the same in every
repo apart from the native lint command:
```sh
#!/bin/sh
# script/lint: run the linter. Inside a container, run it directly; on a
# host, build Dockerfile.lint so it runs in one anyway.
#
# LINT_IN_CONTAINER is set by this repo's Dockerfiles and is the ONLY
# accepted signal. Do not add a /.dockerenv fallback: it is absent inside
# BuildKit RUN steps and present on hosts that are themselves containers,
# so it both misses and false-positives — and a false positive silently
# restores host linting.
set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
main() {
cd "$ROOT"
if [ "${LINT_IN_CONTAINER:-}" = "1" ]; then
# config verify lives here, not in a Dockerfile, so every path
# that lints inherits it — the lint stage of the main image as
# well as Dockerfile.lint. Duplicating it into each Dockerfile
# is how one of them silently loses it.
golangci-lint config verify --config .golangci.yml
exec golangci-lint run --config .golangci.yml ./...
fi
# Own line, and `$$` because busybox `date` drops %N silently.
# Without a fresh nonce the lint layer is cached and this exits 0
# having linted nothing.
epoch="$(date +%s%N)$$"
docker build \
--build-arg CHECK_EPOCH="$epoch" \
-f Dockerfile.lint \
.
}
main "$@"
```
and `Dockerfile.lint`, the standalone path for a developer host:
```dockerfile
# Lint-only image, built by script/lint when not already in a container.
# golangci/golangci-lint:v2.12.2 (Debian-based), 2026-08-07
FROM golangci/golangci-lint@sha256:5cceeef04e53efe1470638d4b4b4f5ceefd574955ab3941b2d9a68a8c9ad5240
WORKDIR /src
ENV LINT_IN_CONTAINER=1
COPY go.mod go.sum ./
RUN go mod download
COPY . .
# ARG after the dependency layer so only the lint re-runs.
ARG CHECK_EPOCH
RUN [ -n "$CHECK_EPOCH" ] || exit 1
RUN echo "lint epoch: ${CHECK_EPOCH}" && make lint
```
Load-bearing properties:
- **Detection rests on `LINT_IN_CONTAINER=1` and nothing else.** Every
Dockerfile in the repo sets it; a host does not. The asymmetry is the
whole design: a **false negative** inside a container tries a nested
`docker build`, finds no daemon and fails loudly, while a **false
positive** on a host silently lints there — the exact defect this rule
exists to kill. So the signal must be one only our own images can produce.
`/.dockerenv` is not such a signal and must not be used, even as a
fallback: measured, it is **absent** inside BuildKit `RUN` steps and
**present** on any host that is itself a container, which is the common
case for CI runners and agent sandboxes. It fails in both directions, and
one of them is the dangerous one.
- **`CHECK_EPOCH`, not `--no-cache`.** `docker build -f Dockerfile.lint .`
on an unchanged tree returns a sub-second cached success having linted
nothing. The `ARG` goes **after** the dependency layer so only the lint
re-runs; `--no-cache` would also reinstall dependencies on every lint.
- **Non-Go repos get the same pattern around their own linter** — `eslint`,
`ruff`, `prettier`, `shellcheck`. Only the base image and the native lint
command change.
- **Keep `golangci-lint config verify`, put it in `script/lint`, and it
costs no network.** It goes in the native branch, not in a Dockerfile, so
the lint stage of the main image inherits it along with `Dockerfile.lint`;
putting it in one Dockerfile leaves the other path unverified. The two
commands catch disjoint classes, measured under the pinned v2.12.2: a
bogus top-level key and a bogus key under `linters.settings.lll` both pass
`golangci-lint run` with **exit 0 and `0 issues`** while `config verify`
exits 3 and names them; an invalid value type fails both; an unknown
linter name fails `run` and passes `config verify`. So `run` alone
silently ignores an unknown key — the mode where a threshold reads as
configured and is not applied. It needs no network: every case reproduced
byte-identically under `docker run --network none`, in a container where
`getent hosts golangci-lint.run` exits 2. The schema is embedded in the
pinned binary. Re-run that control when bumping the pin.
- **A failed `script/lint` that names no finding is not a lint result.** On
the host path `docker build` exits 1 both for findings and for a build
that never got there (daemon down, image unpullable, disk full). BuildKit
names the failing step; read it, fix the environment, re-run. Do not
record a verdict from a run that did not lint.
**Scope: this rule is about linters, and a formatter is not one.**
`script/fmt` writes your working tree, so it can only run on the host, and
`script/fmt-check` is its read-only twin. In a repo whose formatter **is**
its linter (prettier over markdown; this repo), `script/bootstrap` therefore
installs the linter on the host as an ordinary dependency and
`script/fmt-check` runs it there. That is accepted: the version is pinned in
`package.json` and installed into the repo's own `node_modules`, so there is
no shared content-keyed cache, no host-global lock and nothing to skew
against. What is forbidden is taking a **lint verdict** from it —
`script/lint` stays the only source of one. A repo auditing itself will see
those hits and should leave them; anything else the grep finds is a real
second path to the linter and goes.
**What a consuming repo does to adopt this**, in order:
1. Add `Dockerfile.lint`.
2. Replace `script/lint` with the form above, with its own native lint
command.
3. Add `ENV LINT_IN_CONTAINER=1` to **every** stage of every Dockerfile that
runs checks — the lint stage and the build stage both.
4. Delete any golangci-lint install from `script/bootstrap`, with its
version and ref variables and its call site. No lint verdict comes from
the host any more, so it can only reintroduce version skew. A JS repo's
`yarn install` stays.
5. Delete the per-checkout lint state: `GOLANGCI_LINT_CACHE` and `TMPDIR`
exports, `--allow-serial-runners`, the retry/VOID wrapper, and
`.lint-cache/` from both `.gitignore` and `.dockerignore`.
6. Verify by running `make lint` twice on an unchanged tree: the lint layer
must be `DONE` both times, never `CACHED`. Then plant a violation,
confirm it fails naming the finding, revert. A bare
`docker build -f Dockerfile.lint .` must fail on the guard.
`script/check`, `script/cibuild`, `script/docker` and the `Dockerfile` are
unchanged by this: `make check` still runs inside the image, and
`script/lint` there takes the native path.
- **Dockerfiles must use a separate lint stage for fail-fast feedback.** Go - **Dockerfiles must use a separate lint stage for fail-fast feedback.** Go
repos use a multistage build where linting runs in an independent stage based repos use a multistage build where linting runs in an independent stage based
on the `golangci/golangci-lint` image (pinned by hash). This stage runs on the `golangci/golangci-lint` image (pinned by hash), so lint failures
`make fmt-check` and `make lint` before the full build begins. The build stage surface in seconds rather than after a full compile. The build stage declares
then declares an explicit dependency on the lint stage via an explicit dependency on it via `COPY --from=lint /src/go.sum /dev/null`,
`COPY --from=lint /src/go.sum /dev/null`, which forces BuildKit to complete which forces BuildKit — which runs stages in parallel by default — to finish
linting before proceeding to compilation and tests. This ensures lint failures linting first. The canonical Go repo `Dockerfile`:
surface in seconds rather than minutes, without blocking on dependency
download or compilation in the build stage.
The standard pattern for a Go repo Dockerfile is:
```dockerfile ```dockerfile
# Lint stage — fast feedback on formatting and lint issues # Lint stage — fast feedback on formatting and lint issues
# golangci/golangci-lint:v2.x.x, YYYY-MM-DD # golangci/golangci-lint:v2.x.x, YYYY-MM-DD
FROM golangci/golangci-lint@sha256:... AS lint FROM golangci/golangci-lint@sha256:... AS lint
WORKDIR /src WORKDIR /src
ENV LINT_IN_CONTAINER=1
COPY go.mod go.sum ./ COPY go.mod go.sum ./
RUN go mod download RUN go mod download
COPY . . COPY . .
@@ -195,6 +379,7 @@ style conventions are in separate documents:
# golang:1.x-alpine, YYYY-MM-DD # golang:1.x-alpine, YYYY-MM-DD
FROM golang@sha256:... AS builder FROM golang@sha256:... AS builder
WORKDIR /src WORKDIR /src
ENV LINT_IN_CONTAINER=1
# Force BuildKit to run the lint stage before proceeding # Force BuildKit to run the lint stage before proceeding
COPY --from=lint /src/go.sum /dev/null COPY --from=lint /src/go.sum /dev/null
@@ -221,50 +406,36 @@ style conventions are in separate documents:
``` ```
Key points: Key points:
- The lint stage uses the `golangci/golangci-lint` image directly (it - The lint stage uses the `golangci/golangci-lint` image directly (it has
includes both Go and the linter), so there is no need to install the both Go and the linter), so nothing needs installing. `make lint` there
linter separately. runs `script/lint`, which sees `LINT_IN_CONTAINER=1` and invokes
- `COPY --from=lint /src/go.sum /dev/null` is a no-op file copy that creates `golangci-lint` natively instead of building `Dockerfile.lint`. Without
a stage dependency. BuildKit runs stages in parallel by default; without that `ENV` the stage would attempt a nested build and fail.
this line, the build stage would not wait for lint to finish and a lint - `COPY --from=lint /src/go.sum /dev/null` is a no-op copy that exists only
failure might not fail the overall build. to create the stage dependency; without it a lint failure might not fail
the overall build.
- **Re-prove that ordering on a warm cache after adopting `CHECK_EPOCH`.** - **Re-prove that ordering on a warm cache after adopting `CHECK_EPOCH`.**
The cache-bust turns this no-op `COPY` into a content-cache hit, so an The cache-bust turns the no-op `COPY` into a content-cache hit, so an
ordering guarantee established on a cold cache does not automatically ordering guarantee established cold does not automatically carry over. It
carry over; it has to be re-checked warm. This was re-proved in another was re-proved in another org repo using the same trick and held, but that
repo in the org that uses the same file-dependency trick (there with a result does not transfer by assumption — re-check it warm.
marker file in place of `go.sum`), and the ordering held. It has **not** - If the project uses `//go:embed` referencing build artifacts, the lint
been verified in this repo, which is single-stage and has no lint stage to stage must create placeholders so the directives resolve:
order against. Any repo relying on a file-dependency trick for stage `RUN mkdir -p web/dist && touch web/dist/index.html`.
ordering should re-check it warm after adopting the bust rather than - If linting needs CGO or system libraries (e.g. `vips-dev`), `apk add` them
assuming this result transfers. in the lint stage.
- If the project uses `//go:embed` directives that reference build artifacts - Tests run in the build stage, not the lint stage: they may need compiled
(e.g. a web frontend compiled in a separate stage), the lint stage must
create placeholder files so the embed directives resolve. Example:
`RUN mkdir -p web/dist && touch web/dist/index.html web/dist/style.css`.
The lint stage should not depend on the actual build output — it exists to
fail fast.
- If the project requires CGO or system libraries for linting (e.g.
`vips-dev`), install them in the lint stage with `apk add`.
- The build stage runs `make test` after compilation setup. Tests run in the
build stage, not the lint stage, because they may require compiled
artifacts or heavier dependencies. artifacts or heavier dependencies.
- `ARG CHECK_EPOCH` appears in **both** stages, because `ARG` is - `ARG CHECK_EPOCH` appears in **both** stages, because `ARG` is
stage-scoped: declaring it only in the lint stage leaves `make test` stage-scoped: declaring it only in the lint stage leaves `make test`
frozen at the last cached result. In each stage the guard sits immediately frozen at its last cached result. In each stage the guard sits immediately
below the `ARG` so a bare `docker build .` fails instead of reusing the below the `ARG` and the value is expanded into the first check `RUN`.
empty cache key, and the value is expanded into the first check `RUN` so Later `RUN`s in the same stage need no expansion; their parent layer is
the cache miss does not rely on BuildKit's unreferenced-`ARG` handling. already busted.
Both of those lines reference `$CHECK_EPOCH`, so both are value-keyed: - `ARG VERSION=dev` is declared in the build stage and supplied by
each stage is invalidated at two independent points. The later `RUN`s in `script/docker` and `script/cibuild`. **No stage may call
the same stage need no expansion of their own: they are already `git describe`**: `.dockerignore` excludes `.git`, so it yields an empty
invalidated by their busted parent layer. version without failing. See the git-describe rule further down.
- `ARG VERSION=dev` is declared in the build stage, and its value is
supplied on the host by `script/docker` and `script/cibuild` via
`--build-arg VERSION=...`. The `dev` default is a placeholder for a local
build, not a source of truth. **No stage may call `git describe`**:
`.dockerignore` excludes `.git`, so it yields an empty version without
failing. See the git-describe rule further down.
- Every repo should have a Gitea Actions workflow (`.gitea/workflows/`) that - Every repo should have a Gitea Actions workflow (`.gitea/workflows/`) that
runs `script/cibuild` (which runs runs `script/cibuild` (which runs
@@ -274,9 +445,9 @@ style conventions are in separate documents:
cache-bust described above. Without it, an unchanged tree serves the check cache-bust described above. Without it, an unchanged tree serves the check
layer from cache and the build reports a green it never earned. A bare layer from cache and the build reports a green it never earned. A bare
`docker build .` fails closed by design, on the `[ -n "$CHECK_EPOCH" ]` guard; `docker build .` fails closed by design, on the `[ -n "$CHECK_EPOCH" ]` guard;
always go through `script/cibuild` or `script/docker`. Never accept a always go through `script/cibuild` or `script/docker`. Never accept a pass as
`script/cibuild` pass as evidence without confirming it ran: a sub-second wall evidence without confirming it ran: a sub-second wall time, or `CACHED` on the
time, or `CACHED` on the check layer, means nothing was executed. check layer, means nothing was executed.
- Use platform-standard formatters: `black` for Python, `prettier` for - Use platform-standard formatters: `black` for Python, `prettier` for
JS/CSS/Markdown/HTML, `go fmt` for Go. Always use default configuration with JS/CSS/Markdown/HTML, `go fmt` for Go. Always use default configuration with
@@ -498,513 +669,83 @@ style conventions are in separate documents:
- `.golangci.yml` is standardized and must _NEVER_ be modified by an agent, only - `.golangci.yml` is standardized and must _NEVER_ be modified by an agent, only
manually by the user. Fetch from manually by the user. Fetch from
`https://git.eeqj.de/sneak/prompts/raw/branch/main/.golangci.yml`. The `https://git.eeqj.de/sneak/prompts/raw/branch/main/.golangci.yml`. The
canonical golangci-lint version is v2.12.2 (released 2026-05-06), installed canonical golangci-lint version is v2.12.2 (released 2026-05-06), pinned as
commit-pinned via the image digest in `Dockerfile.lint`
`go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@c0d3ddc9cf3faa61a4e378e879ece580256d76e5`. (`golangci/golangci-lint@sha256:5cceeef04e53efe1470638d4b4b4f5ceefd574955ab3941b2d9a68a8c9ad5240`,
which reports
`golangci-lint has version 2.12.2 built with go1.26.2 from c0d3ddc9`). That
digest is the only pin: golangci-lint is not installed on the host by any
repo. Bumping the version means changing that one digest.
- **`script/bootstrap` in Go repos must install the pinned golangci-lint - **`script/bootstrap` must not install golangci-lint.** This supersedes the
whenever the installed version does not match the pin — not merely when the pinned host install that used to be canonical here. `script/lint` never runs
binary is absent — and must then verify the install took effect by it on the host — it either builds `Dockerfile.lint` or is already in a
re-resolving the binary through `PATH`.** The presence test container that ships the binary — so a host install has no caller, and its
`if missing golangci-lint; then go install "$GOLANGCI_LINT_REF"; fi` is wrong: only remaining effect is to put a second, independently-versioned linter where
it tests `PATH` presence and never version, so on any already-provisioned somebody eventually runs it by hand and believes the result. Delete the block,
machine the pin is inert and a version bump is a no-op. Meanwhile the its version and ref variables, and its call site.
Dockerfile installs unconditionally into a clean image, so CI and local
silently disagree about what the linter even is. Observed consequences: a
local `make check` green while `make docker` rejected the same commit with six
`goconst` findings, and a container linter surfacing thirteen findings the
host run missed. A stale host linter does not merely fail to prove the tree is
clean — it hides findings only the container can see. This is a deliberate
departure from the node handling described above, which uses whatever node is
installed: the linter version is the specific thing being held equal between
host and container, so for it, presence is not enough.
Comparing versions is necessary but **not sufficient**, because the obvious This is not a ban on host dependency installs generally. A JS or docs repo's
fix also fails green. `go install` writes to `GOBIN` (or `GOPATH/bin`) while `script/bootstrap` runs `yarn install`, which brings its linter along with
callers resolve `golangci-lint` through `PATH`. If a different binary every other dependency; that is unavoidable and fine. The rule is about a
shadows it earlier in `PATH`, the install genuinely succeeds and changes **dedicated** linter install, and about where a verdict may come from.
nothing any caller will ever see: bootstrap prints success and the next
`make lint` still runs the stale linter. That is worse than no fix, because
it converts a known-stale toolchain into one everyone believes is pinned.
The canonical form, placed in `script/bootstrap` after Go itself is present:
```sh **The version-enforcement principle it established still applies to any
# golangci-lint v2.12.2, 2026-05-06. GOLANGCI_LINT_VERSION must be exactly other tool a repo pins and installs on the host**, and it is the part worth
# what `golangci-lint --version` prints for this ref; update both together. keeping, because each of its four properties guards a failure that otherwise
GOLANGCI_LINT_VERSION="2.12.2" reports success:
GOLANGCI_LINT_REF="github.com/golangci/golangci-lint/v2/cmd/golangci-lint@c0d3ddc9cf3faa61a4e378e879ece580256d76e5" - **Compare the installed version against the pin, never test presence.** A
`if missing <tool>; then install; fi` guard tests `PATH` presence and
# The version golangci-lint reports, resolved the way callers resolve it. never version, so on any already-provisioned machine the pin is inert and
# Prints nothing when the binary is absent, exits non-zero, or prints a version bump is a silent no-op. Compare the **whole** version token,
# something unparseable: all of those must read as "does not match". exactly: a parser that stops at the first `-` reports `2.12.2` for a host
# The capture is the whole version token, not just its numeric prefix. running `2.12.2-rc1` and skips the install — the original defect,
# Stopping at the first `-` would make 2.12.2-rc1 compare equal to 2.12.2 reintroduced through the comparison meant to fix it.
# and skip the install, which is the defect this whole rule exists to close.
# The trailing `|| true` is required, not tidiness. Under `set -o pipefail`
# a non-zero --version would otherwise propagate out of the pipeline and
# kill the script through `set -e` before the diagnostic below is printed.
golangci_lint_version() {
command -v golangci-lint >/dev/null 2>&1 || return 0
golangci-lint --version 2>/dev/null | head -n 1 |
sed -n 's/.*has version v\{0,1\}\([0-9][^ ]*\).*/\1/p' || true
}
ensure_golangci_lint() {
if [ "$(golangci_lint_version)" = "$GOLANGCI_LINT_VERSION" ]; then
echo "bootstrap: golangci-lint $GOLANGCI_LINT_VERSION already installed"
return 0
fi
echo "bootstrap: installing golangci-lint $GOLANGCI_LINT_VERSION"
go install "$GOLANGCI_LINT_REF"
# go install writes to GOBIN (or GOPATH/bin); callers resolve through
# PATH. Re-resolve through PATH and assert the install took effect.
# `hash -r` is load-bearing: without it a shell that already resolved
# a stale golangci-lint answers from its own lookup cache, and this
# check false-fails with the shadowing message below.
hash -r 2>/dev/null || true
gcl_got="$(golangci_lint_version)"
if [ "$gcl_got" = "$GOLANGCI_LINT_VERSION" ]; then
echo "bootstrap: golangci-lint $GOLANGCI_LINT_VERSION installed," \
"and PATH resolves it"
return 0
fi
gcl_bin="$(go env GOBIN)"
[ -n "$gcl_bin" ] || gcl_bin="$(go env GOPATH)/bin"
# Strip a trailing slash: GOBIN=/x/ would otherwise make the
# "$gcl_bin"/* test below miss and misreport shadowing.
while :; do
case "$gcl_bin" in
*/) gcl_bin="${gcl_bin%/}" ;;
*) break ;;
esac
done
gcl_found="$(command -v golangci-lint 2>/dev/null || true)"
echo "bootstrap: installed golangci-lint $GOLANGCI_LINT_VERSION into" \
"$gcl_bin, but that is not what callers will get." >&2
case "$gcl_found" in
"")
echo "bootstrap: PATH resolves no golangci-lint at all." \
"Add $gcl_bin to PATH, then re-run bootstrap." >&2
;;
"$gcl_bin"/*)
echo "bootstrap: PATH resolves $gcl_found, inside that same" \
"directory, reporting version ${gcl_got:-unparseable}." \
"Nothing is shadowing it, so the install itself did not" \
"produce the pinned version: check that" \
"GOLANGCI_LINT_VERSION matches GOLANGCI_LINT_REF." >&2
;;
*)
echo "bootstrap: PATH resolves $gcl_found instead, reporting" \
"version ${gcl_got:-unparseable}. Remove that binary or" \
"put $gcl_bin earlier in PATH, then re-run bootstrap." >&2
;;
esac
exit 1
}
# The definitions above are inert on their own; the call site is part of
# the canonical form. In a script/bootstrap that follows the "define all
# functions, then call main" convention, this line belongs inside main()
# next to the other ensure_* steps.
ensure_golangci_lint
```
Four properties are load-bearing; each guards a failure mode that otherwise
fails green:
- **Compare the installed version against the pin**, never test presence.
This is what makes a version bump propagate to machines that already have
some golangci-lint. Compare the **whole** version token, exactly: a parser
that stops at the first `-` reports `2.12.2` for a host running
`2.12.2-rc1`, which compares equal to a `2.12.2` pin and skips the install
— the original defect, reintroduced through the comparison meant to fix
it.
- **After installing, re-resolve the binary the way callers resolve it** — - **After installing, re-resolve the binary the way callers resolve it** —
through `PATH`, not the path `go install` wrote to — and assert through `PATH`, not the directory the installer wrote to — and assert the
`--version` reports the pin. When it does not, fail non-zero and name the reported version is the pin. An installer that writes to `GOBIN` while a
path `command -v` actually found, the version it reports, and the different binary shadows it earlier in `PATH` genuinely succeeds and
directory the install wrote to. That is a condition a human has to fix by changes nothing any caller sees, which is worse than no fix: it converts a
hand, so bootstrap must not print success in it. Use `hash -r` first so known-stale tool into one everyone believes is pinned. Run `hash -r` first
the shell does not answer from its own lookup cache. Diagnose the cause so the shell does not answer from its own lookup cache, and when the
from the resolved path rather than asserting one: only a path **outside** assertion fails, name the path `command -v` found, the version it reports,
the install directory is shadowing. When the resolved path is inside it, and the directory the install wrote to. Diagnose from the resolved path
nothing is shadowing and telling the operator to delete that binary or rather than asserting a cause: only a path **outside** the install
reorder `PATH` sends them after a fault that does not exist. directory is shadowing.
- **A mis-parse must fall through to reinstall, never to a false match.** - **A mis-parse must fall through to reinstall, never to a false match.**
Absent binary, non-zero exit, empty output, and unrecognised output all Absent binary, non-zero exit, empty output and unrecognised output should
yield an empty string, which compares unequal to the pin. The failure all yield an empty string, which compares unequal to the pin. The failure
direction is always a redundant install, never a skipped one. direction is always a redundant install, never a skipped one.
- **Call it, and say so on success.** Two function definitions with no call - **Call it, and say so on success.** A function defined and never called is
site are a silent no-op that reproduces the original defect exactly: exit a silent no-op indistinguishable from success: exit 0, nothing installed,
0, nothing installed, no output, stale linter still resolved. A success no output. Both success branches must print a line naming the version.
path that prints nothing is byte-identical to that no-op — same exit
status, same empty output — so both success branches must print a Verifying such logic requires a negative control in an environment where a
confirmation naming the version. In a change about undetectable no-ops, shadowing binary exists earlier in `PATH` than the install target — without
"it printed nothing and exited 0" must not be the healthy signal. it the control passes against the naive compare-then-install form too and
proves nothing — plus a mis-parse control that feeds unparseable `--version`
output and confirms a reinstall. Run those controls against the block as a
consuming repo would adopt it: pasted into a `script/bootstrap`-shaped file
that is then executed, never by sourcing it and invoking the function
yourself. Driving the function directly tests something the artifact does
not do, and it is exactly how a missing call site passes every control while
the adopted snippet does nothing.
Keep it POSIX sh: no bashisms, no arrays, no `[[`, no `grep -P`. Keep it POSIX sh: no bashisms, no arrays, no `[[`, no `grep -P`.
**On the hash-pinning rule.** `@c0d3ddc9cf3faa61a4e378e879ece580256d76e5` is - **Superseded: the per-checkout `GOLANGCI_LINT_CACHE`/`TMPDIR` wrapper for
a commit hash, not a server-mutable version tag, and the go command verifies `script/lint`.** It existed only to make a host lint run trustworthy, and the
the fetched module against the checksum database — the mechanism the containerised-lint rule above removes the host run. Delete the wrapper, the
hash-pinning rule at the top of this document already names as acceptable `--allow-serial-runners` flag, and `.lint-cache/` from both `.gitignore` and
for Go modules. Note that `go install pkg@version` runs in module-aware mode `.dockerignore`. Two of its conclusions outlive it: **`GOCACHE` does not need
ignoring the `go.mod` in the current directory or any parent, so no repo isolating** (measured — content-addressed, no foreign paths in its entries, no
`go.sum` is consulted for this install; the checksum database is what global lock), and **verifying lint plumbing requires paired controls** run
verifies it. The linter is a bootstrap prerequisite rather than part of any against the artifact as a consuming repo would adopt it, since a control that
repo's module graph, which is why the canonical form installs it by passes against the broken form proves nothing.
commit-pinned ref instead of declaring it in `go.mod`. Whether a `go.mod`
tool dependency — which would pin the hash in a committed, reviewable file
instead — should replace this is an open decision, tracked at
[prompts#37](https://git.eeqj.de/sneak/prompts/issues/37).
**Keep `GOLANGCI_LINT_VERSION` and the ref in sync.** The ref is a hash and - **Interim rule for reading a lint result produced on the host, in a repo that
carries no readable version, so the expected version is a separate string, has not yet adopted the containerised lint above.** A lint run is **VOID**
and it must be exactly what `--version` prints for that ref — the comparison unless both hold:
is an exact match on the whole version token. When the pinned commit carries
a release tag the go command resolves the hash to that tag, so the string is
simply the release number, `2.12.2` here. When it does not, the go command
falls back to a pseudo-version and the binary reports something like
`2.12.3-0.20260506110758-c0d3ddc9cf3f`; that compares exactly like any other
string, so it works, but it cannot be known without building the binary once
and reading `--version` off it. Prefer pins on tagged releases for that
reason — the expected string is then derivable from the ref — not because
the comparison cannot handle the alternative.
Because the comparison covers the whole token, a pre-release is never
confused with its release: a host carrying `2.12.2-rc1` against a `2.12.2`
pin compares unequal and gets reinstalled. This matters more than it looks,
because a pre-release tag is still a tag, so a rule requiring merely that
the pin be tagged would not catch it.
**Verifying a change to this logic requires a negative control run in an
environment where a shadowing binary exists earlier in `PATH` than the
install target.** Without that, the control passes against the naive
compare-then-install form as well and therefore proves nothing. Also check
the mis-parse direction by feeding it unparseable `--version` output and
confirming it reinstalls rather than reporting a match.
**Run those controls against the block as a consuming repo would adopt it**
— pasted into a `script/bootstrap`-shaped file that is then executed — not
by sourcing it and invoking the function yourself. Driving the function
directly tests something the artifact does not do, and it is exactly how a
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"
# Capture files, per INVOCATION and not per checkout. Two runs in the same
# checkout would otherwise redirect into one pair of fixed paths, opened
# O_TRUNC before the linter even starts, and each would print and scan the
# other's output — a run reporting a result that is not its own, which is
# the whole defect this bullet exists to close, one layer up from where it
# was closed. That case is not hypothetical here: two runs in the same
# checkout is exactly what --allow-serial-runners below exists to support,
# and serialising the linter does not serialise the shell's redirections
# or the grep and cat that read them. mktemp rather than $$: two
# containerised runs over one bind-mounted checkout are in separate PID
# namespaces and can both be PID 7, which puts the collision back.
LINT_OUT="$(mktemp "$LINT_STATE/run.out.XXXXXX")"
LINT_ERR="$(mktemp "$LINT_STATE/run.err.XXXXXX")"
# Print whatever the linter had written before the interruption, then exit
# 128+signal. The exit is what makes this handler TERMINATING, and that is
# the point: a signal-trap handler that returns RESUMES the script. With
# the capture files already deleted, execution would fall into the grep
# below against a missing file, take the not-a-collision branch, and report
# the FINDINGS exit status with empty output for a run that was killed —
# after deleting the findings it was about to print. That is why cleanup
# is on EXIT only. Killing a run is not hypothetical here: it is the stated
# mitigation for the unbounded wait --allow-serial-runners can produce, and
# it is what Ctrl-C on a make check does.
#
# All three writes are best-effort, and the `|| :` on each is load-bearing
# rather than defensive habit. Under set -e a failed write aborts the
# function BEFORE exit "$1", and the shell then exits 1 — the findings
# status, on a run that analysed nothing. SIGHUP is precisely the case
# where writing fails: once the controlling terminal is gone the writes
# return EIO. Guarding only the two cats is not enough, because with the
# capture files empty the cats write nothing and succeed, and the echo is
# what fails.
lint_interrupted() {
if [ -f "$LINT_ERR" ]; then cat "$LINT_ERR" >&2 || :; fi
if [ -f "$LINT_OUT" ]; then cat "$LINT_OUT" || :; fi
echo "lint: interrupted by a signal, so nothing was completed." \
"This is NOT a lint result." >&2 || :
exit "$1"
}
# The EXIT trap still fires on the way out of a terminating handler, so
# cleanup happens exactly once on every path. `|| :` because a failing rm
# — an unwritable state directory is enough — would otherwise change the
# exit status of an otherwise clean run under set -e.
trap 'rm -f "$LINT_OUT" "$LINT_ERR" || :' EXIT
trap 'lint_interrupted 129' HUP
trap 'lint_interrupted 130' INT
trap 'lint_interrupted 143' TERM
# 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() {
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),
# which does separate it from findings at 1, but run.go returns it
# as a plain error that Execute maps to Failure like every other
# error at that level, so 3 cannot separate a collision from a
# genuine linter failure — an unknown linter name, an unknown flag,
# malformed config YAML all exit 3 too. (An unparseable Go source
# file does not: that is reported as typecheck issues and exits 1.)
# Retrying on 3 would retry real failures into a void.
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.
- **Capture stdout and stderr to per-INVOCATION paths, and clean them up.**
Two fixed paths under the checkout are one pair for every run in it, and
the redirections truncate them before the linter starts, so two
overlapping runs print and scan each other's output.
`--allow-serial-runners` does not prevent this — it serialises the linter,
not the shell — and the overlap it exists to support is precisely
`script/precommit` against a `make check` in one checkout. The observed
shapes are a run printing the other's `0 issues.` while its own linter
found something, and a lock error erased before `grep` reads it, so the
retry never fires and the void run returns as a result. Both are a run
reporting a result that is not its own, which is this bullet's entire
subject reintroduced one layer above where it was fixed. Use `mktemp`
under the state directory rather than `$$`: two containerised runs over
one bind-mounted checkout sit in separate PID namespaces and can hold the
same low PID, which puts the collision back on exactly the fleet's
arrangement. `$$` is acceptable only where `mktemp` is unavailable and
that arrangement is ruled out.
- **Clean up on `EXIT` only, and give each signal a TERMINATING handler.** A
signal-trap handler that does not exit **resumes** the script: with the
capture files already deleted, the run falls into the `grep` against a
missing file, takes the not-a-collision branch, and reports the
**findings** exit status with empty output for a run that was killed —
having deleted the findings it was about to print. Measured: `TERM`, `INT`
and `HUP` all returning 1 with empty stdout, against 143, 130 and 129 for
the same block without the handler. Exit `128+signal` instead, and let the
`EXIT` trap do the cleanup on the way out. The handler also makes a
**best-effort** attempt to print what the linter had already written —
best-effort because under `SIGHUP` the terminal is typically gone and
every write returns `EIO`, in which case nothing is printed and only the
status carries the message. Each of those writes needs its own `|| :`:
under `set -e` a failed write aborts the handler before it reaches `exit`,
and the shell then exits 1, which is the findings status on a run that
analysed nothing. Guarding only the `cat`s is not enough — with the
capture files empty they write nothing and succeed, and the `echo` is what
fails. Put `|| :` on the `rm` in the `EXIT` trap for the same reason: an
unwritable state directory makes it fail, and a failing `EXIT` trap under
`set -e` turns an otherwise clean run into exit 1, measured in both `dash`
and `bash`.
- **A signal must reach the LINTER, not just the wrapper.** POSIX defers a
trap until the running foreground command completes, so
`kill -TERM <wrapper pid>` does nothing at all while `golangci-lint` is
running — measured still alive three seconds later, where the same block
without a handler dies immediately at 143. Ctrl-C is unaffected because
the terminal signals the whole process group. This matters exactly where
the handler is supposed to help: the unbounded `--allow-serial-runners`
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.
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 - the output contains no `parallel golangci-lint is running`, and
- no reported file path begins with `../`, and none is an absolute path - no reported file path begins with `../`, and none is an absolute path
outside the tree the run was launched from. outside the tree the run was launched from.
@@ -1030,8 +771,9 @@ style conventions are in separate documents:
that mode has been observed, and nobody should go chasing it**; the point is that mode has been observed, and nobody should go chasing it**; the point is
the reach of the tests, not a claim that the mode exists. They are a filter the reach of the tests, not a claim that the mode exists. They are a filter
for the loud mode, not a proof of soundness — which is the whole argument 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 for containerising the linter instead of documenting a discipline that
depends on every agent remembering to apply it. depends on every agent remembering to apply it. Adopt the rule above and
this one stops applying to the repo entirely.
- When pinning images or packages by hash, add a comment above the reference - When pinning images or packages by hash, add a comment above the reference
with the version and date (YYYY-MM-DD). with the version and date (YYYY-MM-DD).

View File

@@ -1,27 +1,22 @@
#!/bin/sh #!/bin/sh
# script/cibuild: run the CI build. The Dockerfile runs script/check, but # script/cibuild: run the CI build. The Dockerfile runs script/check, but
# that only proves anything because CHECK_EPOCH is a fresh nonce on every # that only proves anything because CHECK_EPOCH is a fresh nonce on every
# invocation: without it Docker serves the check layer from cache on an # invocation: without it Docker serves the check layer from cache and the
# unchanged tree and the build exits 0 without running the suite. # build exits 0 without running the suite.
set -eu set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
main() { main() {
cd "$ROOT" cd "$ROOT"
# Assign on its own line: a failing command substitution inside an # Both assignments on their own line: a failing command substitution
# argument does not trip `set -e`, which would silently degrade the # inside an argument does not trip `set -e`, so the inline form
# nonce to an empty constant. `$$` is required because busybox `date` # degrades silently to an empty constant. `$$` because busybox `date`
# drops %N without erroring. # drops %N without erroring. VERSION is computed here because
# .dockerignore excludes .git, so `git describe` in a build stage
# yields an empty version without failing; the guard below is the
# single place the fallback is applied.
epoch="$(date +%s%N)$$" epoch="$(date +%s%N)$$"
# VERSION must be computed here, on the host: .dockerignore excludes
# .git, so `git describe` cannot run in any build stage and fails
# quietly there rather than erroring. Same own-line discipline as the
# epoch. `|| true` keeps a failing describe from tripping `set -e`
# and leaves the value empty; the guard below is then the single
# place the fallback is applied, and it does fire — on an export with
# no .git, or a repo with no commits yet. `unknown` is visibly wrong
# in a binary in a way that an empty version is not.
version="$(git describe --tags --always --dirty 2>/dev/null || true)" version="$(git describe --tags --always --dirty 2>/dev/null || true)"
[ -n "$version" ] || version="unknown" [ -n "$version" ] || version="unknown"
docker build \ docker build \

View File

@@ -11,19 +11,13 @@ ROOT="$(cd "$SCRIPT_DIR/.." && pwd -P)"
main() { main() {
cd "$ROOT" cd "$ROOT"
# Assign on its own line: a failing command substitution inside an # Both assignments on their own line: a failing command substitution
# argument does not trip `set -e`, which would silently degrade the # inside an argument does not trip `set -e`, so the inline form
# nonce to an empty constant. `$$` is required because busybox `date` # degrades silently to an empty constant. `$$` because busybox `date`
# drops %N without erroring. # drops %N without erroring. VERSION is computed here because
# .dockerignore excludes .git, so `git describe` in a build stage
# yields an empty version without failing.
epoch="$(date +%s%N)$$" epoch="$(date +%s%N)$$"
# VERSION must be computed here, on the host: .dockerignore excludes
# .git, so `git describe` cannot run in any build stage and fails
# quietly there rather than erroring. Same own-line discipline as the
# epoch. `|| true` keeps a failing describe from tripping `set -e`
# and leaves the value empty; the guard below is then the single
# place the fallback is applied, and it does fire — on an export with
# no .git, or a repo with no commits yet. `unknown` is visibly wrong
# in a binary in a way that an empty version is not.
version="$(git describe --tags --always --dirty 2>/dev/null || true)" version="$(git describe --tags --always --dirty 2>/dev/null || true)"
[ -n "$version" ] || version="unknown" [ -n "$version" ] || version="unknown"
docker build \ docker build \

View File

@@ -1,13 +1,34 @@
#!/bin/sh #!/bin/sh
# script/lint: run the linter. # script/lint: run the linter. Inside a container, run it directly;
# on a host, build Dockerfile.lint so it runs in one anyway. The linter
# is never run on a developer host, where a shared result cache, a
# host-global lock and a stale toolchain make its answer untrustworthy.
#
# LINT_IN_CONTAINER is set by this repo's Dockerfiles and is the ONLY
# accepted signal. Do not add a /.dockerenv fallback: it is absent
# inside BuildKit RUN steps and present on hosts that are themselves
# containers, so it both misses and false-positives — and a false
# positive silently restores host linting.
set -eu set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
main() { main() {
cd "$ROOT" cd "$ROOT"
echo "Linting markdown files..."
yarn run prettier --check '**/*.md' --tab-width 4 --prose-wrap always if [ "${LINT_IN_CONTAINER:-}" = "1" ]; then
exec yarn run prettier --check '**/*.md' \
--tab-width 4 --prose-wrap always
fi
# Own line, and `$$` because busybox `date` drops %N silently.
# Without a fresh nonce the lint layer is cached and this exits 0
# having linted nothing.
epoch="$(date +%s%N)$$"
docker build \
--build-arg CHECK_EPOCH="$epoch" \
-f Dockerfile.lint \
.
} }
main "$@" main "$@"