fix(gate): run-tests.sh could not check its own most important precondition

scripts/run-tests.sh

@@ -152,7 +152,11 @@ MIN_TESTS="${MIN_TESTS:-5600}"
 # against a temp directory (rotation, truncation, generation cap, and the .bak
 # scope fence). Those tests FAIL rather than skip when it is absent — a skipped
 # rotation test reports safety it never measured — so it belongs here.
-REQUIRED_TOOLS=(bash curl node rg git awk jq grep setsid python3 nix-instantiate opencode logrotate)
+#
+# 🔴 `python` is listed as well as `python3` because THIS SCRIPT invokes
+# `python -m pytest`, not `python3`. Asserting only `python3` checked a binary
+# the runner never calls.
+REQUIRED_TOOLS=(bash curl node rg git awk jq grep setsid python python3 nix-instantiate opencode logrotate)
 missing_tools=()
 for t in "${REQUIRED_TOOLS[@]}"; do
   command -v "$t" >/dev/null 2>&1 || missing_tools+=("$t")
@@ -172,6 +176,38 @@ if [ "${#missing_tools[@]}" -gt 0 ]; then
   exit 2
 fi
 
+# --- GUARD 1b: pytest ITSELF must be importable --------------------------------
+# 🔴 REQUIRED_TOOLS is a list of BINARIES checked with `command -v`. pytest is
+# not a binary this runner calls — it is a MODULE (`python -m pytest`) — so it
+# was structurally outside the one guard whose entire job is "the thing that
+# runs the tests is present". The precondition that mattered most was the one
+# the precondition mechanism could not express.
+#
+# MEASURED 2026-08-03 on this dev host, with every REQUIRED_TOOLS binary present
+# but no pytest importable from `python`: all 17 targets reported
+#   run-tests: ERROR — could not parse pytest's summary for <dir>.
+# and the run ended `TOTAL collected=0 … RESULT: FAIL`, exit 1.
+#
+# So the gate did NOT go green — GUARD 4 (unparseable summary) and GUARD 3 (the
+# collected floor) both fired, and the "reports per-target PASS with collected=0"
+# version of this hole does not exist on this revision. What it produced instead
+# was SEVENTEEN copies of a message blaming pytest's OUTPUT FORMAT for what was
+# actually "pytest is not installed" — a diagnosis pointing at the wrong
+# subsystem, the shape that has repeatedly cost this repo whole sessions (#276's
+# "missing directory" read as an environment fault). One named error is the fix.
+if [ "$CHECK_TARGETS_ONLY" -eq 0 ]; then
+  if ! python -m pytest --version >/dev/null 2>&1; then
+    echo "run-tests: FATAL — \`python -m pytest\` is not runnable." >&2
+    echo "  \`python\` resolves to $(command -v python 2>/dev/null || echo '(not found)') but the" >&2
+    echo "  pytest MODULE is not importable from it, so every suite would collect 0" >&2
+    echo "  tests and report an unparseable summary. This is NOT a pytest output-format" >&2
+    echo "  change — it is a missing dependency in the caller's environment." >&2
+    echo "  Fix the caller: flake.nix checks.pytests builds a python312.withPackages" >&2
+    echo "  env that includes pytest; the pre-push hook wraps this in a nix-shell." >&2
+    exit 2
+  fi
+fi
+
 # --- HERMETIC set --------------------------------------------------------------
 # Verified to pass in the offline nix sandbox: every third-party call
 # (psycopg2 / requests / minio HTTP) is mocked, so no live DB or network is
@@ -327,8 +363,22 @@ EXPECTED_SKIPS=(
 # instead.
 
 fail=0
-declare -a RESULTS
-declare -a SKIP_LINES
+# 🔴 `RESULTS=()` not `declare -a RESULTS`. Under `set -u`, `declare -a foo`
+# leaves the variable DECLARED BUT UNSET, so the first `${#foo[@]}` or
+# `"${foo[@]}"` on a still-empty array aborts with "foo: unbound variable"
+# (MEASURED on bash 5.3.15; an explicit `=()` assignment makes it set-and-empty).
+#
+# That was live: with zero skips, GUARD 2's reporting block died at
+# `if [ "${#SKIP_LINES[@]}" -eq 0 ]`, printing a raw
+# `run-tests.sh: line 479: SKIP_LINES: unbound variable` instead of the skip
+# list, and the unpinned-skip loop below it never executed. Because there is no
+# `set -e` the script carried on, so the damage was confined to the DIAGNOSTIC
+# path — the skip-total accounting still fired — but the runner emitted a bash
+# internal error at exactly the moment someone is trying to read why the gate is
+# red. That is the #276 lesson repeating: a failure that reads like an
+# environment fault rather than a named finding.
+RESULTS=()
+SKIP_LINES=()
 TOT_COLLECTED=0
 TOT_PASSED=0
 TOT_SKIPPED=0

scripts/tests/test_run_tests_preconditions.py

@@ -0,0 +1,301 @@
+"""Guards for `scripts/run-tests.sh`'s PRECONDITIONS and its empty-array handling.
+
+WHY THIS EXISTS
+---------------
+Two measured holes, both of which let the runner produce output that reads
+reassuring (or merely confusing) rather than naming what is wrong.
+
+1. ``REQUIRED_TOOLS`` could not express its own most important precondition.
+   It is a list of BINARIES checked with ``command -v``. pytest is not a binary
+   this runner calls -- it is a MODULE (``python -m pytest``) -- so the one
+   guard whose whole job is "the thing that runs the tests is present" was
+   structurally unable to check pytest. It also asserted ``python3`` while the
+   runner actually invokes ``python``.
+
+   MEASURED 2026-08-03 with every REQUIRED_TOOLS binary present but no pytest
+   importable: all 17 targets printed ``could not parse pytest's summary`` and
+   the run ended ``TOTAL collected=0 … RESULT: FAIL``, exit 1.
+
+   Being precise, because it changes how much this matters: the gate did NOT go
+   green. GUARD 4 and GUARD 3 both fired. The defect is DIAGNOSTIC -- seventeen
+   copies of a message blaming pytest's OUTPUT FORMAT for what was actually a
+   missing dependency, which is the #276 shape (a real finding that reads like
+   an environment fault).
+
+2. ``declare -a RESULTS`` / ``declare -a SKIP_LINES`` leave the arrays DECLARED
+   BUT UNSET. Under ``set -u`` the first ``${#arr[@]}`` on a still-empty array
+   aborts the command with ``unbound variable`` (measured on bash 5.3.15).
+   With zero skips this printed a raw
+   ``run-tests.sh: line 479: SKIP_LINES: unbound variable`` where the skip list
+   belonged, and the unpinned-skip loop below it never ran. No ``set -e``, so
+   the script continued and the skip-TOTAL accounting still fired -- the damage
+   was confined to the diagnostic path, at exactly the moment someone is trying
+   to read why the gate is red.
+
+WHAT IS REGRESSION COVERAGE HERE AND WHAT IS NOT
+------------------------------------------------
+Per claude/RULES.md:
+
+  * ``test_missing_pytest_module_is_named``, ``test_python_is_a_required_tool``
+    and ``test_empty_arrays_are_initialised_assigned`` are REGRESSION coverage.
+    Each is red at origin/main for its own reason (see the PR matrix).
+
+  * ``test_an_empty_target_directory_is_loud`` and
+    ``test_a_typod_target_entry_is_loud`` are REACHABILITY proofs for guards
+    that already existed before this PR. They are NOT regression coverage for
+    this change -- they were requested as known-bad-state validation, and they
+    pass at origin/main too. Their value is that nothing had ever proven those
+    two paths could actually fire.
+"""
+from __future__ import annotations
+
+import os
+import re
+import subprocess
+import sys
+from pathlib import Path
+
+import pytest
+
+REPO_ROOT = Path(__file__).resolve().parent.parent.parent
+sys.path.insert(0, str(REPO_ROOT / "scripts"))
+from testlib.mockbin import write_exec  # noqa: E402
+RUN_TESTS = REPO_ROOT / "scripts" / "run-tests.sh"
+
+
+def _run(args: list[str], env: dict | None = None, timeout: int = 300):
+    return subprocess.run(
+        ["bash", *args],
+        cwd=str(REPO_ROOT),
+        capture_output=True,
+        text=True,
+        timeout=timeout,
+        env=env,
+    )
+
+
+def _runner_with_targets(tmp_path: Path, targets: list[str]) -> Path:
+    """Copy run-tests.sh with HERMETIC_TARGETS replaced wholesale.
+
+    Replacing the whole block (rather than injecting one entry) keeps these
+    tests FAST: the copy runs only the target under test instead of the full
+    5792-test suite, which is what makes a per-target reachability proof
+    affordable at all.
+    """
+    src = RUN_TESTS.read_text()
+    body = "\n".join(f"  {t}" for t in targets)
+    patched, n = re.subn(
+        r"^HERMETIC_TARGETS=\(.*?^\)", f"HERMETIC_TARGETS=(\n{body}\n)", src, count=1, flags=re.S | re.M
+    )
+    assert n == 1, "failed to replace HERMETIC_TARGETS in the copied runner"
+    assert "HERMETIC_TARGETS=(\n  " in patched, "replacement produced an empty target list"
+    dst = tmp_path / "run-tests.sh"
+    dst.write_text(patched)
+    return dst
+
+
+# --------------------------------------------------------------------------
+# Guard the guard.
+# --------------------------------------------------------------------------
+
+def test_the_runner_copy_helper_actually_works(tmp_path):
+    """POSITIVE CONTROL for _runner_with_targets.
+
+    Every mutation test below rests on this rewrite landing. If the regex
+    stopped matching, the copies would silently be the UNMODIFIED runner and the
+    'known-bad state' tests would be exercising the real target list instead --
+    passing or failing for reasons that have nothing to do with the case they
+    claim to cover. So prove the rewrite both applies and takes effect.
+    """
+    runner = _runner_with_targets(tmp_path, ["scripts/tests"])
+    text = runner.read_text()
+    assert "scripts/dl-router/tests" not in text.split("HERMETIC_TARGETS=(")[1].split("\n)")[0]
+    proc = _run([str(runner), "--check-targets", str(REPO_ROOT)])
+    assert proc.returncode == 0, f"{proc.stdout}\n{proc.stderr}"
+    assert "all 1 hermetic target(s) resolve" in proc.stdout, (
+        f"the copied runner did not use the rewritten 1-entry list.\n{proc.stdout}"
+    )
+
+
+# --------------------------------------------------------------------------
+# REGRESSION: hole 2 -- the pytest module precondition.
+# --------------------------------------------------------------------------
+
+def test_python_is_a_required_tool():
+    """The runner invokes `python -m pytest`; it must assert `python`.
+
+    origin/main listed `python3` -- a binary this script never calls.
+    """
+    src = RUN_TESTS.read_text()
+    m = re.search(r"^REQUIRED_TOOLS=\((.*?)\)", src, re.M | re.S)
+    assert m, "could not find REQUIRED_TOOLS in run-tests.sh"
+    tools = m.group(1).split()
+    assert "python" in tools, (
+        f"REQUIRED_TOOLS does not include `python`, but the runner calls "
+        f"`python -m pytest`. Got: {tools}"
+    )
+
+
+def test_missing_pytest_module_is_named(tmp_path):
+    """RED at origin/main. A pytest-less environment must fail with ONE named error.
+
+    Drives the real runner under a PATH whose `python` cannot import pytest, and
+    asserts the failure names the MODULE -- not pytest's output format. The
+    negative assertion is the point: at origin/main this run produced 17 copies
+    of 'could not parse pytest's summary', a diagnosis pointing at the wrong
+    subsystem entirely.
+    """
+    # A `python` shim that exists and runs but has no pytest.
+    #
+    # 🔴 Written via testlib.mockbin.write_exec, NOT by hand. The first draft
+    # wrote its own `#!{shutil.which("bash")}` shebang and the repo-wide runtime
+    # -shebang scanner (scripts/tests/test_runtime_shebangs.py, added by #306)
+    # failed it IN THE SANDBOX ONLY — the dev-host run of this file alone was
+    # green, because the scanner lives in a different file. Exactly the two-tier
+    # lesson: a per-file dev-host run cannot see a repo-wide guard.
+    bindir = tmp_path / "bin"
+    bindir.mkdir()
+    write_exec(
+        bindir / "python",
+        'if [ "$1" = "-m" ] && [ "$2" = "pytest" ]; then\n'
+        '  echo "python: No module named pytest" >&2; exit 1\n'
+        "fi\n"
+        "exit 0\n",
+    )
+
+    env = dict(os.environ)
+    env["PATH"] = f"{bindir}{os.pathsep}{env.get('PATH', '')}"
+
+    proc = _run([str(RUN_TESTS), str(REPO_ROOT)], env=env)
+    out = proc.stdout + proc.stderr
+
+    assert proc.returncode == 2, (
+        f"expected the precondition to abort with exit 2, got {proc.returncode}.\n{out}"
+    )
+    assert "python -m pytest` is not runnable" in out, (
+        f"the failure did not name the pytest MODULE precondition.\n{out}"
+    )
+    # THIS guard's own reason, not a neighbour's: the old, misleading message
+    # must be gone, and no suite may have been attempted.
+    assert "could not parse pytest's summary" not in out, (
+        "the runner still blames pytest's OUTPUT FORMAT for a missing "
+        f"dependency.\n{out}"
+    )
+    assert "=== pytest " not in proc.stdout, (
+        f"the runner started a suite despite the precondition failing.\n{proc.stdout}"
+    )
+
+
+# --------------------------------------------------------------------------
+# REGRESSION: hole 3 -- set -u vs. an empty array.
+# --------------------------------------------------------------------------
+
+def test_empty_arrays_are_initialised_assigned():
+    """RED at origin/main. `declare -a foo` is unset under `set -u`.
+
+    Pinned as source structure rather than behaviour because reproducing it
+    needs a full zero-skip run of the real suite. The mechanism is exact and
+    measured (bash 5.3.15): `declare -a A; echo ${#A[@]}` under `set -u` aborts
+    with 'A: unbound variable', while `A=()` does not.
+    """
+    src = RUN_TESTS.read_text()
+    offenders = re.findall(r"^\s*declare -a (\w+)\s*$", src, re.M)
+    assert not offenders, (
+        f"{offenders} are declared with bare `declare -a`, which leaves them "
+        "UNSET under `set -u`; the first ${#arr[@]} on a still-empty array "
+        "aborts with 'unbound variable'. Use `NAME=()` instead."
+    )
+    for name in ("RESULTS", "SKIP_LINES"):
+        assert re.search(rf"^{name}=\(\)\s*$", src, re.M), (
+            f"{name} is not initialised with an explicit `{name}=()`"
+        )
+
+
+def test_set_u_empty_array_mechanism_holds():
+    """POSITIVE CONTROL for the test above.
+
+    The source-structure pin is only meaningful if the bash behaviour it
+    describes is real on the bash actually running the gate. Assert BOTH halves
+    -- `declare -a` fails, `=()` succeeds -- so this cannot pass on a bash where
+    neither form errors and the pin above would be guarding nothing.
+    """
+    bad = subprocess.run(
+        ["bash", "-c", 'set -uo pipefail; declare -a A; echo "${#A[@]}"'],
+        capture_output=True, text=True,
+    )
+    good = subprocess.run(
+        ["bash", "-c", 'set -uo pipefail; A=(); echo "${#A[@]}"'],
+        capture_output=True, text=True,
+    )
+    assert bad.returncode != 0 and "unbound variable" in bad.stderr, (
+        "`declare -a A` + `${#A[@]}` did NOT error under set -u on this bash; "
+        f"the pin above may be guarding a non-issue here.\n{bad.stderr}"
+    )
+    assert good.returncode == 0 and good.stdout.strip() == "0", (
+        f"`A=()` did not behave as the fix requires.\n{good.stderr}"
+    )
+
+
+# --------------------------------------------------------------------------
+# REACHABILITY for pre-existing guards (NOT regression coverage for this PR).
+# --------------------------------------------------------------------------
+
+def test_an_empty_target_directory_is_loud(tmp_path):
+    """A target that EXISTS but holds no tests must fail, not pass.
+
+    GUARD 5 accepts it (the directory is real), so the only thing standing
+    between an emptied suite and a green gate is the per-directory
+    ``collected < 1`` floor. Nothing had ever proven that path could fire.
+    """
+    empty = tmp_path / "empty_tests"
+    empty.mkdir()
+    runner = _runner_with_targets(tmp_path, [str(empty)])
+    proc = _run([str(runner), str(REPO_ROOT)])
+    out = proc.stdout + proc.stderr
+    assert proc.returncode != 0, (
+        f"an EMPTY target directory produced a passing run.\n{out}"
+    )
+    assert "collected 0 tests" in out, (
+        f"the run failed but not for the empty-directory reason.\n{out}"
+    )
+    assert "RESULT: FAIL" in proc.stdout, out
+
+
+def test_a_typod_target_entry_is_loud(tmp_path):
+    """A misspelled target must be named, not silently dropped."""
+    typo = "scripts/dl-rooter/tests"
+    runner = _runner_with_targets(tmp_path, [typo])
+    proc = _run([str(runner), "--check-targets", str(REPO_ROOT)])
+    out = proc.stdout + proc.stderr
+    assert proc.returncode == 2, f"a typo'd target did not abort GUARD 5.\n{out}"
+    assert typo in out, f"GUARD 5 failed but never named {typo!r}.\n{out}"
+    assert "does not exist" in out, f"GUARD 5 named {typo!r} but not why.\n{out}"
+
+
+def test_a_failing_run_never_exits_zero(tmp_path):
+    """`RESULT: FAIL` and a zero exit must be impossible.
+
+    The report that motivated this PR said a bare worktree run printed
+    ``RESULT: FAIL`` and exited 0. That did NOT reproduce (measured: exit 1),
+    and the structure forbids it -- ``RESULT: FAIL`` is printed only when
+    ``fail != 0`` and the very next statement is ``exit "$fail"``. The most
+    likely origin is reading the status through a pipeline (``| tail``), which
+    yields the LAST command's status, not the runner's.
+
+    This pins the invariant end-to-end anyway, against a run forced red by the
+    empty-directory case, so the claim can never quietly become true.
+    """
+    empty = tmp_path / "empty_tests"
+    empty.mkdir()
+    runner = _runner_with_targets(tmp_path, [str(empty)])
+    proc = _run([str(runner), str(REPO_ROOT)])
+    if "RESULT: FAIL" in proc.stdout:
+        assert proc.returncode != 0, (
+            "the runner printed RESULT: FAIL and exited 0 — a reassuring exit "
+            f"code over a failed run.\nexit={proc.returncode}\n{proc.stdout}"
+        )
+    else:
+        pytest.fail(
+            "could not force a RESULT: FAIL, so this invariant was never "
+            f"exercised — the test would pass vacuously.\n{proc.stdout}"
+        )