The ShellCheck job passes when a search root disappears, and it never lints the two git hooks

[vladimirrott]

.github/workflows/e2e.yml:77-81 is the only ShellCheck gate in CI, and it
reads its file list from the left-hand side of a pipe:

      - name: ShellCheck maintained scripts
        run: |
          find tests/e2e tests/release scripts assets/demo \
            -type f -name '*.sh' -print0 \
            | xargs -0 shellcheck --severity=warning

GitHub runs a run: block as bash -e {0}, and e2e.yml sets no
defaults.run.shell, so there is no pipefail. find's exit status is
discarded and the step's verdict comes from xargs alone. Rename or move one of
those four roots and the scan quietly shrinks to the survivors.

Why it matters

Reproduced in a scratch tree, running the step body exactly as the workflow
writes it. bad.sh carries three warnings and lives under tests/release:

$ d=/tmp/sk-shellcheck-repro; mkdir -p "$d"/{tests/e2e,tests/release,scripts,assets/demo}
$ printf '#!/usr/bin/env bash\nfoo=1\ncd $undefined_and_unquoted\necho "$fooo"\n' > "$d/tests/release/bad.sh"
$ printf '#!/usr/bin/env bash\necho ok\n' > "$d/scripts/good.sh"
$ cd "$d"
$ out="$(bash -e -c "find tests/e2e tests/release scripts assets/demo -type f -name '*.sh' -print0 | xargs -0 shellcheck --severity=warning" 2>&1)"; rc=$?
$ echo "rc=$rc"; printf '%s\n' "$out" | head -6
rc=123

In tests/release/bad.sh line 2:
foo=1
^-^ SC2034 (warning): foo appears unused. Verify use (or export if used externally).

$ mv tests/release tests/release-suite
$ out="$(bash -e -c "find tests/e2e tests/release scripts assets/demo -type f -name '*.sh' -print0 | xargs -0 shellcheck --severity=warning" 2>&1)"; rc=$?
$ echo "rc=$rc"; printf '%s\n' "$out"
rc=0
find: ‘tests/release’: No such file or directory

Green, with a file carrying three warnings never opened. The whole trace is one
line on stderr inside a passing step.

Two facts bound this honestly, and both are worth knowing before you start.

The local mirror already catches it. scripts/ci-local.sh:265-270 runs the
identical find | xargs, inside a script whose line 2 is set -euo pipefail,
and the subshell inherits it:

$ out="$(bash -eo pipefail -c "find tests/e2e tests/release scripts assets/demo -type f -name '*.sh' -print0 | xargs -0 shellcheck --severity=warning" 2>&1)"; rc=$?
$ echo "rc=$rc"; printf '%s\n' "$out"
rc=1
find: ‘tests/release’: No such file or directory

So CI is the looser of the two copies, which is the wrong way round.

The empty case is fail-closed, so this is about a stale path rather than a
vanished list:

$ out="$(printf '' | xargs -0 shellcheck --severity=warning 2>&1)"; rc=$?
$ echo "rc=$rc"; printf '%s\n' "$out" | head -1
rc=123
No files specified.

The second gap in the same two lines

The roots miss the two hooks that gate every contributor's commit and push.
201 tracked shell files, 199 scanned (194 and 192 when I filed this; recounted at
b2c823e6 on 2026-09-16, and the gap is still exactly the same two files):

$ git ls-files > /tmp/sk-tracked.txt
$ python3 - <<'PY' > /tmp/sk-all-sh.txt
import pathlib
for line in open('/tmp/sk-tracked.txt'):
    p = line.rstrip('\n')
    f = pathlib.Path(p)
    if not f.is_file():
        continue
    if p.endswith('.sh'):
        print(p); continue
    try:
        first = f.open('rb').readline().decode('utf-8', 'replace')
    except Exception:
        continue
    if first.startswith('#!') and ('bash' in first or first.rstrip().endswith('sh')):
        print(p)
PY
$ sort -o /tmp/sk-all-sh.txt /tmp/sk-all-sh.txt; wc -l < /tmp/sk-all-sh.txt
201
$ find tests/e2e tests/release scripts assets/demo -type f -name '*.sh' | sort > /tmp/sk-scanned.txt; wc -l < /tmp/sk-scanned.txt
199
$ comm -23 /tmp/sk-all-sh.txt /tmp/sk-scanned.txt
.githooks/pre-commit
.githooks/pre-push

Neither has a .sh extension, which is why -name '*.sh' skips them.
.githooks/pre-commit is the script that runs scripts/check_no_secrets.sh
before every commit, and it is unlinted.

Scope

• Read the list into an array and refuse an empty one, then lint from the
  array: mapfile -d '' files < <(find ...), (( ${#files[@]} > 0 )) or exit
  non-zero, shellcheck --severity=warning "${files[@]}".
• Add .githooks to the search roots, and select on the shebang rather than
  the extension so a new extensionless hook is covered by construction.
• Make the same change in scripts/ci-local.sh:265-270 so the two copies stay
  the same scan. They are meant to be identical; the comment at :263-264 says so.
• Do not reach for set -o pipefail in the workflow as the whole fix. It turns
  this case red, and it leaves the next reader with the same two lines to
  reason about.

Tests first

tests/release/*.test.sh is where the runnable gates live, and #331 is about
wiring every one of them to a gate, so put the assertion there. A
shellcheck-coverage.test.sh that derives both sets and asserts every tracked
shell file appears in the scanned set, plus a floor on the scanned count.

Prove it bites twice, in a fixture copy rather than in the tree:

1. Rename one search root. The test must fail naming the missing root, not
   pass on the survivors.
2. Point the search at an empty directory. The test must refuse rather than
   report success over zero files.

Difficulty

easy. Two shell edits and one new test script, no Rust and no VM.

Getting started

CONTRIBUTING.md
has the build and test commands. shellcheck comes from apt install shellcheck.
No CLA and no copyright waiver. The project is MIT.

[vladimirrott]

@ITSMERNB holding this for you, offered as part of a batch on #408 so it is not three separate asks.

Two findings in one issue, and the second is the one I would do first: .githooks/pre-commit and .githooks/pre-push are never linted by the sweep at all. Those two run on every contributor's machine, so a shell bug in them is the one that reaches people who never look at CI.

Take it or leave it, and no is a complete answer.

[ITSMERNB]

I will take #409. I am starting with the two git hooks, then the stale-root/empty-scan contract. I will keep CI and ci-local on the same derived shell-file set, add a release coverage test, and prove the missing-root and empty-root mutations both go red before opening the PR. Thanks for holding it.

[vladimirrott]

@ITSMERNB recounted this at b2c823e6 while auditing the tracker, and two numbers in the body had moved. Corrected there; posting the commands here so you can check them against your branch.

$ sort -o /tmp/sk-all-sh.txt /tmp/sk-all-sh.txt; wc -l < /tmp/sk-all-sh.txt
201
$ find tests/e2e tests/release scripts assets/demo -type f -name '*.sh' | sort > /tmp/sk-scanned.txt; wc -l < /tmp/sk-scanned.txt
199
$ comm -23 /tmp/sk-all-sh.txt /tmp/sk-scanned.txt
.githooks/pre-commit
.githooks/pre-push

201 tracked and 199 scanned, up from 194 and 192. The gap is unchanged and still exactly the two hooks, which is the part your fix has to close, so the floor in your test wants to be derived rather than a literal.

A warning about the first number if you write a guard for it: counting with git ls-files | grep '\.sh$' gives 199 and an empty gap, because neither hook carries the extension. The derivation in the body reads the shebang, which is the whole point. I made that mistake this morning and briefly concluded the issue had been fixed.

The local mirror moved too, without changing:

$ sed -n '263,270p' scripts/ci-local.sh
# Same scan as e2e.yml's "ShellCheck maintained scripts" step -- kept here too
# since every script this task adds/touches must stay shellcheck-clean.
hygiene_shellcheck() (
    cd "$repo_root" || exit 1
    find tests/e2e tests/release scripts assets/demo \
        -type f -name '*.sh' -print0 \
        | xargs -0 shellcheck --severity=warning
)

That block is :265-270 now with its comment at :263-264; the body said :254-258 and :253. Seven commits pushed it down and none touched the scan. .github/workflows/e2e.yml:77-81 is unmoved and still sets no defaults.run.shell.