From 24f6e2f9ef946b1f304c0d0d041a9cf1ee17b3d1 Mon Sep 17 00:00:00 2001 From: sneak Date: Sun, 9 Aug 2026 06:08:33 +0000 Subject: [PATCH] Make script/cibuild unable to report an unearned green (closes #85) script/cibuild was a bare `docker build .` with no cache control. The Dockerfile does `COPY . .` and then `RUN make fmt-check` / `RUN make lint` in the lint stage and `COPY . .` / `RUN make test` in the builder stage. On an unchanged tree Docker served those RUN layers from cache, so the checks never executed, and the build still exited 0 -- the exit code, which is the one signal automation trusts, was wrong, and wrong in the direction that matters: the longer a branch sits unchanged, the more likely its "verification" is a replay, which is exactly its state just before a merge. Reproduced on this branch's base at 3bcdbcf. A changed-tree run took 162132ms and produced 14 `ok` lines and `0 issues.`; the immediately following run, with nothing touched, took 221ms and produced 0 `ok` lines and no `0 issues.` line at all, with 19 CACHED layers including `RUN make fmt-check`, `RUN make lint`, and `RUN make test`. Both exited 0. The fix matches the upstream one in sneak/prompts #26 rather than inventing a local variant: an `ARG CHECK_EPOCH` declared immediately above the check RUNs, with script/cibuild passing a fresh `--build-arg CHECK_EPOCH="$(date +%s)"` on every invocation. ARG scope is per-stage in Docker, so the lint stage and the builder stage each declare their own; covering only one would leave half the gate fake. Placement is the substance of the change. The ARG sits below the `apk add`, `COPY go.mod go.sum`, and `go mod download` layers in both stages, so only the check layers are invalidated: earlier and every build would be cold, later and the checks would stay cached. Confirmed by measurement -- on a post-fix build every `apk add` and `go mod download` layer is still reported CACHED, and a changed-tree build went from 162132ms to 176221ms rather than to a cold build's 242727ms. Verified against the original failure mode, not by trusting an exit code: two back-to-back script/cibuild runs on an unchanged tree now take 166745ms and 174025ms, each with 14 `ok` lines and `0 issues.`, and neither reports CACHED on any of the three check layers. Cross-checked host-side with `GOFLAGS=-count=1 make check`: exit 0, 14 `ok` lines, `0 issues.`, with no `parallel golangci-lint is running` and no file paths from outside this worktree, so the lint result is a real one and not a void or contaminated run. .golangci.yml is unchanged (sha256 021cc83f4e6f...643346bcb), as is the lint-stage FROM line that is the single source of truth for the linter version, script/lint's pinned-image logic, and .gitea/workflows/check.yml, whose only step is script/cibuild. --- Dockerfile | 17 +++++++++++++++-- README.md | 14 ++++++++++---- TODO.md | 23 +++++++++++++++++++++++ script/cibuild | 7 ++++++- 4 files changed, 54 insertions(+), 7 deletions(-) diff --git a/Dockerfile b/Dockerfile index 267f8ae..1de5c56 100644 --- a/Dockerfile +++ b/Dockerfile @@ -19,7 +19,17 @@ RUN go mod download # Copy source code COPY . . -# Run formatting check and linter +# Run formatting check and linter. +# +# CHECK_EPOCH must stay immediately above these RUNs. script/cibuild +# passes a fresh value on every build so the check layers can never be +# served from the layer cache: without it an unchanged tree replays +# cached layers, the checks never execute, and the build still exits 0: +# a green nothing earned. ARG scope is per-stage, so the builder +# stage declares its own. Everything above this line (apk, go.mod, +# `go mod download`) is deliberately outside the busted range and keeps +# caching. +ARG CHECK_EPOCH RUN make fmt-check RUN make lint @@ -44,7 +54,10 @@ RUN go mod download # Copy source code COPY . . -# Run tests +# Run tests. See the CHECK_EPOCH comment in the lint stage; ARG scope +# is per-stage, so this stage needs its own declaration, and it must +# stay immediately above the check RUN. +ARG CHECK_EPOCH RUN make test # Build (pure Go, no CGO required since we use modernc.org/sqlite) diff --git a/README.md b/README.md index 552d00c..fb18b16 100644 --- a/README.md +++ b/README.md @@ -617,10 +617,16 @@ them. We provide: lint findings. * `script/docker` — build the Docker image tagged via `script/projectname` -* `script/cibuild` — CI entrypoint: `docker build .` (the Dockerfile - runs the checks). This is the full CI-equivalent gate — it runs the - checks in the same containers CI does, from a clean copy of the tree, - so it also catches anything that depends on host state. +* `script/cibuild` — CI entrypoint: `docker build` (the Dockerfile runs + the checks). This is the full CI-equivalent gate — it runs the checks + in the same containers CI does, from a clean copy of the tree, so it + also catches anything that depends on host state. It passes a fresh + `--build-arg CHECK_EPOCH`, which the `Dockerfile` declares + immediately above the check `RUN`s in both the lint and builder + stages, so those layers can never be served from the Docker layer + cache: a green from this script always means the checks actually + executed. Dependency and module layers sit above the `ARG` and still + cache, so a build is not cold. * `script/precommit` — pre-commit gate: `go mod tidy` + `go fmt` (must not change files), then `script/check` * `script/install-precommit` — install the git pre-commit hook that diff --git a/TODO.md b/TODO.md index 16f67b1..992f4f4 100644 --- a/TODO.md +++ b/TODO.md @@ -19,6 +19,29 @@ or delete the branch. # Completed Steps +- 2026-08-09: Stopped `script/cibuild` from reporting a green it did + not earn (issue #85). A bare `docker build .` let Docker serve the + check layers from the layer cache whenever the tree had not changed: + the checks never executed and the build still exited 0. Reproduced on + this branch's base — a genuine changed-tree run took 162s with 14 + `ok` lines, and the immediately following unchanged-tree run took + 221ms with 0 `ok` lines, 19 cached layers, and the same exit 0, with + `RUN make fmt-check`, `RUN make lint`, and `RUN make test` all + reported `CACHED`. The fix matches the upstream one in + `sneak/prompts` #26: an `ARG CHECK_EPOCH` declared immediately above + the check `RUN`s in both the lint stage and the builder stage (`ARG` + scope is per-stage, so each declares its own), with `script/cibuild` + passing `--build-arg CHECK_EPOCH="$(date +%s)"`. Placement is the + whole point — the `ARG` sits below the `apk add`, `COPY go.mod + go.sum`, and `go mod download` layers, so only the checks are + invalidated and the dependency layers still cache. Verified by + re-running the reproduction: two back-to-back runs on an unchanged + tree took 167s and 174s, each with 14 `ok` lines and `0 issues.`, no + `CACHED` on any of the three check layers, while every `apk add` and + `go mod download` layer stayed `CACHED` in both. A changed-tree build + went from 162s to 176s, so this is not a cold build. `.golangci.yml`, + the lint-stage `FROM` line and its digest, `script/lint`, and + `.gitea/workflows/check.yml` are all untouched. - 2026-08-09: Corrected the `Vaultik.UI` doc comment (issue #84). It claimed the cli layer replaces the writer with a discarding one in `--cron` mode; the actual mechanism is `UI.SetQuiet(true)` in diff --git a/script/cibuild b/script/cibuild index 3da5857..48fd251 100755 --- a/script/cibuild +++ b/script/cibuild @@ -8,7 +8,12 @@ ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" main() { cd "$ROOT" - docker build . + # CHECK_EPOCH changes on every invocation, which invalidates the + # Dockerfile layers that run the checks. Without it an unchanged + # tree replays those layers from cache, the checks never execute, + # and the build still exits 0. The ARG sits immediately above the + # check RUNs, so dependency and module layers still cache. + docker build --build-arg CHECK_EPOCH="$(date +%s)" . } main "$@"