sneak/vaultik
1
0
Fork 0
Code Issues 19 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

script/cibuild reports a green it did not earn when the tree is unchanged #85

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

script/cibuild can exit 0 without running the test suite or the linter.
Not "print less" — exit 0 having executed nothing.

Mechanism

script/cibuild is a bare docker build . with no cache control:

main() {
    cd "$ROOT"
    docker build .
}

The Dockerfile does COPY . . and then RUN make fmt-check /
RUN make lint (lint stage) and COPY . . / RUN make test (builder
stage). When the tree has not changed since the previous build, Docker
serves those RUN layers from cache. The commands never execute. The
build still exits 0.

Reproduction on this repo

Measured on main at 50e20b4, clean tree, back to back.

First run — tree changed since the last build, so the COPY layer
was invalidated and everything genuinely ran:

EXIT=0  ELAPSED_MS=251948
ok lines: 14   "0 issues.": 1   CACHED layers: 0

Second run, immediately after, nothing touched:

EXIT=0  ELAPSED_MS=1151
ok lines: 0    "0 issues.": 1   CACHED layers: 15

1.15 seconds. Fifteen cached layers. Zero ok lines — the test suite
produced no output because it did not run. Identical EXIT=0. A caller
that checks only the exit code cannot distinguish a 252-second real pass
from a 1-second replay.

Why this is worse than a quiet build

A silent build is suspicious. This is a confident green: the exit code
is the one signal most automation trusts, and it is wrong. It also fails
in the most dangerous direction — the longer a tree sits unchanged, the
more likely the "verification" is a replay, which is exactly the state a
branch is in when someone is about to merge it.

This has already bitten twice here: both the review and the re-review of
PR #83 received EXIT=0 from fully-cached builds and correctly discarded
them, re-running uncached instead. It did not cause a bad merge only
because reviewers were explicitly told not to trust a bare exit code.

Context

This is the third distinct way this repo's gate has manufactured an
unearned green:

  1. #78 — script/lint resolved golangci-lint from PATH while CI
    pinned v2.12.2 by digest; two different agents reported false greens.
  2. #80 — the native-lint escape hatch is version-gated rather than
    context-gated (hardening, not yet a live bug).
  3. This.

Reported fleet-wide; the shared template carries the same hole and it is
filed upstream in sneak/prompts as #26, tracked in sneak/dnswatcher as
#115. Fixing it here should match whatever lands upstream rather than
inventing a local variant.

Definition of done

  1. script/cibuild cannot report success without the check stages
    actually executing. The known-good approach: an ARG CHECK_EPOCH
    declared immediately above the RUN make ... check lines, with
    script/cibuild passing --build-arg CHECK_EPOCH="$(date +%s)". This
    busts only the check layers — dependency and module layers stay
    cached, so the build stays fast.
  2. Placement matters: the ARG must sit immediately above the check
    RUNs. Too early and it invalidates dependency layers, making every
    build a cold build; too late and the checks stay cached.
  3. Both check stages are covered — the lint stage (make fmt-check,
    make lint) and the builder stage (make test). Fixing only one
    leaves half the gate fake.
  4. Verify by reproducing the above: run script/cibuild twice back to
    back on an unchanged tree and confirm the second run still executes the
    suite — non-trivial wall time, real ok lines, no CACHED on the
    check layers. Paste both runs' timings into the PR. A fix that is not
    demonstrated against the actual failure mode is not done.
  5. Confirm the dependency layers do still cache, so this does not
    turn every build into a cold build. Report before/after wall times for
    a changed-tree build.
  6. .golangci.yml unchanged (sha256
    021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb);
    the Dockerfile FROM line remains the single source of truth for the
    linter version (#78).

Interim rule

Until this lands, a bare script/cibuild exit code is not evidence.
Verification must either show a non-cached run (real ok lines, plausible
wall time, no CACHED on check layers) or use a host-side
GOFLAGS=-count=1 make check. Every PR claiming green should state which
it was.

`script/cibuild` can exit 0 without running the test suite or the linter. Not "print less" — **exit 0 having executed nothing.** ## Mechanism `script/cibuild` is a bare `docker build .` with no cache control: ```sh main() { cd "$ROOT" docker build . } ``` The `Dockerfile` does `COPY . .` and then `RUN make fmt-check` / `RUN make lint` (lint stage) and `COPY . .` / `RUN make test` (builder stage). When the tree has not changed since the previous build, Docker serves those `RUN` layers from cache. The commands never execute. The build still exits 0. ## Reproduction on this repo Measured on `main` at `50e20b4`, clean tree, back to back. **First run** — tree changed since the last build, so the `COPY` layer was invalidated and everything genuinely ran: ``` EXIT=0 ELAPSED_MS=251948 ok lines: 14 "0 issues.": 1 CACHED layers: 0 ``` **Second run, immediately after, nothing touched:** ``` EXIT=0 ELAPSED_MS=1151 ok lines: 0 "0 issues.": 1 CACHED layers: 15 ``` **1.15 seconds. Fifteen cached layers. Zero `ok` lines** — the test suite produced no output because it did not run. Identical `EXIT=0`. A caller that checks only the exit code cannot distinguish a 252-second real pass from a 1-second replay. ## Why this is worse than a quiet build A silent build is suspicious. This is a *confident green*: the exit code is the one signal most automation trusts, and it is wrong. It also fails in the most dangerous direction — the longer a tree sits unchanged, the more likely the "verification" is a replay, which is exactly the state a branch is in when someone is about to merge it. This has already bitten twice here: both the review and the re-review of PR #83 received `EXIT=0` from fully-cached builds and correctly discarded them, re-running uncached instead. It did not cause a bad merge only because reviewers were explicitly told not to trust a bare exit code. ## Context This is the **third** distinct way this repo's gate has manufactured an unearned green: 1. #78 — `script/lint` resolved `golangci-lint` from `PATH` while CI pinned v2.12.2 by digest; two different agents reported false greens. 2. #80 — the native-lint escape hatch is version-gated rather than context-gated (hardening, not yet a live bug). 3. This. Reported fleet-wide; the shared template carries the same hole and it is filed upstream in `sneak/prompts` as #26, tracked in `sneak/dnswatcher` as #115. Fixing it here should match whatever lands upstream rather than inventing a local variant. ## Definition of done 1. `script/cibuild` cannot report success without the check stages actually executing. The known-good approach: an `ARG CHECK_EPOCH` declared immediately above the `RUN make ...` check lines, with `script/cibuild` passing `--build-arg CHECK_EPOCH="$(date +%s)"`. This busts only the check layers — dependency and module layers stay cached, so the build stays fast. 2. Placement matters: the `ARG` must sit **immediately above** the check `RUN`s. Too early and it invalidates dependency layers, making every build a cold build; too late and the checks stay cached. 3. Both check stages are covered — the lint stage (`make fmt-check`, `make lint`) and the builder stage (`make test`). Fixing only one leaves half the gate fake. 4. **Verify by reproducing the above**: run `script/cibuild` twice back to back on an unchanged tree and confirm the second run still executes the suite — non-trivial wall time, real `ok` lines, no `CACHED` on the check layers. Paste both runs' timings into the PR. A fix that is not demonstrated against the actual failure mode is not done. 5. Confirm the dependency layers **do** still cache, so this does not turn every build into a cold build. Report before/after wall times for a changed-tree build. 6. `.golangci.yml` unchanged (sha256 `021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb`); the `Dockerfile` `FROM` line remains the single source of truth for the linter version (#78). ## Interim rule Until this lands, **a bare `script/cibuild` exit code is not evidence.** Verification must either show a non-cached run (real `ok` lines, plausible wall time, no `CACHED` on check layers) or use a host-side `GOFLAGS=-count=1 make check`. Every PR claiming green should state which it was.
clawbot added this to the 1.0.0 milestone 2026-08-09 07:41:30 +02:00
clawbot commented 2026-08-09 07:45:12 +02:00
Author
Collaborator
Copy Link

Implementation plan

Branch fix-cibuild-cache off main at 3bcdbcf. Matching the
upstream fix in sneak/prompts #26 rather than inventing a local
variant.

1. Dockerfile — lint stage. Insert, immediately above the check
RUNs and below COPY . .:

ARG CHECK_EPOCH
RUN make fmt-check
RUN make lint

2. Dockerfile — builder stage. Same, immediately above
RUN make test. ARG scope is per-stage in Docker, so the builder
stage needs its own declaration; the lint stage's does not carry over.

Placement is below COPY go.mod go.sum ./ / RUN go mod download and
below the apk add layers in both stages, so dependency and toolchain
layers keep caching. Only the check RUNs (and, in the builder stage,
the go build that follows make test) are invalidated per build.

3. script/cibuild. docker build --build-arg CHECK_EPOCH="$(date +%s)" ., POSIX sh, consistent with the other
script/ entrypoints. script/docker is deliberately left alone — it
builds an image, it is not the gate.

4. README.md entrypoints entry for script/cibuild updated to
say the checks are forced to re-execute rather than being served from
the layer cache, so the documented behavior matches.

5. TODO.md updated per the Workflow section, in the same commit.

Verification plan

Directly against the failure mode in the report, not by trusting an
exit code:

  • script/cibuild twice back to back on an unchanged tree, capturing
    $? immediately, wall time, ok line count, and the per-layer
    CACHED status from --progress=plain. The second run must show
    non-trivial wall time, real ok lines with durations, and no
    CACHED on the check layers.
  • The same two-run measurement taken before the change on this
    branch, to confirm the reproduction still holds at 3bcdbcf and that
    the delta is attributable to the fix.
  • Changed-tree wall time before and after the change, plus the
    CACHED status of the go mod download and apk add layers, to
    prove dependency caching survives and this is not a cold build every
    time.
  • Host-side GOFLAGS=-count=1 make check as an independent
    cross-check.
  • .gitea/workflows/check.yml runs script/cibuild and is unchanged;
    confirmed the script still works standalone with no environment
    beyond a Docker daemon.

Out of scope, untouched: .golangci.yml (sha256
021cc83f...46bcb), the lint-stage FROM line and its digest,
script/lint's pinned-image logic.

## Implementation plan Branch `fix-cibuild-cache` off `main` at `3bcdbcf`. Matching the upstream fix in `sneak/prompts` #26 rather than inventing a local variant. **1. `Dockerfile` — lint stage.** Insert, immediately above the check `RUN`s and below `COPY . .`: ``` ARG CHECK_EPOCH RUN make fmt-check RUN make lint ``` **2. `Dockerfile` — builder stage.** Same, immediately above `RUN make test`. `ARG` scope is per-stage in Docker, so the builder stage needs its own declaration; the lint stage's does not carry over. Placement is below `COPY go.mod go.sum ./` / `RUN go mod download` and below the `apk add` layers in both stages, so dependency and toolchain layers keep caching. Only the check `RUN`s (and, in the builder stage, the `go build` that follows `make test`) are invalidated per build. **3. `script/cibuild`.** `docker build --build-arg CHECK_EPOCH="$(date +%s)" .`, POSIX sh, consistent with the other `script/` entrypoints. `script/docker` is deliberately left alone — it builds an image, it is not the gate. **4. `README.md`** entrypoints entry for `script/cibuild` updated to say the checks are forced to re-execute rather than being served from the layer cache, so the documented behavior matches. **5. `TODO.md`** updated per the Workflow section, in the same commit. ## Verification plan Directly against the failure mode in the report, not by trusting an exit code: * `script/cibuild` twice back to back on an unchanged tree, capturing `$?` immediately, wall time, `ok` line count, and the per-layer `CACHED` status from `--progress=plain`. The second run must show non-trivial wall time, real `ok` lines with durations, and no `CACHED` on the check layers. * The same two-run measurement taken *before* the change on this branch, to confirm the reproduction still holds at `3bcdbcf` and that the delta is attributable to the fix. * Changed-tree wall time before and after the change, plus the `CACHED` status of the `go mod download` and `apk add` layers, to prove dependency caching survives and this is not a cold build every time. * Host-side `GOFLAGS=-count=1 make check` as an independent cross-check. * `.gitea/workflows/check.yml` runs `script/cibuild` and is unchanged; confirmed the script still works standalone with no environment beyond a Docker daemon. Out of scope, untouched: `.golangci.yml` (sha256 `021cc83f...46bcb`), the lint-stage `FROM` line and its digest, `script/lint`'s pinned-image logic.
clawbot commented 2026-08-09 09:41:35 +02:00
Author
Collaborator
Copy Link

Closed by PR #89, merged to main as c3bb3b5.

First verification run using the fixed gate, on the merged main:

$ script/cibuild; echo "EXIT=$?"
EXIT=0   ELAPSED_S=190
ok lines: 14   cached check layers: 0   FAIL: 0
#16 63.96 0 issues.

The 190 seconds is now itself the evidence. The identical command on
the identical clean tree returned in 1.15 seconds before this fix,
with zero ok lines and 15 cached layers, and exited 0 both times. The
exit code never distinguished the two states; wall time and ok-line
count did. That is the whole point of the change.

Independent corroboration from the runner side: check / check now takes
2m57s on the fixed head, against the "Successful in 6s" signature
on pre-fix main commits. This was never a local-workstation artifact —
CI itself had been issuing 6-second unearned greens.

What the guarantee actually is

Deliberately stated conditionally, in the README and the Dockerfile,
rather than absolutely: the check layers cannot be replayed when
script/cibuild passes a fresh CHECK_EPOCH. It holds per (build
context, epoch value). After a session spent deleting false claims about
what this gate proves, shipping a new absolute claim that is not absolute
would have been the wrong lesson.

Known residual gap, tracked in #91

A bare docker build . with no --build-arg still replays the check
layers from the second consecutive run onward — reproduced twice at
400ms and 356ms, zero ok lines, all three check layers CACHED, exit 0.
An unset ARG is an empty string, and an empty string is a stable cache
key.

That gap matters more than it sounds: REPO_POLICIES.md names
docker build . verbatim as a command that must be green, so the
documented command is exactly the one that can still lie. #91 is
dispatched and adds RUN [ -n "$CHECK_EPOCH" ] || exit 1 to both check
stages — failed steps are never cached, so it fails on every invocation
rather than once.

Note on the fix's own near-miss

The first attempt inlined --build-arg CHECK_EPOCH="$(date +%s)". Under
set -eu, a failing command substitution in an argument position does
not abort, so a failing date would have yielded an empty — hence
constant — epoch and silently restored the exact false green this issue
exists to eliminate. The guard would have disarmed itself and still
exited 0. Caught in review, fixed by assigning the epoch on its own line,
and verified behaviorally by shadowing date to fail: script/cibuild
exits 1 and no build starts.

Worth recording because it is the same defect one level up. A guard
against unearned greens is exactly the kind of code that can fail green.

Closed by PR #89, merged to `main` as `c3bb3b5`. First verification run using the fixed gate, on the merged `main`: ``` $ script/cibuild; echo "EXIT=$?" EXIT=0 ELAPSED_S=190 ok lines: 14 cached check layers: 0 FAIL: 0 #16 63.96 0 issues. ``` **The 190 seconds is now itself the evidence.** The identical command on the identical clean tree returned in **1.15 seconds** before this fix, with zero `ok` lines and 15 cached layers, and exited 0 both times. The exit code never distinguished the two states; wall time and `ok`-line count did. That is the whole point of the change. Independent corroboration from the runner side: `check / check` now takes **2m57s** on the fixed head, against the "Successful in **6s**" signature on pre-fix `main` commits. This was never a local-workstation artifact — CI itself had been issuing 6-second unearned greens. ## What the guarantee actually is Deliberately stated conditionally, in the README and the `Dockerfile`, rather than absolutely: the check layers cannot be replayed **when `script/cibuild` passes a fresh `CHECK_EPOCH`**. It holds per (build context, epoch value). After a session spent deleting false claims about what this gate proves, shipping a new absolute claim that is not absolute would have been the wrong lesson. ## Known residual gap, tracked in #91 A bare `docker build .` with no `--build-arg` still replays the check layers from the *second* consecutive run onward — reproduced twice at 400ms and 356ms, zero `ok` lines, all three check layers `CACHED`, exit 0. An unset `ARG` is an empty string, and an empty string is a stable cache key. That gap matters more than it sounds: `REPO_POLICIES.md` names `docker build .` **verbatim** as a command that must be green, so the documented command is exactly the one that can still lie. #91 is dispatched and adds `RUN [ -n "$CHECK_EPOCH" ] || exit 1` to both check stages — failed steps are never cached, so it fails on every invocation rather than once. ## Note on the fix's own near-miss The first attempt inlined `--build-arg CHECK_EPOCH="$(date +%s)"`. Under `set -eu`, a failing command substitution in an argument position does **not** abort, so a failing `date` would have yielded an empty — hence constant — epoch and silently restored the exact false green this issue exists to eliminate. The guard would have disarmed itself and still exited 0. Caught in review, fixed by assigning the epoch on its own line, and verified behaviorally by shadowing `date` to fail: `script/cibuild` exits 1 and no build starts. Worth recording because it is the same defect one level up. A guard against unearned greens is exactly the kind of code that can fail green.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
merge-ready
All checks pass, reviewed, rebased, ready for merge
 needs-checks
make check does not pass cleanly
 needs-rebase
PR has merge conflicts or is behind main
 needs-review
Code review not yet done
 needs-rework
Code review found issues to fix
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/vaultik#85
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?