Close three gaps between the containerised-lint rule and its first adopters
All checks were successful
check / check (push) Successful in 22s
All checks were successful
check / check (push) Successful in 22s
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 #40
This commit is contained in:
15
TODO.md
15
TODO.md
@@ -21,6 +21,21 @@ fmt-check, and commit.
|
|||||||
|
|
||||||
# Completed Steps
|
# 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
|
- 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
|
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
|
`docker build -f Dockerfile.lint .` and nothing else; the linter is never
|
||||||
|
|||||||
@@ -52,7 +52,15 @@ with your task.
|
|||||||
containerised-lint rule in `REPO_POLICIES.md` for the canonical file. Its
|
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
|
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,
|
`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
|
- [ ] `.dockerignore` excludes the repo's own host-built artifacts (compiled
|
||||||
binaries, test binaries, coverage output), written root-anchored —
|
binaries, test binaries, coverage output), written root-anchored —
|
||||||
`/myapp`, never `**/myapp`, which would also match `cmd/myapp/`. An
|
`/myapp`, never `**/myapp`, which would also match `cmd/myapp/`. An
|
||||||
@@ -61,8 +69,11 @@ with your task.
|
|||||||
- [ ] `.dockerignore` excludes `.claude`, root-anchored and with no `**/`
|
- [ ] `.dockerignore` excludes `.claude`, root-anchored and with no `**/`
|
||||||
prefix. Agent worktrees are entire checkouts of the repo, so they inflate
|
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
|
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
|
work into an image layer — and `Dockerfile.lint` then lints that checkout
|
||||||
the file — `.gitignore` hides these from `git status` too.
|
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
|
- [ ] **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
|
scratch directory is created in the agent's working directory, so the
|
||||||
canonical anchored entry misses `services/api/.claude/` in a monorepo with
|
canonical anchored entry misses `services/api/.claude/` in a monorepo with
|
||||||
|
|||||||
@@ -93,7 +93,16 @@ Template files can be fetched from:
|
|||||||
`golangci-lint run`; other repos use the same pattern around their own
|
`golangci-lint run`; other repos use the same pattern around their own
|
||||||
linter (eslint, ruff, prettier). Copy the canonical file from
|
linter (eslint, ruff, prettier). Copy the canonical file from
|
||||||
`REPO_POLICIES.md`. The linter is invoked directly there, never via
|
`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
|
- [ ] Gitea Actions workflow at `.gitea/workflows/check.yml` that runs
|
||||||
`script/cibuild` on push — reference
|
`script/cibuild` on push — reference
|
||||||
`https://git.eeqj.de/sneak/prompts/raw/branch/main/.gitea/workflows/check.yml`
|
`https://git.eeqj.de/sneak/prompts/raw/branch/main/.gitea/workflows/check.yml`
|
||||||
|
|||||||
@@ -108,7 +108,10 @@ style conventions are in separate documents:
|
|||||||
build step, where there is no daemon. Lint is not skipped by this: it runs
|
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
|
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
|
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
|
For non-server repos, the Dockerfile should bring up a development
|
||||||
environment and run those checks. For server repos, they should run as an
|
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
|
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
|
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.
|
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
|
- **Keep `golangci-lint config verify`, and it costs no network.** The two
|
||||||
commands catch **disjoint** classes of defect, measured under the pinned
|
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
|
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,
|
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
|
sometimes uncommitted work can be copied into an image layer. The directory is
|
||||||
also created and destroyed constantly, so it invalidates `COPY . .` for
|
also created and destroyed constantly, so it invalidates `COPY . .` for
|
||||||
reasons that have nothing to do with the repo's own content. In `.gitignore`
|
reasons that have nothing to do with the repo's own content. **And because
|
||||||
the entry is `.claude/`, unanchored, which already matches at every depth. In
|
`Dockerfile.lint` and `Dockerfile` run their tooling over the copied context,
|
||||||
`.dockerignore` it is `.claude`, anchored and with **no** `**/` prefix: the
|
a worktree that reaches it is linted and tested as though it were the repo.**
|
||||||
directory occurs exactly once **where agents run at the repo root**, and the
|
Nothing else stops that: language toolchains discover files by walking the
|
||||||
prefixed form would also match any nested directory of that name and delete it
|
tree and do not read `.gitignore`, which is how `sneak/quak` saw a nested
|
||||||
from the build. It is not case-folded the way the secret patterns are, because
|
`.claude/` worktree take its discovered test count from 210 to 1050
|
||||||
tooling creates it in exactly one spelling, so a folded pattern would add no
|
(https://git.eeqj.de/sneak/quak/issues/30). This entry is therefore a
|
||||||
coverage.
|
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
|
**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
|
the agent's working directory, so the "exactly once, at the root" premise is
|
||||||
|
|||||||
Reference in New Issue
Block a user