From dd01f9f27cd349230308130ffa90e84f0857df5c Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Sun, 6 Sep 2026 16:16:23 -0500 Subject: [PATCH] Run shellcheck in the local gate, at CI's exact pin The gate checked ktlint, detekt and Android lint but not shellcheck, so a new or edited .sh file was precisely the case where the hook passed and CI's Static analysis leg still went red. The first file it could not check was itself, and it was caught by hand twice before it was caught here. THE DIGEST IS READ OUT OF status_check.yml RATHER THAN COPIED. shellcheck 0.9.0 and 0.11.0 disagree about how to report a trap handler -- SC2317 on seven body lines against SC2329 once on the declaration, same script, same directive, one red and one green. That is why CI pins by digest, and it is also why a second copy of the digest in this file would be worse than none: when it drifts, the symptom is the gate passing and CI failing, which is the exact failure this section prevents. Runs over `git ls-files '*.sh'` -- all tracked files, not the diff -- because that is what CI does, and the job here is to predict that leg rather than audit the change. podman is preferred over docker for the mount's SELinux relabel; neither present, or the digest unreadable, reports the check as NOT COVERED rather than skipping it quietly. Verified that it bites rather than assumed: a probe script whose only fault was an unquoted `ls $foo` was staged, and the real pre-commit hook blocked on SC2086 before it reached the JVM gate. Probe removed; all tracked .sh are clean under the pinned digest. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 7 ++++++ tools/git-hooks/local-gate.sh | 44 +++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index 77bf98f..91dbff0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -413,6 +413,13 @@ install for code that can never run — and on API 37 the full APK does not fit the level is uncovered when it is not — CI's gating leg being what answers for it then. It never claims five levels having run four. + **It runs shellcheck too, at CI's exact pin, over `git ls-files '*.sh'`** — the same digest and + the same file set that leg uses. That gap was found the hard way: the gate checked ktlint, + detekt and Android lint, so a new `.sh` file was precisely the case where it passed and CI still + went red, and the first file it could not check was itself. **The digest is read out of + `status_check.yml` rather than copied** — two copies drift, and the symptom of that drift is the + gate passing while CI fails, which is the one thing this check exists to prevent. + The sweep is cached under the hash of the **`app/src` subtree**, not the whole repo tree. Keying it on the whole tree was the first cut and it was wrong: editing a comment in `CLAUDE.md` threw away a sweep of byte-identical application code and re-ran forty minutes of emulators to prove diff --git a/tools/git-hooks/local-gate.sh b/tools/git-hooks/local-gate.sh index 3fefc91..ebe0b3f 100755 --- a/tools/git-hooks/local-gate.sh +++ b/tools/git-hooks/local-gate.sh @@ -105,6 +105,50 @@ done <<< "$changed_files" # --- the cheap gate always runs ----------------------------------------------------------- +# --- shellcheck, at CI's exact pin --------------------------------------------------------- +# WHY THIS IS HERE. The gate ran ktlint, detekt and Android lint but not shellcheck, so a new or +# edited `.sh` file was precisely the case where this hook passed and CI's Static analysis leg +# still went red. That is not hypothetical: this script is itself a new `.sh` file, and the first +# thing it could not check was itself. It was caught by hand twice before it was caught here. +# +# THE DIGEST IS READ OUT OF status_check.yml, NOT COPIED INTO THIS FILE. shellcheck 0.9.0 and +# 0.11.0 disagree about how to report a trap handler -- SC2317 on seven body lines versus SC2329 +# once on the declaration, same script, same directive, one red and one green. That disagreement +# is why CI pins by digest, and a second copy of the digest here would drift from it silently. +# When it drifts, the symptom is this gate passing and CI failing: the exact thing this section +# exists to prevent. So there is one digest in the repo and this reads it. +# +# ALL TRACKED FILES, not just changed ones, because that is what CI does -- `git ls-files '*.sh'`. +# The point is to predict that leg, not to audit the diff. +shellcheck_pin="$(grep -oE 'koalaman/shellcheck@sha256:[0-9a-f]{64}' \ + .github/workflows/status_check.yml | head -1)" +runtime="" +for candidate in podman docker; do + if command -v "$candidate" >/dev/null 2>&1; then + runtime="$candidate" + break + fi +done + +if [ -z "$shellcheck_pin" ]; then + say "NOT COVERED: shellcheck. Could not read the pinned digest out of + .github/workflows/status_check.yml -- if that pin moved or was reformatted, fix this grep + rather than leaving the check silently absent." +elif [ -z "$runtime" ]; then + say "NOT COVERED: shellcheck. Neither podman nor docker is on PATH, and there is no shellcheck + system package on this host. CI's Static analysis leg is what answers for .sh files then." +else + # :z is podman's SELinux relabel and is what this host needs; docker on CI does without it. + mount=":z" + [ "$runtime" = "docker" ] && mount="" + say "shellcheck ($runtime, $shellcheck_pin)" + if ! git ls-files -z '*.sh' | + xargs -0 -r "$runtime" run --rm -v "$PWD:/mnt$mount" "docker.io/$shellcheck_pin"; then + die "shellcheck failed. CI runs the same digest over the same files, so this is a red + Static analysis leg waiting to happen." + fi +fi + say "$MODE: running the JVM gate" if ! ./gradlew "${GRADLE_GATE[@]}" --continue; then die "the JVM gate failed (assemble, unit tests, androidTest compile, ktlint, detekt, lint)."