fix: lint every tracked shell script, and prove the set is complete
Filed as cast's record of heavy-duty/box#116: a `shopt -s globstar;
files=(bin/* **/*.sh)` sweep never descends into `.github/`, because globs
do not match dot-prefixed names without `dotglob`. cast has no such sweep —
it has no shellcheck step at all. Its only shell gate was
bash -n install.sh bin/cast scripts/*.sh .github/scripts/*.sh
a syntax check over a hand-maintained list. The reported symptom holds
(release-notes.sh and labels-reconcile.sh ship unlinted) but so does every
other script here, and `bash -n` parses without linting: it would not catch
a quoting or unset-variable bug in any of them.
.github/scripts/shellcheck-all.sh now runs `shellcheck -x` over the tracked
tree, from CI and from `npm run check:shell`. The file list comes from
`git ls-files`, not a glob. `dotglob` was measured and does work today —
cast's dependency tree ships zero `.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 matched by shebang, which covers bin/cast without
naming it.
It carries 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. Verified
by swapping the derivation for the buggy globstar glob, which reports
exactly the two .github/scripts files.
All eight scripts pass 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 in place. No behavior changes.
Refs #118
This commit is contained in:
parent
8f3a9fe468
commit
1c38ef8988
6 changed files with 120 additions and 1 deletions
79
.github/scripts/shellcheck-all.sh
vendored
Executable file
79
.github/scripts/shellcheck-all.sh
vendored
Executable 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"
|
||||
6
.github/workflows/ci.yml
vendored
6
.github/workflows/ci.yml
vendored
|
|
@ -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
|
||||
|
||||
|
|
|
|||
27
CHANGELOG.md
27
CHANGELOG.md
|
|
@ -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
|
||||
|
|
|
|||
1
bin/cast
1
bin/cast
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
|
|
|
|||
Loading…
Reference in a new issue