next -> main #43
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?
Release-cycle branch. One commit per work unit accumulates here; this is the single open
next->mainPR, not a per-issue PR.Landed so far
599286a— run golangci-lint in a pinned container via script/lint (#41)329c03f— add--targetand--output=type=cacheonly, so a bad stage name cannot silently no-op20cfb47— define the stage name once, so the two flags cannot diverge (corrects an overclaim in329c03f)3eb9f81— name the two required flags, and record.dockerignoreas part of the gate (comments and docs only)The last three are reworks after independent review. None of them closes an issue on its own; #41 is closed by
599286a.599286a— run golangci-lint in a pinned container via script/lint (#41)golangci-lintis no longer invoked on the host anywhere in the repo.Dockerfile.lintpinsgolangci/golangci-lint:v2.12.2by digest and runs the linter as a build step, so a successful build IS a clean lint;make lintis now a thin shim overscript/lint(POSIXsh,set -eu, mode100755, resolves its own repo root)..dockerignoreexcludes.gitonly — the lint reads the Go sources,go.mod/go.sumand.golangci.yml, none of which come from there..golangci.ymlis untouched; thegomodguarddeprecation warning in the output is #29 and is sneak's to decide.TODO.md's scaffold-exemption note is narrowed rather than dropped:Dockerfile.lint,script/lintand.dockerignoreare now permitted and required, while CI config,REPO_POLICIES.md, an applicationDockerfileand any otherscript/entrypoint still are not.Two deliberate divergences from the
sneak/homoiconreference shape:depsstage holdsWORKDIR /src,COPY go.mod go.sum ./andRUN go mod download; thenFROM deps AS lintcarriesCOPY . .and the lint run.script/lintbuilds with--no-cache-filterrather than a baredocker build. Caching of the lint result is explicitly waived — a cached build lints nothing — but a single-stage--no-cachewould also re-fetch the whole module cache over the network on every lint. Splitting the stages busts only the stage that lints.golangci-lint config verifystep (homoicon has one). It resolves its JSON schema over a live, unpinned HTTPS call: an unpinned network input inside the one step whose entire purpose is a pinned, reproducible gate, and a schema-host outage would surface as a red build.golangci-lint runalready fails on a malformed config, and the config here is the shared canonical one, verified where it is maintained. The reason is recorded in a comment inDockerfile.lint.Verified at the time: two consecutive runs on an unchanged tree with the
golangci-lint runlayer executing and neverCACHED, a deliberateindent-error-flowviolation failing the build and naming that finding before a revert went clean, andmake checkgreen with real test durations.329c03f—--targetand--output=type=cacheonlyFirst rework, against the first review's finding 1.
--no-cache-filteris silently ignored by BuildKit when no stage matches its argument, so the anti-false-green mechanism of599286arested on one unvalidated string: a rename or typo of thelintstage would have left the lint layer served from cache andscript/lintreporting green having linted nothing — the same false green that started #41 , relocated. Reproduced in this repo: with the filter pointed at a nonexistent stage and no--target, an unchanged tree built withRUN golangci-lint run ... CACHEDat exit 0.--targetwas added because it fails loudly on a name that is not in the file (target stage "nosuchstage" could not be found, exit 1).--output=type=cacheonlywas adopted on the first review's ruling. Nothing consumes the image — the deliverable is an exit code — and the export cost seconds per run and left one dangling image behind every time, on a host where pruning is prohibited. Verified before adoption that the lint stage still executes and a lint failure still exits non-zero; a positive control with the flag removed produced exactly onecmd=[golangci-lint]dangling image plus anexporting layersstep, proving the measurement could detect one.TODO.mdin the same commit: the 2026-08-07 entry's now-false claim that the repo has no linter pin and lints on the host is marked superseded in place rather than rewritten; the narrowed exemption names.dockerignoreas the third file; and the specific wall-clock timings are replaced by the durable property they were evidence for (the lint stage executes every run and is never served from cache), since wall clock varies per host and per run — the same principle as #22 on hard-coded test counts in prose.This commit also introduced an overclaim, corrected by the next one: its comments said the two flags "validate each other's magic string". They did not.
20cfb47— the stage name is defined onceSecond rework, against the second review's finding.
--targetvalidates only its own argument. A typo confined to--no-cache-filterleft the build green and linting nothing — the original defect surviving in the one edit path the previous commit did not cover:Correcting only the prose would have left the hazard live and merely documented, so the duplication itself is gone. The stage name is written once in
script/lintand passed to both flags:Divergence between the flags is now unrepresentable rather than warned about. The true property, which is what the files say: there is a single name to get wrong, and
--targetrejects it loudly when it is not a stage inDockerfile.lint, which covers the filter too because it is the same string. The same typo that previously passed silently now fails:Non-zero, with zero
CACHEDlines anywhere in the build. The third review confirmed the cross-file seam is genuinely guarded by renaming the stage inDockerfile.linttolintstagewhile leavingstage=lint, which also exited 1.3eb9f81— prose accuracy around the gateThird rework, against the third review's two low findings. Comments and documentation only: the
docker buildinvocation and its flags are byte-identical and.dockerignore's effective rules are unchanged (git diffon both files yields no non-comment lines; the only rule is still.git)."Both flags below must stay" had lost its anchor — it ended a paragraph naming only
--no-cache-filter, with--targetintroduced later and three flags on the command below. It now names--targetand--no-cache-filterexplicitly.--output=type=cacheonlyis deliberately not in that pair: removing it would cost efficiency and hygiene, not correctness..dockerignoresits in the same trust boundary as$stageand was missing from the "what the tooling does not check" list. Only what reaches the container is linted, so excluding a Go source there removes it from the lint with no warning. Reproduced rather than asserted: a planted violation plus that one path in.dockerignoregives0 issues.at exit 0 with the violation still in the working tree. The complementary case was also checked so the warning is not overstated — excluding a file other code still references fails loudly onundefined:typecheck errors, so it is specifically the self-contained file that drops out silently. The warning is recorded both inscript/lintand in.dockerignoreitself, since that file is where such an edit gets made and the first review on this PR suggested extending it for build artifacts.TODO.mdgained no new dated paragraph; the existing "Corrected 2026-08-10" entry already enumerated what is left to the editor and was incomplete without.dockerignore, so the clause went into that sentence.Residual hazards, stated in
script/lintandDockerfile.lint--targetverifies that the stage name exists, not that it names the stage which actually runsgolangci-lint, and it stops the build there — so relocating the lint step, or adding a stage afterlint, would not be caught. And.dockerignoregoverns what is linted at all. Both are on whoever edits those files together.Standing notes
Full captured evidence for each rework is in the corresponding comment on this PR: the silent-no-op reproductions, the
stage=lnithard failure, the attribution control showing the same build with--no-cache-filterremoved reportsCOPY . . CACHEDandRUN golangci-lint ... CACHEDon an identical tree, negative controls under four different linters, dangling-image attribution with a positive control, and uncachedmake checkruns.git statuswas empty around every evidence run and all scratch lived outside the clone.No prune of any kind was run at any point during this cycle.
.golangci.ymlis untouched by all four commits, and the branch adds no CI config, noREPO_POLICIES.md, and noscript/entrypoint other thanlint.Porting
--target,--output=type=cacheonlyand the single-variable shape back intosneak/homoiconbefore it is cloned into further repos is tracked separately by the manager and is not part of this cycle.golangci-lint is no longer invoked on the host anywhere in the repo. Dockerfile.lint pins golangci/golangci-lint:v2.12.2 by digest and runs the linter as a build step, so a successful build IS a clean lint, and `make lint` becomes a thin shim over script/lint. This removes the host linter install that produced a false green here, where a branch that was genuinely red with a goconst finding reported "0 issues" off the shared host cache; a container per run has its own cache and lock. Two deliberate divergences from the sneak/homoicon reference shape: - Two stages rather than one. A cached `deps` stage holds `go mod download`, then `FROM deps AS lint` carries the source copy and the lint run, and script/lint builds with `--no-cache-filter=lint`. Caching of the lint result is explicitly waived (a cached build lints nothing), and splitting the stages means busting the lint layer does not re-fetch the module cache over the network on every run. - No `golangci-lint config verify` step. It resolves its JSON schema over a live, unpinned HTTPS call: an unpinned network input inside the one step whose purpose is a pinned, reproducible gate, and a schema-host outage would surface as a red build. `golangci-lint run` already fails on a malformed config. The reason is recorded in a comment in Dockerfile.lint. .dockerignore excludes .git only; the lint reads the Go sources, go.mod/go.sum and .golangci.yml, none of which come from there. The TODO.md scaffold-exemption note is narrowed rather than dropped: Dockerfile.lint and script/lint are now permitted and required, while CI config, REPO_POLICIES.md, an application Dockerfile and any other script/ entrypoint still are not. Verified, since a green docker build is the classic false green: two consecutive script/lint runs on an unchanged tree each showed the `golangci-lint run` layer executing (9.8s and 7.9s, both "0 issues.") while the deps layers reported CACHED; a deliberate indent-error-flow violation failed the build naming that finding and the unused one, and a revert went clean again. `make check` green.next -> mainto next -> mainIndependent adversarial review of
599286a(commit only, not the wholenextbranch). Reproduced from my own fresh clone; the author's pasted evidence was not taken on trust.Verdict: PASS. Two findings below are hardening/doc items, neither a present false-green.
Reproduced, not trusted
script/lintruns, unchanged tree:#10 [lint 2/2] RUN golangci-lint run ...=DONE 14.1sthenDONE 9.5s,0 issues.after 10.2s/7.7s of real work. NeverCACHED.depslayersCACHEDboth times — the two-stage split does what it claims.f.Close()in a newgame/file, not the author'sindent-error-flow): exit 1, naming exactly that finding —game/zzrevcheck.go:7:9: Error return value of f.Close is not checked (errcheck). Removed the file, clean again.make lintandmake checkboth exit 2 (make: *** [Makefile:35: lint] Error 1),make checkstopping beforetest.set -eu,docker buildlast inmain(), nothing swallows status.make checkin full, green: lint layerDONE 15.8s(notCACHED),ok cmd/rogue 1.026s/ok game 3.164s— real durations, no(cached).docker-content-digest: sha256:5cceeef04e53efe1470638d4b4b4f5ceefd574955ab3941b2d9a68a8c9ad5240for tagv2.12.2, OCI image index. Tag and digest agree, and the line is byte-identical tosneak/homoicon@main..golangci.ymlsetsdefault: all, which is whygomodguard's deprecation warning appears — stock golangci-lint would not enable it. That warning is proof the repo's config reached the container and is in force..golangci.ymluntouched by this commit.DOCKER_BUILDKIT=0 ./script/lint: exits 125,unknown flag: --no-cache-filter, no build runs. Fails loudly — no false green.dash -nandsh -nclean, no bashisms. Mode100755in the index. Root resolution works from a subdirectory.origin/main, zero behind,git merge-treeclean), commit title ends exactly(closes #41), no vendor references or attribution trailers anywhere in the commit, tree, or PR body: all clean.Findings
1 (medium, hardening — latent false-green, not a present one).
--no-cache-filter=<stage>is silently ignored when the stage name does not match. Proven on a scratch two-stage Dockerfile:docker build --no-cache-filter=nosuchstagebuilt with the final stage reportingCACHEDand exited 0. BuildKit does not validate the filter argument against the stage list. So the entire anti-false-green mechanism of this change hangs on one unvalidated magic string: rename thelintstage inDockerfile.lint, or typo it, andscript/lintreports green having linted nothing — the exact defect class this PR exists to kill. Nothing in the repo would catch it.Why it matters more than usual: this shape is about to be copied into other repos, so the hazard propagates.
What acceptable looks like: add
--target lintalongside the filter inscript/lint, so both flags name the same stage from the same string. Verified:docker build --target nosuchstagefails loudly —ERROR: failed to build: failed to solve: target stage "nosuchstage" could not be found, exit 1 — and--target lint --no-cache-filter=lintstill re-executes the stage. One flag converts a silent no-op into a hard error. Not a blocker for this commit (the stage is namedlintand I proved it executes), but it should land before the shape is cloned.2 (low, docs).
TODO.mdline 736 now asserts something false. Inside the dated 2026-08-07 Completed Steps entry: "The repo has no golangci-lint version pin to bump (no Dockerfile or CI;make lintruns whatevergolangci-lintis on the host)." That is the last remaining statement in the repo describing a host lint path, and it is written in the present tense. It is a historical log entry, so not a violation of the definition of done, but a reader landing there gets the pre-change answer. Acceptable: append a superseded-by pointer to #41.Minor / anomalies that pass anyway
Dockerfile.lintandscript/lintby name but does not mention.dockerignore, which this commit also adds. The file is justified and in substance part of the lint container, but the note as written reads as though it should not be there. Worth naming it in the note.TODO.mdrecords the two consecutive runs as11.6s and 9.7s; the commit message and PR body say9.8s and 7.9s. Different runs, presumably — but three different numbers for one claim in a change whose entire value is that the evidence is captured rather than asserted. Pick one set.script/lintresolvesROOTto the symlink's directory and fails (failed to read dockerfile: open Dockerfile.lint: no such file or directory, exit 1). Loud failure, not a false green, and identical tosneak/homoicon. Noted, not a defect..dockerignoredoes not exclude.gitignored build artifacts (/rogue,*.test,*.out), so a developer with a built binary ships it into the context each run. Cosmetic.main "$@"takes arguments it never uses. Matches the reference shape.The three documented divergences
Two-stage split: sound, and required by the implementation brief — verified
go mod downloadstayedCACHEDacross every build I ran whilelintre-executed each time.--no-cache-filter: sound, with the caveat in finding 1.Omission of
golangci-lint config verify: I agree with the ruling, and would rule the same way independently. An unpinned live HTTPS fetch inside the one step whose entire value is pinned reproducibility is a contradiction, and it converts a schema-host outage into a red build indistinguishable from a real finding.golangci-lint runrejects an unparseable config on its own, and this repo carries a stronger constraint than schema validity anyway:MEMORY.mdrequires.golangci.ymlto stay byte-identical to the canonical shared config, so drift is caught by comparison, not by a schema. Honestly disclosed inDockerfile.lintlines 27-31.Disclosure: I did not independently confirm that
config verifyperforms a live network fetch — I did not run it. The claim is consistent with the tool's documented behaviour and it is the manager's ruling regardless.Ruling on the open question (dangling images)
Adopt
--output=type=cacheonly. Not a close call.4.2sand3.5sof runs that took 21.8s and 14.1s wall — 20-30% of every lint. The cacheonly variant of the same build finished in10.4swith the lint stage still executing (0 issues.after 8.3s of real linting) and noexporting to imagestep at all.RUNunder--output=type=cacheonly --no-cache-filter=lintexited 1 and named the failing step. The stage still executes; only the export is skipped.--no-cache-filter), for the same reason — the reference shape assumes a build with a product, and this build has none. It requires BuildKit, which--no-cache-filteralready makes a hard requirement.Recommendation: fold
--output=type=cacheonlyand--target lintintoscript/linttogether — one line, both justified by the same argument — and push both back intosneak/homoiconbefore the shape is copied further.Could not verify
Gitea Actions runs are not queryable as
clawbot(403, not repo owner). There is no CI to be green: the repo carries no workflow files by standing exemption, and this change correctly adds none.Manager: review above is a PASS, but I am reworking before merge rather than banking the findings.
Finding 1 is the one that matters and it is worth stating plainly, because it is the same failure class this whole change exists to kill:
--no-cache-filter=lintis silently ignored if no stage by that name exists. The reviewer proved it — a bogus stage name builtCACHEDand exited 0. So a rename or a typo turnsscript/lintinto a green no-op, which is exactly the false green that started #41 , just relocated. Adding--target lintcloses it: a bad target fails loudly instead. One flag, so it goes in now.Also taking the reviewer's
--output=type=cacheonlyruling. Nothing consumes the image — the deliverable is an exit code — and this host already carries 626 dangling images with pruning prohibited.Both flags need porting back into
sneak/homoiconbefore the shape is cloned into further repos; I will open that separately so it is not buried in this repo's cycle.Reworked in
329c03fonnext(new commit;599286auntouched, not amended). Addressing finding 1, the--output=type=cacheonlyruling, finding 2, and the two minor items, point by point.1. Finding 1 (medium) —
--target lintadded. Accepted in full.script/lintnow passes--target lint,--no-cache-filter=lintand--output=type=cacheonly.I reproduced the silent no-op in this repo, not just on a scratch Dockerfile. With the filter pointed at a nonexistent stage and no
--target— i.e. the shape this PR shipped, one typo away — on a genuinely unchanged tree:Green, exit 0, linted nothing. Confirmed exactly as the review described.
With
--targetin place, the same bad name now fails loudly:Reverted after each experiment. The comment in
script/lintstates why both flags are present, that they name the same stage from the same string, and that neither may be "simplified" away. I also added a two-line comment aboveFROM deps AS lintinDockerfile.lint— that is where a rename would actually be typed, so the warning belongs at that end too.2.
--output=type=cacheonlyadopted, with the failure path verified firstThe catastrophic outcome here would be the flag suppressing a lint failure, so that was checked before anything else. Negative control of my own choosing (not the author's
indent-error-flow, not yourerrcheck— a newgame/zzneg_rework_check.gowith an unnecessaryfmt.Sprintf), under all three flags:make checkwith the same violation exits 2 and stops before the tests (zerooklines). After deleting the file,EXIT_AFTER_REVERT=0,0 issues.Dangling images. Raw before/after counts are useless on this host — it gained images from other sessions during every window I measured — so I diffed the image IDs and inspected each new one. Across a
script/lintrun: 4 new dangling images appeared, allcmd=[python3]orcmd=[node], none from this build. Across the fullmake check: 2 new, bothcmd=[python3].To prove the measurement could actually detect one, I ran a positive control — same script with
--output=type=cacheonlyremoved:Exactly one new dangling image with the
golangci-lintsignature, plus 5.3s of layer export. With the flag, zeroexporting to imagesteps in any run. I deleted that one control image by ID (docker rmi 02513b304259, targeted, its own image only). No prune of any kind was run at any point.3. Finding 2 (low) —
TODO.mdline 736 marked superseded in placeThe 2026-08-07 entry keeps its original text; a superseded note is appended pointing at the 2026-08-10 entry and #41 . History not rewritten.
4. Minor —
.dockerignorenamed in the exemption noteFuture Steps note 2 now reads "
Dockerfile.lintandscript/lint... along with the.dockerignorethat scopes their build context", and "those three files only".5. Minor — the three-way timing disagreement
Resolved by deleting the numbers from
TODO.mdprose rather than picking a set. All three measurements were honest; wall clock varies per host and per run, so a specific figure there is a hostage to fortune in exactly the way hard-coded test counts were in #22 . The entry now states the durable property instead: the lint stage executes on every run and is never served from cache. The commit message and PR body history are left alone.Two consecutive runs, all three flags, unchanged tree
Run 1:
Run 2, nothing touched in between:
DONEboth times, neverCACHED,depscached both times, and no export step in either.One methodology correction worth recording, because it would have quietly weakened this evidence: my first attempt kept scratch logs in a directory inside the repo, which put them in the build context and invalidated
COPY . .on every run. The lint stage then re-executes because the context changed, not because--no-cache-filterdid its job — a passing result that proves nothing. Scratch moved outside the tree; the runs above haveCOPY . .and everything above it stable, so the lint layer re-executing is attributable to the filter alone.make checkgreen in fullGo's test cache served
(cached)on a first attempt, which is not evidence, so this is a forced uncached run through the make target (GOFLAGS=-count=1 make check) — real durations:Lint layer executed, not
CACHED.make fmtrun and committed..golangci.ymluntouched; no CI config, noREPO_POLICIES.md, no furtherscript/entrypoint.sh -nanddash -nclean, mode100755preserved.The port of both flags back into
sneak/homoiconbefore the shape is cloned further is the manager's to open and is not in this commit.Independent adversarial review of the rework commit
329c03f, with the verdict owned for the wholenextbranch (599286a+329c03f). Fresh clone of my own; every pasted result on this PR was treated as an unverified claim and re-run.Verdict: PASS. One low finding below (an inaccurate claim in a comment, plus the residual hazard it papers over). Nothing here is a present false green.
Findings
1 (low, accuracy + residual hazard).
script/lintlines 18-19 andTODO.mdclaim the two flags "validate each other's magic string". They do not — only--target's string is validated.--targetchecks its own argument against the stage list.--no-cache-filter's argument is still unchecked, so the guard covers three of the four edit paths and misses one:--targetfails loudly. Caught.--targetfails loudly. Caught.--targetstill correct -> silent no-op, unchanged.Reproduced in this repo on a genuinely unchanged tree, one transposed character in the filter only:
Green, exit 0, linted nothing — the original defect class, surviving in the narrow case. Why it matters beyond pedantry: the prose instruction two lines above ("both must keep naming the stage that
Dockerfile.lintcallslint") is the thing actually holding this together, and the "validate each other" sentence tells a future editor the tooling has their back when it does not. This shape is about to be ported intosneak/homoiconand cloned onward, so the overclaim propagates with it.What acceptable looks like: state the property truthfully —
--targetvalidates the stage name and converts a rename into a hard error; the filter's copy of the name is guarded only by keeping the two strings identical, which is the editor's job. Same correction in theTODO.md"Hardened 2026-08-10" paragraph, which repeats it.Dockerfile.lintlines 22-23 are already correct as written.Reproduced, not trusted
--targetflag guards against — real. Bogus filter name, no--target:RUN golangci-lint run ... CACHED, exit 0. With--target nosuchstage:ERROR: failed to build: failed to solve: target stage "nosuchstage" could not be found, exit 1. Both on the committed tree with no file changed; tree clean after.script/lintruns, unchanged tree, all three flags. Lint layerDONE 15.4s/DONE 15.4s,0 issues.after ~15s of real linting each. NeverCACHED.depslayersCACHEDboth times.COPY . .cannot reportCACHEDhere and its doing so would not be the evidence: it lives inside the cache-bustedlintstage, so--no-cache-filter=lintre-runs it by construction (the rework's own paste showsDONE 0.5s/DONE 2.3sfor the same reason). I substituted a stronger control — the identical build with--no-cache-filterremoved, tree untouched, immediately after:COPY . . CACHEDandRUN golangci-lint run ... CACHED. So the context was genuinely stable and the cache would have served the lint result; the re-execution under the flag is attributable to the flag alone. Build context transferred 2.79kB identically across both runs. My scratch lived outside the repo throughout andgit statuswas empty before and after every run.goconst, notindent-error-flow/errcheck/fmt.Sprintf): three repeated string literals in a newgame/file, under all three flags -> exit 1 naming it exactly —game/zz_w41rev2_probe.go:5:10: string ... has 3 occurrences, make it a constant (goconst)plusunused.make checkwith it present exits 2 with zerooklines. File deleted ->0 issues., exit 0.--output=type=cacheonlymasks nothing, and zeroexporting to imagesteps in the failing run.cmd=[golangci-lint]image roughly every 58s, and two of them landed 0.5s and 2s before my snapshot windows opened. Diffed image IDs and inspected each: no image created inside anyscript/lintwindow, and zeroexporting to imagesteps in any run under the flag. Positive control, same build withcacheonlyremoved:exporting to image/exporting layers 2.8sandnaming to moby-dangling@sha256:5d1e3e3d…— exactly one dangling image, so the measurement can detect one. I deleted that one image by ID (docker rmi sha256:5d1e3e3d…), mine only, and left the concurrent session's alone. No prune of any kind at any point.make checkgreen in full, forced uncached through the make target (GOFLAGS=-count=1): lint layer executed (0 issues.after 7.2s),ok cmd/rogue 1.015s/ok game 3.340s— real durations, no(cached).DOCKER_BUILDKIT=0 ./script/lintstill exits 125 and fails loudly with the added flags.Dockerfile.lint,script/lint,TODO.md(rework) —script/containslintand nothing else; no CI config, noREPO_POLICIES.md..golangci.ymluntouched by both commits. No hostgolangci-lintpath anywhere.dash -n/sh -nclean, mode100755.fmt-checkclean (it gatesmake check). Titles:599286aends exactly(closes #41),329c03fcorrectly carries none. No trailers, no vendor references anywhere in either commit, tree, or PR body.nextis 2 ahead / 0 behindorigin/main,git merge-treeconflict-free.TODO.mdstatement checks out, including the superseded-in-place note at line 749 (original text intact, pointer appended) and the exemption note now naming.dockerignoreas the third file.Anomalies that pass anyway
Dockerfile.lintcomment warns against renaming the stage but not against appending one. With--target lint, any stage added afterlintis silently skipped. Speculative today (there are only the two stages and no reason to add a third) — noted, not a defect.#5 transferring context: 2.79kBon repeat runs vs835.32kBon the cold one confirms.dockerignoreand the context are behaving; the prior review's cosmetic point about build artifacts not being excluded still stands and is still cosmetic.Could not fully verify
sneak/homoiconbyte-identity claim for the tag/digest pair — no access to that repo from here. Indirect support only: the registry accepted thev2.12.2@sha256:5cce…pair on every pull, and the pulled image's labels reportimage.version: 2.12.2.clawbot(403). There is no CI to be green — the repo carries no workflow files by standing exemption and this branch correctly adds none.Manager: second review is a PASS, and its one finding is correct — I wrote the claim it is knocking down, in the manager comment above, and it is wrong.
--targetvalidates its own argument only; a typo confined to--no-cache-filterstill silently no-ops, which the reviewer reproduced with--no-cache-filter=lnitbuildingCACHEDat exit 0.Taking the fix a step further than the reviewer proposed. Correcting the prose would leave a hazard documented but live; the two flags carry the same stage name twice, and duplication is the defect. Hoisting it into one shell variable used by both makes divergence unrepresentable rather than merely warned about, and it is the same one-line class of change. Prose still gets corrected to describe the real property.
Reworking once more, then a third independent review before this merges. Nothing about this shape gets ported to
sneak/homoiconuntil it is settled here.The previous commit claimed --target and --no-cache-filter "validate each other's magic string". They do not. --target validates only its own argument; a typo confined to --no-cache-filter left the build green and linting nothing: docker build --target lint --no-cache-filter=lnit ... #10 [lint 2/2] RUN golangci-lint run ... CACHED exit 0 Three of the four edit paths were caught and one was not, so the original false green survived in the narrow case. The duplication was the defect: the stage name appeared twice on one command line and nothing tied the copies together. Correcting only the prose would have left the hazard live and merely warned about, so the name is now written once, as `stage=lint`, and passed to both flags. Divergence is unrepresentable rather than documented — there is a single name to get wrong, and --target rejects it loudly when it is not a stage in Dockerfile.lint, which now covers the filter too because it is the same string. The comments in script/lint and Dockerfile.lint and the TODO.md entry drop the false "validate each other" claim and state the real property, along with the residual hazard that is genuinely unguarded: --target checks that the name exists, not that it names the stage which actually runs golangci-lint, and it stops the build there, so relocating the lint step or appending a stage after it would go unnoticed.Second rework,
20cfb47onnext(new commit;599286aand329c03funtouched, not amended, not force-pushed).The finding is correct, and the wrong claim was mine
I wrote "the two flags validate each other's magic string" in
script/lintand repeated it inTODO.md. It is false.--targetvalidates its own argument against the stage list and nothing else;--no-cache-filter's argument stays unchecked, so the case where the filter alone is mistyped was never covered. The reviewer's--no-cache-filter=lnitreproduction stands — I re-ran it before changing anything, and it builtCACHEDat exit 0.That is worse than a documentation error. The sentence told a future editor the tooling had their back in a case where it did not, and this shape is queued to be ported into
sneak/homoiconand cloned onward.What changed: the duplication is gone, not annotated
The stage name appeared twice on one command line with nothing tying the copies together.
script/lintnow writes it once:Divergence between the two flags is now unrepresentable rather than warned about. POSIX
sh;sh -nanddash -nclean; expansions quoted; mode100755preserved. No buildx parsing or quoting trouble —--no-cache-filter="$stage"expands exactly as the literal did.Proof 1 — the exact case from the finding is closed
The whole point of the commit, so here it is directly. One typo in the single stage name (
stage=lnit), unchanged tree otherwise:Non-zero, loud, and zero
CACHEDlines in the entire build. Previously this exact typo — confined to the filter — producedRUN golangci-lint run ... CACHEDat exit 0. Reverted immediately;git statusempty after.Note what actually fixed it: not a new check, but the fact that there is now only one name to get wrong, so the name that reaches
--no-cache-filteris the same one--targetrejects.Proof 2 — two consecutive runs, unchanged tree
git statusempty before and after both runs; all scratch outside the clone.depslayersCACHEDboth times, lint layer neverCACHED, zeroexporting to imagesteps in either.Attribution, using the control the second review introduced rather than my own weaker one. The reviewer is right that
COPY . .not reportingCACHEDproves nothing — it lives inside the cache-busted stage, so the filter re-runs it by construction, and I cited it as evidence in my previous comment when it was not. The valid control is the identical build with--no-cache-filterremoved, tree untouched:The cache would have served the lint result, so the context was genuinely stable and the re-execution above is attributable to the flag alone. This control had to be a direct
docker buildinvocation: running it throughscript/lintwould require editingscript/lint, which is itself in the build context and would invalidateCOPY . ., destroying the thing being measured. It is a measurement, not a lint gate — every lint result on this PR came fromscript/lintormake check.Proof 3 — negative control,
wastedassign(new to this PR)Not
indent-error-flow,errcheck,fmt.Sprintf/perfsprint, orgoconst— all used already. A newgame/zz_w41_rework3_probe.gowith a value assigned and then overwritten unused, under the final flag set:make checkwith it present exits 2 with zerooklines. Zeroexporting to imagesteps in the failing run —--output=type=cacheonlystill masks nothing. File deleted,git statusempty,0 issues.at exit 0.Proof 4 —
make checkgreen in fullForced uncached through the make target (
GOFLAGS=-count=1 make check),git statusempty before and after:Lint layer executed, not
CACHED. Real durations, no(cached). No prune of any kind at any point.Prose corrected, and one place where I did not follow the brief literally
script/lint,Dockerfile.lintand theTODO.mdentry now state the property that is true — the name is defined once and shared, so--targetrejecting a bad name covers the filter too because it is the same string — and the "validate each other" claim is gone rather than softened. TheTODO.mdcorrection is a new dated paragraph recording that the earlier claim was wrong and why, not a rewrite of it.The brief asked me to record the cross-file link between
$stageandDockerfile.lintas "a real, unguarded seam". I did not write that, because after the change it is not accurate and I am not willing to replace one overclaim with an underclaim: if the stage inDockerfile.lintis renamed and$stageis not,--targetfails loudly, so that mismatch is guarded. The genuinely unguarded residual is different, and is what the files now say:--targetverifies the name exists, not that it names the stage that actually runsgolangci-lint, and it stops the build at that stage — so relocating the lint step into another stage, or adding a stage afterlint, would pass unnoticed. That last case is the second reviewer's "appends a stage" anomaly, which this makes explicit in both files.Scope unchanged:
script/lint,Dockerfile.lint,TODO.md..golangci.ymluntouched, no CI config, noREPO_POLICIES.md, no furtherscript/entrypoint.make fmtrun and committed.Third independent adversarial review, of
20cfb47, with the verdict owned for the wholenextbranch (599286a+329c03f+20cfb47). Fresh clone of my own; every pasted result on this PR and on #41 was treated as an unverified claim and re-run.git statusempty around every evidence run; all scratch outside the clone.Verdict: PASS. Two low findings, neither a present false green.
The central claim holds
stage=lnit, tree otherwise unchanged, throughscript/lint:Exit 1, zero
CACHEDlines in the entire build. Reverted, clean.I also confirmed the premise independently rather than taking it from the previous review — the pre-fix shape, via a direct
docker build, is genuinely silent:No warning, no diagnostic. The defect was real and it is closed.
Adjudication: the reworker was right to refuse the requested wording
The brief asked for the
$stage<->Dockerfile.lintlink to be recorded as "a real, unguarded seam"; the reworker declined, arguing that would be an underclaim. I tested it. RenamingFROM deps AS linttoFROM deps AS lintstagewhile leavingstage=lint:Exit 1, zero
CACHED. That mismatch is guarded. The refusal was correct and the files say the accurate thing.The residual the files do claim is unguarded is accurate, and I reproduced both halves of it:
stage=deps(a name that exists but is not the lint stage): exit 0,golangci-lintnever invoked, zero occurrences in the build log.FROM lint AS verify/RUN ... && falseafterlint: exit 0, the appended stage silently skipped by--target, lint still ran.So the characterisation is accurate. On completeness, see finding 2.
Attempts to defeat it that failed
.dockerignore+=game/: fails loudly, exit 1, the build errors on the missing package rather than passing green.DOCKER_BUILDKIT/ flag-order / quoting variants:--no-cache-filter="$stage"expands identically to the literal;dash -nandsh -nclean.--output=type=cacheonlymasks nothing (see negative control).Reproduced, not trusted
DONE 7.5sthenDONE 7.8s,0 issues.each, neverCACHED;depslayersCACHEDboth times.--no-cache-filterremoved:COPY . . CACHEDandRUN golangci-lint ... CACHED, exit 0, context2.79kBboth ways. Cache would have served the lint, so re-execution under the flag is attributable to the flag alone. The reworker's use of a directdocker buildfor this measurement is sound:script/lintis itself in the build context (.dockerignoreexcludes only.git), so editing it to drop the flag would invalidateCOPY . .and destroy the thing being measured. Every lint gate on this branch still runs throughscript/lint.misspell+unconvert— notindent-error-flow,errcheck,fmt.Sprintf,goconstorwastedassign): exit 1 naming them exactly (game/zz_w41rev3_probe.go:3:54: 'recieve' is a misspelling of 'receive' (misspell),:5:15: unnecessary conversion (unconvert), plusunused), zeroexporting to imagesteps in the failing run. File removed,0 issues., exit 0.make checkgreen in full, forced uncached (GOFLAGS=-count=1): lint layer executed (0 issues.after 11.2s),ok cmd/rogue 1.017s/ok game 3.266s,real 0m25.892s. No(cached).script/lintrun,cmd=[node], from a concurrent session — not this build; zeroexporting to imagesteps. Positive control with the export enabled produced exactly onecmd=[golangci-lint]image, proving the measurement can detect one; deleted by ID, mine only. No prune of any kind at any point.599286atitle ends exactly(closes #41), the other two correctly carry none; mode100755, expansions quoted, POSIX clean;make fmtidempotent andmake fmt-checkexit 0;next3 ahead / 0 behindorigin/main,git merge-treeconflict-free; scope isDockerfile.lint,script/lint,.dockerignore,Makefile,README.md,TODO.mdonly,script/holdslintand nothing else, no CI config, noREPO_POLICIES.md;.golangci.ymluntouched by all three commits; no hostgolangci-lintpath anywhere; no vendor references and no attribution trailers in any commit, the tree, or the PR body;TODO.mdsuperseded note intact with a pointer appended, exemption note naming all three files, no volatile timings, prettier-clean, all issue references full URLs. PR body accurate for all three commits with nothing previously disclosed dropped.Every statement in
script/lint,Dockerfile.lintand theTODO.mdentry was checked line by line against observed behaviour; all are literally true, except as below.Findings
1 (low, prose — ambiguous in a file about to be ported verbatim).
script/lintline ~10: "Both flags below must stay: do not 'simplify' either one away."The sentence now sits at the end of the paragraph that names only
--no-cache-filter;--targetis not introduced until the following paragraph, and there are three flags in the command below. In329c03fthis instruction was anchored by "--target lintand--no-cache-filter=lintmust BOTH be present";20cfb47removed that anchor, leaving "both" as an unresolved forward reference. A reader could take it as--no-cache-filter+--output=type=cacheonly, which is the wrong pair — only the first two are load-bearing for correctness. Not wrong, but under-specified in the one file whose value is that it is exactly true. Acceptable: name the two flags in the sentence.2 (low, disclosure completeness). The residual-hazard note is accurate but scoped to the
$stage/Dockerfile.lintseam;.dockerignoreis a third file in the same trust boundary and is not named..dockerignoredecides what the linter can see, and narrowing it produces a silent partial lint with no diagnostic. Demonstrated with the violating probe file still present in the working tree and only its path appended to.dockerignore:Green, exit 0, with a real
misspell/unconvertviolation sitting in the tree. This is not introduced by20cfb47— it is inherent to the shape from599286aand has passed two prior reviews — and it needs a deliberate edit. It is worth naming anyway for two reasons: the first review recommended adding build artifacts (/rogue,*.test,*.out) to.dockerignore, so this file is expected to be edited; and the shape is queued for porting. Acceptable: one clause in the same "what the tooling does NOT check" block noting that.dockerignorecan narrow what is linted and must not exclude anythinggolangci-lintshould see.Anomaly that passes
TODO.md's new "Corrected 2026-08-10" paragraph says "the claim first made here that the two flags 'validate each other'", but that sentence was deleted from the "Hardened" paragraph in the same commit, so "here" points at text a reader can no longer find. Defensible as a historical reference, and removing a false claim from live guidance rather than leaving it standing is the right call — noted, not a defect.Could not verify
sneak/homoiconbyte-identity claim for the tag/digest pair — no access to that repo from here.clawbot(403). There is no CI to be green: the repo carries no workflow files by standing exemption and this branch correctly adds none.Third rework,
3eb9f81onnext(new commit;599286a,329c03fand20cfb47untouched, not amended, not force-pushed). Both low findings accepted; I checked each before writing, and both are real.Comments and documentation only. The
docker buildinvocation and its flags are byte-identical, and.dockerignore's effective rules are unchanged — proof below.Finding 1 — "both flags" had lost its anchor
Correct as reported. The sentence ended a paragraph that named only
--no-cache-filter,--targetwas not introduced until the paragraph after, and three flags follow on the command, so "both" pointed at nothing a reader could resolve. Now:>
--no-cache-filterforces the lint stage to re-execute every run ... It and--targetmust both stay, for the reason in the next paragraph: do not "simplify" either of those two away.--output=type=cacheonlyis deliberately not in that pair — it is an efficiency/hygiene flag, and removing it would not create a false green.Finding 2 —
.dockerignoreis in the same trust boundaryReproduced before writing anything, and the hazard is real. A planted violation in a new
game/file is caught normally (exit 1,wastedassign+mnd+unused). Adding only that file's path to.dockerignore, with the violation still sitting in the working tree:0 issues.at exit 0 — a silent partial lint over an incomplete tree.I also checked the other half so the comment states the boundary accurately rather than implying every exclusion is silent. Excluding a file the rest of the package still references fails loudly instead:
So the accurate statement, which is what the files now say: only what reaches the container is linted; excluding a Go source removes it from the lint with no warning, and it surfaces only if other code still references it — a self-contained file drops out silently. Both experiments reverted;
git statusempty after each.The warning is recorded in two places: in
script/lint, where the "what the tooling does NOT check" block is now a numbered list of two items ($stage, then.dockerignore), and in.dockerignoreitself. The second location is beyond the literal brief — you asked for a clause inscript/lint— but.dockerignoreis where the edit actually gets made, and the first review on this PR recommended extending that file for build artifacts, so a reader who never opensscript/lintis exactly the person who needs the warning. It is comment text only; the.gitrule is unchanged.Proof — diff scope
Non-comment changed lines in the two script/config files: none.
Effective ignore rules, before and after — identical:
Files touched:
script/lint,.dockerignore,TODO.md.make fmtrun and committed;sh -nanddash -nclean; mode100755preserved.Proof —
script/lintrun, lint layer executinggit statusempty before and after:Never
CACHED.Proof —
make checkgreen in fullForced uncached through the make target (
GOFLAGS=-count=1 make check),git statusempty before and after:Lint layer executed, not
CACHED; real test durations, no(cached).TODO.mdNo new dated paragraph. The existing "Corrected 2026-08-10" entry already enumerates what is left to the editor and was incomplete without
.dockerignore, so the clause was added to that sentence. Nothing else inTODO.mdchanged.All scratch stayed outside the clone throughout. No prune of any kind at any point.