From dcf75f6c724194b23679f360e363670c80f974f5 Mon Sep 17 00:00:00 2001 From: sneak Date: Mon, 10 Aug 2026 12:59:44 +0000 Subject: [PATCH] Close three gaps between the containerised-lint rule and its first adopters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rule landed in 12e8db8 is right; these are the three places where the canonical text and the repos implementing it can diverge without either side looking wrong. 1. `.dockerignore` excluding the agent scratch directory is now stated as a correctness precondition of containerised linting rather than a context-size measure. `Dockerfile.lint` lints whatever `COPY . .` copies, and language toolchains discover files by walking the tree instead of reading `.gitignore`, so a nested worktree in the context puts the foreign-tree false reds back inside the container — in the convincing form, where the findings are real but belong to another checkout. sneak/quak measured the same discovery mechanism taking a test count from 210 to 1050. 2. The cache-bust build arg is fixed at `CHECK_EPOCH` in `Dockerfile.lint` as well as in `Dockerfile`. A per-file name is invisible to the grep that proves every build is busted, which makes a renamed guard and a missing guard read identically. sneak/quak's lint file currently names it `LINT_EPOCH`. 3. The formatting check must run in exactly one of the two images, and either placement is allowed. Splitting lint out of the `Dockerfile` is precisely the moment `fmt-check` gets dropped from both, and running the formatter beside the linters is the better shape wherever it is the same pinned dependency — it takes the last host toolchain off the checked path for the reason the linter came off it. Both checklists carry the matching items, since a repo that satisfies the policy prose but not the checklist is the drift this is meant to stop. Refs https://git.eeqj.de/sneak/prompts/issues/40 --- TODO.md | 15 ++++++++ prompts/EXISTING_REPO_CHECKLIST.md | 17 +++++++-- prompts/NEW_REPO_CHECKLIST.md | 11 +++++- prompts/REPO_POLICIES.md | 60 +++++++++++++++++++++++++----- 4 files changed, 90 insertions(+), 13 deletions(-) diff --git a/TODO.md b/TODO.md index 71fb17d..56c27ba 100644 --- a/TODO.md +++ b/TODO.md @@ -21,6 +21,21 @@ fmt-check, and commit. # Completed Steps +- 2026-08-10: Closed three gaps the containerised-lint rule left between the + canonical text and the first repos to implement it. `.dockerignore` excluding + the agent scratch directory is now stated as a correctness precondition of + that rule rather than a context-size measure: the lint image lints whatever + `COPY . .` copies, and toolchains discover files by walking the tree instead + of reading `.gitignore`, so a nested worktree puts the foreign-tree false reds + back inside the container — `sneak/quak` measured the same discovery mechanism + taking a test count from 210 to 1050. The cache-bust arg is fixed at + `CHECK_EPOCH` in `Dockerfile.lint` as well, because a per-file name is + invisible to the grep that proves every build is busted, making a renamed + guard indistinguishable from a missing one. And the formatting check is now + required to run in exactly one of the two images, with either placement + allowed: splitting lint out of the `Dockerfile` is precisely when `fmt-check` + gets dropped from both, and running the formatter beside the linters is the + better shape where it is the same pinned dependency. - 2026-08-10: Moved every lint run into a container, on the owner's ruling, and made this repo do it rather than merely document it. `script/lint` is now `docker build -f Dockerfile.lint .` and nothing else; the linter is never diff --git a/prompts/EXISTING_REPO_CHECKLIST.md b/prompts/EXISTING_REPO_CHECKLIST.md index dfe9818..d0d140e 100644 --- a/prompts/EXISTING_REPO_CHECKLIST.md +++ b/prompts/EXISTING_REPO_CHECKLIST.md @@ -52,7 +52,15 @@ with your task. containerised-lint rule in `REPO_POLICIES.md` for the canonical file. Its base image is pinned by sha256 with a version/date comment, it carries `ARG CHECK_EPOCH` **after** the dependency layer with the guard below it, - and it invokes the linter directly rather than through `make lint`. + and it invokes the linter directly rather than through `make lint`. The + arg is named `CHECK_EPOCH` in this file too — a repo that calls it + `LINT_EPOCH` here is missed by the grep that checks every build is + cache-busted. +- [ ] The formatting check runs in exactly one of the two images — either + `script/fmt-check` in the `Dockerfile` or the formatter beside the linters + in `Dockerfile.lint`, whichever puts it on the pinned toolchain. Neither + image running it is the failure to look for here, since moving lint out of + the `Dockerfile` is exactly when it gets dropped. - [ ] `.dockerignore` excludes the repo's own host-built artifacts (compiled binaries, test binaries, coverage output), written root-anchored — `/myapp`, never `**/myapp`, which would also match `cmd/myapp/`. An @@ -61,8 +69,11 @@ with your task. - [ ] `.dockerignore` excludes `.claude`, root-anchored and with no `**/` prefix. Agent worktrees are entire checkouts of the repo, so they inflate the context by a multiple of it and can copy another session's unreviewed - work into an image layer. Confirm by enumerating the image, not by reading - the file — `.gitignore` hides these from `git status` too. + work into an image layer — and `Dockerfile.lint` then lints that checkout + as though it were this one, because toolchains discover files by walking + the tree and never read `.gitignore` (`sneak/quak`: 210 discovered tests + became 1050). Confirm by enumerating the image, not by reading the file — + `.gitignore` hides these from `git status` too. - [ ] **Do agents in this repo run anywhere other than the repo root?** The scratch directory is created in the agent's working directory, so the canonical anchored entry misses `services/api/.claude/` in a monorepo with diff --git a/prompts/NEW_REPO_CHECKLIST.md b/prompts/NEW_REPO_CHECKLIST.md index f6fe45c..7c46a9e 100644 --- a/prompts/NEW_REPO_CHECKLIST.md +++ b/prompts/NEW_REPO_CHECKLIST.md @@ -93,7 +93,16 @@ Template files can be fetched from: `golangci-lint run`; other repos use the same pattern around their own linter (eslint, ruff, prettier). Copy the canonical file from `REPO_POLICIES.md`. The linter is invoked directly there, never via - `make lint`, which would recurse. + `make lint`, which would recurse. The arg keeps the name `CHECK_EPOCH` in + this file as well, so one grep covers both builds. + - The formatting check runs in exactly one of the two images: either + `script/fmt-check` in the `Dockerfile`, or the formatter beside the + linters in `Dockerfile.lint` where that is the same pinned dependency. + Never neither, never both. + - `.dockerignore` must exclude the agent scratch directory before this image + is trusted: it lints whatever is in the build context, and toolchains walk + the tree rather than reading `.gitignore`, so an agent worktree that + reaches the context is linted as though it were the repo. - [ ] Gitea Actions workflow at `.gitea/workflows/check.yml` that runs `script/cibuild` on push — reference `https://git.eeqj.de/sneak/prompts/raw/branch/main/.gitea/workflows/check.yml` diff --git a/prompts/REPO_POLICIES.md b/prompts/REPO_POLICIES.md index bf4bd82..287c294 100644 --- a/prompts/REPO_POLICIES.md +++ b/prompts/REPO_POLICIES.md @@ -108,7 +108,10 @@ style conventions are in separate documents: build step, where there is no daemon. Lint is not skipped by this: it runs in its own container, and `script/cibuild` runs it first. Put a comment to that effect directly above those `RUN` lines, because `make check` is what - the next person will reach for. + the next person will reach for. Of the two, only `script/test` is fixed + here: a repo may run its formatter in `Dockerfile.lint` beside the linters + instead, and some should — see the containerised-lint rule below. It must + then run in that file and not in this one, and never in neither. For non-server repos, the Dockerfile should bring up a development environment and run those checks. For server repos, they should run as an @@ -306,6 +309,37 @@ style conventions are in separate documents: node image, runs `script/bootstrap` as the dependency layer, and lints with the linter from `node_modules`, which is also how it gets the version pinned in `package.json` rather than whatever is on the host. + - **The lint container lints whatever is in the build context, so + `.dockerignore` is part of this rule and not merely hygiene.** `COPY . .` + copies an agent scratch worktree — an entire second checkout of the repo — + into the lint image unless `.dockerignore` excludes it, and language + toolchains discover files by walking the tree rather than by reading + `.gitignore`, so `./...`, `eslint .` and `prettier --check .` all descend + into it. `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). Left in the + context it re-creates _inside_ the container the foreign-tree false reds + that moving lint into a container was adopted to end, and it does so in + the convincing form — the findings are real, they simply belong to another + checkout. See the `.dockerignore` rules below, and verify by enumerating + the image rather than by reading the patterns. + - **The build arg is named `CHECK_EPOCH` in `Dockerfile.lint` too**, not + `LINT_EPOCH` or any other per-file name, and `script/lint` passes it under + that name. Both files guard the same failure under the same contract, and + the single name is what lets a reviewer grep a repo for `CHECK_EPOCH` and + see every cache-bust it has. Rename it in one file and that grep silently + misses it, so a renamed guard and an absent guard read identically without + opening both Dockerfiles. + - **The formatting check runs in exactly one of the two images, and either + one is allowed.** The canonical `Dockerfile` above runs `script/fmt-check` + because that is where the non-lint checks live. A repo may instead run its + formatter in `Dockerfile.lint` beside the linters, which is the better + shape wherever the formatter is the same pinned dependency as the linter + (`prettier` out of `node_modules`, say), because it takes the last host + toolchain off the checked path for the same reason the linter came off it. + What is not allowed is running it in neither image, or in both. Whichever + image runs it carries the epoch guard, and `script/check` still runs all + three targets on the developer's side either way. - **Keep `golangci-lint config verify`, and it costs no network.** The two commands catch **disjoint** classes of defect, measured under the pinned v2.12.2 against a config carrying one planted defect at a time: a bogus @@ -537,14 +571,22 @@ style conventions are in separate documents: inflates by a multiple of the repo, and another session's unreviewed, sometimes uncommitted work can be copied into an image layer. The directory is also created and destroyed constantly, so it invalidates `COPY . .` for - reasons that have nothing to do with the repo's own content. In `.gitignore` - the entry is `.claude/`, unanchored, which already matches at every depth. In - `.dockerignore` it is `.claude`, anchored and with **no** `**/` prefix: the - directory occurs exactly once **where agents run at the repo root**, and the - prefixed form would also match any nested directory of that name and delete it - from the build. It is not case-folded the way the secret patterns are, because - tooling creates it in exactly one spelling, so a folded pattern would add no - coverage. + reasons that have nothing to do with the repo's own content. **And because + `Dockerfile.lint` and `Dockerfile` run their tooling over the copied context, + a worktree that reaches it is linted and tested as though it were the repo.** + Nothing else stops that: language toolchains discover files by walking the + tree and do not read `.gitignore`, which is how `sneak/quak` saw a nested + `.claude/` worktree take its discovered test count from 210 to 1050 + (https://git.eeqj.de/sneak/quak/issues/30). This entry is therefore a + correctness precondition of the containerised-lint rule above and not a size + optimisation — without it the foreign-tree false reds that rule exists to end + simply move inside the container. In `.gitignore` the entry is `.claude/`, + unanchored, which already matches at every depth. In `.dockerignore` it is + `.claude`, anchored and with **no** `**/` prefix: the directory occurs exactly + once **where agents run at the repo root**, and the prefixed form would also + match any nested directory of that name and delete it from the build. It is + not case-folded the way the secret patterns are, because tooling creates it in + exactly one spelling, so a folded pattern would add no coverage. **Known gap that comes with the anchored form.** The directory is created in the agent's working directory, so the "exactly once, at the root" premise is