From 5fa484b5d2a7438f08c05d95837d9733dd8424f5 Mon Sep 17 00:00:00 2001 From: dan-claude-bot Date: Mon, 20 Jul 2026 09:58:41 +0000 Subject: [PATCH] fix: floor the shellcheck sweep on bin/cast, and stop skipping newline-less files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #119's class check asserts the swept set covers `git ls-files '*.sh'`. bin/cast has no `.sh` extension: it enters the set through the shebang scan, so it is covered by the DERIVATION and not by the ASSERTION. Break or delete that scan and the shipped entrypoint drops out of the lint while the check still exits 0 — #118's failure mode (a sweep quietly narrowing while CI stays green) one level in from where #119 closed it. There is no non-circular way to re-derive "every extensionless shell script" inside the script; any second derivation would be the same shebang scan and would break with it. So the floor is named rather than computed: `required=(bin/cast)`, asserted present in the swept set. A rename turns it red, which is correct — the floor is the thing that has to be updated deliberately. A minimum-count assert was considered and declined: given the *.sh class check already floors the set, a count floor's only marginal coverage is "at least one extensionless script exists", which the named floor states more precisely and with a better error message, and it would churn on every script added or removed. Proven to bite. With the shebang allowlist stubbed to match nothing, the *.sh class check still PASSES and the new floor fails: shellcheck-all: 'bin/cast' is not in the swept set it has no .sh extension, so it enters only via the shebang scan above — that scan is broken, or the file moved. See #121. Reverted, the sweep is green over 8 scripts again. Also fixed, from the same review: `IFS= read -r line <"$f" || continue` skipped any file whose FIRST line lacked a trailing newline, because `read` returns 1 at EOF even when it populated `line`. A shebang-only file with no final newline was silently unswept. Now `|| [ -n "$line" ] || continue`, which falls through on a populated partial read and still skips genuinely empty files. Measured against a tracked 9-byte `#!/bin/sh` with no final newline: the fixed scan sweeps 9 scripts including it, the old line sweeps 8 and omits it silently. Closes #121 Co-Authored-By: Claude Opus 4.8 --- .github/scripts/shellcheck-all.sh | 45 ++++++++++++++++++++++++++++++- CHANGELOG.md | 12 +++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/.github/scripts/shellcheck-all.sh b/.github/scripts/shellcheck-all.sh index 29c5409..8d8bbb7 100755 --- a/.github/scripts/shellcheck-all.sh +++ b/.github/scripts/shellcheck-all.sh @@ -38,7 +38,13 @@ mapfile -t files < <( git ls-files | while IFS= read -r f; do case "$f" in *.sh) continue ;; esac [ -f "$f" ] || continue - IFS= read -r line <"$f" || continue + # `read` returns 1 at EOF even when it populated `line` — which is what + # happens on a file whose FIRST line has no trailing newline (a + # shebang-only file with no final newline). A bare `|| continue` would + # skip exactly that file, silently. Fall through whenever `line` is + # non-empty; the empty case is a genuinely empty file, which has no + # shebang and is meant to be skipped. + IFS= read -r line <"$f" || [ -n "$line" ] || continue case "$line" in '#!'*) ;; *) continue ;; esac # reduce the shebang to a bare interpreter name: drop the '#!', drop any # flags, then keep the last path/word component — so both '#!/bin/sh -e' @@ -46,6 +52,15 @@ mapfile -t files < <( interp="${line#\#!}" interp="${interp%% -*}" interp="${interp##*[ /]}" + # Two known limits of this reduction, both theoretical in this repo: + # - `zsh` is on the allowlist, but shellcheck has no zsh support and + # emits SC1071 for it. So a tracked zsh script makes the sweep fail + # HARD rather than get linted. That is the right end state — a + # script nobody can lint should be loud, not skipped — but the + # outcome is "blocked", not "clean". Drop zsh here only if the repo + # ever gains one and the answer is to exempt it on purpose. + # - `#!/usr/bin/env -S bash` reduces to `env` and is not matched. + # Nothing in the repo uses `-S`; see #121 for why that is left. case "$interp" in sh | bash | dash | ksh | zsh) printf '%s\n' "$f" ;; esac done } | sort -u @@ -71,6 +86,34 @@ if [ -n "$unlinted" ]; then exit 1 fi +# --- the floor: extensionless scripts -------------------------------------- +# +# The check above is derived from `git ls-files '*.sh'`, so it says nothing +# about scripts that have no `.sh` extension — those enter the set only via +# the shebang scan. `bin/cast` is one, and it is the shipped entrypoint. So +# it is covered by the DERIVATION and not by the ASSERTION: break or delete +# the shebang branch and bin/cast drops out of the sweep while this script +# still exits 0. That is #118's failure mode — a lint quietly narrowing while +# CI stays green — one level in from where the *.sh check closed it (#121). +# +# There is no non-circular way to re-derive "every extensionless shell +# script" here; any second derivation would be the same shebang scan, and +# would break with it. So the floor is named rather than computed: the known +# extensionless scripts are listed, and the sweep must contain them. A rename +# turns this red, which is correct — the floor is the thing that has to be +# updated deliberately. + +required=(bin/cast) + +for req in "${required[@]}"; do + if ! printf '%s\n' "${files[@]}" | grep -qxF "$req"; then + echo "shellcheck-all: '$req' is not in the swept set" >&2 + echo "it has no .sh extension, so it enters only via the shebang scan above —" >&2 + echo "that scan is broken, or the file moved. See #121." >&2 + exit 1 + fi +done + # --- lint ------------------------------------------------------------------ printf 'shellcheck: linting %d tracked scripts\n' "${#files[@]}" diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ecf24b..24c44b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -292,6 +292,18 @@ actually cutting it, and this file starts there. with a class check in box#112's shape: the sweep asserts its own list covers `git ls-files '*.sh'` and fails naming the strays otherwise, so a future sweep that quietly narrows is red rather than green over nothing. +- **The shellcheck sweep's own blind spot: extensionless scripts** (#121) — + the class check above asserts the swept set covers `git ls-files '*.sh'`, + which says nothing about scripts with no `.sh` extension. `bin/cast` is + one, and it enters the set only through the shebang scan — covered by the + derivation, not by the assertion. Break that scan and the shipped + entrypoint drops out of the lint while the check still exits 0: #118's + failure mode one level in. The sweep now also asserts a named floor of + known extensionless scripts, verified to go red when the shebang branch is + broken. Fixed alongside it: `IFS= read -r line <"$f" || continue` skipped + any file whose first line had no trailing newline, because `read` returns + 1 at EOF even having populated `line` — a shebang-only file with no final + newline was silently unswept. ## 0.1.1 — 2026-07-19