Run all linting in Docker via Dockerfile.lint (closes #30) #31
Reference in New Issue
Block a user
Delete Branch "next"
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?
Closes #30.
What changed
New root
Dockerfile.lint. It copies the repo into the node image already pinned by digest inDockerfile(node 22.22.0 on Alpine 3.23.3) and runs eslint and prettier as build steps, so a successful build IS a clean lint.script/lintis reduced to building it, which also works where the docker daemon is remote and bind mounts are impossible. Manifests (package.json,yarn.lock) are copied before the sources, soscript/bootstrapand itsyarn install --frozen-lockfilecache as their own layer while the lint steps do not.No host lint path survives.
script/lintno longer invokes a linter against the working tree, and"lint": "eslint ."is gone frompackage.json— that was a second, unpinned way to get a lint verdict out of whatever eslint the tree happened to have installed.Cache is waived for lint, fail closed.
LINT_EPOCHhas exactly the contractCHECK_EPOCHhas today:ARG LINT_EPOCHfollowed byRUN [ -n "$LINT_EPOCH" ] || exit 1, with both linters below the guard so a fresh epoch forces them to execute. An unset ARG is the empty string, which is a perfectly stable cache key, so a baredocker build -f Dockerfile.lint .hard-fails rather than serving a cached green.Recursion broken.
script/lintis now adocker build, so nothing inside a container may call it, andscript/checkcallsscript/lint:lintstage inDockerfileis deleted, including itsCOPY --from=lint /app/yarn.lock /dev/nullordering hack —Dockerfile.lintreplaces it, and two definitions of how to lint is one too many.Dockerfilestage no longer runsmake check. It runsmake testandmake buildunder the unchangedCHECK_EPOCHguard.script/cibuildis the composite gate: it runsscript/lintfirst (one definition of the lint build, and a lint failure is reported before the slower suite), then theDockerfileimage.script/checkandscript/precommitstill callscript/lint; on the host that now routes to docker, which is the ruling.Build context.
.dockerignoreis unchanged and applies to the-f Dockerfile.lintbuild too, including the.claude/exclusion (eslint's flat config does not ignore dot-directories, so a nested worktree in the context would be linted) and the deliberate exception that keeps.gitignorein the context because prettier 3 reads it as a default ignore file.Tests.
test/packaging/lint-docker.test.tsasserts the whole shape;test/packaging/build-context.test.tsgains an assertion that no<Dockerfile>.dockerignoreshadows the root one for either image (BuildKit supports per-Dockerfile ignore files, and one would silently give the lint build a different, unreviewed context).Docs. README Entrypoints section rewritten for the new split, with new "Linting" and "Build epochs" subsections;
TODO.mdgains a Completed Steps entry. Its Next Step is the README API reference, which is unrelated, so that was left alone.Verification
Exit codes prove nothing here, so this is the output.
Fail-closed guard: bare build is rejected
docker build -f Dockerfile.lint .with noLINT_EPOCH, exit 1:Two consecutive
make lintruns on an unchanged tree, both executingRun 1, exit 0 in 45s. Run 2, immediately after with nothing touched, exit 0 in 12s — and the linter layers ran again rather than being served:
CACHEDappears only on layers 6 to 9, which are the bootstrap and dependency install — exactly the intended split. The guard line echoes the epoch it received (RUN [ -n "1786365306" ]), which is what differs run to run.Negative control 1: eslint violation
Added
src/lint-negative-control.tswith an unused local.make lintexit 2, failing on that specific finding:Reverted,
make lintexit 0 with both linters executing again.Negative control 2: formatting violation
Added
src/fmt-negative-control.tswith mangled spacing.make lintexit 2, failing on that specific file at the prettier step:Reverted, exit 0 and
All matched files use Prettier code style!.The container linted this tree only
From inside the built lint image:
Both halves of the
.dockerignorecontract hold: no.claude/, and.gitignorestill present for prettier.As an active control, a foreign tree was planted at
.claude/worktrees/foreign/test/foreign.test.tscontaining both an unused-variable error and mangled formatting — violations that the two negative controls above prove this build catches.make lintstill passed, exit 0 withAll matched files use Prettier code style!, so the container did not see it. The tree was removed afterwards.make checkExit 0 in 19s: 22 test files, 244 tests passed, then the containerised lint, then
fmt-check.script/cibuildExit 0 in 37s, and it executed rather than being served from cache. Lint image first (
RUN [ -n "1786365490" ], then eslint and prettier running), then the test and build image:make fmtwas run and its result is in the commit. Nodocker builder prunewas run at any point; cache invalidation was scoped to the epoch arguments only.Notes
maketargets andscript/entrypoints. The only directdockercalls were the deliberate bare-build negative control and a one-offdocker runto list the image contents, neither of which can be expressed as an entrypoint.Dockerfileimage no longer runsmake check, whichREPO_POLICIES.mdasks for. That is unavoidable oncescript/checkreachesscript/lintandscript/lintis adocker build— the alternatives are docker-in-docker or a second, cached lint path. Coverage is preserved byscript/cibuildbuilding both images.make check).script/fmt-checkstill runs prettier on the host, unchanged, since that issue is what decides who owns the prettier invocation.Review: PASS
Independent review of #31 (head
fed39d19cf3cdbfc695e50045a0055cc743c66d1) against #30. All evidence below was reproduced in a fresh clone, not read from the PR body.Verified
docker build -f Dockerfile.lint .exits 1 at[7/9] RUN [ -n "$LINT_EPOCH" ] || exit 1. No green served.make lintruns, unchanged tree. 15.6s and 15.3s.CACHEDappears on layers 2-6 only (WORKDIR,COPY script/,COPY package.json yarn.lock,RUN script/bootstrap,COPY . .); the guard and both linter layers executed both times with real output ($ /app/node_modules/.bin/eslint .,Checking formatting... All matched files use Prettier code style!) and different epochs (1786365763,1786365779). The epoch busts exactly the lint layers and nothing else — the split works as designed.error 'neverUsedByReviewer' is assigned a value but never used ... @typescript-eslint/no-unused-vars, build failed atRUN yarn run eslint .. Reverted, green.[warn] src/pr31-review-fmt-probe.ts, failed atRUN yarn run prettier --check .. Reverted, green..claude/worktrees/pr31foreign/carrying both an unused-variable error and a formatting error — violations this build provably catches.make lintexit 0: the container never saw it. Confirmed from inside the image:.claudeabsent,.gitabsent,.gitignorepresent (the deliberate exception preserved), 46 TS/JS files, 23 files undertest/.make fmt-check+make lint; check stage ranmake check+make build. After:Dockerfile.lintruns eslint + prettier,Dockerfilerunsmake test+make build. Net executed set is identical — eslint,prettier --check, vitest,tsc+ entrypoint verification. Confirmed in onescript/cibuildrun (42.9s, exit 0): eslint ran, prettier ran,Test Files 22 passed / Tests 244 passed,build: verified ./dist/src/index.jsetc.make testandmake buildwere notCACHED.script/checkreachesscript/lint; nothing inside either container reachesscript/checkorscript/lint.Dockerfile.lintinvokes the linters directly rather than viamake lint, which is what would have recursed.script/cibuildcompleted, which is the empirical proof.script/lint, missingLINT_EPOCHguard, per-Dockerfile ignore file, linter hoisted above the guard, tag instead of digest,make checkreintroduced intoDockerfile,script/cibuildorder swapped,lintscript restored topackage.json, manifests/sources order swapped, eslint step removed. Each fired its own named test and no others. These are not vacuous.Dockerfile(sha256:e4bf2a82...26e34), with the house-style# node 22.22.0 on Alpine 3.23.3 (node:22-alpine), 2026-08-09comment on the preceding line.make checkexit 0 (244 tests).make fmtproduces no diff. CI green on head. Fast-forward mergeable againstmain(156fe87), zero conflicts. Commit subject carries(closes #30), no trailers, no attribution anywhere in the diff or commit metadata.make lintandmake fmt-checkboth still do what their names say. Nothing else referenced the removedyarn lint. Naming and terminology are consistent.Non-blocking
REPO_POLICIES.mdis now internally inconsistent with the repo, and was not updated. Lines 92-108 still state "All Dockerfiles must runmake check" and mandate the separate lint stage with theCOPY --from=lintordering dependency. This change deliberately does neither, correctly, per the newer owner ruling in #30. The implementer disclosed the deviation. Raising as a question rather than a defect, sinceREPO_POLICIES.mdlooks like a shared canonical document rather than a per-repo file: does the ruling supersede those two clauses repo-wide, and where should that be written down? Nothing enforcesREPO_POLICIES.mdtoday, so the drift is silent.script/fmt-checkstill runsprettier --checkon the host, which is in literal tension with the definition of done's "no host lint path remains". My view: defensible, not a violation. It is a formatting entrypoint required byREPO_POLICIES.md, not a lint verdict, and #29 explicitly owns this question as the next unit. I checked specifically whether this change lets the host and container verdicts disagree, and it does not: prettier 3 reads.gitignore(which excludes.claude/),.dockerignoremirrors it while deliberately keeping.gitignorein the context, and prettier is pinned to 3.8.1 installed under--frozen-lockfile. Empirically, with the foreign tree planted, hostmake fmt-checkand the containerised prettier both reported clean. This change does not make #29 harder — it arguably makes it easier, sinceDockerfile.lintis now the single owner of prettier in CI.Pre-existing and out of scope, flagged for tracking, not filed here. The foreign-tree exposure is now closed for lint but remains open for the host test path. With
.claude/worktrees/pr31foreign/planted, hostmake testpicked the tree up — 23 test files instead of 22, exit 2 — while the containerised run was unaffected. CI is safe because.dockerignoreexcludes.claude/; only a developer's hostmake checkis affected. Same exposure class the issue cites, different entrypoint. Not introduced by this change and not in scope for it.Disclosure
Docker is now a hard requirement for
make checkand for the pre-commit hook. That is the ruling, and it is documented in the README "Linting" section, thescript/checkheader and thescript/precommitheader, so it is not a silent change. Two evidence items I took on inspection rather than execution: the claim that a<Dockerfile>.dockerignorewould shadow the root one is BuildKit-documented behaviour I did not exercise (the test asserts absence of such a file, which is the useful assertion either way), and I did not test behaviour on a host with no docker installed beyond confirming the failure would be a loudcommand not foundrather than a silent skip.No blocking defects found. Recommend
merge-ready.Unit #30 landed on
nextand passed independent review; all evidence was reproduced by the reviewer in a separate clone rather than taken from the PR body.This PR stays open and accumulates the rest of the cycle — it is not merge-ready as a whole until the 1.0.0 milestone lands. Next commit: #29.
Two items from the review are being handled outside this PR: the
REPO_POLICIES.mddrift goes to the canonical copy rather than being patched per-repo, and the hostmake testforeign-tree exposure the reviewer reproduced is already tracked as #25.Added commit
a73f0ab, "Check formatting once per make check, in the container", closing #29. It is a direct follow-on to the change this PR already carries: moving lint intoDockerfile.lintis what turned the duplicate prettier pass from two identical host runs into one container run and one host run, and this removes the host one.What changed
script/checkno longer callsscript/fmt-check. It runsscript/testthenscript/lint, andscript/lintis the build ofDockerfile.lint, which runsprettier --check .as a build step.script/precommithad the same pair and gets the same fix:script/lintonly.script/fmt-checkis untouched and still wired tomake fmt-check, asREPO_POLICIES.mdrequires — it is now a standalone entrypoint for asking the formatting question by itself, without docker, rather than a step inside two other scripts.test/packaging/lint-once.test.tsis new and is what makes the guarantee non-vacuous. It statically walks the invocation graph from an entrypoint, following the edges this repo actually uses:"$SCRIPT_DIR/<name>"andscript/<name>into other scripts,make <target>through the Makefile shims,yarn run <name>through thepackage.jsonscripts, anddocker build -f <file>into that Dockerfile'sRUNsteps. Comments are stripped first, since the headers of these very scripts nameprettierandscript/fmt-checkwhile explaining why they must not run twice. It asserts one prettier invocation permake check, one perscript/precommit, and one each formake lintandmake fmt-checkalone — the last two so a count of 1 cannot be achieved by gutting the targets. It also asserts which nodes the walk reached, so a restructure that defeats the resolver fails loudly instead of counting zero and reading as the tidiest possible result; and it re-asserts the three alignment invariants below.README and
TODO.mdupdated to match;make fmtrun and included.Prettier invocation count, observed
Not inferred from the diff — counted in the output of a real run, before and after.
Before,
make checkonfed39d1, exit 0 in 19s, two verdicts:grep -c "All matched files use Prettier code style"= 2 (one container, one host).After,
make checkona73f0ab, exit 0 in 18s:grep -c= 1.Test Files 23 passed (23),Tests 258 passed (258).The test can fail
Mutated three ways, each run through
script/test:"$SCRIPT_DIR/fmt-check"toscript/check— 2 failures:expected 2 to be 1, andexpected [ 'make:check', 'script/check', …(5) ] to not include 'script/fmt-check'.RUN yarn run prettier --check .fromDockerfile.lint— 5 failures across both new and existing tests:invokes prettier once for the whole of make check,keeps the surviving invocation inside the lint container,still checks formatting under make lint,script/precommit checks formatting exactly once, plus the pre-existingDockerfile.lint runs prettier as a build step.script/fmt-checkwith anecho— 1 failure:still checks formatting under make fmt-check.All three reverted; suite green afterwards.
Worth recording that the test caught a real defect in its own first draft:
node.slice("make:")(a string where a number is required, coercing toNaN) made everymake-rooted walk return zero, and the reachability assertions failed rather than the count quietly passing at 0. That is the failure mode those assertions exist for.Standalone targets
Each invoked on its own, on
a73f0ab:make fmt-check— exit 0 in 2s,All matched files use Prettier code style!, one prettier invocation. Still does what its name says, on the host, without docker.make lint— exit 0 in 74s, one prettier verdict, both linters executing rather than served:#12 [8/9] RUN yarn run eslint .DONE 3.6s,#13 [9/9] RUN yarn run prettier --check .DONE 2.7s.make check— exit 0 in 18s, as above.What catches a formatting problem before a commit lands
make fmt-checkis now the only host-side formatting check, but it is not what gates commits and never was the only gate. The pre-commit hook runsscript/precommit, which runsscript/lint, which buildsDockerfile.lint, which runsprettier --check .as a build step under theLINT_EPOCHguard. A badly formatted tree fails that build, fails the hook, and fails the commit — exactly as before this change, since the hook already ranscript/lintfirst and would have failed there before ever reachingscript/fmt-check. Negative control 2 in this PR's description is the evidence that the container step rejects a formatting violation. The hook lost a redundant second pass, not a capability.Host and container verdicts still cannot disagree
Re-verified on
a73f0abrather than assumed, and now asserted by the test:package.json:"prettier": "3.8.1"— an exact pin, not a range.script/bootstrap, which is whatDockerfile.lintruns to install:yarn install --frozen-lockfile, both code paths..dockerignoredoes not list.gitignore, so it stays in the build context — prettier 3 reads it as a default ignore file, and both sides therefore see the same file set.Nothing in this change touches any of the three.
script/cibuild, shown to have executed
Exit 0 in 19s. A cached green would show
CACHEDon the layers that matter; hereCACHEDappears on 10 layers, all of them the dependency layers above the epoch guards (WORKDIR,COPY script/,COPY package.json yarn.lock,RUN script/bootstrap,COPY . .) — which is the intended split. Every layer below the guards ran, with real durations, and the guard lines echo the epoch they received:Two distinct epochs, one per image, both non-empty. Exactly one prettier invocation in the whole of
script/cibuild.No
docker builder prunewas run at any point, in any form; no cache was invalidated beyond the epoch build arguments. All verification went throughmaketargets andscript/entrypoints — there was no directdocker buildthis time, since no guard or cache probe was needed beyond whatscript/cibuildalready demonstrates.REPO_POLICIES.mdwas left alone, per instruction.Review: PASS
Independent review of commit
a73f0abonly (diffed against its already-reviewed parentfed39d1), against #29. Reproduced in a fresh clone; nothing taken from the PR body.Central claim reproduced
make checkprettier verdicts, counted in real runs: 2 onfed39d1(container#13 [9/9] RUN yarn run prettier --check ., then hostnode_modules/.bin/prettier --check .), 1 ona73f0ab, and the survivor is the container one. Both exit 0. In thea73f0abrunCACHEDcovers layers 6-10 only; the guard echoedRUN [ -n "1786367380" ]and eslint/prettier ran for 10.2s/8.9s.Negative controls, executed:
make checkexit 2,[warn] src/q29rev-fmt-probe.tsatRUN yarn run prettier --check .. Coverage not lost.git committhrough the hook installed bymake hooks: exit 1, same prettier failure, HEAD unmoved. The "lost a redundant pass, not a capability" claim holds under test, not just in prose.make fmt-checkalone exit 0 in 1.6s (host, no docker).make lintalone exit 0 in 7.5s with eslint 3.0s and prettier 2.9s executing below the epoch guard.script/cibuildexit 0 in 24.7s:CACHEDonly on the dependency layers of both images,RUN [ -n "1786367878" ]/RUN [ -n "1786367884" ],make test9.8s (258 tests),make build5.9s, exactly one prettier invocation across the whole run.Test falsifiability — six mutations, each fired its own named assertion and no unrelated one: re-adding
fmt-checktoscript/check(expected 2 to be 1+ the not-reached assertion); deleting prettier fromDockerfile.lint(count 0, not 1 — it cannot pass on an empty walk); guttingscript/fmt-check(onlystill checks formatting under make fmt-check); droppingscript/testfromscript/check(only thereaches script/testcase); droppingscript/lint(count + two reachability cases); renaming the Makefilecheck:target (Error: no such Makefile target: check, exit 2). Not vacuous, not a hardcoded count.Also verified: CI green on head (
check / check (push), successful in 59s); fast-forward mergeable ontomain(156fe87);make fmtproduces no diff; commit subject carries(closes #29); no attribution trailers or vendor references anywhere in the diff or commit metadata; inclusive terminology; README Entrypoints/Linting/workflow-item-8/required-checks bullets all match the scripts as they now are;TODO.mdNext Step correctly untouched.script/fmt-checkis now reachable only by a human typingmake fmt-check(oryarn fmt-check) — not dead, but no longer called by anything.Non-blocking findings
1. The new test never walks
script/cibuild, which is what CI runs.test/packaging/lint-once.test.tswalksmake:check,make:lint,make:fmt-check,script/precommitanddocker:Dockerfile.lint— neverscript/cibuild, and therefore neverdocker:Dockerfile. I appendedRUN yarn run prettier --check .toDockerfile(afterRUN make build) andmake teststayed green, 258/258, exit 0. That arrangement givesscript/cibuildtwo prettier passes — the exact duplication this test exists to prevent, reintroduced in the one place CI executes. Why it matters: the file header at lines 20-22 claims "A prettier call added anywhere in that graph is therefore caught, wherever it is added", which overstates what is covered. Acceptable: addexpect(walk("script/cibuild").prettier).toBe(1)and a reachability case fordocker:Dockerfile, which closes the hole in two lines.2.
installs it from the lockfile in the container(line 261) is a whole-filetoContainand misses the branch the container actually takes.script/bootstraphas two install sites: line 130 (nvm path) and line 132 (elsepath).Dockerfile.lintrunsscript/bootstrapinnode:22-alpine, whereyarnis present, somissing yarnis false and line 132 is the path taken. I changed line 132 to a bareyarn installand left line 130 alone: suite green, 258/258. The assertion that is supposed to guarantee the container installs from the lockfile passes while the container's own install has stopped doing so. Acceptable: assert on the branch the container reaches, or assert that no bareyarn installoccurs in the file.3. Prettier is counted per line, not per occurrence.
walkdoesresult.prettier += 1; continue;on the first/\bprettier\b/match in a line.RUN yarn run prettier --check . && yarn run prettier --check srcinDockerfile.lintcounts as one — verified, suite green. Thecontinuealso means anyscript/,makeoryarnedge sharing a line with a prettier call is never followed. Contrived, but it is a hole in a test whose entire job is counting.4. Stale comment made stale by this commit.
test/packaging/entrypoints.test.ts:3still reads "make checkruns test, lint and fmt-check but never the build". The clause the sentence is arguing for still stands, but the premise is now false and the file was not touched.Raised as a question, not filed
REPO_POLICIES.mdline 38 (make check"runstest,lint,fmt-check"), line 65 (script/checkruns those three) and line 182 (a reducedscript/precommit"may skipscript/testand run onlyscript/lintandscript/fmt-check") now all describe a repo that no longer exists. Per the ruling on #31 this drift goes to the canonical copy rather than being patched per-repo, so it is not a defect here — noting only that this commit widens the same gap on three more lines, and nothing enforces that file.Disclosure
keeps .gitignore in the build contextassertion (line 268) only detects a literal.gitignoreline in.dockerignore; an exclusion written as a glob (.git*) would drop it from the context and the assertion would still pass. I confirmed the literal case fails as intended but did not execute a build with a glob form — inspection only.Test Files 1 failed | 22 passed,Tests 244 passed— the 14 assertions vanish rather than fail individually. Exit code 2, so it is loud; noting the shape because the count in the summary silently drops.prettierpinned3.8.1,--frozen-lockfileon bothscript/bootstrappaths as shipped,.claude/in.gitignoreand.dockerignore,.gitignorekept in the context,.prettierignorepresent in both. Unchanged by this commit; finding 2 above is about the test guarding it, not about the property itself.docker builder prunein any form.No blocking defects. Recommend
merge-readyfor this commit; findings 1 and 2 are worth tracking as a follow-up to tighten the new test, not a reason to hold it.Unit #29 landed and passed independent review (a different reviewer from the previous commit).
The review's non-blocking findings are tracked as #33 and go in as the next commit here: the enforcing test added by this unit does not walk
script/cibuild, which is what CI actually runs, and its lockfile assertion checks the wrong bootstrap branch — both shown by mutation, both leaving the suite green. Taking them now rather than later, because a test that reports a guarantee it does not provide is worse than no test.REPO_POLICIES.mdin this repo is now stale in three more places. That file is a copy of the canonical one and gets resynced there, not patched here — see #32.2bfa11c— #33Fixes all four findings in
test/packaging/lint-once.test.ts. Files touched:test/packaging/lint-once.test.ts,test/packaging/entrypoints.test.ts,TODO.md. Nothing else —REPO_POLICIES.mdand theLINT_EPOCHnaming are #32 and were left alone.What changed
1. The walk now starts where CI starts. A
workflow:<path>node kind resolves therun:steps of.gitea/workflows/check.yml, and a second walk is rooted there. It reachesscript/cibuild, and through it bothdocker:Dockerfile.lint(viascript/lint) anddocker:Dockerfile(the baredocker build .), which themake checkwalk never sees. Rooting at the workflow rather than at a hand-picked script is deliberate: the previous version's blind spot was an assumption about what CI runs, so that is now read out of the repo and asserted. New assertions: prettier is invoked exactly once across the whole CI build;script/cibuild,script/lint,docker:Dockerfile.lintanddocker:Dockerfileare all reached; and the test and build image invokes prettier zero times.2. The lockfile assertion targets the executed branch.
install_js_depsis resolved out ofscript/bootstrapand split at itsmissing yarnguard, so the two install sites are separately addressable. The pinned node image ships yarn, so the container takes theelsebranch. Each branch is asserted separately, and everyyarn installoccurrence in a branch must carry--frozen-lockfile, so an unpinned install cannot hide beside a pinned one. A companion assertion requiresdocker:Dockerfile.lintto reachscript/bootstrapat all — without it the lockfile assertions could end up describing a script the image never executes.3. Counting is per occurrence, and edges survive counting.
countPrettiercounts matches on a line rather than answering yes/no, and thecontinueis gone, soedgesOfruns on every line including counted ones. Both are asserted directly as well as through the graph.4. The stale
make checkcomment intest/packaging/entrypoints.test.tsnow says the suite and the lint container, not test/lint/fmt-check.Anti-vacuity. Every way for the walk to reach nothing is now a thrown error rather than a quiet zero: unknown Makefile target, unknown
package.jsonscript, missing script file, node resolving to no commands, unknown node kind. Theyarn:resolver previously returned[""]for a missing script — a silent zero of exactly the kind thenode.slice("make:")/NaNdraft produced. All five are tested, plus a walk that legitimately counts zero (make:clean) and the cycle guard.Mutation matrix
Every assertion in the file, 35 in total. Source-file mutations were applied one at a time. Expectation mutations — changing what an assertion asserts, inside the test file — were applied in four batches; each is a local change to one
it's expectation and cannot influence another test, and vitest names every result, so the evidence is that the failing set was exactly the mutated set in each run. Every mutation was reverted; the final tree is byte-identical to the committed one (diffagainst a pre-mutation copy) and green at 279/279.Source mutations (one run each)
RUN yarn run prettier --check .toDockerfileafterRUN make build— the mutation from the issueinvokes prettier once for the whole CI build(expected 2 to be 1);keeps prettier out of the test and build image(expected 1 to be 0)script/bootstrapsecond install site to bareyarn install, first left alone — the mutation from the issueinstalls from the lockfile on the branch the container takes(expected'yarn install'to contain'--frozen-lockfile')installs from the lockfile on the nvm branch too— and not the container-branch test, which is the cross-check that the two branches are genuinely distinguishedDockerfile.lint:RUN yarn run prettier --check . && yarn run prettier --check src(two invocations, one line)invokes prettier once for the whole of make check;invokes prettier once for the whole CI build;keeps the surviving invocation inside the lint container;still checks formatting under make lint;script/precommit checks formatting exactly once— all "expected 2 to be 1". This is finding 3: the old counter scored this arrangement as 1"$SCRIPT_DIR/fmt-check"toscript/check(the original bug)invokes prettier once for the whole of make check;does not reach the host formatting check from make checkscript/lintdirectly and nevermake check.gitignoreto.dockerignorekeeps .gitignore in the build contextbuild-context.test.ts > leaves .gitignore in the build context for prettier, a pre-existing assertion of the same fact in another file; not incidentalpackage.jsonprettier3.8.1to^3.8.1pins the same prettier for bothcheck:tocheck-all:no such Makefile target: checkand the suite goes red — the loud-failure guarantee against theNaNfailure mode.gitea/workflows/check.ymlrun: script/cibuildtorun: script/checkreaches script/cibuild while counting;reaches docker:Dockerfile while counting;reads the run steps of the CI workflow and not its uses stepsDockerfile.lint: replaceRUN script/bootstrapwithRUN yarn install --frozen-lockfileruns script/bootstrap inside the lint containerscript/lint: drop-f Dockerfile.lintfrom thedocker buildinvokes prettier once for the whole of make check;invokes prettier once for the whole CI build;reaches Dockerfile.lint while counting;reaches docker:Dockerfile.lint while counting;still checks formatting under make lint;script/precommit checks formatting exactly once;gets that check from the lint containerlint-docker.test.ts > lints by building Dockerfile.lint, a pre-existing assertion of the same factExpectation mutations (four runs)
toContaintonot.toContainin both reachabilityit.eachblocks;make fmt-checkcount1to2reaches script/check,reaches script/test,reaches script/lint,reaches Dockerfile.lint(make check);reaches script/cibuild,reaches script/lint,reaches docker:Dockerfile.lint,reaches docker:Dockerfile(CI);still checks formatting under make fmt-check0to1; each of the sixtoThrowpatterns prefixed withMUTANTreports zero for a subgraph that does not run prettier;refuses a Makefile target that does not exist;refuses a package.json script that does not exist;refuses a script that does not exist;refuses a node that resolves to no commands;refuses a node kind it does not understand;refuses to walk in circles. Each failure printed the real error alongside the unmatched pattern, confirming it throws for its own named reason2to1; config-file count0to1;toContaintonot.toContainon the counted-line edge; the spelling list shortened; baredocker buildexpectation todocker:Nope; bothnot.toHaveLength(0)guards totoHaveLength(0)-f Dockerfile.lintexpectation todocker:Nope(the second assertion in that test, unreachable while the first was mutated)follows a bare docker build to Dockerfile and -f to its fileNothing in the file survived mutation, so there is no assertion here that reads as a guarantee without being one.
Verification
make check: green. The lint container executed — the guard line carried a live epoch and the linters ran rather than being served:script/cibuild: exit 0, and executed. Both images, per-layer timings, distinct epochs.CACHEDappears only on the bootstrap and dependency layers, which is the intended split:The suite ran inside the container (13.8s, 279 passed), not from cache.
make fmtwas run and its result is in the commit. Nodocker builder pruneat any point; no cache invalidation beyond the epoch arguments. All verification went throughmaketargets andscript/entrypoints — no rawvitest,prettier,eslintortscinvocation, and no directdocker build.Note
Two facts are now asserted in two places:
.gitignorestaying in the build context (here and intest/packaging/build-context.test.ts) andscript/lintbuildingDockerfile.lint(here and intest/packaging/lint-docker.test.ts). Both showed up as extra failures under mutations F and L. They are deliberate — this file needs them as premises for its own claims — but flagging the overlap in case you would rather they were consolidated.Review of
2bfa11c(implements #33) — FAIL,needs-reworkReviewed only
2bfa11cagainsta73f0ab, in a fresh clone. Every claim below was reproduced by mutation, not read.Definition of done: met. Both original holes are now red, each on its own named test — appending
RUN yarn run prettier --check .toDockerfilefiresinvokes prettier once for the whole CI build(2 vs 1) andkeeps prettier out of the test and build image; a bareyarn installat the secondscript/bootstrapsite firesinstalls from the lockfile on the branch the container takesand nothing else, and mutating the nvm site instead fires onlyinstalls from the lockfile on the nvm branch too— the two branches are genuinely distinguished.prettier --check . && prettier --check srcon one line now scores 2 and fires five assertions. Renaming the Makefilecheck:target errors the whole file withno such Makefile target: checkrather than reporting zero.make checkgreen (37s, eslint 10.5s / prettier 3.9s both executed,CACHEDonly on the bootstrap and dependency layers);script/cibuildexit 0 in 44s with fresh epoch guard values (RUN [ -n "1786370184" ],RUN [ -n "1786370198" ]),make testreporting 279 passed inside the image andmake buildverifying the entrypoints. CI green on the head commit; fast-forwardable ontomain;make fmtclean; commit message carries(closes #33); no attribution trailers;TODO.mdNext Step untouched;entrypoints.test.ts:3corrected.Batched-mutation deviation: verified, not accepted on trust. Six expectation mutations re-run individually —
ci.prettiertoBe(2); the CIit.eachreachability flipped to.not.toContain(all four cases fired, nothing else);walk("docker:Dockerfile").prettiertoBe(1);expect(installs).toHaveLength(0)on the container branch; the workflowtoEqualgiven a second element; the cycletoThrowregex changed. Each failed alone, on its own test, with no collateral. The batching argument holds for this sample.Blocking
1.
make checkcan still run prettier twice with the suite green — two idiomatic Makefile edges are not followed.test/packaging/lint-once.test.ts:86-103(makeRecipes) records only tab-indented recipe lines, andedgesOfat line 180 matches only a literal lowercasemake <target>. Neither prerequisites nor$(MAKE)is an edge. Both reproduced on this commit:Makefile:21changed tocheck: fmt-check—make -n checkprintsscript/fmt-checkthenscript/check, i.e. one host prettier pass plus the container pass. Suite: 23 files passed, green.Makefilegiven@$(MAKE) fmt-checkabove@script/checkin thecheckrecipe —make -n checkprintsmake fmt-check→script/fmt-check, thenscript/check. Suite: 23 files passed, green.That is precisely the duplication this file exists to prevent, reintroduced by the two most ordinary ways to compose make targets — and the most likely way someone "restores"
fmt-checktocheck. It also means the header claim at lines 21-22, "A prettier call added anywhere in that graph is therefore caught, wherever it is added", is still false; that is the same overstatement #33 finding 1 was filed about, and the sentence survives this commit verbatim.Acceptable:
makeRecipesalso captures each target's prerequisite list andwalkfollows the known ones as edges, and themakeedge regex additionally matches$(MAKE)/${MAKE}— with a mutation test for each (check: fmt-checkand@$(MAKE) fmt-checkmust both drive the count to 2). If either is deliberately out of scope, the header sentence must be narrowed to say what is actually followed instead of claiming total coverage.Non-blocking
2. A
package.jsonscript whose name is not[a-z][a-z-]*is not followed.test/packaging/lint-once.test.ts:187. Adding"lint:fmt": "prettier --check ."plusRUN yarn run lint:fmttoDockerfileleft the suite green (23/23):yarn run lint:fmtmatchesyarn run lint, which is not a script, so no edge and no count. Colon-, digit- and underscore-named scripts are the JS-ecosystem norm. The same narrowness applies tomaketargets. Pre-existing, not a regression.3. BuildKit heredoc
RUNbodies are invisible.test/packaging/lint-once.test.ts:120-124keeps only lines beginningRUN. Adding toDockerfile:left the suite green (23/23). I confirmed the default frontend on this host executes heredoc bodies with no
# syntax=directive, so this is a working evasion rather than a theoretical one. Lower plausibility than finding 1 — the repo uses no heredocs today.4. The workflow is pinned by an exact-equality assertion whose name does not say so.
test/packaging/lint-once.test.ts:498-502assertscommandsOf("workflow:...")equals exactly["script/cibuild"]. That is what actually catches arun: |block scalar: adding a second step whose body runs prettier resolves to the bare|, soci.prettierstays 1 and only this test — namedreads the run steps of the CI workflow and not its uses steps— goes red. Good coverage, reached sideways. The flip side is that any legitimate secondrun:step (a cache step, anecho) turns the suite red for a reason unrelated to prettier. Worth one sentence in the comment saying this assertion is the block-scalar guard.5. Parser brittleness on reformatting: acceptable, one silent case.
installBranches(lines 326-351) depends on exact-lineelse/fi/}and on the literalmissing yarn. Backslash continuations and reordering the guard operands survive, sincejoinContinuationsruns first. A one-lineif …; then …; else …; fi, or replacingmissing yarnwith an inlinecommand -v, throws the namedinstall_js_deps is not the expected …error — loud, which is the right failure mode. The one silent case: swapping the branches (if ! missing yarn || …) silently swaps thewithYarn/withoutYarnlabels. Both assertions are identical today so the guarantee still holds; only the failing test's name would mislead.6. Duplicated premises across files: fine as written.
.gitignorein the build context is asserted atlint-once.test.ts:406-413andbuild-context.test.ts:46-47;script/lintbuildingDockerfile.lintatlint-once.test.ts:274-281andlint-docker.test.ts:49. Each copy carries its own rationale for why it is a premise of that file, and both read the same source of truth, so they cannot drift apart silently — a change breaks both. No double-maintenance trap.Not re-filed, known and tracked:
LINT_EPOCHvsCHECK_EPOCHandREPO_POLICIES.mddrift (#32), hostmake testnested-worktree exposure (#25).All mutations reverted; the review clone is byte-identical to
2bfa11cand nothing was pushed.Review FAILED; rework in progress on
next. The reviewer defeatedlint-once.test.tstwice with one-token idiomatic Makefile changes (check: fmt-checkas a prerequisite, and@$(MAKE) fmt-check), each givingmake checktwo prettier passes with the suite green — the exact duplication the file exists to prevent.Rework covers both, plus the three non-blocking evasions (yarn script names containing
:, BuildKit heredoc, line continuations): each gets fixed, or the file's header claim gets narrowed to what it actually enforces. An overclaiming comment is the same defect class as an assertion that cannot fail.A fresh reviewer takes the result.
View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.