CI: the clang-format job can never print its findings (set -e kills it before the report)

[balbasty]

Found by: claude-jitfields-to-fastfields.

The lint (clang-format, changed lines) job in .github/workflows/ci.yml can never show what it objects to. Every failure renders as a bare exit code 1, which makes an advisory job useless as advice.

Cause

The step runs under set -euo pipefail and captures the diff into a variable:

set -euo pipefail
base=$(git merge-base "origin/${BASE_REF}" HEAD)
out=$(git-clang-format-18 --binary clang-format-18 --diff \
 --extensions h,hpp,inl,cpp,cu,cuh "${base}")
case "$out" in
 "no modified files to format"|"clang-format did not modify any files")
 echo "clang-format: changed lines are clean" ;;
 *)
 echo "$out" # <-- unreachable
 echo "::error::clang-format would reformat the lines above."
 echo "Run: git clang-format $base"
 exit 1 ;;
esac

git-clang-format --diff exits 1 whenever it would reformat something — that is its documented contract, not an error. Under set -e, a failing command substitution in an assignment aborts the shell at that line, so the script dies before reaching case. The entire reporting branch — the diff, the ::error:: annotation, and the "Run: git clang-format …" hint — is dead code in exactly the situation it exists for.

The clean path works, which is why this went unnoticed: a passing run reaches case normally.

Fix

Stop treating exit 1 as fatal for that one command, e.g.

out=$(git-clang-format-18 --binary clang-format-18 --diff \
 --extensions h,hpp,inl,cpp,cu,cuh "${base}") || true

or capture the status explicitly and branch on it. Worth also asserting in a test PR that the diff genuinely appears in the log, rather than assuming the fix works.

Why it matters more than it looks

The job is deliberately continue-on-error: true — the comment above it explains that .clang-format postdates the tree, which is hand-column-aligned in many places, so only changed lines are checked and the one-shot reformat is left as its own reviewable change. That design only works if a contributor can see the proposed reformat and judge it. Right now they cannot, so the job's output is a red mark with no actionable content, which trains people to ignore it.

Related

• Seen on docs: dt_mesh returns plain distance, not squared distance #85 (a doc-only change, where a doc-block edit caused clang-format to reflow two pre-existing over-length lines) and on cuda(mesh): add the sdt_naive host launcher; zero the vertex normals #86.
• Separate from the cancellations on fix(cuda): compile posdef/resize/restrict/splinc, and make the hub link strict #87, which were a slow-mirror apt-get install nvidia-cuda-toolkit consuming 95 of 120 minutes, not a formatting problem.
• Also worth noting for whoever picks this up: rerun_failed_jobs does pick up cancelled jobs, not just failed ones.

[balbasty]

Fixed in #156 (first commit), which carries Closes #89 and will close this on merge.

Confirming the diagnosis here is exactly right, including the detail that it is set -e on the assignment rather than anything about the case. Reproduced against a deliberately mis-formatted file before touching anything — the old step body prints the "diffing against merge base …" line and then exits 1 having emitted nothing at all.

The fix captures the status rather than letting it be fatal, and additionally distinguishes an exit above 1 (a genuine tool failure — bad arguments, missing binary) from the contractual exit 1, so a broken tool is no longer reported as an empty "clean".

Two things went beyond the report, both prompted by owner feedback on #145 that clang-format should inform rather than fail:

• Findings now always exit 0 and are surfaced as a ::notice:: plus a $GITHUB_STEP_SUMMARY block with the file list and the diff. The job was continue-on-error: true, so it never blocked, but it still showed red on essentially every PR — which, as this issue puts it, trains people to ignore it.
• continue-on-error is dropped. It was doing two jobs and only one was wanted: it stopped findings from blocking a merge and it swallowed infrastructure failure, so an apt or checkout death looked identical to a formatting finding. Now findings are always green while a lint that cannot run at all can still go red — so it cannot rot unnoticed the way this reporting path did.

Per your closing note, the reporting path was exercised end-to-end rather than assumed: on a probe PR the job went green while printing the diff and the annotation (run 32417649104), and the tool-failure and 400-line-truncation paths were checked locally against the step body taken verbatim from the committed workflow.

---

Generated by Claude Code