Make script/cibuild unable to report an unearned green (closes #85)
All checks were successful
check / check (pull_request) Successful in 2m57s

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.

The fix is an `ARG CHECK_EPOCH` declared immediately above the check
RUNs, with script/cibuild passing a fresh value 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.

The epoch is assigned to its own variable rather than substituted
inline into the --build-arg:

    epoch="$(date +%s)"
    docker build --build-arg CHECK_EPOCH="$epoch" .

Under `set -eu` a command substitution that fails inside an argument
does not abort the script. Inline, a failing `date` would leave
CHECK_EPOCH an empty string; an empty string is a constant; and a
constant CHECK_EPOCH is precisely the cached-check false green this
commit exists to eliminate -- so the guard would have carried a silent
path to the defect it guards against. As a bare assignment, `set -e`
aborts before any build starts.

The guarantee is conditional, and README.md and the Dockerfile now say
so instead of claiming the check layers can never be cached. They are
keyed on CHECK_EPOCH, so they re-run for any value not yet built
against this tree -- but a build that omits --build-arg gets the empty
default, and on an unchanged tree every build after the first then
replays them, executes nothing, and exits 0. That state was produced
by measurement rather than reasoned about. Issue #91 tracks the
upstream hardening that would make the missing-arg case fail loudly,
along with the expanded ARG form, a per-invocation epoch, and
script/docker.

The bare unreferenced ARG form is kept deliberately, not because it
matches upstream -- upstream has since settled on expanding the value
into the check command. A declared-but-unreferenced ARG does enter
BuildKit's cache key, which is measured on this host rather than
assumed, and upstream records that repos on the bare form need no
rework. Moving to the expanded form is hardening, tracked in #91.

The script/cibuild header comment claimed the Dockerfile runs
script/check via make check. It does not: it runs make fmt-check and
make lint in the lint stage and make test in the builder stage.
Corrected.

Measurements are recorded once, in the verification comment on PR #89:
a back-to-back script/cibuild pair on an unchanged tree, and the
counterfactual that withholds --build-arg and reproduces the original
false green on its second run. They are deliberately not restated here
or in TODO.md, so there is a single record that cannot disagree with
itself.

.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.
This commit is contained in:
2026-08-09 06:08:33 +00:00
parent 3bcdbcfd83
commit 09dbe6f4c9
4 changed files with 97 additions and 9 deletions

View File

@@ -19,7 +19,34 @@ 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. These layers are
# keyed on its value, so they are cache-eligible only for a value
# already built against this same tree. script/cibuild passes a fresh
# value on every invocation, which is what makes its green mean the
# checks really executed.
#
# The guarantee is conditional on that fresh value, not absolute. A
# build that omits --build-arg -- a bare `docker build .` -- gets an
# empty CHECK_EPOCH, and an empty string is a constant: the first such
# build runs the checks, and every one after it on an unchanged tree
# replays these layers from cache, never executing a check and still
# exiting 0, a green nothing earned. Gate through script/cibuild.
# Making the missing-arg case fail loudly instead is tracked in #91.
#
# CHECK_EPOCH is deliberately not referenced by the commands below: a
# declared-but-unreferenced ARG does enter BuildKit's cache key, which
# is measured on this host rather than assumed (PR #89). Upstream
# sneak/prompts #26 prefers expanding the value into the command so
# that the miss is contractual rather than dependent on that behavior
# staying as it is; adopting that here is tracked in #91. Do not delete
# this ARG as dead code -- the gate depends on it.
#
# 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 +71,11 @@ RUN go mod download
# Copy source code
COPY . .
# Run tests
# Run tests. See the CHECK_EPOCH comment in the lint stage, including
# the conditions the guarantee depends on; 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)

View File

@@ -617,10 +617,24 @@ 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 `make fmt-check` and `make lint` in its lint stage and `make
test` in its builder stage). 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 stages. Those
layers are keyed on that value, so a new value re-runs them even on a
byte-identical tree, and a green from this script means the checks
executed. Dependency and module layers sit above the `ARG` and still
cache, so a build is not cold.
That guarantee is conditional on the fresh value, so **run the gate
through `script/cibuild`, not by invoking `docker build` yourself**. A
bare `docker build .` supplies no `CHECK_EPOCH`; the empty default is
a constant, so the second and every later build on an unchanged tree
serves all three check layers from cache, executes nothing, and still
exits 0. Issue #91 tracks making that case fail loudly instead.
* `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

25
TODO.md
View File

@@ -19,6 +19,31 @@ 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. The fix is 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` assigning
`epoch="$(date +%s)"` and passing `--build-arg CHECK_EPOCH="$epoch"`.
The assignment is separate on purpose: under `set -eu` a command
substitution that fails inside an argument does not abort the script,
which would leave an empty constant `CHECK_EPOCH` and restore the
very false green being fixed. Placement is the rest of the 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. The guarantee is conditional on a
fresh value rather than absolute: a bare `docker build .` gets an
empty `CHECK_EPOCH` and can still serve the check layers from cache,
which `README.md` and the `Dockerfile` now say plainly, with issue
#91 tracking the upstream hardening (expanded `ARG` form, unset
guard, per-invocation epoch, `script/docker`) that would close it.
Verified by re-running the reproduction plus the withheld-`--build-arg`
counterfactual; the measurements are recorded once, in the PR #89
verification comment, rather than restated here. `.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

View File

@@ -1,6 +1,9 @@
#!/bin/sh
# script/cibuild: run the CI build. The Dockerfile runs script/check
# (via make check), so a successful build implies all checks pass.
# script/cibuild: run the CI build. The Dockerfile does not run
# script/check; it runs `make fmt-check` and `make lint` in its lint
# stage and `make test` in its builder stage. A successful build
# implies those three passed, provided they actually ran -- which is
# what the CHECK_EPOCH below is for.
# Generic: needs no adaptation. The Gitea workflow runs this on push.
set -eu
@@ -8,7 +11,22 @@ ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
main() {
cd "$ROOT"
docker build .
# The Dockerfile's check layers are keyed on CHECK_EPOCH, so a
# fresh value here is what forces them to re-run: without it an
# unchanged tree replays them 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.
#
# Assign the epoch on its own line rather than inline in the
# argument. Under `set -eu` a command substitution that fails
# inside an argument does NOT abort the script: CHECK_EPOCH would
# become an empty string, an empty string is a constant, and a
# constant CHECK_EPOCH is exactly the cached-check false green this
# script exists to prevent -- so the guard would disarm itself and
# still exit 0. As a bare assignment, `set -e` catches a failing
# `date` and no build starts.
epoch="$(date +%s)"
docker build --build-arg CHECK_EPOCH="$epoch" .
}
main "$@"