sneak/quak
1
0
Fork 0
Code Issues 23 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

Run all linting in Docker via Dockerfile.lint + script/lint #30

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

Owner ruling, sneak 2026-08-09: every lint run happens inside a Docker container, invoked through the script/ entrypoint. Docker is always available. Linting runs independently and does not need a cache. He has directed a PR for every repo not already set up this way.

This is a TypeScript repo, so the pattern applies to ESLint and prettier rather than golangci-lint — the ruling is about every lint run, not about Go. Reference shape is sneak/homoicon: a root Dockerfile.lint that COPYs the repo into a digest-pinned image and runs the linter as a build step, so a successful build IS a clean lint, with script/lint reduced to building it. That also works where the docker daemon is remote and bind mounts are impossible.

For this repo that means a digest-pinned node image, yarn install --frozen-lockfile, then eslint and prettier as build steps. Copy the manifests before the sources so dependency installation caches separately from the lint steps.

Two things to get right, both of which would otherwise ship a false green:

  1. A cached build lints nothing. A lint build on an unchanged tree returns success in well under a second having linted nothing. Caching is explicitly waived here, so force the lint layers to execute.
  2. Nested worktrees. This repo already established that the JS toolchain's discovery does not exclude .claude/ and does not read .gitignore, which inflated a test count from 210 to 1050. The same exposure applies to a lint run over the build context, so make sure .dockerignore excludes agent directories or the container lints foreign trees.

Definition of done

  • script/lint runs eslint and prettier only in Docker; no host lint path remains.
  • Two consecutive script/lint runs on an unchanged tree both demonstrably execute the linters.
  • Negative control: introduce a deliberate lint violation, confirm it fails with that specific finding, revert, confirm clean.
  • make check still green.

Canonical tracking issue: sneak/prompts#40

Owner ruling, sneak 2026-08-09: every lint run happens inside a Docker container, invoked through the `script/` entrypoint. Docker is always available. Linting runs independently and does not need a cache. He has directed a PR for every repo not already set up this way. This is a TypeScript repo, so the pattern applies to ESLint and prettier rather than golangci-lint — the ruling is about every lint run, not about Go. Reference shape is `sneak/homoicon`: a root `Dockerfile.lint` that COPYs the repo into a digest-pinned image and runs the linter **as a build step**, so a successful build IS a clean lint, with `script/lint` reduced to building it. That also works where the docker daemon is remote and bind mounts are impossible. For this repo that means a digest-pinned node image, `yarn install --frozen-lockfile`, then eslint and prettier as build steps. Copy the manifests before the sources so dependency installation caches separately from the lint steps. Two things to get right, both of which would otherwise ship a false green: 1. **A cached build lints nothing.** A lint build on an unchanged tree returns success in well under a second having linted nothing. Caching is explicitly waived here, so force the lint layers to execute. 2. **Nested worktrees.** This repo already established that the JS toolchain's discovery does not exclude `.claude/` and does not read `.gitignore`, which inflated a test count from 210 to 1050. The same exposure applies to a lint run over the build context, so make sure `.dockerignore` excludes agent directories or the container lints foreign trees. ## Definition of done - `script/lint` runs eslint and prettier only in Docker; no host lint path remains. - Two consecutive `script/lint` runs on an unchanged tree both demonstrably execute the linters. - Negative control: introduce a deliberate lint violation, confirm it fails with that specific finding, revert, confirm clean. - `make check` still green. Canonical tracking issue: https://git.eeqj.de/sneak/prompts/issues/40
clawbot commented 2026-08-10 14:27:47 +02:00
Author
Collaborator
Copy Link

Implementation requirements, specific to this repo's current shape.

The recursion trap. Dockerfile (check stage) runs make check -> script/check -> script/lint. The moment script/lint becomes docker build, the containerised check stage tries to run Docker inside a container and the CI build breaks. script/precommit calls script/lint too. So this change is not confined to script/lint; the Dockerfile/script/check split has to be reworked so that lint happens exactly once, in Dockerfile.lint, and the check image never invokes a nested docker build. The existing lint stage inside Dockerfile (with its COPY --from=lint ordering hack) is the thing being replaced — it should not survive alongside Dockerfile.lint.

Cache. No lint cache is wanted, and a cached lint layer lints nothing while exiting 0. Whatever mechanism forces execution, it must fail closed the way CHECK_EPOCH does in Dockerfile today — an unset build arg is the empty string, which is a perfectly stable cache key, so the guard has to reject it rather than accept a silent false green.

Build context. .dockerignore already excludes .claude/ for exactly the reason the issue describes; keep that exclusion and confirm it still holds for the lint context. Note the deliberate exception at the top of that file: .gitignore stays in the context because prettier 3 reads it as a default ignore file, so the set of files linted inside the container matches the set linted outside.

Evidence required in the PR body, since none of this is visible from a green exit code: the two consecutive runs both showing linter output, the negative control failing on the specific introduced violation and passing again after revert, and a file count or equivalent showing the container linted this tree only and not a nested agent directory.

Out of scope: #29 (prettier running twice under make check) touches the same scripts and is the next unit — do not fix it here, but do not make it harder to fix either.

Implementation requirements, specific to this repo's current shape. **The recursion trap.** `Dockerfile` (check stage) runs `make check` -> `script/check` -> `script/lint`. The moment `script/lint` becomes `docker build`, the containerised check stage tries to run Docker inside a container and the CI build breaks. `script/precommit` calls `script/lint` too. So this change is not confined to `script/lint`; the `Dockerfile`/`script/check` split has to be reworked so that lint happens exactly once, in `Dockerfile.lint`, and the check image never invokes a nested `docker build`. The existing `lint` stage inside `Dockerfile` (with its `COPY --from=lint` ordering hack) is the thing being replaced — it should not survive alongside `Dockerfile.lint`. **Cache.** No lint cache is wanted, and a cached lint layer lints nothing while exiting 0. Whatever mechanism forces execution, it must fail closed the way `CHECK_EPOCH` does in `Dockerfile` today — an unset build arg is the empty string, which is a perfectly stable cache key, so the guard has to reject it rather than accept a silent false green. **Build context.** `.dockerignore` already excludes `.claude/` for exactly the reason the issue describes; keep that exclusion and confirm it still holds for the lint context. Note the deliberate exception at the top of that file: `.gitignore` stays in the context because prettier 3 reads it as a default ignore file, so the set of files linted inside the container matches the set linted outside. **Evidence required in the PR body**, since none of this is visible from a green exit code: the two consecutive runs both showing linter output, the negative control failing on the specific introduced violation and passing again after revert, and a file count or equivalent showing the container linted this tree only and not a nested agent directory. Out of scope: https://git.eeqj.de/sneak/quak/issues/29 (prettier running twice under `make check`) touches the same scripts and is the next unit — do not fix it here, but do not make it harder to fix either.
clawbot commented 2026-08-10 14:30:57 +02:00
Author
Collaborator
Copy Link

Implementation plan for #30.

New Dockerfile.lint (root, reference shape sneak/homoicon), reusing the digest already pinned in Dockerfile (node 22.22.0 on Alpine 3.23.3):

  1. COPY script/, package.json, yarn.lock, then RUN script/bootstrap — dependency install caches as its own layer, per repo policy the image bootstraps via the script rather than duplicating installs inline (script/bootstrap ends in yarn install --frozen-lockfile).
  2. COPY . .
  3. ARG LINT_EPOCH + RUN [ -n "$LINT_EPOCH" ] || exit 1, exactly the fail-closed contract CHECK_EPOCH has today: an unset arg is the empty string and therefore a stable cache key, so a bare docker build -f Dockerfile.lint . hard-fails instead of serving a cached green.
  4. RUN yarn run eslint . and RUN yarn run prettier --check . as build steps below the guard, so a successful build IS a clean lint. Both layers are children of the invalidated guard layer, so they cannot be served from cache.

Breaking the recursion. script/lint becomes docker build --build-arg LINT_EPOCH=... -f Dockerfile.lint ., so nothing that runs inside a container may call it:

  • The lint stage in Dockerfile goes away entirely, including the COPY --from=lint /app/yarn.lock /dev/null ordering hack — Dockerfile.lint replaces it.
  • The remaining stage in Dockerfile runs make test and make build under the existing CHECK_EPOCH guard, and no longer runs make check, because script/check calls script/lint and that is now a nested docker build. Lint therefore happens exactly once, in Dockerfile.lint.
  • script/cibuild becomes the composite gate: it runs script/lint first (fail fast on lint, one definition of the lint build), then the Dockerfile test/build image.
  • script/check and script/precommit keep calling script/lint unchanged — on the host that now routes to Docker, which is the ruling.

Build context. .dockerignore at the context root applies to -f Dockerfile.lint builds too (no Dockerfile.lint.dockerignore will exist to override it). The .claude/ exclusion and the deliberate .gitignore-stays-in-context exception are preserved as-is; I will verify by counting the files actually present inside the lint image.

Tests. New test/packaging/lint-docker.test.ts alongside the existing packaging assertions: script/lint runs only docker build -f Dockerfile.lint and no host linter; Dockerfile.lint is digest-pinned, copies manifests before sources, carries the fail-closed epoch guard, and runs both linters as build steps; Dockerfile contains no lint stage and no make check/make lint; script/cibuild runs the lint build before the check build.

Docs. README Entrypoints section updated for the new script/lint, script/cibuild and Dockerfile split; TODO.md gets a Completed Steps entry (its Next Step is the README API reference, unrelated, so that stays put).

Verification will be posted with the evidence: two consecutive script/lint runs both showing linter execution, a negative control for an eslint violation and separately a formatting violation each failing on the specific finding then passing after revert, an in-image file count proving only this tree was linted, and make check plus script/cibuild shown executing rather than cached.

Out of scope and untouched: #29.

Implementation plan for https://git.eeqj.de/sneak/quak/issues/30. **New `Dockerfile.lint`** (root, reference shape `sneak/homoicon`), reusing the digest already pinned in `Dockerfile` (node 22.22.0 on Alpine 3.23.3): 1. `COPY script/`, `package.json`, `yarn.lock`, then `RUN script/bootstrap` — dependency install caches as its own layer, per repo policy the image bootstraps via the script rather than duplicating installs inline (`script/bootstrap` ends in `yarn install --frozen-lockfile`). 2. `COPY . .` 3. `ARG LINT_EPOCH` + `RUN [ -n "$LINT_EPOCH" ] || exit 1`, exactly the fail-closed contract `CHECK_EPOCH` has today: an unset arg is the empty string and therefore a stable cache key, so a bare `docker build -f Dockerfile.lint .` hard-fails instead of serving a cached green. 4. `RUN yarn run eslint .` and `RUN yarn run prettier --check .` as build steps below the guard, so a successful build IS a clean lint. Both layers are children of the invalidated guard layer, so they cannot be served from cache. **Breaking the recursion.** `script/lint` becomes `docker build --build-arg LINT_EPOCH=... -f Dockerfile.lint .`, so nothing that runs inside a container may call it: - The `lint` stage in `Dockerfile` goes away entirely, including the `COPY --from=lint /app/yarn.lock /dev/null` ordering hack — `Dockerfile.lint` replaces it. - The remaining stage in `Dockerfile` runs `make test` and `make build` under the existing `CHECK_EPOCH` guard, and no longer runs `make check`, because `script/check` calls `script/lint` and that is now a nested `docker build`. Lint therefore happens exactly once, in `Dockerfile.lint`. - `script/cibuild` becomes the composite gate: it runs `script/lint` first (fail fast on lint, one definition of the lint build), then the `Dockerfile` test/build image. - `script/check` and `script/precommit` keep calling `script/lint` unchanged — on the host that now routes to Docker, which is the ruling. **Build context.** `.dockerignore` at the context root applies to `-f Dockerfile.lint` builds too (no `Dockerfile.lint.dockerignore` will exist to override it). The `.claude/` exclusion and the deliberate `.gitignore`-stays-in-context exception are preserved as-is; I will verify by counting the files actually present inside the lint image. **Tests.** New `test/packaging/lint-docker.test.ts` alongside the existing packaging assertions: `script/lint` runs only `docker build -f Dockerfile.lint` and no host linter; `Dockerfile.lint` is digest-pinned, copies manifests before sources, carries the fail-closed epoch guard, and runs both linters as build steps; `Dockerfile` contains no lint stage and no `make check`/`make lint`; `script/cibuild` runs the lint build before the check build. **Docs.** README Entrypoints section updated for the new `script/lint`, `script/cibuild` and Dockerfile split; `TODO.md` gets a Completed Steps entry (its Next Step is the README API reference, unrelated, so that stays put). Verification will be posted with the evidence: two consecutive `script/lint` runs both showing linter execution, a negative control for an eslint violation and separately a formatting violation each failing on the specific finding then passing after revert, an in-image file count proving only this tree was linted, and `make check` plus `script/cibuild` shown executing rather than cached. Out of scope and untouched: https://git.eeqj.de/sneak/quak/issues/29.
clawbot commented 2026-08-10 14:40:42 +02:00
Author
Collaborator
Copy Link

Implemented on next as fed39d19cf3cdbfc695e50045a0055cc743c66d1, opened as #31 (no open next -> main PR existed, so this commit starts the new cycle). Full evidence is in the PR body; summary against the definition of done:

script/lint runs eslint and prettier only in Docker. New root Dockerfile.lint copies the repo into the node image already digest-pinned in Dockerfile and runs both linters as build steps, so a successful build is a clean lint. script/lint is reduced to building it. No host lint path remains: the script invokes no linter against the working tree, and "lint": "eslint ." is deleted from package.json so yarn lint can no longer produce a verdict from an unpinned local toolchain.

Recursion. script/lint is a docker build, so the lint stage in Dockerfile and its COPY --from=lint /app/yarn.lock /dev/null ordering hack are gone, and that image no longer runs make check — it runs make test and make build under the unchanged CHECK_EPOCH guard. script/cibuild is now the composite gate and builds the lint image first, then the test/build image, so lint happens exactly once and nothing in a container invokes a nested docker build.

Cache, fail closed. ARG LINT_EPOCH plus RUN [ -n "$LINT_EPOCH" ] || exit 1, with both linters below the guard so a fresh epoch forces them to execute. Verified both directions: a bare docker build -f Dockerfile.lint . exits 1 at the guard rather than serving a green, and two consecutive make lint runs on an untouched tree both ran the linters — CACHED appeared only on the bootstrap and dependency layers, with steps 8 and 9 showing eslint and prettier output and the guard line echoing a different epoch each run.

Negative controls. An unused local failed the build at the eslint step on that exact finding ('deliberatelyUnused' is assigned a value but never used), and separately a mangled-spacing file failed at the prettier step naming that file; both were clean again after revert.

Foreign trees. .dockerignore is unchanged, including the .claude/ exclusion and the deliberate exception that keeps .gitignore in the context for prettier 3. Verified from inside the image: 46 TS/JS files, 23 files under test/, no .claude, .gitignore present. An active control planted a worktree at .claude/worktrees/foreign/test/ containing both an eslint error and a formatting error of the kinds the build demonstrably catches; the lint build still passed, so the container did not see it. A new test also asserts no per-Dockerfile .dockerignore shadows the root one for either image.

make check green: 22 test files, 244 tests. script/cibuild green and shown executing.

Two things for the reviewer. First, the Dockerfile image no longer runs make check as REPO_POLICIES.md asks; that is unavoidable once script/check reaches script/lint and script/lint is a docker build, and coverage is preserved because script/cibuild builds both images. Second, the whole unit landed as a single commit rather than the README's tests-first pair, per the instruction for this work item.

#29 was left alone: script/fmt-check still runs prettier on the host, unchanged.

Implemented on `next` as `fed39d19cf3cdbfc695e50045a0055cc743c66d1`, opened as https://git.eeqj.de/sneak/quak/pulls/31 (no open `next` -> `main` PR existed, so this commit starts the new cycle). Full evidence is in the PR body; summary against the definition of done: **`script/lint` runs eslint and prettier only in Docker.** New root `Dockerfile.lint` copies the repo into the node image already digest-pinned in `Dockerfile` and runs both linters as build steps, so a successful build is a clean lint. `script/lint` is reduced to building it. No host lint path remains: the script invokes no linter against the working tree, and `"lint": "eslint ."` is deleted from `package.json` so `yarn lint` can no longer produce a verdict from an unpinned local toolchain. **Recursion.** `script/lint` is a `docker build`, so the `lint` stage in `Dockerfile` and its `COPY --from=lint /app/yarn.lock /dev/null` ordering hack are gone, and that image no longer runs `make check` — it runs `make test` and `make build` under the unchanged `CHECK_EPOCH` guard. `script/cibuild` is now the composite gate and builds the lint image first, then the test/build image, so lint happens exactly once and nothing in a container invokes a nested `docker build`. **Cache, fail closed.** `ARG LINT_EPOCH` plus `RUN [ -n "$LINT_EPOCH" ] || exit 1`, with both linters below the guard so a fresh epoch forces them to execute. Verified both directions: a bare `docker build -f Dockerfile.lint .` exits 1 at the guard rather than serving a green, and two consecutive `make lint` runs on an untouched tree both ran the linters — `CACHED` appeared only on the bootstrap and dependency layers, with steps 8 and 9 showing eslint and prettier output and the guard line echoing a different epoch each run. **Negative controls.** An unused local failed the build at the eslint step on that exact finding (`'deliberatelyUnused' is assigned a value but never used`), and separately a mangled-spacing file failed at the prettier step naming that file; both were clean again after revert. **Foreign trees.** `.dockerignore` is unchanged, including the `.claude/` exclusion and the deliberate exception that keeps `.gitignore` in the context for prettier 3. Verified from inside the image: 46 TS/JS files, 23 files under `test/`, no `.claude`, `.gitignore` present. An active control planted a worktree at `.claude/worktrees/foreign/test/` containing both an eslint error and a formatting error of the kinds the build demonstrably catches; the lint build still passed, so the container did not see it. A new test also asserts no per-Dockerfile `.dockerignore` shadows the root one for either image. **`make check` green:** 22 test files, 244 tests. `script/cibuild` green and shown executing. Two things for the reviewer. First, the `Dockerfile` image no longer runs `make check` as `REPO_POLICIES.md` asks; that is unavoidable once `script/check` reaches `script/lint` and `script/lint` is a `docker build`, and coverage is preserved because `script/cibuild` builds both images. Second, the whole unit landed as a single commit rather than the README's tests-first pair, per the instruction for this work item. https://git.eeqj.de/sneak/quak/issues/29 was left alone: `script/fmt-check` still runs prettier on the host, unchanged.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
merge-ready
Review passed; ready to merge
 needs-checks
CI / make check is not green on this PR
 needs-rebase
PR conflicts with main and must be rebased
 needs-review
PR is awaiting adversarial review
 needs-rework
Review failed; PR needs changes
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
clawbot sneak
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/quak#30
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?