CI can report a green it did not earn: script/cibuild serves make check from the Docker layer cache
#37
Reference in New Issue
Block a user
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?
Problem
script/cibuildis a plaindocker build .with no cache control. TheDockerfiledoesCOPY . .thenRUN 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:514 milliseconds, exit 0, and neither
vite buildnorprettier --checkran. Compare a genuine run: ~74 seconds with real output.script/cibuild's own comment states the assumption this breaks: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: "mainmust always passmake 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.ymlwas 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 checkdoes 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/cibuildtemplate 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 thepromptsrepo.That does not mean waiting.
script/cibuildin 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
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-cacheis 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.DockerfileandDockerfile.backend. The backend has the identicalCOPY . .+RUN make checkshape.script/cibuild's comment is corrected so it no longer asserts an implication that is only conditionally true.make checkandcd backend && make checkstill pass.TODO.mdupdated in the same commit.(closes #N).Implementation requirements
script/cibuildto 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.--no-cache-filteron the check stage, or similar) must be deterministic and explainable, not a timestamp hack that also busts the dependency layers as a side effect.maketargets andscript/entrypoints only.