diff --git a/.dockerignore b/.dockerignore index 8df76b7..4f59329 100644 --- a/.dockerignore +++ b/.dockerignore @@ -1,9 +1,4 @@ -# The lint build reads the Go sources, go.mod/go.sum and .golangci.yml; -# none of that comes out of .git, so keep the build context small. -# -# This file is part of the lint gate, not housekeeping: only what reaches -# the container gets linted, so excluding a Go source here silently drops -# it from the lint (a self-contained file yields `0 issues.` at exit 0 with -# the violation still in the tree; it fails loudly only if other code still -# references it). Never exclude Go sources, go.mod/go.sum or .golangci.yml. +# Part of the lint gate: only what reaches the container is linted, so +# excluding a self-contained Go source here drops it from the lint silently. +# Never exclude Go sources, go.mod/go.sum or .golangci.yml. .git diff --git a/Dockerfile.lint b/Dockerfile.lint index 28febae..dac4ecd 100644 --- a/Dockerfile.lint +++ b/Dockerfile.lint @@ -1,39 +1,20 @@ -# Lint-only image: used by script/lint on machines where the docker -# daemon is remote (no bind mounts possible) — the repo is COPYed into -# the build context and golangci-lint runs as a build step, so a -# successful build means a clean lint. -# -# Two stages on purpose. script/lint builds with --no-cache-filter=lint so -# that the lint stage re-executes on every run, including on an unchanged -# tree (caching of the lint result is explicitly waived: a cached build -# lints nothing). Keeping `go mod download` in a separate `deps` stage -# means busting the lint stage does not also re-fetch the module cache -# over the network every time. +# Lint image, built by script/lint: golangci-lint runs as a build step, so +# a successful build is a clean lint. # golangci/golangci-lint:v2.12.2 (Debian-based), 2026-08-07 FROM golangci/golangci-lint:v2.12.2@sha256:5cceeef04e53efe1470638d4b4b4f5ceefd574955ab3941b2d9a68a8c9ad5240 AS deps WORKDIR /src -# Copy go mod files first for better layer caching COPY go.mod go.sum ./ RUN go mod download # This stage must stay the one that runs golangci-lint, and its name must -# match $stage in script/lint, which passes that single name to both -# --target and --no-cache-filter. Renaming here without updating script/lint -# fails the build loudly (--target rejects a name that is not in this file), -# so a mismatch cannot pass silently — but moving the lint step to another -# stage, or adding a stage after this one, would not be caught. Change the -# two files together. +# match $stage in script/lint. --target halts the build at this stage, so +# moving the lint step to another stage, or adding a stage after this one, +# is not caught. FROM deps AS lint -# Copy source code COPY . . -# No `golangci-lint config verify` step here, deliberately (sneak/homoicon -# has one). It resolves its JSON schema over a live, unpinned HTTPS call: -# an unpinned network input inside the one step whose whole purpose is a -# pinned, reproducible gate, and a schema-host outage would show up as a -# red build. `golangci-lint run` already fails on a malformed config. RUN golangci-lint run --config .golangci.yml ./... diff --git a/TODO.md b/TODO.md index f14deb8..7c2e24c 100644 --- a/TODO.md +++ b/TODO.md @@ -44,57 +44,25 @@ is finished. 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. The image - has two stages rather than one — a cached `deps` stage holding - `go mod download`, then `FROM deps AS lint` with the 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 also re-fetch the module - cache over the network on every run. And `golangci-lint config verify` is - left out: it resolves its JSON schema over a live, unpinned HTTPS call, - which is 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. + `script/lint` builds with `--target "$stage"`, `--no-cache-filter="$stage"` + and `--output=type=cacheonly`. The durable property to check when touching + any of this: the lint stage executes on every run and is never served from + cache. Three things no tooling checks, left to whoever edits the gate — + `$stage` must match the stage name in `Dockerfile.lint`; that stage must + stay the one running `golangci-lint`, since `--target` halts the build + there; and `.dockerignore` governs what reaches the container, so excluding + a self-contained Go source drops it from the lint silently. Verified rather than assumed, since a green docker build is the classic false green: two consecutive runs on an unchanged tree each showed the `golangci-lint run` layer executing and reporting `0 issues.` while the - `deps` layers reported `CACHED`, and a deliberate `indent-error-flow` - violation failed the build naming that finding plus the `unused` one before - a revert went clean again. The durable property to check when touching any - of this is that the lint stage executes on every run and is never served - from cache; wall-clock durations vary per host and per run, so they are not - recorded here. - - Hardened 2026-08-10 after review. `script/lint` now also passes `--target` - and `--output=type=cacheonly`. `--target` is what makes `--no-cache-filter` - trustworthy: BuildKit silently ignores the filter when no stage matches its - argument, so a rename or typo of the `lint` stage would have left the lint - layer cached and `script/lint` green having linted nothing — the same false - green in a new place. `--target` fails loudly on a name that is not in the - file. `--output=type=cacheonly` skips the image export: nothing consumes the - image (the deliverable is an exit code), and exporting it cost seconds per - run and left a dangling image behind each time. - - Corrected 2026-08-10 after a second review, which was right to reject the - claim first made here that the two flags "validate each other". They did - not: `--target` validates only its own argument, so a typo confined to - `--no-cache-filter` still built `CACHED` at exit 0 — the original defect, - surviving in the narrow case. The duplicated stage name was the defect, so - it is now written once, as `stage=lint` in `script/lint`, and passed to both - flags. The true property is that there is only one name to get wrong, and - `--target` rejects it loudly if it is not a stage in `Dockerfile.lint`, so a - typo or a stale rename is a hard error rather than a silent skip. What - remains on the editor, and is not checked by anything: `$stage` must name - the stage that actually runs `golangci-lint`. `--target` verifies the name - exists, not that it is the right stage, and it stops the build there — so - moving the lint step to another stage, or adding a stage after it, would go - unnoticed. The same applies to `.dockerignore`, which is part of this gate - rather than housekeeping: only what reaches the container is linted, so - excluding a Go source there drops it from the lint silently — verified, a - planted violation plus that one path in `.dockerignore` gives `0 issues.` at - exit 0 with the violation still in the tree, and it fails loudly only when - other code still references the excluded file. + `deps` layers reported `CACHED`; the same build with `--no-cache-filter` + removed reported that layer `CACHED`, so the re-execution is attributable to + the flag rather than to a changed context; deliberate violations failed the + build naming the specific finding and reverted clean; a stage-name typo + failed loudly at exit 1; and a Go file excluded via `.dockerignore` reported + `0 issues.` at exit 0 with the violation still in the tree. Wall-clock + durations vary per host and per run, so they are not recorded here. - 2026-08-09 `TestAutoSaveOnSignalRacesTurnLoop` de-flaked at the cause (`fix/autosave-turn-budget-36`, closes #36). The failure text was captured diff --git a/script/lint b/script/lint index a4b837c..b2262ae 100755 --- a/script/lint +++ b/script/lint @@ -1,51 +1,23 @@ #!/bin/sh -# script/lint: run the linter. golangci-lint is never installed locally: -# it runs via docker only, one way, everywhere — script/lint builds -# Dockerfile.lint, which COPYs the repo into the pinned golangci-lint -# image and lints as a build step. This works even when the docker daemon -# is remote and bind mounts are impossible. +# script/lint: lint in docker. golangci-lint is never installed on the host. # -# --no-cache-filter forces the lint stage to re-execute every run, so an -# unchanged tree is still actually linted; the deps stage keeps its cache, -# so the module download is not repeated. It and --target must both stay, -# for the reason in the next paragraph: do not "simplify" either of those -# two away. +# Traps, each of which yields a green run over an unlinted or partly linted +# tree: # -# The stage name is written ONCE, in $stage, and passed to both --target -# and --no-cache-filter, so the two flags cannot come to name different -# stages. That is the whole point of the variable. BuildKit silently -# ignores --no-cache-filter when no stage matches its argument: a filter -# naming a stage that does not exist is a no-op, the lint layer is served -# from cache, and script/lint reports green having linted nothing — the -# false green this setup exists to prevent. --target, by contrast, fails -# loudly on a name that is not in the file. With a single shared name, a -# typo or a stale rename therefore becomes a hard error instead of a -# silent skip, because the one name reaches both flags. +# 1. --target and --no-cache-filter must both stay, and $stage must match +# the stage name in Dockerfile.lint. BuildKit ignores --no-cache-filter +# when no stage matches its argument, serving the lint layer from cache +# without a word; --target rejects a name that is not in the file, which +# is what makes the single $stage safe. # -# What the tooling does NOT check, and is left to whoever edits this: +# 2. --target checks that the stage exists, not that it is the stage +# running golangci-lint, and it halts the build there. Moving the lint +# step to another stage, or adding a stage after it, is not caught. # -# 1. $stage must name the stage in Dockerfile.lint that actually runs -# golangci-lint. --target verifies that the name exists, not that it is -# the right stage, and it stops the build at that stage — so moving the -# lint step into a different stage, or adding a stage after this one, -# would not be caught here. Keep this file and Dockerfile.lint in sync. -# -# 2. .dockerignore decides what reaches the container, and only what -# reaches it gets linted. Excluding a Go file there removes it from the -# lint with no warning: verified by planting a real violation and adding -# just that file's path to .dockerignore, which produced `0 issues.` at -# exit 0 with the violation still sitting in the working tree. It shows -# up only if the rest of the package still references the excluded file, -# in which case the build fails loudly on `undefined:` typecheck errors; -# a self-contained file drops out silently. So .dockerignore is part of -# this gate, not housekeeping — keep it to build inputs the lint does -# not read, and never exclude Go sources, go.mod/go.sum or +# 3. .dockerignore decides what reaches the container, and only what +# reaches it is linted. Excluding a self-contained Go file drops it from +# the lint silently. Never exclude Go sources, go.mod/go.sum or # .golangci.yml. -# -# --output=type=cacheonly skips the image export. Nothing consumes the -# image — the deliverable of this build is an exit code — and exporting it -# costs seconds per run and leaves a dangling image behind every time. The -# lint stage still executes and a lint failure still exits non-zero. set -eu ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"