fix(ci): unbreak the pre-push gate — strict-mode leak and apm-owned settings drift

Two pre-existing failures, both red at HEAD before ADR-0020 work began,
both invisible in an ordinary local run.

RUN_TESTS_STRICT leaked from the environment into test-run-tests.sh's
fixture children. The meta-test is itself a suite the runner discovers,
so under the gate's own invocation the variable propagated outer runner
-> batch_run -> the fixture's copy of run-tests.sh, flipping it strict.
Case 10c (a deliberate control asserting a skip is tolerated WITHOUT
strict) then failed. Six further cases were silently running strict too
and asserting against the wrong stream — case 9 was matching the stderr
strict block rather than the stdout skip list it was written to check.
run_fake now spawns via 'env -u RUN_TESTS_STRICT', so fixture strictness
is a property of the case, never of how the file was launched. No
assertion weakened; run-tests.sh itself is untouched.

pretty-format-json --autofix was re-sorting apm's output on the way into
every commit. .claude/settings.json is apm-owned (ADR-0018/0019) and its
exclude list named fifteen generated manifests but not this file, so
since 2e395a4 it has been committed in a key order apm would never write
— permanent drift on a file with an empty git diff. Content was always
byte-identical; only JSON key order differed. The exclude ships in the
same commit as the corrected file because otherwise the hook re-breaks
it during staging.

apm.lock.yaml: generated_at churn, plus lint's exec_status corrected from
'deployed' to 'gated_pending_approval' — executables.allow grants only
kyberforge#1.5.0, so lint's hooks/bin are genuinely gated.

New coverage: an ambient RUN_TESTS_STRICT must not reach a fixture that
did not ask for it, and under --strict the skip report goes to stderr
only with the stdout list suppressed. Neither was pinned.
This commit is contained in:
2026-08-14 21:54:00 +00:00
parent 36ba7a18f8
commit 76075223c7
5 changed files with 122 additions and 8 deletions

View File

@@ -76,6 +76,18 @@ add_case() {
# Runs the fixture's run-tests.sh over its cases/ directory, capturing output and
# exit code separately. TMPDIR is private per run so a case script can locate the
# scratch directory run-tests.sh mktemp -d's for itself -- see case 6.
#
# RUN_TESTS_STRICT is REMOVED from the child environment, not merely left alone.
# This suite is itself discovered and run by run-tests.sh, so when the outer run
# is the gate (`bash tests/run-tests.sh --strict`, or RUN_TESTS_STRICT=1 in CI)
# the variable is exported down the whole process tree and every fixture below
# silently became a strict run. Case 10c -- the control asserting a skip is
# tolerated WITHOUT strict -- then failed, and the suite passed ad hoc while
# failing under the exact invocation the run-tests pre-push hook uses. The
# fixture's strictness must be a property of the case, never of how this file
# happened to be launched, so strict is opted into per case: `--strict` on the
# argv (cases 10, 10d, 10f, 10h) or an explicit RUN_TESTS_STRICT=1 on the one
# invocation testing the env var (case 10b). Case 10g pins the scrub itself.
FAKE_OUT=""
FAKE_RC=0
run_fake() {
@@ -84,7 +96,8 @@ run_fake() {
priv="$(mktemp -d)"
FIXTURES+=("$priv")
FAKE_RC=0
FAKE_OUT="$(TMPDIR="$priv" TEST_DIR="$dir/cases" bash "$dir/tests/run-tests.sh" "$@" 2>&1)" || FAKE_RC=$?
FAKE_OUT="$(env -u RUN_TESTS_STRICT TMPDIR="$priv" TEST_DIR="$dir/cases" \
bash "$dir/tests/run-tests.sh" "$@" 2>&1)" || FAKE_RC=$?
}
# --- 1. A healthy run is green, runs the bats leg, and says so ---
@@ -524,6 +537,73 @@ else
fail "--strict failed but lost the stderr skip reason: $FAKE_OUT"
fi
# --- 10g. An ambient RUN_TESTS_STRICT=1 must not reach a fixture that did not ask
# for it. This suite is discovered and run by run-tests.sh itself, so under the
# gate the variable is exported into every child here. That is not a hypothetical:
# `bash tests/run-tests.sh` reported 19 passed while
# `RUN_TESTS_STRICT=1 bash tests/run-tests.sh` reported this file as the one
# failure, because case 10c's deliberately-non-strict run inherited strict and
# went red. The gate was therefore RED for everyone, and the only way to see it
# was to run the gate.
#
# The variable is exported here rather than passed as a prefix on purpose: a
# prefix (`RUN_TESTS_STRICT=1 run_fake ...`) applies to the function call, and
# `env -u` inside it would strip it either way, so the prefix form cannot tell a
# working scrub from a broken one. Exporting reproduces the real leak -- the
# ambient environment this whole script runs in -- which is the state that broke.
echo ""
echo "--- an ambient RUN_TESTS_STRICT=1 does not leak into a non-strict fixture ---"
export RUN_TESTS_STRICT=1
run_fake "$DIR10"
unset RUN_TESTS_STRICT
if [[ $FAKE_RC -ne 0 ]]; then
fail "an inherited RUN_TESTS_STRICT=1 turned a non-strict fixture strict — this suite's own result depends on how it was launched: $FAKE_OUT"
elif echo "$FAKE_OUT" | grep -q "a skip is a SETUP ERROR"; then
fail "the fixture ran strict despite not asking for it: $FAKE_OUT"
elif echo "$FAKE_OUT" | grep -q "^=== Summary: 1 passed, 1 skipped, 0 failed ===$"; then
pass "an ambient RUN_TESTS_STRICT=1 is scrubbed from fixtures that did not ask for strict"
else
fail "the scrubbed run produced the wrong summary: $FAKE_OUT"
fi
# --- 10h. Under --strict the skip report goes to STDERR, and the stdout skip list
# is suppressed rather than printed twice ---
# Both halves are deliberate and neither was pinned. The stream matters because
# this block is what a pre-push reader is handed: pre-commit prints nothing for a
# passing hook and shows the failing hook's output, and the runner routes the
# strict diagnostic to stderr so it survives a caller that redirects stdout. The
# suppression matters because without it the same suites are listed twice a few
# lines apart, which is exactly the noise that trains a reader to scroll past the
# list instead of reading it. Every other strict case captures `2>&1`, which
# folds the two streams together and so cannot see either property.
echo ""
echo "--- --strict reports skips on stderr only, without duplicating the stdout list ---"
DIR10H="$(make_fake_repo)"
FIXTURES+=("$DIR10H")
install_healthy_bats_runner "$DIR10H"
add_case "$DIR10H" test-needs-a-binary.sh <<'EOF'
#!/usr/bin/env bash
echo "SKIP: frobnicator is not installed"
exit 77
EOF
H_PRIV="$(mktemp -d)"
FIXTURES+=("$H_PRIV")
H_RC=0
env -u RUN_TESTS_STRICT TMPDIR="$H_PRIV" TEST_DIR="$DIR10H/cases" \
bash "$DIR10H/tests/run-tests.sh" --strict \
>"$H_PRIV/stdout" 2>"$H_PRIV/stderr" || H_RC=$?
if [[ $H_RC -eq 0 ]]; then
fail "--strict passed with a skipped suite"
elif ! grep -q "a skip is a SETUP ERROR" "$H_PRIV/stderr"; then
fail "the strict skip report is not on stderr — a caller redirecting stdout loses the only explanation of the failure: $(cat "$H_PRIV/stderr")"
elif grep -q "a skip is a SETUP ERROR" "$H_PRIV/stdout"; then
fail "the strict skip report was also written to stdout"
elif grep -q "^Skipped scripts:$" "$H_PRIV/stdout"; then
fail "--strict printed the stdout skip list as well as the stderr report — the same suites are named twice"
else
pass "--strict reports skipped suites on stderr and suppresses the duplicate stdout list"
fi
echo ""
echo "Results: $PASS passed, $FAIL failed"
[[ $FAIL -eq 0 ]]