CI can report a green it did not earn: script/cibuild serves make check from the Docker layer cache #37

Open
opened 2026-08-09 07:39:59 +02:00 by clawbot · 0 comments
Collaborator

Problem

script/cibuild is a plain docker build . with no cache control. The Dockerfile does COPY . . then RUN make check. On a tree Docker has seen before, the check layer is served from cache: the suite never executes and the build still exits 0.

Reproduced first-hand in this repo. After one warm build, a repeat docker build . on an unchanged tree:

9 CACHED layers
ELAPSED_MS=514
#13 [build 7/7] RUN make check      <- CACHED, no vite output, no prettier output

514 milliseconds, exit 0, and neither vite build nor prettier --check ran. Compare a genuine run: ~74 seconds with real output.

script/cibuild's own comment states the assumption this breaks:

# script/cibuild: run the CI build. The Dockerfile runs make check, so
# a successful build implies all checks pass.

That implication does not hold. A successful build implies either all checks passed or Docker decided nothing changed.

Why this is a 1.0 blocker

REPO_POLICIES.md: "main must always pass make check, no exceptions." That guarantee is only as good as the mechanism enforcing it, and the mechanism can currently return a green without running anything.

This is not theoretical for netwatch. It compounds with #14: until PR #31 lands, backend/.golangci.yml was schema-invalid, so golangci-lint silently fell back to default thresholds. Stack the two and the repo's historical "0 issues, CI green" was two independent layers of nothing — a linter running at defaults, behind a build that may not have run the linter at all.

It also compounds with #16: root make check does not cover the backend. So there have been three separate ways for this repo to report a green it had not earned.

Scope note — this is an org-wide defect

The same hole exists in the shared script/cibuild template and was independently observed on at least one other repo in the org (a SUCCESS in 0.262s with every layer cached, versus 64.3s forced uncached). An upstream fix is tracked in the prompts repo.

That does not mean waiting. script/cibuild in this repo is already divergent from the model in other ways (see #28), and a green CI badge that means nothing is worth closing here regardless of upstream timing. Prefer whatever fix upstream settles on if it lands first; otherwise fix it here and note that the change is a candidate for upstreaming.

Definition of done

  • A repeat CI build on an unchanged tree executes the checks rather than serving them from cache. Demonstrate it: run the CI entrypoint twice in a row on an unchanged tree and show the second run producing real check output and a realistic duration, not a sub-second cached exit.
  • The fix does not throw away all build caching. Dependency layers (yarn install, go mod download) should stay cached — those are expensive and their cache keys are already correct, keyed on the manifests. Only the check step must be forced to run. A blanket --no-cache is the lazy fix and makes every CI run pay full dependency-download cost; call it out explicitly if you choose it anyway and say why.
  • Both images are covered — the frontend Dockerfile and Dockerfile.backend. The backend has the identical COPY . . + RUN make check shape.
  • script/cibuild's comment is corrected so it no longer asserts an implication that is only conditionally true.
  • Docker builds still complete in under 5 minutes.
  • Root make check and cd backend && make check still pass.
  • TODO.md updated in the same commit.
  • Commit title ends with (closes #N).

Implementation requirements

  • Coordinate with #16, which rewrites script/cibuild to build both images and route CI through the script layer. These two overlap directly. Land #16 first and build this on top, or fold this into #16 — say which in the PR. Do not implement them in parallel.
  • Whatever mechanism is chosen (a cache-busting build arg, --no-cache-filter on the check stage, or similar) must be deterministic and explainable, not a timestamp hack that also busts the dependency layers as a side effect.
  • Verify the claim rather than asserting it. This issue exists because a green was trusted that had not been earned; the fix must not be accepted on the same basis.
  • make targets and script/ entrypoints only.
  • No attribution trailers in the commit message.
## Problem `script/cibuild` is a plain `docker build .` with no cache control. The `Dockerfile` does `COPY . .` then `RUN make check`. On a tree Docker has seen before, the check layer is served from cache: **the suite never executes and the build still exits 0.** Reproduced first-hand in this repo. After one warm build, a repeat `docker build .` on an unchanged tree: ``` 9 CACHED layers ELAPSED_MS=514 #13 [build 7/7] RUN make check <- CACHED, no vite output, no prettier output ``` 514 milliseconds, exit 0, and neither `vite build` nor `prettier --check` ran. Compare a genuine run: ~74 seconds with real output. `script/cibuild`'s own comment states the assumption this breaks: ```sh # script/cibuild: run the CI build. The Dockerfile runs make check, so # a successful build implies all checks pass. ``` That implication does not hold. A successful build implies *either* all checks passed *or* Docker decided nothing changed. ## Why this is a 1.0 blocker `REPO_POLICIES.md`: "`main` must always pass `make check`, no exceptions." That guarantee is only as good as the mechanism enforcing it, and the mechanism can currently return a green without running anything. This is not theoretical for netwatch. It compounds with #14: until PR #31 lands, `backend/.golangci.yml` was schema-invalid, so golangci-lint silently fell back to default thresholds. Stack the two and the repo's historical "0 issues, CI green" was two independent layers of nothing — a linter running at defaults, behind a build that may not have run the linter at all. It also compounds with #16: root `make check` does not cover the backend. So there have been three separate ways for this repo to report a green it had not earned. ## Scope note — this is an org-wide defect The same hole exists in the shared `script/cibuild` template and was independently observed on at least one other repo in the org (a SUCCESS in 0.262s with every layer cached, versus 64.3s forced uncached). An upstream fix is tracked in the `prompts` repo. That does **not** mean waiting. `script/cibuild` in this repo is already divergent from the model in other ways (see #28), and a green CI badge that means nothing is worth closing here regardless of upstream timing. Prefer whatever fix upstream settles on if it lands first; otherwise fix it here and note that the change is a candidate for upstreaming. ## Definition of done - [ ] A repeat CI build on an unchanged tree **executes the checks** rather than serving them from cache. Demonstrate it: run the CI entrypoint twice in a row on an unchanged tree and show the second run producing real check output and a realistic duration, not a sub-second cached exit. - [ ] The fix does not throw away *all* build caching. Dependency layers (`yarn install`, `go mod download`) should stay cached — those are expensive and their cache keys are already correct, keyed on the manifests. Only the check step must be forced to run. A blanket `--no-cache` is the lazy fix and makes every CI run pay full dependency-download cost; call it out explicitly if you choose it anyway and say why. - [ ] Both images are covered — the frontend `Dockerfile` and `Dockerfile.backend`. The backend has the identical `COPY . .` + `RUN make check` shape. - [ ] `script/cibuild`'s comment is corrected so it no longer asserts an implication that is only conditionally true. - [ ] Docker builds still complete in under 5 minutes. - [ ] Root `make check` and `cd backend && make check` still pass. - [ ] `TODO.md` updated in the same commit. - [ ] Commit title ends with ` (closes #N)`. ## Implementation requirements - Coordinate with **#16**, which rewrites `script/cibuild` to build both images and route CI through the script layer. These two overlap directly. **Land #16 first** and build this on top, or fold this into #16 — say which in the PR. Do not implement them in parallel. - Whatever mechanism is chosen (a cache-busting build arg, `--no-cache-filter` on the check stage, or similar) must be deterministic and explainable, not a timestamp hack that also busts the dependency layers as a side effect. - Verify the claim rather than asserting it. This issue exists because a green was trusted that had not been earned; the fix must not be accepted on the same basis. - `make` targets and `script/` entrypoints only. - No attribution trailers in the commit message.
clawbot added this to the 1.0.0 milestone 2026-08-09 07:39:59 +02:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/netwatch#37