Files
improvise/scripts/checkers-test.sh
T
Edward Langley cb6866b2c6 fix(scripts): one shell and one environment for every checker (improvise-f9qf)
`scripts/pr-reviews-test.sh` passed 19/19 locally and 2/19 in CI on the same
commit, because a multi-assignment `local` is not left-to-right on bash 5
while macOS's bash 3.2 tolerates it. The obvious half of the fix is pinning
the shell. The half that decides the design is not.

.envrc DOES already put a nix bash 5.3.15 on PATH, which is CI's semantics --
and it is not enough. `pkgs.mkShell` exports `name`, `out`, `outputs`,
`system`, `shell`, `builder`, `stdenv`, `phases` and about twenty more
generically-named variables that CI does not have, so a script reading one by
accident under `set -u` is SILENT in the dev shell and fatal in CI. That is
exactly what happened: the broken clause read `$name`, and in the dev shell
`$name` was `nix-shell-env`. Measured at 051f541^ on one machine:

  macOS /bin/bash 3.2                             19 passed, 0 failed
  direnv exec . ./scripts/pr-reviews-test.sh      19 passed, 0 failed
  the same, with `name` unset                      2 passed, 17 failed

Two reasons for green, one of them the correct interpreter. A dev shell is a
third environment, not a reproduction of CI.

A nix shebang was measured and declined, on availability rather than speed:
warm, `nix shell nixpkgs#bash --command true` is 61.8ms against an ambient
5.1ms, and `nix-shell -p bash --run true` -- the manpage form -- is 675.2ms,
132x, inside git's merge machinery for beads-merge-driver.sh. But only the
`kb` job installs nix; build, test, wasm, js, harness-lint and browser have
none, so a nix shebang needs an installer step on three more jobs to reach a
bash those runners already ship. zsh is not a shebang change and the runner
does not have it either. Both declined in context/code/tooling.md section 5.

What lands instead pins interpreter and environment together, in one place:
scripts/checkers.sh is the one entry point for every check_the_checker, used
verbatim by CI and by the local gate. It pins a bash >= 5 (ambient first, nix
only when the ambient one is older, exit 3 when neither), scrubs the
environment to an allowlist, and refuses any shell shebang in scripts/ that
is not `#!/usr/bin/env bash`. The pinned bash goes on the front of PATH so
inner invocations get it too -- which is where the fault actually lived.

The second undetected split is closed with it: the four `#!/bin/sh` and one
`#!/usr/bin/env sh` scripts are now bash, and the six places that ran a
script through an explicit `sh` -- overriding the callee's own shebang, which
is the same disease -- now invoke it directly.

Verification, injected against the real script: restoring pr-reviews'
multi-assignment `local` leaves `sh scripts/pr-reviews-test.sh` at 22/22 and
`direnv exec . ./scripts/pr-reviews-test.sh` at 22/22, while
`./scripts/checkers.sh pr-reviews` reports 2 passed, 20 failed, exit 1 --
matching the 2/19 CI actually reported. scripts/checkers-test.sh is twelve
cases, each injecting rather than asserting; the pair that carries it runs
one fixture both ways, so the `set -u` case is a test of the runner and not
of `set -u`.

The old local gate was `for t in scripts/*-test.sh; do sh "$t"; done`, which
overrides every shebang with /bin/sh -- so it ran every suite under bash 3.2
in sh-mode regardless of what the file said. It is replaced by
`./scripts/checkers.sh`.

Workspace suite unchanged at 1689 passing over 38 binaries.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
2026-09-22 15:50:14 -07:00

181 lines
6.3 KiB
Bash
Executable File

#!/usr/bin/env bash
# check_the_checker for scripts/checkers.sh. Every case injects a fault that
# a bare `./scripts/X-test.sh` cannot see, and asserts the runner does.
# See context/test-evidence/tooling.md.
set -uo pipefail
here="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
RUNNER="$here/checkers.sh"
work="$(mktemp -d)"
trap 'rm -rf "$work"' EXIT
pass=0; fail=0
ok() { printf ' ok %s\n' "$1"; pass=$((pass+1)); }
bad() { printf ' FAIL %s (%s)\n' "$1" "$2"; fail=$((fail+1)); }
# A sandbox is a copy of the runner beside fixture suites, so `$HERE` inside it
# is the sandbox: the shebang scan and the suite list are both the fixtures'.
sandbox() {
local d="$work/$1"
mkdir -p "$d"
cp "$RUNNER" "$d/checkers.sh"
chmod +x "$d/checkers.sh"
printf '%s' "$d"
}
# A suite is only discovered when the thing it checks is present, so every
# fixture gets a stub subject beside it.
suite() {
local d="$1" name="$2"
cat > "$d/$name-test.sh"
chmod +x "$d/$name-test.sh"
printf '#!/usr/bin/env bash\nexit 0\n' > "$d/$name.sh"
chmod +x "$d/$name.sh"
}
# $1 name, $2 expected exit, rest: the command. Nothing follows the runner in
# its own pipeline, which is the rule its own header states.
expect() {
local what="$1" want="$2"; shift 2
"$@" > "$work/out" 2>&1
local got=$?
[ "$got" = "$want" ] && ok "$what" || bad "$what" "exit $got, wanted $want"
}
# ---------------------------------------------------------------------------
# 1. The fault this runner exists for. `nix develop` exports `name`; a suite
# that reads an unset variable under `set -u` is therefore SILENT in the dev
# shell and fatal in CI. That is improvise-f9qf, reproduced exactly.
d="$(sandbox setu)"
suite "$d" masked <<'EOF'
#!/usr/bin/env bash
set -euo pipefail
echo "read: $name"
EOF
export name=nix-shell-env
expect "a suite reading an inherited dev-shell variable fails under the runner" \
1 "$d/checkers.sh" masked
# The same suite run the way the old gate ran it PASSES, which is what makes
# the case above discriminating rather than a test of `set -u`.
"$d/masked-test.sh" > "$work/out" 2>&1
direct=$?
[ "$direct" = 0 ] \
&& ok "and passes when run directly with that variable exported" \
|| bad "and passes when run directly with that variable exported" \
"exit $direct, wanted 0"
unset name
# ---------------------------------------------------------------------------
# 2. The bash-5 `local` semantics. A multi-assignment `local` does not read
# left to right on bash 5, so this suite passes on macOS bash 3.2 and dies
# under `set -u` on bash 5 -- PR #133's 19/19 local against 2/19 in CI.
d="$(sandbox bash5)"
suite "$d" vers <<'EOF'
#!/usr/bin/env bash
set -euo pipefail
f() { local a="$1" b="$a/x"; echo "$b"; }
f one
EOF
expect "a bash-3.2-only construct fails under the runner's pinned bash" \
1 "$d/checkers.sh" vers
# ---------------------------------------------------------------------------
# 3. The shebang rule. `#!/bin/sh` is dash in CI and bash-in-sh-mode on macOS.
d="$(sandbox shebang)"
suite "$d" fine <<'EOF'
#!/usr/bin/env bash
exit 0
EOF
printf '#!/bin/sh\nexit 0\n' > "$d/legacy.sh"
chmod +x "$d/legacy.sh"
expect "a #!/bin/sh script in the scripts directory is refused" 1 \
"$d/checkers.sh" fine
grep -q 'wanted #!/usr/bin/env bash' "$work/out" \
&& ok "and the refusal names the shebang it wanted" \
|| bad "and the refusal names the shebang it wanted" "no such message"
# Without the injected file the same sandbox passes, so the case above is not
# passing on some other complaint.
rm -f "$d/legacy.sh"
expect "and the same sandbox passes once it is removed" 0 "$d/checkers.sh" fine
# ---------------------------------------------------------------------------
# 4. A failing suite is reported, and a passing one is not.
d="$(sandbox rc)"
suite "$d" red <<'EOF'
#!/usr/bin/env bash
exit 1
EOF
expect "a failing suite fails the runner" 1 "$d/checkers.sh" red
d="$(sandbox green)"
suite "$d" blue <<'EOF'
#!/usr/bin/env bash
exit 0
EOF
expect "a passing suite passes the runner" 0 "$d/checkers.sh" blue
# ---------------------------------------------------------------------------
# 5. No bash >= 5 and no nix is exit 3, not a silent run under whatever is
# there. This is the arm that would have refused the macOS system bash.
d="$(sandbox oldbash)"
suite "$d" any <<'EOF'
#!/usr/bin/env bash
exit 0
EOF
mkdir -p "$work/fakebin"
cat > "$work/fakebin/bash" <<'EOF'
#!/bin/sh
case "$*" in *BASH_VERSINFO*) echo 3; exit 0 ;; esac
exit 0
EOF
chmod +x "$work/fakebin/bash"
# The runner is invoked through a REAL bash, so the fake one below is only
# what its own `command -v bash` probe finds -- not its interpreter.
# /usr/bin:/bin carries the utilities the runner itself needs and carries no
# nix, so the fake bash is the only candidate and the nix fallback is absent.
env PATH="$work/fakebin:/usr/bin:/bin" HOME="$HOME" \
"${BASH:-bash}" "$d/checkers.sh" any > "$work/out" 2>&1
got=$?
[ "$got" = 3 ] && ok "an old bash with no nix is exit 3" \
|| bad "an old bash with no nix is exit 3" "exit $got, wanted 3"
# ---------------------------------------------------------------------------
# 6. A named suite that does not exist is a usage error, not a silent no-op.
d="$(sandbox missing)"
suite "$d" present <<'EOF'
#!/usr/bin/env bash
exit 0
EOF
expect "an unknown suite name is exit 4" 4 "$d/checkers.sh" absent
# ---------------------------------------------------------------------------
# 7. --list names every suite in the directory and nothing else.
d="$(sandbox listing)"
suite "$d" alpha <<'EOF'
#!/usr/bin/env bash
exit 0
EOF
suite "$d" beta <<'EOF'
#!/usr/bin/env bash
exit 0
EOF
"$d/checkers.sh" --list > "$work/out" 2>&1
listed="$(tr '\n' ' ' < "$work/out")"
[ "$listed" = "alpha beta " ] && ok "--list names exactly the suites present" \
|| bad "--list names exactly the suites present" "got [$listed]"
# A test RUNNER also ends in -test.sh. It is not a suite, and running it as
# one is how `scripts/node-test.sh` printed its usage inside a green gate.
printf '#!/usr/bin/env bash\nexit 4\n' > "$d/gamma-test.sh"
chmod +x "$d/gamma-test.sh"
"$d/checkers.sh" --list > "$work/out" 2>&1
listed="$(tr '\n' ' ' < "$work/out")"
[ "$listed" = "alpha beta " ] \
&& ok "an X-test.sh with no X beside it is not a suite" \
|| bad "an X-test.sh with no X beside it is not a suite" "got [$listed]"
printf '\ncheckers-test: %d passed, %d failed\n' "$pass" "$fail"
[ "$fail" -eq 0 ]