ci: make a failed Clang download fail the step that downloads it, at all ten sites (fixes #681, refs #674)

[Yaraslaut]

One commit, one ticket. #674 is handed back rather than implemented — the reasoning, the measurements and a working design are in a comment on the issue, summarised at the bottom.

#681 — a failed Clang install reported success, at ten sites

Ten steps across ci.yml and mutation.yml installed the pinned Clang with

wget -qO- https://apt.llvm.org/llvm.sh | sudo bash -s -- ${{ env.CLANG_VERSION }}

wget -q writes no error document, so on an HTTP 4xx/5xx it exits 8 having written zero bytes; bash reads an empty script, does nothing, exits 0. GitHub runs a bare run: as bash -e {0} — errexit, not pipefail — so the pipeline's status is bash's. The step reported success having installed no compiler, and the leg went red several steps later at Configure, naming a missing clang++ rather than the download that never happened.

All ten now run the same three lines #683 gave the Install sccache steps, so the repository has one download idiom rather than two:

curl -sSL --fail -o /tmp/llvm.sh https://apt.llvm.org/llvm.sh
test -s /tmp/llvm.sh
sudo bash /tmp/llvm.sh ${{ env.CLANG_VERSION }}

--fail makes curl report the HTTP status as its own exit code; a file means nothing downstream runs when the download did not arrive; test -s covers the one case --fail cannot see, a 200 with an empty body.

Sites: linux-compilers, linux-sanitizers, linux-coverage, kanban-tsan, bank-sanitizers, ladder-sanitizers, linux-all-features (the else branch of its gcc/clang split), clang-format, clang-tidy, and mutation.yml's install step. The full argument lives once on linux-compilers; the other nine carry a three-line pointer to it. The ticket's triage comment says nine sites and the issue body says ten — ten is right, and all ten are done.

Proof that a 404 now fails the step

Not "verified against a working URL", which is the defect the ticket is. Each shape was written to a file and run as bash -e <file> — what GitHub actually does — against a local origin serving /llvm.sh and 404ing everything else:

=========== OLD SHAPE, 404  (the defect) ===========
step exit=0

=========== OLD SHAPE, 200  (the happy path) ===========
llvm.sh ran, version argument = 22
step exit=0

=========== NEW SHAPE, 404  (must be non-zero) ===========
curl: (22) The requested URL returned error: 404
step exit=22

=========== NEW SHAPE, 200 EMPTY BODY (must be non-zero) ===========
step exit=1

=========== NEW SHAPE, 200  (must be zero and must run the script) ===========
llvm.sh ran, version argument = 22
step exit=0

The old shape's 404 and its 200 are indistinguishable. And against apt.llvm.org itself rather than a stand-in:

$ curl -sSL -o llvm.sh https://apt.llvm.org/does-not-exist.sh
exit=0   564 bytes of error page
$ curl -sSL --fail -o llvm.sh https://apt.llvm.org/does-not-exist.sh
curl: (22) The requested URL returned error: 404
exit=22

Review reasoning, inline (no /code-review, no /simplify)

• Is the check vacuous? No — mutating the feature (dropping --fail) restores exit 0 on the 404, which is the OLD-SHAPE row above. The two rows are the same command differing only in the flag.
• Does test -s earn its line? Yes, and it is the row --fail alone fails: a 200 with an empty body exits 1 only because of it.
• sudo bash <file> vs sudo bash -s --. Argument passing is unchanged: sudo bash /tmp/llvm.sh 22 passes 22 as $1 exactly as -s -- 22 did. The 200 rows above assert it (version argument = 22).
• Does curl exist on every runner that runs these steps? All ten are Linux (ubuntu-24.04 or self-hosted), and ci: three robustness gaps — a check that refused the narrow case, a download that blamed tar, and a sanitizer matrix the bank example was never in (fixes #675, fixes #679, refs #672) #683 already put curl --fail on ten Install sccache steps on the same runners, several of them in the same jobs. Reusing the idiom rather than inventing a wget -O one keeps that to a single assumption.
• /tmp/llvm.sh is written unprivileged and then run under sudo. Unchanged from the piped form, which also fed sudo bash bytes fetched unprivileged over the same TLS connection.
• docs/superpowers/plans/2026-08-18-kanban-rung4-completion.md still contains the old line and is deliberately untouched: it is a dated plan document, not a live workflow.

What I did not verify

No CI run has yet pointed a step at a 404 on a runner. Everything above is local: GNU wget 1.25.0, curl 8.22.0, bash 5.3.15, Arch Linux, measured on 7d4ca453. The behaviour of ubuntu-24.04's own curl build is unverified.

The one claim this branch rests on: curl --fail exits non-zero on an HTTP error status. That is curl's documented contract, it is what the apt.llvm.org measurement above shows, and it is the same property ten Install sccache steps already depend on on these runners.

Gates run before pushing

check_workflow_job_banners.py, check_workflow_option_coverage.py, check_catch2_pin.sh, check_ci_clang_pin.sh, check_bidi_controls.py — all exit 0 — and every workflow parses as YAML.

#674 — handed back, with a working design and the numbers

Not on build-time cost: that is acceptable. Cold, no compiler cache, -j4, the shallow clone is 1.7 s, the configure delta 0.5 s, and Catch2's 108 objects build in 98.5 s; 17 Linux legs install catch2 from apt today, so the critical path grows ~1.7 min (3-7% of a 26-74 min run). Warm cost should be near zero via sccache — inferred, not measured.

Three things came out of the measurements that the ticket does not have:

1. The stated mechanism is wrong in detail. The compile database records no Catch2 include path at all — find_package resolves to /usr/include, which CMake omits as an implicit directory. The divergence is real; it is the default system include path, one level below where the ticket puts it. Read literally, the ticket's own "close as invalid" condition is satisfied by this measurement, and it should not close as invalid.
2. CMAKE_DISABLE_FIND_PACKAGE_Catch2 + a fetched Catch2, exactly as the ticket names it, turns the clang-tidy job permanently red. A fetched include dir arrives as -I, so clang-tidy treats Catch2 macro expansions as user code: on one test TU, 1 finding becomes 45 (21 cppcoreguidelines-avoid-do-while, 11 misc-use-anonymous-namespace out of REQUIRE/TEST_CASE). FetchContent_Declare(... SYSTEM) restores exact parity — measured at 1 finding, the same one — and the suite passes: 100% tests passed out of 1564.
3. v3.8.1 already carries the NOLINT(bugprone-chained-comparison) the nine */tests/.clang-tidy files say arrived in 3.15.3. Pinning it makes all nine suppressions inert and their prose (CI pins catch2 3.4.0 -- ubuntu-24.04's package) false.

Which is why it is handed back rather than half-landed: finishing it requires rewriting those nine files and retiring scripts/check_catch2_pin.sh in the same change, and one of the nine is examples/bank/tests/.clang-tidy, held by a concurrent lane. Deleting the gate leaves a false claim unchecked; reducing the gate makes it fail on that file; editing the file crosses a lane boundary. Shipping the CMake half alone is precisely the "two mechanisms claiming the same thing" the ticket warns against.

A side finding recorded on the issue: because the current code already fetches v3.8.1 when no system Catch2 is present, those nine suppressions are already inert for any contributor without the distro package, on 7d4ca453, before anything changes.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VptDWG2fKr2vBnLSJcgzgW

[Yaraslaut]

Runner verification

All ten converted, zero executable occurrences left:

executable (non-comment) 'llvm.sh | sudo bash':   ci.yml 0   mutation.yml 0
curl -sSL --fail -o /tmp/llvm.sh sites added:     ci.yml 9   mutation.yml 1

I briefly thought one had been missed — ci.yml:282 still matches the old pattern — but it is a comment documenting the old shape and citing morph#681. My grep counted it; the conversion is complete.

The before/after proof is the right shape and the mutation makes it evidence:

OLD SHAPE, 404   -> step exit=0        <- the defect: indistinguishable from success
OLD SHAPE, 200   -> step exit=0
NEW SHAPE, 404   -> step exit=22
NEW SHAPE, 200 EMPTY BODY -> exit=1    <- test -s catches a truncated download
NEW SHAPE, 200   -> ran the script, exit=0

Running each shape as bash -e <file> — what GitHub actually does for a bare run: — rather than in an interactive shell is what makes this trustworthy, since the whole defect turns on the absence of pipefail in that default. Dropping --fail restoring exit 0 confirms the check is not vacuous.

On the count: nine versus ten — the tree moved, neither of us was wrong

My triage said nine sites; this PR converted ten and says my number was wrong. It was correct when measured and went stale within the hour:

llvm.sh | sudo bash in ci.yml @ 95aa5686 (when I counted): 8   (+1 in mutation.yml = 9)
                    @ 7d4ca453 (when the lane worked):    9   (+1 = 10)

#683's own new bank-sanitizers job added the tenth, between my triage and this lane starting. That is the same staleness hazard I have been applying to other people's figures all day, on a one-hour timescale — and it is an argument for the lane's instinct to re-derive the count rather than trust the ticket.

#674 — the handback is right, and the blocker is mine to resolve

Handing it back was correct, and for a better reason than cost: the measured build-time delta (~1.7 min on the critical path, ~3–7% of a run) is acceptable, and the lane says so rather than hiding behind it.

The real blocker is a lane boundary I drew: finishing #674 means rewriting nine */tests/.clang-tidy files and retiring scripts/check_catch2_pin.sh, and one of the nine is examples/bank/tests/.clang-tidy, which I assigned to the concurrent BANK lane. Shipping the CMake half alone would leave exactly the "two mechanisms claiming the same thing" #674 warns against. I will sequence that rather than have a lane cross the boundary — noted on the issue.

Three findings there are worth more than the fix, and all are measured on 7d4ca453:

1. The ticket's stated mechanism is wrong in detail — the compile database records no Catch2 include path; find_package resolves to /usr/include, which CMake omits as an implicit directory. The divergence is real but lives in the default system include path. Read literally that satisfies clang-tidy-diff still analyses the runner's Catch2, not one the repository pins (#666's first closing condition) #674's own "close as invalid" condition, and the lane explicitly says it should not close as invalid. That is the right call: the finding survives, its explanation does not.
2. The shape the ticket names would turn clang-tidy permanently red — a fetched Catch2 arrives as -I, so macro expansions become user code: 1 finding → 45 on one TU. FetchContent_Declare(... SYSTEM) restores exact parity, measured back at 1.
3. v3.8.1 already carries the NOLINT the nine files attribute to 3.15.3, so pinning it makes all nine suppressions inert and their prose false — and that is already true today for any contributor without the distro package.

Leaving check_catch2_pin.sh untouched was correct: retiring it is coupled to a fix that did not land.

Not verified: no runner has pointed a step at a 404 — all #681 evidence is local (curl 8.22.0 on Arch), and ubuntu-24.04's curl build is unconfirmed. The warm sccache cost for #674 is inferred from CMakeLists.txt:117, not measured.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VptDWG2fKr2vBnLSJcgzgW

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!