sneak/sfdupes
1
0
Fork 0
Code Issues 23 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

script/cibuild can report a green it did not earn (Docker layer cache) #32

New Issue
Closed
opened 2026-08-09 07:37:46 +02:00 by clawbot · 2 comments
clawbot commented 2026-08-09 07:37:46 +02:00
Collaborator
Copy Link

script/cibuild is a bare docker build . with no cache control, and the Dockerfile does COPY . . followed by RUN make check. When the tree is unchanged, Docker serves the check layer from cache: the suite never runs, and the build still exits 0. A green that nobody earned.

Reported fleet-wide by the manager coordinating the other repos, and observed concretely on dnswatcher: SUCCESS in 0.262 seconds with every layer CACHED; the same build forced uncached took 64.3 seconds and was a real pass.

This is the same class of defect as #24 — a shared template manufacturing greens. #24 is the local gate lying because script/bootstrap never checks the linter's version; this is the CI gate lying because the check layer is cached.

How exposed is this repo, concretely

Narrower than it first looks, but real. COPY . . is invalidated by any change to the tree, so a branch with actual changes does get a genuine run. The hole opens when the tree is unchanged between builds — and that is exactly what a merge commit is here. Every merge this repo has done is a non-fast-forward merge of a branch with no divergence, so the merge commit's tree is byte-identical to the branch head's:

main     60edf5e49ea5a064e32138648932e28e88d62732
2a055c0  60edf5e49ea5a064e32138648932e28e88d62732   merge of PR #29
73841c9  60edf5e49ea5a064e32138648932e28e88d62732   PR #29 branch head
ce6d29d  de288560295d9de0e4c9b461417d22c2a05927ca   merge of PR #28
a729575  de288560295d9de0e4c9b461417d22c2a05927ca   PR #28 branch head
38a01bd  0ed58e230d21602b06b8fad6fa1b8a99a9cd3146   merge of PR #2
814bdad  0ed58e230d21602b06b8fad6fa1b8a99a9cd3146   PR #2 branch head

So the CI run on each merge commit was almost certainly a full cache hit. The branch-head runs are the ones that carried real signal, and for the merges already landed there is independent evidence they executed for real (recorded on the PRs): PR #2's reviewer executed the pinned lint image by digest and ran a controlled three-way config experiment inside it; PR #29's make docker failed with six goconst findings and then passed after the fix, which a cached layer cannot do. main's current tree is the one PR #29 validated.

The danger is forward-looking: a re-run of CI on an unchanged commit, or any future merge, reports green without executing anything.

Definition of done

  1. ARG CHECK_EPOCH is declared immediately above each RUN that constitutes a gate, and referenced so it busts that layer. Note this Dockerfile has three such steps across two stages — RUN make fmt-check and RUN make lint in the lint stage, and RUN make check in the build stage — and ARG is per-stage, so it must be declared in both.
  2. script/cibuild passes --build-arg CHECK_EPOCH="$(date +%s)". Dependency layers (go mod download, base images) must stay cached — the point is to bust only the check layers, not to make CI slow.
  3. script/docker gets the same treatment, so a developer running make docker locally cannot be fooled either. This matters here: make docker is currently the only trustworthy gate in this repo while #24 is open.
  4. Demonstrated, not asserted: run script/cibuild twice in a row on an unchanged tree and show in the PR that the check steps executed both times, with timings. A sub-second second run is a failure of this issue.
  5. Confirm the fix does not defeat the BuildKit stage dependency that forces the lint stage to complete before the build stage proceeds.
  6. make check and make docker green.

Upstream: tracked as prompts #26 and dnswatcher #115, which fold in #24 and #19 as related template defects. If the canonical Dockerfile and script/cibuild templates are fixed upstream first, prefer re-vendoring them over a bespoke local fix — but do not wait on that if it stalls.

`script/cibuild` is a bare `docker build .` with no cache control, and the `Dockerfile` does `COPY . .` followed by `RUN make check`. When the tree is unchanged, Docker serves the check layer from cache: the suite never runs, and the build still exits 0. A green that nobody earned. Reported fleet-wide by the manager coordinating the other repos, and observed concretely on `dnswatcher`: SUCCESS in 0.262 seconds with every layer `CACHED`; the same build forced uncached took 64.3 seconds and was a real pass. This is the same class of defect as #24 — a shared template manufacturing greens. #24 is the local gate lying because `script/bootstrap` never checks the linter's version; this is the CI gate lying because the check layer is cached. ## How exposed is this repo, concretely Narrower than it first looks, but real. `COPY . .` is invalidated by any change to the tree, so a branch with actual changes does get a genuine run. The hole opens when the tree is *unchanged* between builds — and that is exactly what a merge commit is here. Every merge this repo has done is a non-fast-forward merge of a branch with no divergence, so the merge commit's tree is byte-identical to the branch head's: main 60edf5e49ea5a064e32138648932e28e88d62732 2a055c0 60edf5e49ea5a064e32138648932e28e88d62732 merge of PR #29 73841c9 60edf5e49ea5a064e32138648932e28e88d62732 PR #29 branch head ce6d29d de288560295d9de0e4c9b461417d22c2a05927ca merge of PR #28 a729575 de288560295d9de0e4c9b461417d22c2a05927ca PR #28 branch head 38a01bd 0ed58e230d21602b06b8fad6fa1b8a99a9cd3146 merge of PR #2 814bdad 0ed58e230d21602b06b8fad6fa1b8a99a9cd3146 PR #2 branch head So the CI run on each *merge* commit was almost certainly a full cache hit. The branch-head runs are the ones that carried real signal, and for the merges already landed there is independent evidence they executed for real (recorded on the PRs): PR #2's reviewer executed the pinned lint image by digest and ran a controlled three-way config experiment inside it; PR #29's `make docker` **failed** with six `goconst` findings and then passed after the fix, which a cached layer cannot do. `main`'s current tree is the one PR #29 validated. The danger is forward-looking: a re-run of CI on an unchanged commit, or any future merge, reports green without executing anything. ## Definition of done 1. `ARG CHECK_EPOCH` is declared immediately above each `RUN` that constitutes a gate, and referenced so it busts that layer. Note this Dockerfile has **three** such steps across two stages — `RUN make fmt-check` and `RUN make lint` in the lint stage, and `RUN make check` in the build stage — and `ARG` is per-stage, so it must be declared in both. 2. `script/cibuild` passes `--build-arg CHECK_EPOCH="$(date +%s)"`. Dependency layers (`go mod download`, base images) must stay cached — the point is to bust only the check layers, not to make CI slow. 3. `script/docker` gets the same treatment, so a developer running `make docker` locally cannot be fooled either. This matters here: `make docker` is currently the only trustworthy gate in this repo while #24 is open. 4. Demonstrated, not asserted: run `script/cibuild` twice in a row on an unchanged tree and show in the PR that the check steps executed both times, with timings. A sub-second second run is a failure of this issue. 5. Confirm the fix does not defeat the BuildKit stage dependency that forces the lint stage to complete before the build stage proceeds. 6. `make check` and `make docker` green. Upstream: tracked as `prompts` #26 and `dnswatcher` #115, which fold in #24 and #19 as related template defects. If the canonical `Dockerfile` and `script/cibuild` templates are fixed upstream first, prefer re-vendoring them over a bespoke local fix — but do not wait on that if it stalls.
clawbot added this to the 1.0.0 milestone 2026-08-09 07:37:46 +02:00
clawbot commented 2026-08-09 07:47:30 +02:00
Author
Collaborator
Copy Link

Confirmed live in this repo, independently and by accident.

The reviewer of PR #31 went to run the authoritative gate and found that make docker was a 17-layer cache hit that proved nothing — it exited 0 without executing either the lint stage or the builder-stage make check. They forced a cold build instead, which took 48.2 seconds for the lint stage on pinned golangci-lint v2.12.2 and 57.6 seconds for make check as the unprivileged user. Both genuinely passed, so PR #31 was safe to merge — but only because the reviewer noticed and refused to accept the cached result.

That is the whole issue in one observation: the gate returned success in a fraction of a second, and the only thing standing between that and a false green was a reviewer being suspicious. The next reviewer might not be.

It also sharpens the priority. While #24 is open (script/bootstrap never checking the linter version, so local make check runs v2.10.1 against a v2.12.2 pin), make docker is supposed to be the one trustworthy gate in this repo — and this issue means it is only trustworthy when it happens to miss cache. Two independent ways to get a green nobody earned, layered on top of each other.

Raising this above the other tooling issues in the milestone. #24 and #32 together should land before the remaining code work, so that the gates protecting that work are real.

Confirmed live in this repo, independently and by accident. The reviewer of PR #31 went to run the authoritative gate and found that `make docker` was **a 17-layer cache hit that proved nothing** — it exited 0 without executing either the lint stage or the builder-stage `make check`. They forced a cold build instead, which took 48.2 seconds for the lint stage on pinned golangci-lint v2.12.2 and 57.6 seconds for `make check` as the unprivileged user. Both genuinely passed, so PR #31 was safe to merge — but only because the reviewer noticed and refused to accept the cached result. That is the whole issue in one observation: the gate returned success in a fraction of a second, and the only thing standing between that and a false green was a reviewer being suspicious. The next reviewer might not be. It also sharpens the priority. While #24 is open (`script/bootstrap` never checking the linter version, so local `make check` runs v2.10.1 against a v2.12.2 pin), `make docker` is supposed to be the one trustworthy gate in this repo — and this issue means it is only trustworthy when it happens to miss cache. Two independent ways to get a green nobody earned, layered on top of each other. Raising this above the other tooling issues in the milestone. #24 and #32 together should land before the remaining code work, so that the gates protecting that work are real.
clawbot commented 2026-08-09 08:41:37 +02:00
Author
Collaborator
Copy Link

Implementation plan

Branch cibuild-cache-bust off main (b8ebe5f), in a scratch
worktree.

Checked upstream first, per the issue's closing note: prompts #26 is
still open and the canonical Dockerfile in prompts still reads
COPY . . / RUN make check with no ARG. There is nothing to
re-vendor, so this is the bespoke local fix. It matches the shape #26
recommends, so re-vendoring later should be a no-op or close to it.

1. Dockerfile — three gate steps, two stages

ARG is per-stage, so one declaration would leave the other stage
cacheable. Declaring it in both:

  • lint stage — ARG CHECK_EPOCH after COPY . ., immediately
    above RUN make fmt-check and RUN make lint.
  • build stage — ARG CHECK_EPOCH after USER builder,
    immediately above RUN make check.

In both places the value is referenced in the RUN command itself
(the command string is what BuildKit hashes; a declared-but-unreferenced
ARG invalidates nothing). Reference is an echo of the epoch, which
doubles as evidence in BUILDKIT_PROGRESS=plain output that the layer
really executed.

Position matters as much as presence: below COPY go.mod go.sum /
RUN go mod download / RUN apk add / COPY . ., so the dependency
layers above are untouched and stay cached. Only the gate layers and
what follows them (RUN make build) go cold. A build that goes fully
cold every time would be a regression, not a fix.

2. script/cibuild and script/docker

Both get --build-arg CHECK_EPOCH="$(date +%s)". POSIX sh,
set -eu, no bashisms. script/docker matters as much as cibuild
here — make docker is the gate a human runs by hand, and per the
comment above it is the one that actually fooled a reviewer on PR #31.
script/cibuild's header comment currently asserts the guarantee it
did not provide; it gets rewritten to say why the implication now
holds.

3. Two things I will not break

  • The COPY --from=lint /usr/bin/golangci-lint at Dockerfile:29 is
    what forces BuildKit to finish the lint stage before the build stage
    proceeds. Busting the lint stage's trailing layers must not let
    BuildKit satisfy that copy from cache without running the stage. I
    will verify the lint gates appear in the plain progress output of the
    build-stage run, not just assume it.
  • The build stage is deliberately non-root: make check runs as
    builder because the permission-denied tests rely on chmod(0),
    which root ignores. ARG/USER ordering is chosen so the drop to
    builder still happens before make check.

4. Verification to be posted on the PR

  • script/cibuild twice back to back on an unchanged tree, with
    BUILDKIT_PROGRESS=plain, showing all three gates executing on
    both runs, with wall-clock times. A sub-second second run is a
    failure.
  • The cached-step count and identity for the second run, to prove the
    dependency layers were still served from cache.
  • The same two-run check for script/docker.
  • Explicit confirmation the lint stage still gates the build stage.
  • make check green.

TODO.md gets a Completed Steps entry in the same commit; commit
title ends with (closes #32).

Out of scope

Propagating the fix to the canonical templates — that is prompts #26
and its follow-up. Nothing here touches #24, #19, or the test suite.

## Implementation plan Branch `cibuild-cache-bust` off `main` (`b8ebe5f`), in a scratch worktree. Checked upstream first, per the issue's closing note: `prompts` #26 is still open and the canonical `Dockerfile` in `prompts` still reads `COPY . .` / `RUN make check` with no `ARG`. There is nothing to re-vendor, so this is the bespoke local fix. It matches the shape #26 recommends, so re-vendoring later should be a no-op or close to it. ### 1. `Dockerfile` — three gate steps, two stages `ARG` is per-stage, so one declaration would leave the other stage cacheable. Declaring it in both: - **lint stage** — `ARG CHECK_EPOCH` after `COPY . .`, immediately above `RUN make fmt-check` and `RUN make lint`. - **build stage** — `ARG CHECK_EPOCH` after `USER builder`, immediately above `RUN make check`. In both places the value is *referenced in the RUN command itself* (the command string is what BuildKit hashes; a declared-but-unreferenced `ARG` invalidates nothing). Reference is an `echo` of the epoch, which doubles as evidence in `BUILDKIT_PROGRESS=plain` output that the layer really executed. Position matters as much as presence: below `COPY go.mod go.sum` / `RUN go mod download` / `RUN apk add` / `COPY . .`, so the dependency layers above are untouched and stay cached. Only the gate layers and what follows them (`RUN make build`) go cold. A build that goes fully cold every time would be a regression, not a fix. ### 2. `script/cibuild` and `script/docker` Both get `--build-arg CHECK_EPOCH="$(date +%s)"`. POSIX `sh`, `set -eu`, no bashisms. `script/docker` matters as much as `cibuild` here — `make docker` is the gate a human runs by hand, and per the comment above it is the one that actually fooled a reviewer on PR #31. `script/cibuild`'s header comment currently asserts the guarantee it did not provide; it gets rewritten to say why the implication now holds. ### 3. Two things I will not break - The `COPY --from=lint /usr/bin/golangci-lint` at `Dockerfile:29` is what forces BuildKit to finish the lint stage before the build stage proceeds. Busting the lint stage's trailing layers must not let BuildKit satisfy that copy from cache without running the stage. I will verify the lint gates appear in the plain progress output of the build-stage run, not just assume it. - The build stage is deliberately non-root: `make check` runs as `builder` because the permission-denied tests rely on `chmod(0)`, which root ignores. `ARG`/`USER` ordering is chosen so the drop to `builder` still happens before `make check`. ### 4. Verification to be posted on the PR - `script/cibuild` twice back to back on an unchanged tree, with `BUILDKIT_PROGRESS=plain`, showing all three gates executing on **both** runs, with wall-clock times. A sub-second second run is a failure. - The cached-step count and identity for the second run, to prove the dependency layers were still served from cache. - The same two-run check for `script/docker`. - Explicit confirmation the lint stage still gates the build stage. - `make check` green. `TODO.md` gets a Completed Steps entry in the same commit; commit title ends with ` (closes #32)`. ### Out of scope Propagating the fix to the canonical templates — that is `prompts` #26 and its follow-up. Nothing here touches #24, #19, or the test suite.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
merge-ready
Review passed; ready to merge
 needs-checks
CI checks failing or not yet green
 needs-rebase
Branch conflicts with main; rebase required
 needs-review
PR awaiting independent adversarial review
 needs-rework
Review found blocking problems; author must rework
No Label
Milestone
No items
No Milestone 1.0.0
Projects
Clear projects
No project
Assignees
Clear assignees
clawbot sneak
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/sfdupes#32
Delete Branch "%!s()"

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?