Merge pull request #119 from dan-claude-bot/fix/shellcheck-dotglob

fix: lint every tracked shell script, and prove the set is complete
This commit is contained in:
Daniel Marin 2026-07-21 13:56:52 +01:00 committed by GitHub
commit 2118756ccb
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 120 additions and 1 deletions

79
.github/scripts/shellcheck-all.sh vendored Executable file
View file

@ -0,0 +1,79 @@
#!/usr/bin/env bash
set -euo pipefail
# Lint every tracked shell script in the repo, and prove the set is complete
# (#118). Note for editors: a comment line here must not BEGIN with the word
# "shellcheck" — that is directive syntax, and prose in that position is a
# parse error. This file's own sweep catches it, which is how this note exists.
#
# WHY THE LIST COMES FROM GIT, NOT FROM A GLOB
#
# The obvious sweep is a globstar one:
#
# shopt -s globstar; files=(bin/* **/*.sh)
#
# and it is quietly wrong. Globs do not match dot-prefixed names without
# `dotglob`, so `**/` never descends into `.github/` — which is where
# release-notes.sh lives, the script that produces the published release body.
#
# `shopt -s globstar dotglob` does fix that, and measured in this repo it
# pulls in nothing unwanted: the sweep runs after `npm ci`, but cast's current
# dependency tree happens to ship zero `.sh` files, so `**/*.sh` stays clean.
# "Happens to" is the problem — that is a property of somebody else's package
# tree, re-decided by every `npm install`, and the day a transitive dep vendors
# a shell script the lint silently becomes partly about their code. `git
# ls-files` does not depend on that: it sees the tracked tree exactly, with no
# dotfile blind spot and no untracked noise, and it stays right when files move.
#
# Extensionless scripts (bin/cast) are found by shebang rather than named, so
# adding one does not require editing this file.
cd "$(git rev-parse --show-toplevel)"
# --- the set to lint -------------------------------------------------------
mapfile -t files < <(
{
git ls-files '*.sh'
git ls-files | while IFS= read -r f; do
case "$f" in *.sh) continue ;; esac
[ -f "$f" ] || continue
IFS= read -r line <"$f" || 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'
# and '#!/usr/bin/env bash' come out as 'sh' and 'bash'.
interp="${line#\#!}"
interp="${interp%% -*}"
interp="${interp##*[ /]}"
case "$interp" in sh | bash | dash | ksh | zsh) printf '%s\n' "$f" ;; esac
done
} | sort -u
)
[ "${#files[@]}" -gt 0 ] || { echo "shellcheck-all: found no shell scripts — the sweep is broken" >&2; exit 1; }
# --- the class check -------------------------------------------------------
#
# Assert the set we are about to lint COVERS every tracked *.sh. Today this
# can't fail, because the list above is derived from the same `git ls-files`
# — and that is the point. It is a guard on the STATE ("no tracked script
# goes unlinted"), not on the instance that broke: the day someone rewrites
# the derivation above into something cheaper that skips a directory, this
# fails loudly instead of the lint silently passing over nothing. #118 stayed
# latent precisely because a shrinking sweep looks exactly like a green one.
unlinted="$(comm -23 <(git ls-files '*.sh' | sort -u) <(printf '%s\n' "${files[@]}" | sort -u))"
if [ -n "$unlinted" ]; then
echo "shellcheck-all: tracked shell scripts that this sweep does not lint:" >&2
printf '%s\n' "$unlinted" | sed 's/^/ /' >&2
echo "the sweep must cover every tracked *.sh — see #118" >&2
exit 1
fi
# --- lint ------------------------------------------------------------------
printf 'shellcheck: linting %d tracked scripts\n' "${#files[@]}"
printf ' %s\n' "${files[@]}"
shellcheck -x "${files[@]}"
echo "shellcheck: clean"

View file

@ -34,6 +34,12 @@ jobs:
- run: npm test
- name: installer is valid bash
run: bash -n install.sh bin/cast scripts/*.sh .github/scripts/*.sh
# `bash -n` above is a syntax check on a hand-maintained list; it is not
# a linter and it does not notice a script it was never told about.
# This sweep derives its list from git and asserts the list covers every
# tracked *.sh, so a new script cannot go unlinted quietly (#118).
- name: shellcheck — every tracked shell script
run: bash .github/scripts/shellcheck-all.sh
- name: labels state-machine tests
run: bash test/labels-reconcile.sh

View file

@ -266,6 +266,33 @@ actually cutting it, and this file starts there.
wind-down window where the two overlap — and the whole check-outcome enum are
pinned in `test/labels-reconcile.sh` (fixtures 19 → 51).
- **CI now lints every tracked shell script, and proves the set is
complete** (#118) — filed as cast's record of heavy-duty/box#116, whose
defect is a `shopt -s globstar; files=(bin/* **/*.sh)` sweep that skips
`.github/` because globs do not match dot-prefixed names without
`dotglob`. cast's CI turned out never to have had a shellcheck step at
all: the only shell gate was `bash -n install.sh bin/cast scripts/*.sh
.github/scripts/*.sh`, a syntax check over a hand-maintained list. So the
reported symptom was right — `release-notes.sh` and
`labels-reconcile.sh` shipped unlinted — but so did every other script in
the repo, including `install.sh` and `bin/cast`, and `bash -n` would not
have caught a quoting or unset-variable bug in any of them.
[.github/scripts/shellcheck-all.sh](.github/scripts/shellcheck-all.sh)
now runs `shellcheck -x` over the tracked tree, from CI and from
`npm run check:shell`. It takes its file list from `git ls-files` rather
than from a glob. `shopt -s globstar dotglob` was measured and does work
here — cast's dependency tree ships no `.sh` files, so sweeping after
`npm ci` pulls in nothing — but that is a property of somebody else's
package tree, re-decided by every install; `git ls-files` does not depend
on it. Extensionless scripts are found by shebang, which is how
`bin/cast` is covered without being named. All seven scripts passed as
they stood — the three findings were intentional (`$PATH` written
literally into a profile, advice text in backticks) or a false positive,
and are annotated as such, so this lands as a no-op on behavior. It ships
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.
## 0.1.1 — 2026-07-19
### Fixed

View file

@ -263,6 +263,7 @@ command -v node >/dev/null 2>&1 || {
}
if [ ! -f "$ROOT/dist/cli.js" ]; then
# shellcheck disable=SC2016 # intentional: the backticked text is advice for a human to type, not something to expand here
printf 'cast: %s/dist/cli.js is missing — run the installer, or `npm ci && npm run build` in %s\n' "$ROOT" "$ROOT" >&2
exit 1
fi

View file

@ -410,6 +410,10 @@ profile_for_shell() {
}
path_line_for() {
# $PATH below must land in the profile LITERALLY — it is expanded by the
# user's future shells, not by this one. The directive sits on the whole
# case because a directive on an individual branch is rejected (SC1124).
# shellcheck disable=SC2016
case "$1" in
*/config.fish) printf 'fish_add_path %s\n' "$BINDIR" ;;
*) printf 'export PATH="%s:$PATH"\n' "$BINDIR" ;;
@ -427,6 +431,7 @@ else
log "$PROFILE already puts $BINDIR on PATH — this shell just predates it."
else
mkdir -p "$(dirname "$PROFILE")"
# shellcheck disable=SC2094 # false positive: path_line_for only cases on its argument's NAME, it never reads the file
{ printf '\n%s\n' "$MARKER"; path_line_for "$PROFILE"; } >>"$PROFILE" \
|| die "could not write $PROFILE — add this line yourself: $(path_line_for "$PROFILE")"
log "wired $BINDIR onto PATH in $PROFILE"

View file

@ -13,7 +13,8 @@
"build": "tsc",
"test": "vitest run",
"format": "biome format --write .",
"check": "biome check --error-on-warnings ."
"check": "biome check --error-on-warnings .",
"check:shell": "bash .github/scripts/shellcheck-all.sh"
},
"dependencies": {
"yaml": "^2.5.0",