script/cibuild and script/docker were bare docker build
invocations with no cache control, and the Dockerfile copies the tree
before running its gates. On an unchanged tree Docker served those
layers from cache: the gates never executed and the build still exited
0.
What changed
Dockerfile — ARG CHECK_EPOCH declared in both stages,
because ARG is scoped per stage and this file has three gates across
two of them. One declaration would have left the other stage silently
cacheable, which is the easiest way to ship a fix that does not fix
anything.
lint stage: below COPY . ., above RUN make fmt-check and RUN make lint
build stage: below USER builder, above RUN make check
Each gate RUNreferences the value (echo "gate ..., epoch ${CHECK_EPOCH}" && make ...). BuildKit hashes the expanded command,
not the declaration, so a declared-but-unreferenced ARG invalidates
nothing. The echo doubles as evidence in the build log that the layer
really executed, which is what let me verify the rest of this rather
than assert it.
Both declarations sit below the dependency layers on purpose. The
goal is to bust the gates, not to go cold — a build that recompiles the
world every invocation would be a different bug.
The build-stage ARG is placed after USER builder, so the drop to
the unprivileged user still happens before make check.
script/cibuild, script/docker — both pass --build-arg CHECK_EPOCH="$(date +%s)". POSIX sh, set -eu, no
bashisms; both verified with dash -n. script/cibuild's header
comment asserted the guarantee it did not provide, so it now says why
the implication actually holds. script/docker matters as much: per
the issue's follow-up comment, make docker is the gate a reviewer
runs by hand and it is the one that actually fooled someone on PR #31.
Verification
All runs on an unchanged tree with BUILDKIT_PROGRESS=plain, cache
pre-warmed.
1. script/cibuild twice in a row — gates ran both times
run
wall
exit
gate fmt-check
gate lint
gate check
epoch in log
1
78.8s
0
4.6s
29.7s
33.1s
1786258744
2
61.1s
0
1.6s
24.7s
31.7s
1786258823
Distinct epochs, three gate layers executed each time. No sub-second
run anywhere.
2. Dependency layers still cached
Twelve steps served CACHED in the steady state (run 2 above, and both script/docker runs below):
[lint 3/7] COPY go.mod go.sum ./
[lint 4/7] RUN go mod download
[lint 5/7] COPY . .
[builder 2/11] RUN apk add --no-cache make
[builder 3/11] RUN adduser -D -u 1000 builder
[builder 4/11] WORKDIR /src
[builder 5/11] COPY --from=lint /usr/bin/golangci-lint /usr/local/bin/golangci-lint
[builder 6/11] COPY go.mod go.sum ./
[builder 7/11] RUN go mod download
[builder 8/11] COPY . .
[builder 9/11] RUN chown -R builder:builder /src /home/builder
[stage-2 2/2] COPY --from=builder /src/sfdupes /usr/local/bin/sfdupes
The pinned base images resolve from the local store on top of that.
Only the three gates and the RUN make build downstream of them go
cold, which is the intended blast radius.
One honest wrinkle: the first run after editing the Dockerfile showed
9 CACHED rather than 12, because the Dockerfile is itself part of
the build context and BuildKit re-materialised three lint-stage layers.
Every subsequent run on a stable tree settled at 12. That is cache
churn from editing the file, not from CHECK_EPOCH.
3. script/docker twice in a row
run
wall
exit
gate fmt-check
gate lint
gate check
CACHED
1
61.1s
0
2.9s
24.7s
31.9s
12
2
53.4s
0
3.3s
21.1s
26.5s
12
4. The lint stage still gates the build stage
Not asserted — tested. I planted a deliberate unused finding in the
tree and ran script/cibuild:
#17 24.86 lintprobe_scratch.go:5:6: func lintProbeScratch is unused (unused)
#17 ERROR: process "/bin/sh -c echo \"gate lint, epoch ${CHECK_EPOCH}\" && make lint" did not complete successfully: exit code: 2
Exit 1 after 36.1s, and grep -c "gate check, epoch" over the whole
build log returned 0 — the build-stage make check never started.
The COPY --from=lint at Dockerfile:29 still forces the lint stage
to complete first. Worth noting explicitly: that COPY step itself
shows as CACHED, because the copied binary is byte-identical every
time. The lint stage still runs — BuildKit has to build it to resolve
the copy. Probe file removed; it is not in the commit.
5. Non-root build stage preserved, and it is load-bearing
Root reads straight through the chmod(0), so the test fails. It
passes in the real build, which is only possible as builder.
6. make check
Green in 17.3s: ok sneak.berlin/go/sfdupes 1.799s coverage: 88.3% of statements, 0 issues. Run with a fresh GOLANGCI_LINT_CACHE per #36, so no findings are attributable to stale worktree paths. Also
green inside the container on every build above (88.5% there).
Out of scope
Propagating this to the canonical Dockerfile / script/cibuild
templates. I checked upstream first per the issue's closing note: prompts#26 is still open and the canonical Dockerfile there is
unchanged, so there was nothing to re-vendor. This is the bespoke
local fix, shaped to match what #26 recommends.
A deprecation warning surfaced on every lint run and is not
addressed here, since it is outside this issue's scope: The linter 'gomodguard' is deprecated (since v2.12.0) ... Replaced by gomodguard_v2. I will file it as its own issue.
Closes #32.
`script/cibuild` and `script/docker` were bare `docker build`
invocations with no cache control, and the `Dockerfile` copies the tree
before running its gates. On an unchanged tree Docker served those
layers from cache: the gates never executed and the build still exited
0.
## What changed
**`Dockerfile`** — `ARG CHECK_EPOCH` declared in **both** stages,
because `ARG` is scoped per stage and this file has three gates across
two of them. One declaration would have left the other stage silently
cacheable, which is the easiest way to ship a fix that does not fix
anything.
- lint stage: below `COPY . .`, above `RUN make fmt-check` and
`RUN make lint`
- build stage: below `USER builder`, above `RUN make check`
Each gate `RUN` **references** the value (`echo "gate ..., epoch
${CHECK_EPOCH}" && make ...`). BuildKit hashes the expanded command,
not the declaration, so a declared-but-unreferenced `ARG` invalidates
nothing. The echo doubles as evidence in the build log that the layer
really executed, which is what let me verify the rest of this rather
than assert it.
Both declarations sit **below** the dependency layers on purpose. The
goal is to bust the gates, not to go cold — a build that recompiles the
world every invocation would be a different bug.
The build-stage `ARG` is placed after `USER builder`, so the drop to
the unprivileged user still happens before `make check`.
**`script/cibuild`, `script/docker`** — both pass
`--build-arg CHECK_EPOCH="$(date +%s)"`. POSIX `sh`, `set -eu`, no
bashisms; both verified with `dash -n`. `script/cibuild`'s header
comment asserted the guarantee it did not provide, so it now says why
the implication actually holds. `script/docker` matters as much: per
the issue's follow-up comment, `make docker` is the gate a reviewer
runs by hand and it is the one that actually fooled someone on PR #31.
## Verification
All runs on an unchanged tree with `BUILDKIT_PROGRESS=plain`, cache
pre-warmed.
### 1. `script/cibuild` twice in a row — gates ran both times
| run | wall | exit | `gate fmt-check` | `gate lint` | `gate check` | epoch in log |
| --- | --- | --- | --- | --- | --- | --- |
| 1 | 78.8s | 0 | 4.6s | 29.7s | 33.1s | `1786258744` |
| 2 | 61.1s | 0 | 1.6s | 24.7s | 31.7s | `1786258823` |
Distinct epochs, three gate layers executed each time. No sub-second
run anywhere.
### 2. Dependency layers still cached
Twelve steps served `CACHED` in the steady state (run 2 above, and both
`script/docker` runs below):
```
[lint 3/7] COPY go.mod go.sum ./
[lint 4/7] RUN go mod download
[lint 5/7] COPY . .
[builder 2/11] RUN apk add --no-cache make
[builder 3/11] RUN adduser -D -u 1000 builder
[builder 4/11] WORKDIR /src
[builder 5/11] COPY --from=lint /usr/bin/golangci-lint /usr/local/bin/golangci-lint
[builder 6/11] COPY go.mod go.sum ./
[builder 7/11] RUN go mod download
[builder 8/11] COPY . .
[builder 9/11] RUN chown -R builder:builder /src /home/builder
[stage-2 2/2] COPY --from=builder /src/sfdupes /usr/local/bin/sfdupes
```
The pinned base images resolve from the local store on top of that.
Only the three gates and the `RUN make build` downstream of them go
cold, which is the intended blast radius.
One honest wrinkle: the first run after editing the `Dockerfile` showed
9 `CACHED` rather than 12, because the `Dockerfile` is itself part of
the build context and BuildKit re-materialised three lint-stage layers.
Every subsequent run on a stable tree settled at 12. That is cache
churn from editing the file, not from `CHECK_EPOCH`.
### 3. `script/docker` twice in a row
| run | wall | exit | `gate fmt-check` | `gate lint` | `gate check` | `CACHED` |
| --- | --- | --- | --- | --- | --- | --- |
| 1 | 61.1s | 0 | 2.9s | 24.7s | 31.9s | 12 |
| 2 | 53.4s | 0 | 3.3s | 21.1s | 26.5s | 12 |
### 4. The lint stage still gates the build stage
Not asserted — tested. I planted a deliberate `unused` finding in the
tree and ran `script/cibuild`:
```
#17 24.86 lintprobe_scratch.go:5:6: func lintProbeScratch is unused (unused)
#17 ERROR: process "/bin/sh -c echo \"gate lint, epoch ${CHECK_EPOCH}\" && make lint" did not complete successfully: exit code: 2
```
Exit 1 after 36.1s, and `grep -c "gate check, epoch"` over the whole
build log returned **0** — the build-stage `make check` never started.
The `COPY --from=lint` at `Dockerfile:29` still forces the lint stage
to complete first. Worth noting explicitly: that `COPY` step itself
shows as `CACHED`, because the copied binary is byte-identical every
time. The lint stage still runs — BuildKit has to build it to resolve
the copy. Probe file removed; it is not in the commit.
### 5. Non-root build stage preserved, and it is load-bearing
`docker run --rm <builder stage> id` reports:
```
uid=1000(builder) gid=1000(builder) groups=1000(builder)
```
To confirm the `chmod(0)` tests are genuinely exercised rather than
merely present, I forced the same image to run them as root:
```
--- FAIL: TestScanHardlinkRunFailsTogether (0.01s)
scan_test.go:817: stats = {added:2 ... skipped:0}, want both hardlink paths skipped
```
Root reads straight through the `chmod(0)`, so the test fails. It
passes in the real build, which is only possible as `builder`.
### 6. `make check`
Green in 17.3s: `ok sneak.berlin/go/sfdupes 1.799s coverage: 88.3% of
statements`, `0 issues`. Run with a fresh `GOLANGCI_LINT_CACHE` per
#36, so no findings are attributable to stale worktree paths. Also
green inside the container on every build above (88.5% there).
## Out of scope
- Propagating this to the canonical `Dockerfile` / `script/cibuild`
templates. I checked upstream first per the issue's closing note:
`prompts` #26 is still open and the canonical `Dockerfile` there is
unchanged, so there was nothing to re-vendor. This is the bespoke
local fix, shaped to match what #26 recommends.
- A deprecation warning surfaced on every lint run and is **not**
addressed here, since it is outside this issue's scope: `The linter
'gomodguard' is deprecated (since v2.12.0) ... Replaced by
gomodguard_v2`. I will file it as its own issue.
script/cibuild and script/docker were bare docker build invocations
with no cache control, and the Dockerfile copies the tree before
running its gates. On an unchanged tree Docker served those layers
from cache, so the gates never executed and the build still exited 0.
A merge commit here has a tree byte-identical to the branch head it
merges, so every merge CI run was almost certainly a full cache hit,
and PR #31's reviewer caught make docker returning success as a
17-layer cache hit that proved nothing.
Declare ARG CHECK_EPOCH in both stages and have the scripts pass
--build-arg CHECK_EPOCH="$(date +%s)". ARG is scoped per stage and
this Dockerfile has three gates across two of them (make fmt-check and
make lint in the lint stage, make check in the build stage), so one
declaration would have left a stage silently cacheable. BuildKit
hashes the expanded command rather than the declaration, so each gate
RUN echoes the epoch: an unreferenced ARG invalidates nothing, and the
echo doubles as evidence in the build log that the layer really ran.
Both declarations sit below the dependency layers, so the pinned base
images, go mod download, apk add and the source copies keep their
cache and only the gates go cold. The build-stage declaration sits
after USER, so the drop to the unprivileged builder user still happens
before make check and the chmod(0) permission tests stay real.
Notes for the reviewer, since this PR is specifically about not
trusting a build that says it passed.
Reproduce the verification yourself
Do not take the tables in the description on faith — the whole point of
the issue is that a green can be counterfeit. Recipe:
git checkout cibuild-cache-bust
BUILDKIT_PROGRESS=plain script/cibuild # primetimeBUILDKIT_PROGRESS=plain script/cibuild 2>&1| tee /tmp/a.log
timeBUILDKIT_PROGRESS=plain script/cibuild 2>&1| tee /tmp/b.log
grep -E 'gate .*, epoch [0-9]+' /tmp/a.log /tmp/b.log # 3 lines each, different epochs
grep -c CACHED /tmp/b.log # should be 12, not 0
Three gate ..., epoch N lines per run means all three gates executed;
the epoch differing between the two logs means it was not a replay of
the same cached command. A CACHED count near zero would mean I traded
one bug for a cold build, which the issue explicitly calls a failure.
The counterfactual, if you want it
To see the old behaviour and confirm the fix is actually doing
something, run the same two-run loop against main. The second run
should return in well under a second with every layer CACHED and no
gate output at all. That is the 17-layer cache hit PR #31's reviewer
hit.
Where I would attack this if I were reviewing
Four things I deliberately went looking for, so you can check my work
rather than repeat it:
Only one stage busted.ARG is per-stage. A single declaration
leaves the other stage cacheable and the fix looks fine in a diff.
There are two declarations here, Dockerfile lint stage and build
stage. Confirm both, and confirm both gates in the lint stage sit
below the declaration.
ARG declared but not referenced. BuildKit hashes the expanded
command, so ARG CHECK_EPOCH with a plain RUN make check under it
busts nothing and still reads as a fix. Every gate RUN here
interpolates ${CHECK_EPOCH}.
Fully cold build. Check the CACHED count, not just the gate
output. If go mod download and apk add are re-running, the ARGs are too high in the file.
Root creeping back in. The build stage must still be builder
at make check time. If ARG had gone above USER, nothing would
visibly break — but TestScanHardlinkRunFailsTogether and the main_test.go unreadable-file path would stop testing anything,
because root reads through chmod(0). I verified that as root the
suite genuinely fails, so a passing containerised run is itself
proof the drop still happens.
One thing I could not fully close
date +%s has one-second resolution. Two builds starting within the
same wall-clock second would share an epoch and the second could serve
the gates from cache. Not reachable in practice here — the build takes
50 to 80 seconds, so back-to-back invocations cannot collide — and I
kept $(date +%s) because the issue's definition of done names that
exact form. If you would rather have the guarantee not depend on build
duration, date +%s%N or $(date +%s)-$$ would close it, and I will
make that change on request rather than argue about it.
Also filed
#38, for the gomodguard deprecation warning that appears on every
lint run. Noticed here, deliberately not fixed here.
Notes for the reviewer, since this PR is specifically about not
trusting a build that says it passed.
## Reproduce the verification yourself
Do not take the tables in the description on faith — the whole point of
the issue is that a green can be counterfeit. Recipe:
```sh
git checkout cibuild-cache-bust
BUILDKIT_PROGRESS=plain script/cibuild # prime
time BUILDKIT_PROGRESS=plain script/cibuild 2>&1 | tee /tmp/a.log
time BUILDKIT_PROGRESS=plain script/cibuild 2>&1 | tee /tmp/b.log
grep -E 'gate .*, epoch [0-9]+' /tmp/a.log /tmp/b.log # 3 lines each, different epochs
grep -c CACHED /tmp/b.log # should be 12, not 0
```
Three `gate ..., epoch N` lines per run means all three gates executed;
the epoch differing between the two logs means it was not a replay of
the same cached command. A `CACHED` count near zero would mean I traded
one bug for a cold build, which the issue explicitly calls a failure.
## The counterfactual, if you want it
To see the old behaviour and confirm the fix is actually doing
something, run the same two-run loop against `main`. The second run
should return in well under a second with every layer `CACHED` and no
gate output at all. That is the 17-layer cache hit PR #31's reviewer
hit.
## Where I would attack this if I were reviewing
Four things I deliberately went looking for, so you can check my work
rather than repeat it:
1. **Only one stage busted.** `ARG` is per-stage. A single declaration
leaves the other stage cacheable and the fix looks fine in a diff.
There are two declarations here, `Dockerfile` lint stage and build
stage. Confirm both, and confirm both gates in the lint stage sit
below the declaration.
2. **`ARG` declared but not referenced.** BuildKit hashes the expanded
command, so `ARG CHECK_EPOCH` with a plain `RUN make check` under it
busts nothing and still reads as a fix. Every gate `RUN` here
interpolates `${CHECK_EPOCH}`.
3. **Fully cold build.** Check the `CACHED` count, not just the gate
output. If `go mod download` and `apk add` are re-running, the
`ARG`s are too high in the file.
4. **Root creeping back in.** The build stage must still be `builder`
at `make check` time. If `ARG` had gone above `USER`, nothing would
visibly break — but `TestScanHardlinkRunFailsTogether` and the
`main_test.go` unreadable-file path would stop testing anything,
because root reads through `chmod(0)`. I verified that as root the
suite genuinely fails, so a passing containerised run is itself
proof the drop still happens.
## One thing I could not fully close
`date +%s` has one-second resolution. Two builds starting within the
same wall-clock second would share an epoch and the second could serve
the gates from cache. Not reachable in practice here — the build takes
50 to 80 seconds, so back-to-back invocations cannot collide — and I
kept `$(date +%s)` because the issue's definition of done names that
exact form. If you would rather have the guarantee not depend on build
duration, `date +%s%N` or `$(date +%s)-$$` would close it, and I will
make that change on request rather than argue about it.
## Also filed
#38, for the `gomodguard` deprecation warning that appears on every
lint run. Noticed here, deliberately not fixed here.
Independent review of head 964fc29 against base main (b8ebe5f).
Every claim in the description was re-executed from a throwaway
worktree; nothing in any checkout was modified and nothing was
committed.
Environment note. A host-wide BuildKit cache prune happened
mid-review, unrelated to this PR. Every cached-step count below comes
from a steady state I re-established myself after the prune (build
once, then count CACHED on the next run on an unchanged tree), not
from the author's numbers. Wall-clock times are therefore not
comparable to the description's and are reported only to show that no
run was a cache hit.
All three gate steps executed with real wall time in all four runs, no
gate step reported CACHED in any of them, and every run carried a
distinct epoch. No sub-second run anywhere.
2. Is the invalidation real — yes, confirmed by counterfactual
Held the value constant and built twice with --build-arg CHECK_EPOCH=CONSTANT:
first build: all three gates executed;
second build: 17 CACHED, all three gate RUNs among them, zero gate ..., epoch lines in the log, exit 0 in seconds.
That is precisely the 17-layer cache hit recorded on #32. So the
invalidation comes from the interpolated value changing, not from
anything cosmetic, and the ARG is genuinely referenced rather than
merely declared. Nothing above the gates is invalidated — see 3.
3. Dependency layers still cached — yes; count is 13 here, not 12
Steady-state CACHED steps, identical in run B and both script/docker
runs:
[lint 2/7] WORKDIR /src
[lint 3/7] COPY go.mod go.sum ./
[lint 4/7] RUN go mod download
[lint 5/7] COPY . .
[builder 2/11] RUN apk add --no-cache make
[builder 3/11] RUN adduser -D -u 1000 builder
[builder 4/11] WORKDIR /src
[builder 5/11] COPY --from=lint /usr/bin/golangci-lint ...
[builder 6/11] COPY go.mod go.sum ./
[builder 7/11] RUN go mod download
[builder 8/11] COPY . .
[builder 9/11] RUN chown -R builder:builder /src /home/builder
[stage-2 2/2] COPY --from=builder /src/sfdupes /usr/local/bin/sfdupes
Both go mod downloads, apk add, adduser and the source copies stay
cached; the pinned base images resolve from the local store. Only the
three gates and the RUN make build below them go cold. The blast
radius is what the issue asks for; this is not a cold build.
The description and TODO.md:59 say twelve — the thirteenth is the
lint stage's WORKDIR /src. Cosmetic, and possibly BuildKit-version
dependent, but it is wrong in committed content. Non-blocking.
4. Lint stage still gates the build stage — reproduced
Planted a deliberate unused function in a copy of the tree and ran script/cibuild: exit 1 after 24.0s, failing at RUN echo "gate lint, epoch ${CHECK_EPOCH}" && make lint with func lintProbeScratch is unused (unused), and grep -c 'gate check, epoch' over the entire build log returned 0 —
the build-stage make check never started.
The author's reasoning about COPY --from=lint is correct, and I
verified both halves in a single build: that COPY reported CACHED
while both lint gates executed in the same run. The copy's key is
content-addressed on the copied binary, which is byte-identical every
time, so the copy result is reused; BuildKit still has to bring the
lint stage to its final state to resolve it, which is why a lint
failure aborts before compilation. Fail-fast is intact.
5. Non-root quirk — intact and load-bearing
--target builder image reports uid=1000(builder) gid=1000(builder) groups=1000(builder); ARG after USER does not reset the user, so make check still runs unprivileged.
In that same image:
as builder: ok sneak.berlin/go/sfdupes for TestScanHardlinkRunFailsTogether;
as --user 0:0: FAIL ... stats = {added:2 ... skipped:0}, want both hardlink paths skipped.
Root reads straight through the chmod(0), as claimed. The permission
tests are genuinely exercised, and only because the drop is still there.
6. make check
Green locally on the head commit with a fresh GOLANGCI_LINT_CACHE per #36: 13.4s, coverage: 88.5% of statements, 0 issues, and git status clean afterwards (the gate modifies nothing). Local golangci-lint is 2.12.2, matching the pin.
7. Scope and hygiene
Four files: Dockerfile, TODO.md, script/cibuild, script/docker.
No scope creep.
dash -n clean on both scripts; set -eu preserved; no bashisms.
git show --check: no whitespace errors. No over-long added lines.
No non-inclusive terminology in the diff.
Commit subject ends with (closes #32); body is wrapped and
accurate; no attribution trailers of any kind anywhere in the diff or
the message.
CI green on 964fc29 (check / check (push), 1m50s).
Mergeable: head contains current main, no conflicts.
Cold build 2m53s, inside the 5-minute policy limit.
TODO.md entry is in the right place and accurate apart from the
count in 3.
Non-blocking findings
Dockerfile:24, Dockerfile:64 — an unset CHECK_EPOCH degrades
silently. The ARG has no default and nothing asserts it is
non-empty. Measured on this branch: a bare docker build . run twice
gives 17 CACHED, all three gates CACHED, no gate output, exit 0 —
the exact defect this PR removes, quietly restored, with the log
showing gate check, epoch and an empty value. Not a regression
(docker build . was equally broken before) and both sanctioned
entrypoints pass the arg, so the DoD is met. But REPO_POLICIES.md
itself describes CI as "runs docker build . on push", so the raw
command is a plausible thing for a human to type. Cheap hardening for
a follow-up: have the first gate assert the value, e.g. RUN test -n "${CHECK_EPOCH}" || { echo "CHECK_EPOCH unset; use script/docker or script/cibuild" >&2; exit 1; },
so the failure is loud instead of a cached green.
Second-resolution epoch — agreed non-blocking, but for a different
reason than the one given. "Builds take 50 to 80 seconds" is an
argument about one process, and it does not cover two concurrent
Gitea Actions jobs sharing a runner — pushing a branch and its merge
commit together is exactly the byte-identical-tree case #32
documents, and those two jobs can plausibly reach date +%s in the
same second. What makes it acceptable is narrower: a collision needs
the same second and an identical tree, and in that case the reused
result was genuinely computed for that exact tree, so it cannot
certify a tree nobody checked. The residual cost is that one of the
two runs proves nothing on its own. If it is ever tightened, prefer "$(date +%s)-$$" over date +%s%N: %N is a GNU extension, and on
macOS/BSD date it emits a literal trailing N, i.e. it would
silently stay at one-second resolution on developer machines — a
fix that looks like a fix.
Noted, not a finding: the gomodguard deprecation warning appears on
every lint run, is correctly out of scope here, and is already filed
as #38.
No blocking defects. The gate does what it claims, fails when it should,
and keeps the dependency cache.
## Review: PASS
Independent review of head `964fc29` against base `main` (`b8ebe5f`).
Every claim in the description was re-executed from a throwaway
worktree; nothing in any checkout was modified and nothing was
committed.
**Environment note.** A host-wide BuildKit cache prune happened
mid-review, unrelated to this PR. Every cached-step count below comes
from a steady state I re-established myself after the prune (build
once, then count `CACHED` on the *next* run on an unchanged tree), not
from the author's numbers. Wall-clock times are therefore not
comparable to the description's and are reported only to show that no
run was a cache hit.
### 1. Do the gates execute every time — yes
`script/cibuild`, unchanged tree, `BUILDKIT_PROGRESS=plain`:
| run | wall | exit | gates seen | epoch | `CACHED` |
| --- | --- | --- | --- | --- | --- |
| A (first post-prune, cold) | 2m53.4s | 0 | fmt-check, lint, check | `1786260290` | 1 |
| B (steady state) | 1m08.9s | 0 | fmt-check, lint, check | `1786260463` | 13 |
`script/docker`, same conditions:
| run | wall | exit | gates seen | epoch | `CACHED` |
| --- | --- | --- | --- | --- | --- |
| 1 | 57.5s | 0 | fmt-check, lint, check | `1786261295` | 13 |
| 2 | 1m11.1s | 0 | fmt-check, lint, check | `1786261353` | 13 |
All three gate steps executed with real wall time in all four runs, no
gate step reported `CACHED` in any of them, and every run carried a
distinct epoch. No sub-second run anywhere.
### 2. Is the invalidation real — yes, confirmed by counterfactual
Held the value constant and built twice with
`--build-arg CHECK_EPOCH=CONSTANT`:
- first build: all three gates executed;
- second build: **17 `CACHED`**, all three gate `RUN`s among them, zero
`gate ..., epoch` lines in the log, exit 0 in seconds.
That is precisely the 17-layer cache hit recorded on #32. So the
invalidation comes from the interpolated value changing, not from
anything cosmetic, and the `ARG` is genuinely referenced rather than
merely declared. Nothing above the gates is invalidated — see 3.
### 3. Dependency layers still cached — yes; count is 13 here, not 12
Steady-state `CACHED` steps, identical in run B and both `script/docker`
runs:
```
[lint 2/7] WORKDIR /src
[lint 3/7] COPY go.mod go.sum ./
[lint 4/7] RUN go mod download
[lint 5/7] COPY . .
[builder 2/11] RUN apk add --no-cache make
[builder 3/11] RUN adduser -D -u 1000 builder
[builder 4/11] WORKDIR /src
[builder 5/11] COPY --from=lint /usr/bin/golangci-lint ...
[builder 6/11] COPY go.mod go.sum ./
[builder 7/11] RUN go mod download
[builder 8/11] COPY . .
[builder 9/11] RUN chown -R builder:builder /src /home/builder
[stage-2 2/2] COPY --from=builder /src/sfdupes /usr/local/bin/sfdupes
```
Both `go mod download`s, `apk add`, `adduser` and the source copies stay
cached; the pinned base images resolve from the local store. Only the
three gates and the `RUN make build` below them go cold. The blast
radius is what the issue asks for; this is not a cold build.
The description and `TODO.md:59` say twelve — the thirteenth is the
lint stage's `WORKDIR /src`. Cosmetic, and possibly BuildKit-version
dependent, but it is wrong in committed content. Non-blocking.
### 4. Lint stage still gates the build stage — reproduced
Planted a deliberate `unused` function in a copy of the tree and ran
`script/cibuild`: exit 1 after 24.0s, failing at
`RUN echo "gate lint, epoch ${CHECK_EPOCH}" && make lint` with
`func lintProbeScratch is unused (unused)`, and
`grep -c 'gate check, epoch'` over the entire build log returned **0** —
the build-stage `make check` never started.
The author's reasoning about `COPY --from=lint` is correct, and I
verified both halves in a single build: that `COPY` reported `CACHED`
while both lint gates executed in the same run. The copy's key is
content-addressed on the copied binary, which is byte-identical every
time, so the copy result is reused; BuildKit still has to bring the
lint stage to its final state to resolve it, which is why a lint
failure aborts before compilation. Fail-fast is intact.
### 5. Non-root quirk — intact and load-bearing
`--target builder` image reports
`uid=1000(builder) gid=1000(builder) groups=1000(builder)`; `ARG` after
`USER` does not reset the user, so `make check` still runs unprivileged.
In that same image:
- as `builder`: `ok sneak.berlin/go/sfdupes` for
`TestScanHardlinkRunFailsTogether`;
- as `--user 0:0`:
`FAIL ... stats = {added:2 ... skipped:0}, want both hardlink paths skipped`.
Root reads straight through the `chmod(0)`, as claimed. The permission
tests are genuinely exercised, and only because the drop is still there.
### 6. `make check`
Green locally on the head commit with a fresh `GOLANGCI_LINT_CACHE` per
#36: 13.4s, `coverage: 88.5% of statements`, `0 issues`, and
`git status` clean afterwards (the gate modifies nothing). Local
`golangci-lint` is 2.12.2, matching the pin.
### 7. Scope and hygiene
- Four files: `Dockerfile`, `TODO.md`, `script/cibuild`, `script/docker`.
No scope creep.
- `dash -n` clean on both scripts; `set -eu` preserved; no bashisms.
- `git show --check`: no whitespace errors. No over-long added lines.
- No non-inclusive terminology in the diff.
- Commit subject ends with ` (closes #32)`; body is wrapped and
accurate; no attribution trailers of any kind anywhere in the diff or
the message.
- CI green on `964fc29` (`check / check (push)`, 1m50s).
- Mergeable: head contains current `main`, no conflicts.
- Cold build 2m53s, inside the 5-minute policy limit.
- `TODO.md` entry is in the right place and accurate apart from the
count in 3.
### Non-blocking findings
1. **`Dockerfile:24`, `Dockerfile:64` — an unset `CHECK_EPOCH` degrades
silently.** The `ARG` has no default and nothing asserts it is
non-empty. Measured on this branch: a bare `docker build .` run twice
gives 17 `CACHED`, all three gates `CACHED`, no gate output, exit 0 —
the exact defect this PR removes, quietly restored, with the log
showing `gate check, epoch` and an empty value. Not a regression
(`docker build .` was equally broken before) and both sanctioned
entrypoints pass the arg, so the DoD is met. But `REPO_POLICIES.md`
itself describes CI as "runs `docker build .` on push", so the raw
command is a plausible thing for a human to type. Cheap hardening for
a follow-up: have the first gate assert the value, e.g.
`RUN test -n "${CHECK_EPOCH}" || { echo "CHECK_EPOCH unset; use script/docker or script/cibuild" >&2; exit 1; }`,
so the failure is loud instead of a cached green.
2. **Second-resolution epoch — agreed non-blocking, but for a different
reason than the one given.** "Builds take 50 to 80 seconds" is an
argument about one process, and it does not cover two concurrent
Gitea Actions jobs sharing a runner — pushing a branch and its merge
commit together is exactly the byte-identical-tree case #32
documents, and those two jobs can plausibly reach `date +%s` in the
same second. What makes it acceptable is narrower: a collision needs
the same second *and* an identical tree, and in that case the reused
result was genuinely computed for that exact tree, so it cannot
certify a tree nobody checked. The residual cost is that one of the
two runs proves nothing on its own. If it is ever tightened, prefer
`"$(date +%s)-$$"` over `date +%s%N`: `%N` is a GNU extension, and on
macOS/BSD `date` it emits a literal trailing `N`, i.e. it would
silently stay at one-second resolution on developer machines — a
fix that looks like a fix.
3. **`TODO.md:59`** says twelve `CACHED` steps; measured thirteen here.
4. Noted, not a finding: the `gomodguard` deprecation warning appears on
every lint run, is correctly out of scope here, and is already filed
as #38.
No blocking defects. The gate does what it claims, fails when it should,
and keeps the dependency cache.
Manager note — merged as a102b8f. main green, 0 issues., 88.3% coverage, origin carries only main.
The decisive piece of this review was the counterfactual. Rather than only checking that the gates now run, the reviewer built with --build-arg CHECK_EPOCH=CONSTANT and watched the second build produce 17 CACHED steps with all three gate RUNs cached, zero gate output, exit 0 — reproducing precisely the false green #32 was filed for. That establishes the fix is load-bearing rather than incidental, which counting successful runs alone could never show.
It also verified the two things most likely to have been quietly broken: the lint stage still gates the build stage (planted an unused finding; build failed at the lint gate in 24s and grep -c 'gate check, epoch' over the full log returned 0, so the build-stage make check never started), and the non-root drop is still load-bearing (TestScanHardlinkRunFailsTogether passes as builder and fails as --user 0:0 with skipped:0, because root reads through chmod(0)). That second one is the repo quirk we were told to preserve, and it is now demonstrated rather than assumed.
Worth recording how the review handled the environment. A host-wide docker builder prune -af on another repo destroyed roughly 41 GB of shared BuildKit cache mid-review. I warned the reviewer that its steady-state cached-step measurement had become meaningless and that a cold-cache test timeout would look like a bug. It re-established its own baseline — build once, count on the next run — and reported 13 cached steps rather than repeating the author's 12, correctly identifying the extra as the lint stage's WORKDIR /src. Measurements taken pre- and post-prune are labelled as such in its comment. Without that, this would very likely have been a false FAIL on a PR that is correct.
Non-blocking findings, disposed:
CHECK_EPOCH has no default and nothing asserts it is non-empty, so a bare docker build . silently restores the cached-gate behaviour — 17 CACHED, exit 0. Filed as #39. Not a regression, but one habit away from the failure mode this PR exists to prevent.
The second-resolution epoch stands, with a better justification than the PR gave: safety comes from a collision requiring the same second and a byte-identical tree, not from builds taking 50-80 seconds. Recorded on #39, along with the note that date +%s%N is GNU-only and degrades silently on macOS.
TODO.md twelve-versus-thirteen correction, also on #39.
The gomodguard deprecation is #26, not #38 — #38 was the third independent rediscovery of it and is closed as a duplicate. #26 is assigned to sneak, since the fix belongs in the canonical config upstream.
With this and #24 landed, two of the three ways this repo's gates could lie are closed. #36 (linter cache attributing findings to deleted worktrees) is the remaining one, and it produces noise rather than false confidence.
Manager note — merged as `a102b8f`. `main` green, `0 issues.`, 88.3% coverage, `origin` carries only `main`.
The decisive piece of this review was the counterfactual. Rather than only checking that the gates now run, the reviewer built with `--build-arg CHECK_EPOCH=CONSTANT` and watched the second build produce **17 `CACHED` steps with all three gate `RUN`s cached, zero gate output, exit 0** — reproducing precisely the false green #32 was filed for. That establishes the fix is load-bearing rather than incidental, which counting successful runs alone could never show.
It also verified the two things most likely to have been quietly broken: the lint stage still gates the build stage (planted an `unused` finding; build failed at the lint gate in 24s and `grep -c 'gate check, epoch'` over the full log returned 0, so the build-stage `make check` never started), and the non-root drop is still load-bearing (`TestScanHardlinkRunFailsTogether` passes as `builder` and fails as `--user 0:0` with `skipped:0`, because root reads through `chmod(0)`). That second one is the repo quirk we were told to preserve, and it is now demonstrated rather than assumed.
Worth recording how the review handled the environment. A host-wide `docker builder prune -af` on another repo destroyed roughly 41 GB of shared BuildKit cache mid-review. I warned the reviewer that its steady-state cached-step measurement had become meaningless and that a cold-cache test timeout would look like a bug. It re-established its own baseline — build once, count on the next run — and reported 13 cached steps rather than repeating the author's 12, correctly identifying the extra as the lint stage's `WORKDIR /src`. Measurements taken pre- and post-prune are labelled as such in its comment. Without that, this would very likely have been a false FAIL on a PR that is correct.
Non-blocking findings, disposed:
- `CHECK_EPOCH` has no default and nothing asserts it is non-empty, so a bare `docker build .` silently restores the cached-gate behaviour — 17 `CACHED`, exit 0. Filed as #39. Not a regression, but one habit away from the failure mode this PR exists to prevent.
- The second-resolution epoch stands, with a better justification than the PR gave: safety comes from a collision requiring the same second *and* a byte-identical tree, not from builds taking 50-80 seconds. Recorded on #39, along with the note that `date +%s%N` is GNU-only and degrades silently on macOS.
- `TODO.md` twelve-versus-thirteen correction, also on #39.
- The `gomodguard` deprecation is #26, not #38 — #38 was the third independent rediscovery of it and is closed as a duplicate. #26 is assigned to `sneak`, since the fix belongs in the canonical config upstream.
With this and #24 landed, two of the three ways this repo's gates could lie are closed. #36 (linter cache attributing findings to deleted worktrees) is the remaining one, and it produces noise rather than false confidence.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Closes #32.
script/cibuildandscript/dockerwere baredocker buildinvocations with no cache control, and the
Dockerfilecopies the treebefore running its gates. On an unchanged tree Docker served those
layers from cache: the gates never executed and the build still exited
0.
What changed
Dockerfile—ARG CHECK_EPOCHdeclared in both stages,because
ARGis scoped per stage and this file has three gates acrosstwo of them. One declaration would have left the other stage silently
cacheable, which is the easiest way to ship a fix that does not fix
anything.
COPY . ., aboveRUN make fmt-checkandRUN make lintUSER builder, aboveRUN make checkEach gate
RUNreferences the value (echo "gate ..., epoch ${CHECK_EPOCH}" && make ...). BuildKit hashes the expanded command,not the declaration, so a declared-but-unreferenced
ARGinvalidatesnothing. The echo doubles as evidence in the build log that the layer
really executed, which is what let me verify the rest of this rather
than assert it.
Both declarations sit below the dependency layers on purpose. The
goal is to bust the gates, not to go cold — a build that recompiles the
world every invocation would be a different bug.
The build-stage
ARGis placed afterUSER builder, so the drop tothe unprivileged user still happens before
make check.script/cibuild,script/docker— both pass--build-arg CHECK_EPOCH="$(date +%s)". POSIXsh,set -eu, nobashisms; both verified with
dash -n.script/cibuild's headercomment asserted the guarantee it did not provide, so it now says why
the implication actually holds.
script/dockermatters as much: perthe issue's follow-up comment,
make dockeris the gate a reviewerruns by hand and it is the one that actually fooled someone on PR #31.
Verification
All runs on an unchanged tree with
BUILDKIT_PROGRESS=plain, cachepre-warmed.
1.
script/cibuildtwice in a row — gates ran both timesgate fmt-checkgate lintgate check17862587441786258823Distinct epochs, three gate layers executed each time. No sub-second
run anywhere.
2. Dependency layers still cached
Twelve steps served
CACHEDin the steady state (run 2 above, and bothscript/dockerruns below):The pinned base images resolve from the local store on top of that.
Only the three gates and the
RUN make builddownstream of them gocold, which is the intended blast radius.
One honest wrinkle: the first run after editing the
Dockerfileshowed9
CACHEDrather than 12, because theDockerfileis itself part ofthe build context and BuildKit re-materialised three lint-stage layers.
Every subsequent run on a stable tree settled at 12. That is cache
churn from editing the file, not from
CHECK_EPOCH.3.
script/dockertwice in a rowgate fmt-checkgate lintgate checkCACHED4. The lint stage still gates the build stage
Not asserted — tested. I planted a deliberate
unusedfinding in thetree and ran
script/cibuild:Exit 1 after 36.1s, and
grep -c "gate check, epoch"over the wholebuild log returned 0 — the build-stage
make checknever started.The
COPY --from=lintatDockerfile:29still forces the lint stageto complete first. Worth noting explicitly: that
COPYstep itselfshows as
CACHED, because the copied binary is byte-identical everytime. The lint stage still runs — BuildKit has to build it to resolve
the copy. Probe file removed; it is not in the commit.
5. Non-root build stage preserved, and it is load-bearing
docker run --rm <builder stage> idreports:To confirm the
chmod(0)tests are genuinely exercised rather thanmerely present, I forced the same image to run them as root:
Root reads straight through the
chmod(0), so the test fails. Itpasses in the real build, which is only possible as
builder.6.
make checkGreen in 17.3s:
ok sneak.berlin/go/sfdupes 1.799s coverage: 88.3% of statements,0 issues. Run with a freshGOLANGCI_LINT_CACHEper#36, so no findings are attributable to stale worktree paths. Also
green inside the container on every build above (88.5% there).
Out of scope
Dockerfile/script/cibuildtemplates. I checked upstream first per the issue's closing note:
prompts#26 is still open and the canonicalDockerfilethere isunchanged, so there was nothing to re-vendor. This is the bespoke
local fix, shaped to match what #26 recommends.
addressed here, since it is outside this issue's scope:
The linter 'gomodguard' is deprecated (since v2.12.0) ... Replaced by gomodguard_v2. I will file it as its own issue.Notes for the reviewer, since this PR is specifically about not
trusting a build that says it passed.
Reproduce the verification yourself
Do not take the tables in the description on faith — the whole point of
the issue is that a green can be counterfeit. Recipe:
Three
gate ..., epoch Nlines per run means all three gates executed;the epoch differing between the two logs means it was not a replay of
the same cached command. A
CACHEDcount near zero would mean I tradedone bug for a cold build, which the issue explicitly calls a failure.
The counterfactual, if you want it
To see the old behaviour and confirm the fix is actually doing
something, run the same two-run loop against
main. The second runshould return in well under a second with every layer
CACHEDand nogate output at all. That is the 17-layer cache hit PR #31's reviewer
hit.
Where I would attack this if I were reviewing
Four things I deliberately went looking for, so you can check my work
rather than repeat it:
ARGis per-stage. A single declarationleaves the other stage cacheable and the fix looks fine in a diff.
There are two declarations here,
Dockerfilelint stage and buildstage. Confirm both, and confirm both gates in the lint stage sit
below the declaration.
ARGdeclared but not referenced. BuildKit hashes the expandedcommand, so
ARG CHECK_EPOCHwith a plainRUN make checkunder itbusts nothing and still reads as a fix. Every gate
RUNhereinterpolates
${CHECK_EPOCH}.CACHEDcount, not just the gateoutput. If
go mod downloadandapk addare re-running, theARGs are too high in the file.builderat
make checktime. IfARGhad gone aboveUSER, nothing wouldvisibly break — but
TestScanHardlinkRunFailsTogetherand themain_test.gounreadable-file path would stop testing anything,because root reads through
chmod(0). I verified that as root thesuite genuinely fails, so a passing containerised run is itself
proof the drop still happens.
One thing I could not fully close
date +%shas one-second resolution. Two builds starting within thesame wall-clock second would share an epoch and the second could serve
the gates from cache. Not reachable in practice here — the build takes
50 to 80 seconds, so back-to-back invocations cannot collide — and I
kept
$(date +%s)because the issue's definition of done names thatexact form. If you would rather have the guarantee not depend on build
duration,
date +%s%Nor$(date +%s)-$$would close it, and I willmake that change on request rather than argue about it.
Also filed
#38, for the
gomodguarddeprecation warning that appears on everylint run. Noticed here, deliberately not fixed here.
Review: PASS
Independent review of head
964fc29against basemain(b8ebe5f).Every claim in the description was re-executed from a throwaway
worktree; nothing in any checkout was modified and nothing was
committed.
Environment note. A host-wide BuildKit cache prune happened
mid-review, unrelated to this PR. Every cached-step count below comes
from a steady state I re-established myself after the prune (build
once, then count
CACHEDon the next run on an unchanged tree), notfrom the author's numbers. Wall-clock times are therefore not
comparable to the description's and are reported only to show that no
run was a cache hit.
1. Do the gates execute every time — yes
script/cibuild, unchanged tree,BUILDKIT_PROGRESS=plain:CACHED17862602901786260463script/docker, same conditions:CACHED17862612951786261353All three gate steps executed with real wall time in all four runs, no
gate step reported
CACHEDin any of them, and every run carried adistinct epoch. No sub-second run anywhere.
2. Is the invalidation real — yes, confirmed by counterfactual
Held the value constant and built twice with
--build-arg CHECK_EPOCH=CONSTANT:CACHED, all three gateRUNs among them, zerogate ..., epochlines in the log, exit 0 in seconds.That is precisely the 17-layer cache hit recorded on #32. So the
invalidation comes from the interpolated value changing, not from
anything cosmetic, and the
ARGis genuinely referenced rather thanmerely declared. Nothing above the gates is invalidated — see 3.
3. Dependency layers still cached — yes; count is 13 here, not 12
Steady-state
CACHEDsteps, identical in run B and bothscript/dockerruns:
Both
go mod downloads,apk add,adduserand the source copies staycached; the pinned base images resolve from the local store. Only the
three gates and the
RUN make buildbelow them go cold. The blastradius is what the issue asks for; this is not a cold build.
The description and
TODO.md:59say twelve — the thirteenth is thelint stage's
WORKDIR /src. Cosmetic, and possibly BuildKit-versiondependent, but it is wrong in committed content. Non-blocking.
4. Lint stage still gates the build stage — reproduced
Planted a deliberate
unusedfunction in a copy of the tree and ranscript/cibuild: exit 1 after 24.0s, failing atRUN echo "gate lint, epoch ${CHECK_EPOCH}" && make lintwithfunc lintProbeScratch is unused (unused), andgrep -c 'gate check, epoch'over the entire build log returned 0 —the build-stage
make checknever started.The author's reasoning about
COPY --from=lintis correct, and Iverified both halves in a single build: that
COPYreportedCACHEDwhile both lint gates executed in the same run. The copy's key is
content-addressed on the copied binary, which is byte-identical every
time, so the copy result is reused; BuildKit still has to bring the
lint stage to its final state to resolve it, which is why a lint
failure aborts before compilation. Fail-fast is intact.
5. Non-root quirk — intact and load-bearing
--target builderimage reportsuid=1000(builder) gid=1000(builder) groups=1000(builder);ARGafterUSERdoes not reset the user, somake checkstill runs unprivileged.In that same image:
builder:ok sneak.berlin/go/sfdupesforTestScanHardlinkRunFailsTogether;--user 0:0:FAIL ... stats = {added:2 ... skipped:0}, want both hardlink paths skipped.Root reads straight through the
chmod(0), as claimed. The permissiontests are genuinely exercised, and only because the drop is still there.
6.
make checkGreen locally on the head commit with a fresh
GOLANGCI_LINT_CACHEper#36: 13.4s,
coverage: 88.5% of statements,0 issues, andgit statusclean afterwards (the gate modifies nothing). Localgolangci-lintis 2.12.2, matching the pin.7. Scope and hygiene
Dockerfile,TODO.md,script/cibuild,script/docker.No scope creep.
dash -nclean on both scripts;set -eupreserved; no bashisms.git show --check: no whitespace errors. No over-long added lines.(closes #32); body is wrapped andaccurate; no attribution trailers of any kind anywhere in the diff or
the message.
964fc29(check / check (push), 1m50s).main, no conflicts.TODO.mdentry is in the right place and accurate apart from thecount in 3.
Non-blocking findings
Dockerfile:24,Dockerfile:64— an unsetCHECK_EPOCHdegradessilently. The
ARGhas no default and nothing asserts it isnon-empty. Measured on this branch: a bare
docker build .run twicegives 17
CACHED, all three gatesCACHED, no gate output, exit 0 —the exact defect this PR removes, quietly restored, with the log
showing
gate check, epochand an empty value. Not a regression(
docker build .was equally broken before) and both sanctionedentrypoints pass the arg, so the DoD is met. But
REPO_POLICIES.mditself describes CI as "runs
docker build .on push", so the rawcommand is a plausible thing for a human to type. Cheap hardening for
a follow-up: have the first gate assert the value, e.g.
RUN test -n "${CHECK_EPOCH}" || { echo "CHECK_EPOCH unset; use script/docker or script/cibuild" >&2; exit 1; },so the failure is loud instead of a cached green.
Second-resolution epoch — agreed non-blocking, but for a different
reason than the one given. "Builds take 50 to 80 seconds" is an
argument about one process, and it does not cover two concurrent
Gitea Actions jobs sharing a runner — pushing a branch and its merge
commit together is exactly the byte-identical-tree case #32
documents, and those two jobs can plausibly reach
date +%sin thesame second. What makes it acceptable is narrower: a collision needs
the same second and an identical tree, and in that case the reused
result was genuinely computed for that exact tree, so it cannot
certify a tree nobody checked. The residual cost is that one of the
two runs proves nothing on its own. If it is ever tightened, prefer
"$(date +%s)-$$"overdate +%s%N:%Nis a GNU extension, and onmacOS/BSD
dateit emits a literal trailingN, i.e. it wouldsilently stay at one-second resolution on developer machines — a
fix that looks like a fix.
TODO.md:59says twelveCACHEDsteps; measured thirteen here.Noted, not a finding: the
gomodguarddeprecation warning appears onevery lint run, is correctly out of scope here, and is already filed
as #38.
No blocking defects. The gate does what it claims, fails when it should,
and keeps the dependency cache.
Manager note — merged as
a102b8f.maingreen,0 issues., 88.3% coverage,origincarries onlymain.The decisive piece of this review was the counterfactual. Rather than only checking that the gates now run, the reviewer built with
--build-arg CHECK_EPOCH=CONSTANTand watched the second build produce 17CACHEDsteps with all three gateRUNs cached, zero gate output, exit 0 — reproducing precisely the false green #32 was filed for. That establishes the fix is load-bearing rather than incidental, which counting successful runs alone could never show.It also verified the two things most likely to have been quietly broken: the lint stage still gates the build stage (planted an
unusedfinding; build failed at the lint gate in 24s andgrep -c 'gate check, epoch'over the full log returned 0, so the build-stagemake checknever started), and the non-root drop is still load-bearing (TestScanHardlinkRunFailsTogetherpasses asbuilderand fails as--user 0:0withskipped:0, because root reads throughchmod(0)). That second one is the repo quirk we were told to preserve, and it is now demonstrated rather than assumed.Worth recording how the review handled the environment. A host-wide
docker builder prune -afon another repo destroyed roughly 41 GB of shared BuildKit cache mid-review. I warned the reviewer that its steady-state cached-step measurement had become meaningless and that a cold-cache test timeout would look like a bug. It re-established its own baseline — build once, count on the next run — and reported 13 cached steps rather than repeating the author's 12, correctly identifying the extra as the lint stage'sWORKDIR /src. Measurements taken pre- and post-prune are labelled as such in its comment. Without that, this would very likely have been a false FAIL on a PR that is correct.Non-blocking findings, disposed:
CHECK_EPOCHhas no default and nothing asserts it is non-empty, so a baredocker build .silently restores the cached-gate behaviour — 17CACHED, exit 0. Filed as #39. Not a regression, but one habit away from the failure mode this PR exists to prevent.date +%s%Nis GNU-only and degrades silently on macOS.TODO.mdtwelve-versus-thirteen correction, also on #39.gomodguarddeprecation is #26, not #38 — #38 was the third independent rediscovery of it and is closed as a duplicate. #26 is assigned tosneak, since the fix belongs in the canonical config upstream.With this and #24 landed, two of the three ways this repo's gates could lie are closed. #36 (linter cache attributing findings to deleted worktrees) is the remaining one, and it produces noise rather than false confidence.