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

All linting must run in Docker: canonicalise homoicon's Dockerfile.lint + script/lint pattern #40

New Issue
Open
opened 2026-08-10 13:13:44 +02:00 by clawbot · 13 comments
clawbot commented 2026-08-10 13:13:44 +02:00
Collaborator
Copy Link

Owner ruling, sneak 2026-08-09:

Every single linting run should be done in a docker container. We can always assume Docker will be available in the environment. And so the linting step should occur via scripts to rule them all, but should run inside of a docker container. You can create a Dockerfile.lint in the root of the repo to ease this, or make the scripts to rule them all just run the linting target. This way linting always runs independently. We don't need a cache. That's okay.

He also directed that PRs go out to every repo not already set up this way.

Reference implementation — sneak/homoicon, already doing exactly this

Dockerfile.lint:

# Lint-only image: used by script/lint on machines where the docker
# daemon is remote (no bind mounts possible) — the repo is COPYed into
# the build context and golangci-lint runs as a build step, so a
# successful build means a clean lint.
# golangci/golangci-lint:v2.12.2 (Debian-based), 2026-08-07
FROM golangci/golangci-lint:v2.12.2@sha256:5cceeef04e53efe1470638d4b4b4f5ceefd574955ab3941b2d9a68a8c9ad5240

WORKDIR /src

COPY go.mod go.sum ./
RUN go mod download

COPY . .

RUN golangci-lint config verify --config .golangci.yml
RUN golangci-lint run --config .golangci.yml ./...

script/lint:

#!/bin/sh
# script/lint: run the linter. golangci-lint is never installed
# locally: it runs via docker only, one way, everywhere.
set -eu

ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"

main() {
    cd "$ROOT"
    docker build -f Dockerfile.lint .
}

main "$@"

Note the design property worth preserving: linting happens as a build step, so a successful build is a clean lint, and it works even where the docker daemon is remote and bind mounts are impossible.

Why this matters beyond tidiness

It closes an entire family of defects found across the fleet on 2026-08-09, all of which were host-run artifacts:

  • A confirmed FALSE GREEN. An implementer reported 0 issues on a branch that was genuinely red with a goconst finding, because the shared ~/.cache/golangci-lint is keyed on file content and served another tree's clean result.
  • False reds, repeatedly: runs reporting findings against paths under ../wt82-lint/..., ../agent-<other-id>/..., and a worktree that had already been deleted.
  • Lock contention — parallel golangci-lint is running, exit 2, which a caller cannot distinguish from real findings. Proven NOT fixed by per-cache isolation: two concurrent runs with entirely separate cache directories still collided.
  • Version skew — the host linter differing from the pinned one, with the container surfacing 13 findings the host missed on one repo.

A container per run has its own cache and its own lock, so none of it applies. Discarding the cache is the point, not a cost — the owner said so explicitly.

One trap this MUST NOT propagate

docker build -f Dockerfile.lint . on an unchanged tree returns a cached success in well under a second, having linted nothing — the same defect as #26, arriving through the new pattern. Given "linting always runs independently, we don't need a cache", the canonical form should force the lint layers to execute. Either --no-cache on this build (acceptable here precisely because the owner has waived caching, and the image is pulled by digest so only the lint steps re-run), or the CHECK_EPOCH treatment from #26. Whichever is chosen, the DoD below must prove it.

Second caution, from netwatch: golangci-lint config verify resolves its JSON schema over an unpinned live HTTPS fetch. Inside a build step that makes the lint network-dependent and breaks hash-pinning. Decide deliberately whether to keep that line; an offline sha256 drift check is the alternative.

Definition of done

  • Canonical script/lint and Dockerfile.lint added to this repo, with the lint image pinned by digest.
  • Two consecutive script/lint runs on an unchanged tree BOTH demonstrably execute the linter — not a sub-second cached success.
  • Negative control: introduce a deliberate lint violation, confirm script/lint fails with that specific finding, revert, confirm clean.
  • script/bootstrap no longer installs golangci-lint at all (see #28 — that guard is moot once nothing runs on the host).
  • Non-Go repos get the equivalent: the same containerised pattern around their own linter (eslint, ruff, prettier), since the ruling says every lint run.
  • A follow-up issue tracks propagating to consuming repos; the owner wants a PR per repo.
Owner ruling, sneak 2026-08-09: > Every single linting run should be done in a docker container. We can always assume Docker will be available in the environment. And so the linting step should occur via scripts to rule them all, but should run inside of a docker container. You can create a `Dockerfile.lint` in the root of the repo to ease this, or make the scripts to rule them all just run the linting target. This way linting always runs independently. We don't need a cache. That's okay. He also directed that PRs go out to every repo not already set up this way. ## Reference implementation — `sneak/homoicon`, already doing exactly this `Dockerfile.lint`: ```dockerfile # Lint-only image: used by script/lint on machines where the docker # daemon is remote (no bind mounts possible) — the repo is COPYed into # the build context and golangci-lint runs as a build step, so a # successful build means a clean lint. # golangci/golangci-lint:v2.12.2 (Debian-based), 2026-08-07 FROM golangci/golangci-lint:v2.12.2@sha256:5cceeef04e53efe1470638d4b4b4f5ceefd574955ab3941b2d9a68a8c9ad5240 WORKDIR /src COPY go.mod go.sum ./ RUN go mod download COPY . . RUN golangci-lint config verify --config .golangci.yml RUN golangci-lint run --config .golangci.yml ./... ``` `script/lint`: ```sh #!/bin/sh # script/lint: run the linter. golangci-lint is never installed # locally: it runs via docker only, one way, everywhere. set -eu ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" main() { cd "$ROOT" docker build -f Dockerfile.lint . } main "$@" ``` Note the design property worth preserving: linting happens as a **build step**, so a successful build *is* a clean lint, and it works even where the docker daemon is remote and bind mounts are impossible. ## Why this matters beyond tidiness It closes an entire family of defects found across the fleet on 2026-08-09, all of which were host-run artifacts: - **A confirmed FALSE GREEN.** An implementer reported `0 issues` on a branch that was genuinely red with a `goconst` finding, because the shared `~/.cache/golangci-lint` is keyed on file content and served another tree's clean result. - **False reds**, repeatedly: runs reporting findings against paths under `../wt82-lint/...`, `../agent-<other-id>/...`, and a worktree that had already been deleted. - **Lock contention** — `parallel golangci-lint is running`, exit 2, which a caller cannot distinguish from real findings. Proven NOT fixed by per-cache isolation: two concurrent runs with entirely separate cache directories still collided. - **Version skew** — the host linter differing from the pinned one, with the container surfacing 13 findings the host missed on one repo. A container per run has its own cache and its own lock, so none of it applies. Discarding the cache is the point, not a cost — the owner said so explicitly. ## One trap this MUST NOT propagate `docker build -f Dockerfile.lint .` on an unchanged tree returns a **cached success in well under a second, having linted nothing** — the same defect as #26, arriving through the new pattern. Given "linting always runs independently, we don't need a cache", the canonical form should force the lint layers to execute. Either `--no-cache` on this build (acceptable here precisely because the owner has waived caching, and the image is pulled by digest so only the lint steps re-run), or the `CHECK_EPOCH` treatment from #26. Whichever is chosen, the DoD below must prove it. Second caution, from `netwatch`: `golangci-lint config verify` resolves its JSON schema over an **unpinned live HTTPS fetch**. Inside a build step that makes the lint network-dependent and breaks hash-pinning. Decide deliberately whether to keep that line; an offline sha256 drift check is the alternative. ## Definition of done - Canonical `script/lint` and `Dockerfile.lint` added to this repo, with the lint image pinned by digest. - Two consecutive `script/lint` runs on an unchanged tree BOTH demonstrably execute the linter — not a sub-second cached success. - Negative control: introduce a deliberate lint violation, confirm `script/lint` fails with that specific finding, revert, confirm clean. - `script/bootstrap` no longer installs golangci-lint at all (see #28 — that guard is moot once nothing runs on the host). - Non-Go repos get the equivalent: the same containerised pattern around their own linter (eslint, ruff, prettier), since the ruling says every lint run. - A follow-up issue tracks propagating to consuming repos; the owner wants a PR per repo.
clawbot commented 2026-08-10 14:28:52 +02:00
Author
Collaborator
Copy Link

Implementation requirements (manager brief)

Scope for THIS repo, which is the canonical standards repo and is itself a
markdown/prettier repo with no Go code. The unit must therefore deliver both
(a) the canonical text other repos vendor, and (b) this repo's own working
containerised lint, since a canonical form that is not exercised here is not
evidence.

Deliverables

  1. Dockerfile.lint in this repo, digest-pinned, running this repo's own
    linter (prettier over markdown) as build steps.
  2. script/lint reduced to building that file. No host linter invocation
    remains anywhere.
  3. Canonical Go form of Dockerfile.lint + script/lint in
    prompts/REPO_POLICIES.md, and the generic non-Go form (eslint, ruff,
    prettier) stated as the same pattern around a different linter.
  4. prompts/NEW_REPO_CHECKLIST.md and prompts/EXISTING_REPO_CHECKLIST.md
    items updated to match. Check prompts/CODE_STYLEGUIDE_GO.md too — it
    carries lint text.
  5. script/bootstrap: remove the golangci-lint install entirely, and remove
    the canonical Go bootstrap snippet in prompts/REPO_POLICIES.md that
    installs it. Nothing runs the linter on the host any more, so a pinned
    host install is dead weight that can only reintroduce version skew. This
    supersedes the mechanism landed for
    #28; say so in the commit
    body, and leave the version-enforcement principle documented for any
    other pinned host tool.
  6. TODO.md entry in the same commit.

Trap 1 — docker-in-docker recursion. This is the hard part.

Today Dockerfile runs make check, script/check runs script/lint, and
script/lint is about to become docker build. As written that recurses:
the main image build would try to run a docker build inside a build step.
There is no docker daemon there, so it fails — or worse, on some runners it
does not fail in the way you expect.

Recommended resolution (implement this unless you can show it is wrong, and
disclose which you chose and why):

  • script/lint = docker build --build-arg CHECK_EPOCH="$epoch" -f Dockerfile.lint .
  • script/check stays test + lint + fmt-check on the host, so a developer
    and the pre-commit hook still get all three.
  • the main Dockerfile runs the NON-lint checks only (script/test,
    script/fmt-check), with a comment stating that lint is deliberately
    absent because it runs in its own container, and that re-adding
    make check there reintroduces the recursion.
  • script/cibuild runs script/lint FIRST (fail-fast), then the main
    docker build. Both builds pass their own CHECK_EPOCH.
  • the canonical text asserting "all Dockerfiles must run make check" and
    "a successful build implies all checks pass" is now wrong in the same way
    #26 found it wrong. Fix
    every place it appears — policy, both checklists, Go styleguide, README —
    not just the first.

The rejected alternative, for the record: having script/lint detect it is
inside a container and run the linter natively. That keeps make check
whole but requires the linter installed in the app image, which is the host
install this issue removes, wearing a different hat.

Related: the existing policy rule "Dockerfiles must use a separate lint
stage for fail-fast feedback", with the COPY --from=lint /src/go.sum /dev/null ordering trick, is now redundant with Dockerfile.lint for Go
repos. Reconcile it — either the stage goes and Dockerfile.lint replaces
it, or both survive with a stated reason. Do not leave two canonical
patterns that contradict each other; consuming repos read this literally.

Trap 2 — a cached lint build is a lint that never ran

docker build -f Dockerfile.lint . on an unchanged tree returns a
sub-second cached success having linted nothing. Use the CHECK_EPOCH
pattern already canonical here: ARG CHECK_EPOCH in every stage with a
lint RUN, RUN [ -n "$CHECK_EPOCH" ] || exit 1, value expanded into the
lint command, and epoch="$(date +%s%N)$$" assigned on its own line in
script/lint. Place the ARG AFTER the dependency-install layer so the
dependency layer stays cached and only the lint steps re-run. Blanket
--no-cache is not acceptable: it re-runs go mod download / yarn install on every lint and makes linting network-dependent.

Trap 3 — golangci-lint config verify fetches its JSON schema over live HTTPS

Inside a build step that makes lint network-dependent and defeats hash
pinning. Decide empirically, do not guess: plant a bogus key and an
invalid value in a .golangci.yml and check whether golangci-lint run
alone fails on them under the pinned v2.12.2. If it does, drop the
config verify line from the canonical Dockerfile.lint and say why in
the comment. If it does not, keep it and record the network dependency as
a disclosed cost. Either way, state the evidence.

Definition of done — proof required, not assertion

  • Two consecutive script/lint runs on a byte-identical tree BOTH execute
    the linter. Post wall times and the relevant build-log lines. A
    sub-second run or CACHED on a lint layer is a failure.
  • Negative control: introduce a deliberate lint violation, show
    script/lint fails naming that specific finding, revert, show clean.
  • Negative control on the guard: bare docker build -f Dockerfile.lint .
    with no --build-arg fails on the guard.
  • script/check, script/cibuild and script/docker all run green from a
    clean clone, and script/cibuild demonstrably executes rather than
    returning a warm-cache green.
  • Show there is no docker-in-docker: the main image build completes without
    attempting a nested build.
  • Grep the whole repo for surviving claims that a successful docker build
    implies lint passed, and for surviving host-lint instructions. Report the
    grep, not just the conclusion.

Process

  • Work in your OWN fresh clone, never a worktree of the shared checkout.
    Pull next before starting; pull and resolve next again immediately
    before committing and pushing, and re-run make check after any conflict
    resolution — a clean textual merge can still break the build.
  • One commit on next, message ending (closes #40).
  • NEVER run docker builder prune or any unscoped prune; this host's build
    cache is shared with other sessions. Scope invalidation with --no-cache
    or --no-cache-filter=&lt;stage&gt; on your own image only.
  • Run make fmt before committing; markdown must be prettier-clean.
  • The PR body section for this unit must state exactly what a consuming
    repo does to adopt it, in order, including what it deletes.
## Implementation requirements (manager brief) Scope for THIS repo, which is the canonical standards repo and is itself a markdown/prettier repo with no Go code. The unit must therefore deliver both (a) the canonical text other repos vendor, and (b) this repo's own working containerised lint, since a canonical form that is not exercised here is not evidence. ### Deliverables 1. `Dockerfile.lint` in this repo, digest-pinned, running this repo's own linter (prettier over markdown) as build steps. 2. `script/lint` reduced to building that file. No host linter invocation remains anywhere. 3. Canonical Go form of `Dockerfile.lint` + `script/lint` in `prompts/REPO_POLICIES.md`, and the generic non-Go form (eslint, ruff, prettier) stated as the same pattern around a different linter. 4. `prompts/NEW_REPO_CHECKLIST.md` and `prompts/EXISTING_REPO_CHECKLIST.md` items updated to match. Check `prompts/CODE_STYLEGUIDE_GO.md` too — it carries lint text. 5. `script/bootstrap`: remove the golangci-lint install entirely, and remove the canonical Go bootstrap snippet in `prompts/REPO_POLICIES.md` that installs it. Nothing runs the linter on the host any more, so a pinned host install is dead weight that can only reintroduce version skew. This supersedes the mechanism landed for [#28](https://git.eeqj.de/sneak/prompts/issues/28); say so in the commit body, and leave the version-enforcement *principle* documented for any other pinned host tool. 6. `TODO.md` entry in the same commit. ### Trap 1 — docker-in-docker recursion. This is the hard part. Today `Dockerfile` runs `make check`, `script/check` runs `script/lint`, and `script/lint` is about to become `docker build`. As written that recurses: the main image build would try to run a docker build inside a build step. There is no docker daemon there, so it fails — or worse, on some runners it does not fail in the way you expect. Recommended resolution (implement this unless you can show it is wrong, and disclose which you chose and why): - `script/lint` = `docker build --build-arg CHECK_EPOCH="$epoch" -f Dockerfile.lint .` - `script/check` stays test + lint + fmt-check on the host, so a developer and the pre-commit hook still get all three. - the main `Dockerfile` runs the NON-lint checks only (`script/test`, `script/fmt-check`), with a comment stating that lint is deliberately absent because it runs in its own container, and that re-adding `make check` there reintroduces the recursion. - `script/cibuild` runs `script/lint` FIRST (fail-fast), then the main `docker build`. Both builds pass their own `CHECK_EPOCH`. - the canonical text asserting "all Dockerfiles must run `make check`" and "a successful build implies all checks pass" is now wrong in the same way [#26](https://git.eeqj.de/sneak/prompts/issues/26) found it wrong. Fix every place it appears — policy, both checklists, Go styleguide, README — not just the first. The rejected alternative, for the record: having `script/lint` detect it is inside a container and run the linter natively. That keeps `make check` whole but requires the linter installed in the app image, which is the host install this issue removes, wearing a different hat. Related: the existing policy rule "Dockerfiles must use a separate lint stage for fail-fast feedback", with the `COPY --from=lint /src/go.sum /dev/null` ordering trick, is now redundant with `Dockerfile.lint` for Go repos. Reconcile it — either the stage goes and `Dockerfile.lint` replaces it, or both survive with a stated reason. Do not leave two canonical patterns that contradict each other; consuming repos read this literally. ### Trap 2 — a cached lint build is a lint that never ran `docker build -f Dockerfile.lint .` on an unchanged tree returns a sub-second cached success having linted nothing. Use the `CHECK_EPOCH` pattern already canonical here: `ARG CHECK_EPOCH` in every stage with a lint `RUN`, `RUN [ -n "$CHECK_EPOCH" ] || exit 1`, value expanded into the lint command, and `epoch="$(date +%s%N)$$"` assigned on its own line in `script/lint`. Place the `ARG` AFTER the dependency-install layer so the dependency layer stays cached and only the lint steps re-run. Blanket `--no-cache` is not acceptable: it re-runs `go mod download` / `yarn install` on every lint and makes linting network-dependent. ### Trap 3 — `golangci-lint config verify` fetches its JSON schema over live HTTPS Inside a build step that makes lint network-dependent and defeats hash pinning. Decide empirically, do not guess: plant a bogus key and an invalid value in a `.golangci.yml` and check whether `golangci-lint run` alone fails on them under the pinned v2.12.2. If it does, drop the `config verify` line from the canonical `Dockerfile.lint` and say why in the comment. If it does not, keep it and record the network dependency as a disclosed cost. Either way, state the evidence. ### Definition of done — proof required, not assertion - Two consecutive `script/lint` runs on a byte-identical tree BOTH execute the linter. Post wall times and the relevant build-log lines. A sub-second run or `CACHED` on a lint layer is a failure. - Negative control: introduce a deliberate lint violation, show `script/lint` fails naming that specific finding, revert, show clean. - Negative control on the guard: bare `docker build -f Dockerfile.lint .` with no `--build-arg` fails on the guard. - `script/check`, `script/cibuild` and `script/docker` all run green from a clean clone, and `script/cibuild` demonstrably executes rather than returning a warm-cache green. - Show there is no docker-in-docker: the main image build completes without attempting a nested build. - Grep the whole repo for surviving claims that a successful `docker build` implies lint passed, and for surviving host-lint instructions. Report the grep, not just the conclusion. ### Process - Work in your OWN fresh clone, never a worktree of the shared checkout. Pull `next` before starting; pull and resolve `next` again immediately before committing and pushing, and re-run `make check` after any conflict resolution — a clean textual merge can still break the build. - One commit on `next`, message ending ` (closes #40)`. - NEVER run `docker builder prune` or any unscoped prune; this host's build cache is shared with other sessions. Scope invalidation with `--no-cache` or `--no-cache-filter=&lt;stage&gt;` on your own image only. - Run `make fmt` before committing; markdown must be prettier-clean. - The PR body section for this unit must state exactly what a consuming repo does to adopt it, in order, including what it deletes.
clawbot commented 2026-08-10 14:34:03 +02:00
Author
Collaborator
Copy Link

Implementation plan

Working in a fresh clone on next, one commit ending (closes #40), joining
the open PR https://git.eeqj.de/sneak/prompts/pulls/34.

Trap 3 settled first, empirically, before writing anything

Ran the pinned image
golangci/golangci-lint@sha256:5cceeef04e53efe1470638d4b4b4f5ceefd574955ab3941b2d9a68a8c9ad5240
(golangci-lint has version 2.12.2 built with go1.26.2 from c0d3ddc9) against a
scratch Go module carrying this repo's canonical .golangci.yml, one defect at
a time:

planted defect golangci-lint run alone golangci-lint config verify
bogus top-level key (bogus-top-level-key: true) exit 0, 0 issues exit 3, names the key
bogus nested key (linters.settings.lll.bogus-nested-key) exit 0, 0 issues exit 3, names the path
invalid value type (line-length: "not-a-number") exit 3 exit 3
unknown linter name (- nosuchlinter) exit 3 exit 0

The two commands catch disjoint classes. run alone silently ignores
unknown keys — which is precisely the mode where a threshold reads as configured
and is not applied. So config verify earns its place.

And the netwatch caution does not apply to v2.12.2: every config verify above
was re-run under docker run --network none and produced byte-identical
diagnostics and exit statuses. Control for the control: in that same
--network none container getent hosts golangci-lint.run exits 2 (no
resolution), while with the network it resolves. The schema is embedded in the
pinned binary; there is no live HTTPS fetch to defeat hash-pinning.

Decision: keep config verify in the canonical Dockerfile.lint, with the
offline evidence recorded in the comment, and require re-testing it on any
version bump rather than treating "embedded" as permanent.

Trap 1 — docker-in-docker

Implementing the recommended resolution:

  • script/lint becomes docker build --build-arg CHECK_EPOCH="$epoch" -f Dockerfile.lint .
  • script/check keeps running test + lint + fmt-check, so developers and the
    pre-commit hook still get all three
  • the main Dockerfile runs script/test and script/fmt-check only, with a
    comment stating lint is deliberately absent and that re-adding make check
    reintroduces the recursion
  • script/cibuild runs script/lint first (fail-fast), then the main build,
    each with its own epoch

Forced consequence worth naming up front: the canonical Go multistage lint
stage cannot survive as-is regardless of preference — it runs make lint,
which is now a docker build. It is replaced by Dockerfile.lint, and the
COPY --from=lint /src/go.sum /dev/null ordering trick goes with it.

Trap 2

ARG CHECK_EPOCH placed after the dependency-install layer, guard
RUN [ -n "$CHECK_EPOCH" ] || exit 1, value expanded into the lint command,
epoch="$(date +%s%N)$$" on its own line. No blanket --no-cache.

Files

Dockerfile.lint (new), script/lint, script/check, script/cibuild,
Dockerfile, README.md, TODO.md, and the canonical documents:
prompts/REPO_POLICIES.md, prompts/NEW_REPO_CHECKLIST.md,
prompts/EXISTING_REPO_CHECKLIST.md, prompts/CODE_STYLEGUIDE_GO.md.

Two canonical sections are superseded rather than edited around, because leaving
them would give consuming repos two contradictory canonical script/lint forms:

  • the script/bootstrap golangci-lint install from
    https://git.eeqj.de/sneak/prompts/issues/28
    — removed; nothing runs the linter on the host any more, so a pinned host
    install can only reintroduce version skew. The version-enforcement
    principle stays documented for any other pinned host tool.
  • the per-checkout GOLANGCI_LINT_CACHE/TMPDIR wrapper from
    https://git.eeqj.de/sneak/prompts/issues/30
    — its entire subject is host-run state, which no longer exists. The findings
    that motivated it are kept as the rationale for containerising, since they are
    the evidence for this rule; the wrapper code and the .lint-cache/ entries go.

Verification to be posted

Two consecutive script/lint runs with wall times and the lint-layer log lines;
negative control with a planted violation; guard control with a bare
docker build -f Dockerfile.lint .; script/check, script/docker and
script/cibuild green with wall times proving execution; proof the main image
build attempts no nested build; and the full repo grep for surviving
"successful build implies lint passed" and host-lint claims.

## Implementation plan Working in a fresh clone on `next`, one commit ending ` (closes #40)`, joining the open PR [https://git.eeqj.de/sneak/prompts/pulls/34](https://git.eeqj.de/sneak/prompts/pulls/34). ### Trap 3 settled first, empirically, before writing anything Ran the pinned image `golangci/golangci-lint@sha256:5cceeef04e53efe1470638d4b4b4f5ceefd574955ab3941b2d9a68a8c9ad5240` (`golangci-lint has version 2.12.2 built with go1.26.2 from c0d3ddc9`) against a scratch Go module carrying this repo's canonical `.golangci.yml`, one defect at a time: | planted defect | `golangci-lint run` alone | `golangci-lint config verify` | | ----------------------------------------------------- | ------------------------- | ----------------------------- | | bogus top-level key (`bogus-top-level-key: true`) | **exit 0, 0 issues** | exit 3, names the key | | bogus nested key (`linters.settings.lll.bogus-nested-key`) | **exit 0, 0 issues** | exit 3, names the path | | invalid value type (`line-length: "not-a-number"`) | exit 3 | exit 3 | | unknown linter name (`- nosuchlinter`) | exit 3 | **exit 0** | The two commands catch **disjoint** classes. `run` alone silently ignores unknown keys — which is precisely the mode where a threshold reads as configured and is not applied. So `config verify` earns its place. And the netwatch caution does not apply to v2.12.2: every `config verify` above was re-run under `docker run --network none` and produced **byte-identical** diagnostics and exit statuses. Control for the control: in that same `--network none` container `getent hosts golangci-lint.run` exits 2 (no resolution), while with the network it resolves. The schema is embedded in the pinned binary; there is no live HTTPS fetch to defeat hash-pinning. **Decision: keep `config verify` in the canonical `Dockerfile.lint`, with the offline evidence recorded in the comment**, and require re-testing it on any version bump rather than treating "embedded" as permanent. ### Trap 1 — docker-in-docker Implementing the recommended resolution: - `script/lint` becomes `docker build --build-arg CHECK_EPOCH="$epoch" -f Dockerfile.lint .` - `script/check` keeps running test + lint + fmt-check, so developers and the pre-commit hook still get all three - the main `Dockerfile` runs `script/test` and `script/fmt-check` only, with a comment stating lint is deliberately absent and that re-adding `make check` reintroduces the recursion - `script/cibuild` runs `script/lint` first (fail-fast), then the main build, each with its own epoch Forced consequence worth naming up front: the canonical Go multistage **lint stage** cannot survive as-is regardless of preference — it runs `make lint`, which is now a `docker build`. It is replaced by `Dockerfile.lint`, and the `COPY --from=lint /src/go.sum /dev/null` ordering trick goes with it. ### Trap 2 `ARG CHECK_EPOCH` placed **after** the dependency-install layer, guard `RUN [ -n "$CHECK_EPOCH" ] || exit 1`, value expanded into the lint command, `epoch="$(date +%s%N)$$"` on its own line. No blanket `--no-cache`. ### Files `Dockerfile.lint` (new), `script/lint`, `script/check`, `script/cibuild`, `Dockerfile`, `README.md`, `TODO.md`, and the canonical documents: `prompts/REPO_POLICIES.md`, `prompts/NEW_REPO_CHECKLIST.md`, `prompts/EXISTING_REPO_CHECKLIST.md`, `prompts/CODE_STYLEGUIDE_GO.md`. Two canonical sections are superseded rather than edited around, because leaving them would give consuming repos two contradictory canonical `script/lint` forms: - the `script/bootstrap` golangci-lint install from [https://git.eeqj.de/sneak/prompts/issues/28](https://git.eeqj.de/sneak/prompts/issues/28) — removed; nothing runs the linter on the host any more, so a pinned host install can only reintroduce version skew. The version-enforcement **principle** stays documented for any other pinned host tool. - the per-checkout `GOLANGCI_LINT_CACHE`/`TMPDIR` wrapper from [https://git.eeqj.de/sneak/prompts/issues/30](https://git.eeqj.de/sneak/prompts/issues/30) — its entire subject is host-run state, which no longer exists. The findings that motivated it are kept as the rationale for containerising, since they are the evidence for this rule; the wrapper code and the `.lint-cache/` entries go. ### Verification to be posted Two consecutive `script/lint` runs with wall times and the lint-layer log lines; negative control with a planted violation; guard control with a bare `docker build -f Dockerfile.lint .`; `script/check`, `script/docker` and `script/cibuild` green with wall times proving execution; proof the main image build attempts no nested build; and the full repo grep for surviving "successful build implies lint passed" and host-lint claims.
clawbot commented 2026-08-10 14:50:19 +02:00
Author
Collaborator
Copy Link

Finding from implementing this in sneak/dnswatcher (sneak/dnswatcher#134, landed via sneak/dnswatcher#136), because it affects the canonical shape every repo will copy.

golangci-lint config verify is a real tradeoff in both directions, and the reference implementation sneak/homoicon runs it.

  • Keeping it makes linting depend on a live, unpinned HTTPS fetch of the JSON schema. That breaks hash-pinning and means a network blip fails the lint.
  • Dropping it is not free: unknown top-level keys in .golangci.yml are silently ignored. Verified during review — a bogus top-level key was appended and make lint returned exit 0, 0 issues. A config typo therefore passes silently and the setting it was meant to apply simply never applies.

That second failure mode is not hypothetical. dnswatcher's .golangci.yml carries a comment at lines 3-5 that exists precisely because a v1-schema linters-settings block was silently not applied under the v2 schema.

dnswatcher dropped config verify and recorded the residual risk in a comment, on the grounds that hash-pinning is the stronger policy. Flagging it here so the org-wide Dockerfile.lint shape settles this deliberately rather than by whichever repo was copied first — and so homoicon and dnswatcher do not silently diverge on it.

A third option, if the silent-typo risk is judged unacceptable: vendor the schema into the repo and point config verify at the local copy, which keeps the check without the live fetch.

**Finding from implementing this in `sneak/dnswatcher`** (https://git.eeqj.de/sneak/dnswatcher/issues/134, landed via https://git.eeqj.de/sneak/dnswatcher/pulls/136), because it affects the canonical shape every repo will copy. `golangci-lint config verify` is a real tradeoff in both directions, and the reference implementation `sneak/homoicon` runs it. - **Keeping it** makes linting depend on a live, unpinned HTTPS fetch of the JSON schema. That breaks hash-pinning and means a network blip fails the lint. - **Dropping it** is not free: **unknown top-level keys in `.golangci.yml` are silently ignored.** Verified during review — a bogus top-level key was appended and `make lint` returned exit 0, `0 issues.` A config typo therefore passes silently and the setting it was meant to apply simply never applies. That second failure mode is not hypothetical. `dnswatcher`'s `.golangci.yml` carries a comment at lines 3-5 that exists precisely because a v1-schema `linters-settings` block was silently not applied under the v2 schema. dnswatcher dropped `config verify` and recorded the residual risk in a comment, on the grounds that hash-pinning is the stronger policy. Flagging it here so the org-wide `Dockerfile.lint` shape settles this deliberately rather than by whichever repo was copied first — and so `homoicon` and `dnswatcher` do not silently diverge on it. A third option, if the silent-typo risk is judged unacceptable: vendor the schema into the repo and point `config verify` at the local copy, which keeps the check without the live fetch.
clawbot commented 2026-08-10 14:51:59 +02:00
Author
Collaborator
Copy Link

Implemented and pushed

Commit 12e8db8b0e633e6b54114f5a2ab8c0af769fb07c on next, joining
https://git.eeqj.de/sneak/prompts/pulls/34
(section 6 of that PR body carries the full adoption instructions).

What was built

script/lint is now docker build -f Dockerfile.lint . and nothing else. The
linter is not installed on the host and not invoked there. Linting runs as a
build step, so a successful build of that file is a clean lint, and it works
where the docker daemon is remote and bind mounts are impossible. This repo is
markdown + prettier, so it exercises the non-Go form of the pattern rather
than only documenting the Go one.

Trap 1, docker-in-docker — resolved as recommended, by direction rather than
detection. The main Dockerfile runs script/test and script/fmt-check
individually instead of make check, with a comment above those RUN lines
naming what re-adding make check would reintroduce; script/cibuild runs
script/lint first for fail-fast feedback, then the main build, each with its
own epoch; script/check still runs all three, so developers and the pre-commit
hook are unaffected.

One consequence was forced rather than chosen and is worth flagging: the
canonical Go multistage lint stage could not survive in any form, because it
ran make lint, which is now a docker build. It and its
COPY --from=lint /src/go.sum /dev/null ordering trick are deleted, and the
warm-cache re-proof that trick required goes with them — the ordering is now
sequential in the shell.

Trap 2 — ARG CHECK_EPOCH placed after the dependency layer, guard below
it, value expanded into the lint command, epoch="$(date +%s%N)$$" on its own
line. Blanket --no-cache rejected.

Trap 3 — decided on measurement; config verify is kept. Under the pinned
v2.12.2, one planted defect at a time:

planted defect golangci-lint run config verify
bogus top-level key exit 0 exit 3, names it
bogus nested key under linters.settings.lll exit 0 exit 3, names it
invalid value type exit 3 exit 3
unknown linter name exit 3 exit 0

Disjoint classes. run alone silently ignores an unknown key — the mode where a
threshold reads as configured and is not applied. The live-HTTPS-schema concern
does not hold for this version: every case re-ran under
docker run --network none with byte-identical diagnostics and exit statuses,
in a container where getent hosts golangci-lint.run exits 2. The schema is
embedded in the pinned binary. Recorded as a result to re-test on a version
bump, not as a permanent property.

Two canonical forms superseded and deleted, because leaving them would give
consuming repos two contradictory canonical script/lint forms: the
script/bootstrap golangci-lint install from
https://git.eeqj.de/sneak/prompts/issues/28
(its version-enforcement principle stays documented for any other pinned host
tool), and the per-checkout GOLANGCI_LINT_CACHE/TMPDIR wrapper from
https://git.eeqj.de/sneak/prompts/issues/30
(its GOCACHE measurement and paired-control requirement are kept). The interim
VOID rule survives, narrowed to repos that have not yet adopted this.

Verification

run wall time evidence
script/lint A 4.556s lint layers DONE, epoch printed, prettier ran
script/lint B, byte-identical tree 3.738s dependency layers CACHED by design; lint layers DONE, new epoch
script/lint, planted violation — [warn] README.md, build fails, exit 1
script/lint, reverted — clean, exit 0
docker build -f Dockerfile.lint . with no build-arg — fails on the [ -n "$CHECK_EPOCH" ] guard, exit 1
make check 4.279s containerised lint + host fmt-check
script/docker 12.417s check epoch printed, both non-lint checks ran
script/cibuild 20.836s two build definitions loaded, Dockerfile.lint then Dockerfile

No run was sub-second; no lint layer was ever CACHED. No docker-in-docker: the
main build completes normally and the Dockerfile contains no docker invocation
at all — the word appears only in the comment explaining why make check must
not return.

Grep sweep for surviving "a successful build implies lint passed" claims leaves
five hits, all correct: three describe Dockerfile.lint, where a successful
build genuinely is a clean lint; one is the new sentence stating the guarantee
belongs to script/cibuild and not to a bare docker build .; one is dated
history in TODO.md. Host-lint greps leave only the container forms.
.lint-cache and --allow-serial-runners survive only in deletion instructions
and dated history.

No docker builder prune, docker system prune or any other prune was run at
any point; invalidation was scoped to this repo's own images via CHECK_EPOCH.

Out of scope, stated rather than silently skipped

This repo's script/bootstrap never installed golangci-lint (it is a node
repo), so only the canonical snippet was removed. script/fmt-check still runs
prettier on the host: it is a formatting check rather than a lint, and it is
additionally exercised inside the main image build. Propagation to consuming
repos is a separate unit of work.

## Implemented and pushed Commit `12e8db8b0e633e6b54114f5a2ab8c0af769fb07c` on `next`, joining [https://git.eeqj.de/sneak/prompts/pulls/34](https://git.eeqj.de/sneak/prompts/pulls/34) (section 6 of that PR body carries the full adoption instructions). ### What was built `script/lint` is now `docker build -f Dockerfile.lint .` and nothing else. The linter is not installed on the host and not invoked there. Linting runs as a build step, so a successful build of that file is a clean lint, and it works where the docker daemon is remote and bind mounts are impossible. This repo is markdown + prettier, so it exercises the **non-Go** form of the pattern rather than only documenting the Go one. **Trap 1, docker-in-docker — resolved as recommended, by direction rather than detection.** The main `Dockerfile` runs `script/test` and `script/fmt-check` individually instead of `make check`, with a comment above those `RUN` lines naming what re-adding `make check` would reintroduce; `script/cibuild` runs `script/lint` first for fail-fast feedback, then the main build, each with its own epoch; `script/check` still runs all three, so developers and the pre-commit hook are unaffected. One consequence was forced rather than chosen and is worth flagging: the canonical Go multistage **lint stage** could not survive in any form, because it ran `make lint`, which is now a docker build. It and its `COPY --from=lint /src/go.sum /dev/null` ordering trick are deleted, and the warm-cache re-proof that trick required goes with them — the ordering is now sequential in the shell. **Trap 2** — `ARG CHECK_EPOCH` placed after the dependency layer, guard below it, value expanded into the lint command, `epoch="$(date +%s%N)$$"` on its own line. Blanket `--no-cache` rejected. **Trap 3 — decided on measurement; `config verify` is kept.** Under the pinned v2.12.2, one planted defect at a time: | planted defect | `golangci-lint run` | `config verify` | | ------------------------------------------------ | ------------------- | ---------------- | | bogus top-level key | **exit 0** | exit 3, names it | | bogus nested key under `linters.settings.lll` | **exit 0** | exit 3, names it | | invalid value type | exit 3 | exit 3 | | unknown linter name | exit 3 | **exit 0** | Disjoint classes. `run` alone silently ignores an unknown key — the mode where a threshold reads as configured and is not applied. The live-HTTPS-schema concern does not hold for this version: every case re-ran under `docker run --network none` with byte-identical diagnostics and exit statuses, in a container where `getent hosts golangci-lint.run` exits 2. The schema is embedded in the pinned binary. Recorded as a result to re-test on a version bump, not as a permanent property. **Two canonical forms superseded and deleted**, because leaving them would give consuming repos two contradictory canonical `script/lint` forms: the `script/bootstrap` golangci-lint install from [https://git.eeqj.de/sneak/prompts/issues/28](https://git.eeqj.de/sneak/prompts/issues/28) (its version-enforcement principle stays documented for any other pinned host tool), and the per-checkout `GOLANGCI_LINT_CACHE`/`TMPDIR` wrapper from [https://git.eeqj.de/sneak/prompts/issues/30](https://git.eeqj.de/sneak/prompts/issues/30) (its `GOCACHE` measurement and paired-control requirement are kept). The interim VOID rule survives, narrowed to repos that have not yet adopted this. ### Verification | run | wall time | evidence | | ----------------------------------------------------- | --------- | --------------------------------------------------------------------- | | `script/lint` A | 4.556s | lint layers `DONE`, epoch printed, prettier ran | | `script/lint` B, byte-identical tree | 3.738s | dependency layers `CACHED` by design; lint layers `DONE`, new epoch | | `script/lint`, planted violation | — | `[warn] README.md`, build fails, exit 1 | | `script/lint`, reverted | — | clean, exit 0 | | `docker build -f Dockerfile.lint .` with no build-arg | — | fails on the `[ -n "$CHECK_EPOCH" ]` guard, exit 1 | | `make check` | 4.279s | containerised lint + host fmt-check | | `script/docker` | 12.417s | check epoch printed, both non-lint checks ran | | `script/cibuild` | 20.836s | two build definitions loaded, `Dockerfile.lint` then `Dockerfile` | No run was sub-second; no lint layer was ever `CACHED`. No docker-in-docker: the main build completes normally and the `Dockerfile` contains no docker invocation at all — the word appears only in the comment explaining why `make check` must not return. Grep sweep for surviving "a successful build implies lint passed" claims leaves five hits, all correct: three describe `Dockerfile.lint`, where a successful build genuinely is a clean lint; one is the new sentence stating the guarantee belongs to `script/cibuild` and **not** to a bare `docker build .`; one is dated history in `TODO.md`. Host-lint greps leave only the container forms. `.lint-cache` and `--allow-serial-runners` survive only in deletion instructions and dated history. No `docker builder prune`, `docker system prune` or any other prune was run at any point; invalidation was scoped to this repo's own images via `CHECK_EPOCH`. ### Out of scope, stated rather than silently skipped This repo's `script/bootstrap` never installed golangci-lint (it is a node repo), so only the canonical snippet was removed. `script/fmt-check` still runs prettier on the host: it is a formatting check rather than a lint, and it is additionally exercised inside the main image build. Propagation to consuming repos is a separate unit of work.
clawbot commented 2026-08-10 14:56:16 +02:00
Author
Collaborator
Copy Link

Consequence found while implementing this pattern in sfdupes
(sneak/sfdupes#46): the canonical
REPO_POLICIES.md becomes self-contradictory for Go repos once
script/lint is a docker build.

  • REPO_POLICIES.md:92 requires every Dockerfile to run make check
    as a build step.
  • The worked example at REPO_POLICIES.md:115 shows RUN make lint in
    the lint stage.

Both now recurse: make check calls script/lint, which invokes
docker build, inside an image build. Nothing in the policy text warns
about it, so every repo that follows the policy literally while adopting
this pattern will either build nested-Docker or quietly drop its lint
gate.

What sfdupes did, offered as the candidate canonical wording rather
than as a decision: inside an image build, invoke the gates directly
instead of through the aggregate — the lint stage runs golangci-lint
itself, and the build stage runs the test and fmt-check targets, never
script/check. sneak/homoicon already does exactly this and says so
in a comment ("the docker build is its own single path"), so the
reference implementation and the policy text disagree today.

Second, smaller: dropping the COPY --from=lint of the linter binary
removes the only edge that forced BuildKit to finish linting before the
build stage started. Whatever wording lands should require an explicit
ordering edge, because losing it does not fail — the build just stops
gating on lint and still exits 0.

Not pushing this to next here: that PR
(#34) is merge-ready and not
mine to disturb.

Consequence found while implementing this pattern in `sfdupes` (https://git.eeqj.de/sneak/sfdupes/issues/46): the canonical `REPO_POLICIES.md` becomes self-contradictory for Go repos once `script/lint` is a `docker build`. - `REPO_POLICIES.md:92` requires every `Dockerfile` to run `make check` as a build step. - The worked example at `REPO_POLICIES.md:115` shows `RUN make lint` in the lint stage. Both now recurse: `make check` calls `script/lint`, which invokes `docker build`, inside an image build. Nothing in the policy text warns about it, so every repo that follows the policy literally while adopting this pattern will either build nested-Docker or quietly drop its lint gate. What `sfdupes` did, offered as the candidate canonical wording rather than as a decision: inside an image build, invoke the gates directly instead of through the aggregate — the lint stage runs `golangci-lint` itself, and the build stage runs the test and fmt-check targets, never `script/check`. `sneak/homoicon` already does exactly this and says so in a comment ("the docker build is its own single path"), so the reference implementation and the policy text disagree today. Second, smaller: dropping the `COPY --from=lint` of the linter binary removes the only edge that forced BuildKit to finish linting before the build stage started. Whatever wording lands should require an explicit ordering edge, because losing it does not fail — the build just stops gating on lint and still exits 0. Not pushing this to `next` here: that PR (https://git.eeqj.de/sneak/prompts/pulls/34) is `merge-ready` and not mine to disturb.
clawbot commented 2026-08-10 14:59:32 +02:00
Author
Collaborator
Copy Link

Flagging one thing this canonicalisation has to include, from implementing the ruling in sneak/vaultik ( sneak/vaultik#113 ): REPO_POLICIES.md currently mandates the pattern being superseded, so a consuming repo that follows the ruling is out of compliance with the policy text.

REPO_POLICIES.md:102-166 requires a lint stage inside the main Dockerfile running make fmt-check and make lint, with the build stage forced to wait on it via COPY --from=lint /src/go.sum /dev/null, and gives a worked example. Under the ruling script/lint builds Dockerfile.lint, so a Dockerfile lint stage running make lint would shell out to docker build from inside a BuildKit step with no daemon. The two cannot both stand.

Also worth deciding centrally rather than per repo, since each will otherwise answer it differently: with the lint stage gone from Dockerfile, make fmt-check needs a new home in the container gate (vaultik moved it beside make test in the builder stage), and the COPY --from=lint ordering trick no longer applies, so script/cibuild has to build both files and fail on either.

Not raising the cache-busting point separately — the issue body already names it, and it is the item most likely to be dropped when this is copied around.

Flagging one thing this canonicalisation has to include, from implementing the ruling in `sneak/vaultik` ( https://git.eeqj.de/sneak/vaultik/issues/113 ): `REPO_POLICIES.md` currently mandates the pattern being superseded, so a consuming repo that follows the ruling is out of compliance with the policy text. `REPO_POLICIES.md:102-166` requires a lint stage **inside** the main `Dockerfile` running `make fmt-check` and `make lint`, with the build stage forced to wait on it via `COPY --from=lint /src/go.sum /dev/null`, and gives a worked example. Under the ruling `script/lint` builds `Dockerfile.lint`, so a `Dockerfile` lint stage running `make lint` would shell out to `docker build` from inside a BuildKit step with no daemon. The two cannot both stand. Also worth deciding centrally rather than per repo, since each will otherwise answer it differently: with the lint stage gone from `Dockerfile`, `make fmt-check` needs a new home in the container gate (vaultik moved it beside `make test` in the builder stage), and the `COPY --from=lint` ordering trick no longer applies, so `script/cibuild` has to build both files and fail on either. Not raising the cache-busting point separately — the issue body already names it, and it is the item most likely to be dropped when this is copied around.
clawbot commented 2026-08-10 15:00:53 +02:00
Author
Collaborator
Copy Link

Two questions for you, and a speculative PR that answers them one way

I was sent to reconcile prompts/REPO_POLICIES.md with this ruling, on the
premise that the doc still mandates the old shape — the clause requiring every
Dockerfile to run make check as a build step, and the clause headed
"Dockerfiles must use a separate lint stage for fail-fast feedback" with its
COPY --from=lint /src/go.sum /dev/null ordering trick.

Both are already rewritten, in 12e8db8 on next, which is in
#34. The contradiction is visible on
main only because next has not merged. I did not open a second PR rewriting
them, since that would duplicate and conflict with that one. Answering the
question this issue's title implies: yes, this issue does cover the
policy-document rewrite, and it has been done — so I filed no new issue
either.

What I did instead was check that rewritten text against the two repos that have
actually implemented the ruling, sneak/homoicon and sneak/quak, rather than
against the tracker. Three places diverge, and two of them need a decision from
you rather than an edit from me.

1. Does the sneak/quak division of responsibility get to be the shape?

sneak/quak#31 splits it as:

  • Dockerfile — make test, then make build. No lint, no fmt-check.
  • Dockerfile.lint — eslint ., then prettier --check ..
  • script/cibuild — script/lint first, then the main build. That composite is
    where "all checks ran" is now true.

The canonical text on next says the main Dockerfile runs the individual
non-lint checks, script/test and script/fmt-check. Read literally,
sneak/quak is out of compliance: its fmt-check moved into the lint image
instead.

I think the doc should move rather than quak, and the PR implements that:
the formatting check must run in exactly one of the two images, either
placement allowed, never neither and never both. Where the formatter is the
same pinned dependency as the linter — prettier out of node_modules — the
lint image is the better home, because it takes the last host toolchain off the
checked path for exactly the reason the linter came off it. The failure to
guard against is it running in neither, which is the live risk: splitting lint
out of the Dockerfile is precisely the moment fmt-check gets dropped from
both.

If you would rather have one shape enforced, say so and quak changes instead.
Either answer is implementable; what does not work is the doc and the
first adopter disagreeing silently.

2. Is the cache-bust arg one name or per-file?

quak's Dockerfile.lint names it LINT_EPOCH; the canonical text says
CHECK_EPOCH in both files. quak's guard is functionally correct, so this is
drift and not a defect — but a per-file name is invisible to the grep that
proves every build in a repo is cache-busted, so a renamed guard and a missing
guard read identically without opening both Dockerfiles. The PR fixes the name
at CHECK_EPOCH everywhere. If you prefer LINT_EPOCH in the lint file, that
is fine too and the doc should say it; one of the two has to give.

3. Not a question — a gap I would land regardless

The policy requires .dockerignore to exclude the agent scratch directory,
justified on build-context bloat and on another session's unreviewed work
reaching an image layer. Both true, neither load-bearing now. Dockerfile.lint
lints whatever COPY . . copies, and language toolchains discover files by
walking the tree rather than by reading .gitignore — ./..., eslint . and
prettier --check . all descend into a nested worktree. sneak/quak measured
this on the same discovery mechanism in its test runner: a nested .claude/
worktree took the discovered test count from 210 to 1050
(sneak/quak#30).

So a repo that containerises its lint and skips that entry re-creates the
foreign-tree false reds inside the container — in the convincing form, where
the findings are real and simply belong to another checkout. The PR restates
that entry as a correctness precondition of the containerised-lint rule rather
than a size optimisation, and both checklists get the matching item.

The PR

#43, assigned to you, based on and
targeting next because that is the branch the rule lives on. Four files,
documentation only, make check green, make fmt run. Speculative and awaiting
your decision — close it and delete the branch if you disagree. No other repo
was touched.

## Two questions for you, and a speculative PR that answers them one way I was sent to reconcile `prompts/REPO_POLICIES.md` with this ruling, on the premise that the doc still mandates the old shape — the clause requiring every Dockerfile to run `make check` as a build step, and the clause headed "Dockerfiles must use a separate lint stage for fail-fast feedback" with its `COPY --from=lint /src/go.sum /dev/null` ordering trick. **Both are already rewritten**, in `12e8db8` on `next`, which is in https://git.eeqj.de/sneak/prompts/pulls/34. The contradiction is visible on `main` only because `next` has not merged. I did not open a second PR rewriting them, since that would duplicate and conflict with that one. Answering the question this issue's title implies: **yes, this issue does cover the policy-document rewrite, and it has been done** — so I filed no new issue either. What I did instead was check that rewritten text against the two repos that have actually implemented the ruling, `sneak/homoicon` and `sneak/quak`, rather than against the tracker. Three places diverge, and two of them need a decision from you rather than an edit from me. ### 1. Does the `sneak/quak` division of responsibility get to be the shape? https://git.eeqj.de/sneak/quak/pulls/31 splits it as: - `Dockerfile` — `make test`, then `make build`. No lint, no `fmt-check`. - `Dockerfile.lint` — `eslint .`, then `prettier --check .`. - `script/cibuild` — `script/lint` first, then the main build. That composite is where "all checks ran" is now true. The canonical text on `next` says the main `Dockerfile` runs the individual non-lint checks, **`script/test` and `script/fmt-check`**. Read literally, `sneak/quak` is out of compliance: its `fmt-check` moved into the lint image instead. I think the doc should move rather than quak, and the PR implements that: the formatting check must run in exactly **one** of the two images, either placement allowed, never neither and never both. Where the formatter is the same pinned dependency as the linter — `prettier` out of `node_modules` — the lint image is the better home, because it takes the last host toolchain off the checked path for exactly the reason the linter came off it. The failure to guard against is it running in neither, which is the live risk: splitting lint out of the `Dockerfile` is precisely the moment `fmt-check` gets dropped from both. If you would rather have one shape enforced, say so and quak changes instead. Either answer is implementable; what does not work is the doc and the first adopter disagreeing silently. ### 2. Is the cache-bust arg one name or per-file? `quak`'s `Dockerfile.lint` names it `LINT_EPOCH`; the canonical text says `CHECK_EPOCH` in both files. quak's guard is functionally correct, so this is drift and not a defect — but a per-file name is invisible to the grep that proves every build in a repo is cache-busted, so a renamed guard and a missing guard read identically without opening both Dockerfiles. The PR fixes the name at `CHECK_EPOCH` everywhere. If you prefer `LINT_EPOCH` in the lint file, that is fine too and the doc should say it; one of the two has to give. ### 3. Not a question — a gap I would land regardless The policy requires `.dockerignore` to exclude the agent scratch directory, justified on build-context bloat and on another session's unreviewed work reaching an image layer. Both true, neither load-bearing now. `Dockerfile.lint` lints whatever `COPY . .` copies, and language toolchains discover files by walking the tree rather than by reading `.gitignore` — `./...`, `eslint .` and `prettier --check .` all descend into a nested worktree. `sneak/quak` measured this on the same discovery mechanism in its test runner: a nested `.claude/` worktree took the discovered test count from 210 to 1050 (https://git.eeqj.de/sneak/quak/issues/30). So a repo that containerises its lint and skips that entry re-creates the foreign-tree false reds **inside** the container — in the convincing form, where the findings are real and simply belong to another checkout. The PR restates that entry as a correctness precondition of the containerised-lint rule rather than a size optimisation, and both checklists get the matching item. ### The PR https://git.eeqj.de/sneak/prompts/pulls/43, assigned to you, based on and targeting `next` because that is the branch the rule lives on. Four files, documentation only, `make check` green, `make fmt` run. Speculative and awaiting your decision — close it and delete the branch if you disagree. No other repo was touched.
clawbot commented 2026-08-10 15:01:08 +02:00
Author
Collaborator
Copy Link

Evidence against the second caution in this issue, from implementing it in
sneak/cattbox (sneak/cattbox#33).

> golangci-lint config verify resolves its JSON schema over an unpinned live
> HTTPS fetch. Inside a build step that makes the lint network-dependent and
> breaks hash-pinning.

Not true for the digest this issue pins. I acted on that caution, told the
implementer to omit the line, and the reviewer then demonstrated it had cost us
a live false green. So I tested the premise instead of propagating it. Same
image, golangci/golangci-lint@sha256:5cceeef0…, run with the network removed
entirely:

$ docker run --rm --network none ... config verify --config .golangci.yml
exit=0

$ # identical invocation, config with `linters:` renamed to `lintersX:`
$ docker run --rm --network none ... config verify --config .golangci.yml
jsonschema: "" does not validate with "/additionalProperties":
  additional properties 'lintersX' not allowed
The command is terminated due to an error: the configuration contains invalid elements
exit=3

The schema is embedded in v2.12.2. --network none is a hard control: nothing
could have been fetched. And it genuinely validates rather than degrading to a
no-op when offline, which the second run proves.

Why the line matters more than it looks

Omitting it is not neutral, because golangci-lint run does not cover the
same ground. Measured on cattbox:

  • Unparseable YAML → run fails, exit 3. Fine either way.
  • Unrecognised key → run silently ignores it, exit 0. Two demonstrations
    with an identical probe file: line-length mistyped as line-lenght took
    lll back to its 120 default and reported 0 issues.; mistyping
    linters.default as linters.defaults dropped the key that enables the whole
    non-standard linter set, collapsing it to standard, lll never running,
    0 issues., exit 0.

So a one-character typo in .golangci.yml silently downgrades the gate to
default linters and reports green. That is the same class of defect this whole
issue exists to eliminate, arriving through the config file instead of the
cache. config verify catches it; nothing else in the pipeline does.

Suggest striking the caution from the canonical guidance and keeping
RUN golangci-lint config verify --config .golangci.yml in Dockerfile.lint as
sneak/homoicon already has it, with a note that it is offline for a
digest-pinned v2.12.2 and should be re-checked if the pin moves. Happy to send
that as a PR here if the wording is wanted from me rather than decided by you.

One related note for the canonical template, found in the same work: it
prescribes RUN make lint in the main Dockerfile's lint stage
(REPO_POLICIES.md:104-127 as vendored into cattbox). Once script/lint is
itself a docker build, that line is docker-in-docker inside an image build and
cannot work. cattbox resolved it by invoking golangci-lint directly in that
stage, which is what sneak/homoicon does. Any repo adopting this pattern with
a lint stage in its main Dockerfile will hit it.

Evidence against the second caution in this issue, from implementing it in `sneak/cattbox` (https://git.eeqj.de/sneak/cattbox/issues/33). &gt; `golangci-lint config verify` resolves its JSON schema over an **unpinned live &gt; HTTPS fetch**. Inside a build step that makes the lint network-dependent and &gt; breaks hash-pinning. **Not true for the digest this issue pins.** I acted on that caution, told the implementer to omit the line, and the reviewer then demonstrated it had cost us a live false green. So I tested the premise instead of propagating it. Same image, `golangci/golangci-lint@sha256:5cceeef0…`, run with the network removed entirely: $ docker run --rm --network none ... config verify --config .golangci.yml exit=0 $ # identical invocation, config with `linters:` renamed to `lintersX:` $ docker run --rm --network none ... config verify --config .golangci.yml jsonschema: "" does not validate with "/additionalProperties": additional properties 'lintersX' not allowed The command is terminated due to an error: the configuration contains invalid elements exit=3 The schema is embedded in v2.12.2. `--network none` is a hard control: nothing could have been fetched. And it genuinely validates rather than degrading to a no-op when offline, which the second run proves. ## Why the line matters more than it looks Omitting it is not neutral, because `golangci-lint run` does **not** cover the same ground. Measured on cattbox: - Unparseable YAML → `run` fails, exit 3. Fine either way. - **Unrecognised key → `run` silently ignores it, exit 0.** Two demonstrations with an identical probe file: `line-length` mistyped as `line-lenght` took `lll` back to its 120 default and reported `0 issues.`; mistyping `linters.default` as `linters.defaults` dropped the key that enables the whole non-standard linter set, collapsing it to standard, `lll` never running, `0 issues.`, exit 0. So a one-character typo in `.golangci.yml` silently downgrades the gate to default linters and reports green. That is the same class of defect this whole issue exists to eliminate, arriving through the config file instead of the cache. `config verify` catches it; nothing else in the pipeline does. Suggest striking the caution from the canonical guidance and keeping `RUN golangci-lint config verify --config .golangci.yml` in `Dockerfile.lint` as `sneak/homoicon` already has it, with a note that it is offline for a digest-pinned v2.12.2 and should be re-checked if the pin moves. Happy to send that as a PR here if the wording is wanted from me rather than decided by you. One related note for the canonical template, found in the same work: it prescribes `RUN make lint` in the main `Dockerfile`&#39;s lint stage (`REPO_POLICIES.md:104-127` as vendored into cattbox). Once `script/lint` is itself a `docker build`, that line is docker-in-docker inside an image build and cannot work. cattbox resolved it by invoking `golangci-lint` directly in that stage, which is what `sneak/homoicon` does. Any repo adopting this pattern with a lint stage in its main `Dockerfile` will hit it.
clawbot commented 2026-08-10 15:01:12 +02:00
Author
Collaborator
Copy Link

Owner ruling on the scope boundary, sneak 2026-08-10, verbatim:

> fmt and fmt check arent docker, just linting.

Posting it here because it is org-wide and because it is an easy line to cross: implementing this in sneak/lora.vegas (sneak/lora.vegas#38) the implementer containerised script/fmt-check too, reasoning that leaving prettier on the host would leave make check with a host-run path. That reasoning is coherent and is now overruled — only linting is containerised.

The canonical text already on next here matches the ruling, so nothing needs changing upstream. The value of the ruling is that it closes the question for the repos still adopting the pattern: script/fmt and script/fmt-check stay on the host, and because they do, the main Dockerfile can still run the format check directly with no recursion.

Owner ruling on the scope boundary, sneak 2026-08-10, verbatim: &gt; fmt and fmt check arent docker, just linting. Posting it here because it is org-wide and because it is an easy line to cross: implementing this in `sneak/lora.vegas` (https://git.eeqj.de/sneak/lora.vegas/issues/38) the implementer containerised `script/fmt-check` too, reasoning that leaving prettier on the host would leave `make check` with a host-run path. That reasoning is coherent and is now overruled — only linting is containerised. The canonical text already on `next` here matches the ruling, so nothing needs changing upstream. The value of the ruling is that it closes the question for the repos still adopting the pattern: `script/fmt` and `script/fmt-check` stay on the host, and because they do, the main `Dockerfile` can still run the format check directly with no recursion.
clawbot commented 2026-08-10 15:09:03 +02:00
Author
Collaborator
Copy Link

New defect class in the canonical pattern, found by adversarial review in sneak/lora.vegas (sneak/lora.vegas#39). Worth a line in the canonical text because it is silent, and because the canonical script/lint form is what triggers it.

If Dockerfile.lint has more than one stage and the lint stage is not the last one, docker build -f Dockerfile.lint . never runs the lint and exits 0. Sibling stages off a shared base are not instantiated unless something depends on them or --target names them, so BuildKit builds only the final stage. Demonstrated: a whole-file build ran only the trailing stage and returned success with the lint stage absent from the graph entirely.

The CHECK_EPOCH guard does not catch it. The guard is ARG-scoped per stage, so it is satisfied by whichever stage actually ran, and a build that skipped the lint stage skipped its guard too. Every existing proof of "the epoch forces execution" remains true and simply does not apply to a stage that was never instantiated.

Two things follow for the canonical shape:

  1. Keep Dockerfile.lint single-stage wherever possible. Then the canonical docker build -f Dockerfile.lint . cannot skip anything and the file's usual claim — a successful build is a clean lint — is true for every invocation rather than only the one the repo's own script/lint happens to use.
  2. If a repo genuinely needs multiple lint stages, the last stage must depend on all the others (an explicit ordering edge, the same requirement already noted for the Go lint stage), or the claim in the file header is false. Relying on script/lint passing the right --target is not sufficient: it makes correctness a property of one caller rather than of the file, and the canonical caller passes no target at all.

This is the same failure shape already flagged here as "present but no longer gating" — a check that is nominally configured, passes, and gates nothing.

New defect class in the canonical pattern, found by adversarial review in `sneak/lora.vegas` (https://git.eeqj.de/sneak/lora.vegas/pulls/39). Worth a line in the canonical text because it is silent, and because the canonical `script/lint` form is what triggers it. **If `Dockerfile.lint` has more than one stage and the lint stage is not the last one, `docker build -f Dockerfile.lint .` never runs the lint and exits 0.** Sibling stages off a shared base are not instantiated unless something depends on them or `--target` names them, so BuildKit builds only the final stage. Demonstrated: a whole-file build ran only the trailing stage and returned success with the lint stage absent from the graph entirely. The `CHECK_EPOCH` guard does not catch it. The guard is `ARG`-scoped per stage, so it is satisfied by whichever stage actually ran, and a build that skipped the lint stage skipped its guard too. Every existing proof of "the epoch forces execution" remains true and simply does not apply to a stage that was never instantiated. Two things follow for the canonical shape: 1. Keep `Dockerfile.lint` **single-stage** wherever possible. Then the canonical `docker build -f Dockerfile.lint .` cannot skip anything and the file's usual claim — a successful build is a clean lint — is true for every invocation rather than only the one the repo's own `script/lint` happens to use. 2. If a repo genuinely needs multiple lint stages, the last stage must depend on all the others (an explicit ordering edge, the same requirement already noted for the Go lint stage), or the claim in the file header is false. Relying on `script/lint` passing the right `--target` is not sufficient: it makes correctness a property of one caller rather than of the file, and the canonical caller passes no target at all. This is the same failure shape already flagged here as "present but no longer gating" — a check that is nominally configured, passes, and gates nothing.
clawbot commented 2026-08-10 15:18:27 +02:00
Author
Collaborator
Copy Link

Correction to the second caution in the issue body, with evidence — it matters because acting on it as written leaves a false green in every repo that adopts this pattern.

The claim that golangci-lint config verify resolves its JSON schema over an unpinned live HTTPS fetch is false at the pinned v2.12.2. Tested in the pinned image with the network off:

  • docker run --network none ... golangci-lint config verify --config .golangci.yml exits 0 on a real config.
  • The same command with one key typo'd exits 3: additional properties 'linterz' not allowed.

It validates offline and it is not silently skipping validation when offline. So the hash-pinning objection to including that line does not apply, and there is no reason to drop it.

Dropping it is not neutral, because golangci-lint run does not catch the same thing. Unparseable YAML it rejects. An unknown top-level key it silently ignores, exit 0. Reproduced against a real containerised gate in sneak/vaultik: changing .golangci.yml's linters: to linterz: — one character — makes script/lint exit 0 reporting 0 issues. in a run whose lint layer demonstrably executed. default: all, the disable list and every threshold are discarded, and only golangci-lint's small default set runs. A repo could sit in that state indefinitely with a green gate.

So the canonical Dockerfile.lint should keep RUN golangci-lint config verify --config .golangci.yml, and the caution in the issue body should be struck rather than propagated. One thing to decide when canonicalising it: whatever cache-busting the lint RUN gets, the config verify RUN needs too — a cached verify layer validates nothing, which is the same trap one level down.

Found by an adversarial reviewer of the vaultik implementation ( sneak/vaultik#114 ), where the omission had been made deliberately on the strength of the caution above.

Correction to the second caution in the issue body, with evidence — it matters because acting on it as written leaves a false green in every repo that adopts this pattern. **The claim that `golangci-lint config verify` resolves its JSON schema over an unpinned live HTTPS fetch is false at the pinned v2.12.2.** Tested in the pinned image with the network off: - `docker run --network none ... golangci-lint config verify --config .golangci.yml` exits **0** on a real config. - The same command with one key typo'd exits **3**: `additional properties 'linterz' not allowed`. It validates offline and it is not silently skipping validation when offline. So the hash-pinning objection to including that line does not apply, and there is no reason to drop it. **Dropping it is not neutral, because `golangci-lint run` does not catch the same thing.** Unparseable YAML it rejects. An unknown *top-level key* it silently ignores, exit 0. Reproduced against a real containerised gate in `sneak/vaultik`: changing `.golangci.yml`'s `linters:` to `linterz:` — one character — makes `script/lint` exit **0** reporting `0 issues.` in a run whose lint layer demonstrably executed. `default: all`, the disable list and every threshold are discarded, and only golangci-lint's small default set runs. A repo could sit in that state indefinitely with a green gate. So the canonical `Dockerfile.lint` should keep `RUN golangci-lint config verify --config .golangci.yml`, and the caution in the issue body should be struck rather than propagated. One thing to decide when canonicalising it: whatever cache-busting the lint `RUN` gets, the `config verify` `RUN` needs too — a cached verify layer validates nothing, which is the same trap one level down. Found by an adversarial reviewer of the vaultik implementation ( https://git.eeqj.de/sneak/vaultik/pulls/114 ), where the omission had been made deliberately on the strength of the caution above.
clawbot commented 2026-08-10 15:27:00 +02:00
Author
Collaborator
Copy Link

Second finding, and this one is a direct contradiction inside canonical REPO_POLICIES.md once Docker-only linting lands.

REPO_POLICIES.md (around lines 266-271 in the version on #42) mandates installing golangci-lint on the host via go install ...@c0d3ddc9. That instruction is incompatible with the Docker-only linting ruling this issue tracks: after the change, nothing installs or runs golangci-lint on the host at all.

Confirmed concretely in sneak/dnswatcher, which has now landed Docker-only linting (sneak/dnswatcher#134). Its script/bootstrap deliberately installs golangci-lint nowhere, so the repo satisfies the version the canonical text pins (v2.12.2 / c0d3ddc9) while violating the mechanism it prescribes. Every repo converted to Docker-only linting will land in the same state.

The vendored copy cannot be fixed downstream — it is canonical and must not be hand-edited per policy — so the fix belongs in sneak/prompts.

Suggested resolution when the Dockerfile.lint shape is settled here: replace the host go install mandate with the Docker-only lint requirement, keeping the pinned version but attaching it to the digest-pinned lint image rather than a host install. Worth doing in the same change as whatever is decided about golangci-lint config verify (raised in my earlier comment), since both edit the same section.

**Second finding, and this one is a direct contradiction inside canonical `REPO_POLICIES.md` once Docker-only linting lands.** `REPO_POLICIES.md` (around lines 266-271 in the version on https://git.eeqj.de/sneak/prompts/pulls/42) mandates installing golangci-lint on the host via `go install ...@c0d3ddc9`. That instruction is incompatible with the Docker-only linting ruling this issue tracks: after the change, nothing installs or runs golangci-lint on the host at all. Confirmed concretely in `sneak/dnswatcher`, which has now landed Docker-only linting (https://git.eeqj.de/sneak/dnswatcher/issues/134). Its `script/bootstrap` deliberately installs golangci-lint nowhere, so the repo satisfies the **version** the canonical text pins (v2.12.2 / `c0d3ddc9`) while violating the **mechanism** it prescribes. Every repo converted to Docker-only linting will land in the same state. The vendored copy cannot be fixed downstream — it is canonical and must not be hand-edited per policy — so the fix belongs in `sneak/prompts`. Suggested resolution when the `Dockerfile.lint` shape is settled here: replace the host `go install` mandate with the Docker-only lint requirement, keeping the pinned version but attaching it to the digest-pinned lint image rather than a host install. Worth doing in the same change as whatever is decided about `golangci-lint config verify` (raised in my earlier comment), since both edit the same section.
sneak commented 2026-08-10 16:20:11 +02:00
Owner
Copy Link

Don't do the config check step. That's not necessary. We can assume the config is valid.
It's okay to make linting its own Docker phase. That way you can put it into the main Docker file.
You don't need a Docker file dot lint separately.
Then the linting script entry point can just run that single build phase in Docker using no caching.
Additionally, there is a way to create an artificial dependency between the linter and the main build.
I would do the same thing for the testing phase in the main build.
That way you can ensure that the main build will not run unless linting and testing both pass first.
This pattern exists in one of the existing reposed Docker files and you should find it and use that one.

then both testing and linting happen inside docker and you can simply disable all caching.

Don't do the config check step. That's not necessary. We can assume the config is valid. It's okay to make linting its own Docker phase. That way you can put it into the main Docker file. You don't need a Docker file dot lint separately. Then the linting script entry point can just run that single build phase in Docker using no caching. Additionally, there is a way to create an artificial dependency between the linter and the main build. I would do the same thing for the testing phase in the main build. That way you can ensure that the main build will not run unless linting and testing both pass first. This pattern exists in one of the existing reposed Docker files and you should find it and use that one. then both testing and linting happen inside docker and you can simply disable all caching.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
merge-ready
PR approved and ready to merge
 needs-review
PR needs code review
 needs-rework
PR needs changes based on review
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
clawbot sneak
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/prompts#40
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?