diff --git a/.github/scripts/shellcheck-all.sh b/.github/scripts/shellcheck-all.sh new file mode 100755 index 0000000..29c5409 --- /dev/null +++ b/.github/scripts/shellcheck-all.sh @@ -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" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index dac6604..a44759e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -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 diff --git a/CHANGELOG.md b/CHANGELOG.md index 8929dce..8ecf24b 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/bin/cast b/bin/cast index bb69c71..ebf9fed 100755 --- a/bin/cast +++ b/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 diff --git a/install.sh b/install.sh index 98ce3e1..5d58295 100644 --- a/install.sh +++ b/install.sh @@ -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" diff --git a/package.json b/package.json index d697f75..64da5ea 100644 --- a/package.json +++ b/package.json @@ -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",