fix(gates): the runner enabled errexit 27 times and called it a restore (#243) - #256
Merged
Merged
Conversation
…re (#243) run-hydra-gates.sh runs under `set -u` only — errexit is deliberately OFF, because a gate returning non-zero is how a gate reports findings. Twenty-seven blocks wrapped a helper call in `set +e … set -e` and read the trailing `set -e` as "restore". It is not a restore, it is an unconditional ENABLE: errexit was never on. The first offender is in gate-19, so gates 20-64 — forty-five gates — ran under an errexit the surrounding code does not expect. Two comments in the file (gate-27, gate-53) already documented the leak and worked around it locally instead of fixing it. Measured against a fixture with a `python3` that exits 127: gate-39's unguarded `python3 - "$vue" <<'PYBN'` returned non-zero, errexit was live, and the run DIED there — 37 of 64 gates emitted a verdict and 27 never executed. The abort banner does fire, so the run is not silently green; it is a whole-suite outage triggered by one checker having a bad day. - every restore site now says `set +e`, the state the script actually runs in - `_pass` / `_fail` / `_skip` re-assert `set +e` as a backstop, so a future gate that leaks cannot carry the leak past its own verdict line - scripts/lib/test_gate_errexit_discipline.sh asserts both arms, and takes HYDRA_GATES_RUNNER_UNDER_TEST so the fix can be mutation-checked against a known-bad copy without editing the shipped file Mutation-checked against the pre-fix runner at origin/main: aborts at gate-38, 37 of 64 gates report, no coverage summary. After: 64 of 64 report, no abort. A single-site mutation is NOT enough to red the behavioural arm — the remaining `set +e` sites switch errexit back off a few gates later, which is exactly why this leak survived long enough to be documented twice and fixed neither time.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #243.
The defect
run-hydra-gates.shruns underset -uonly. errexit is deliberately OFF, because a gate returning non-zero is how a gate reports findings.Twenty-seven blocks wrapped a helper call in
set +e … set -eand read the trailingset -eas "restore". It is not a restore — it is an unconditional ENABLE, because errexit was never on in the first place. The first offender sits in gate-19, so gates 20–64 — forty-five gates — ran under an errexit the surrounding code does not expect.The file already knew. Two comments work around the leak instead of fixing it:
NOTE: gates 25/26 above leave 'set -e' ENABLED, so a 'grep -c .' on an empty log would kill the script here.NOTE: 'set -e' is still enabled at this point in the script (gates 25/26 leave it on) — every command below is guarded.Reproduction
A fixture app plus a
python3shim that exits 127 — standing in for the checker being uninstalled, OOM-killed, argv-too-long, or syntactically broken. gate-39's unguardedpython3 - "$vue" <<'PYBN'returns non-zero, errexit is live, and the run dies there:[gate-38] skip-link: PASS[gate-64] …The abort banner does fire, so the run is not silently green. It is a whole-suite outage triggered by one checker having a bad day.
The fix
set +e— the state the script actually runs in_pass/_fail/_skipre-assertset +eas a backstop: every gate ends at one of these three, so a future gate that leaks cannot carry the leak past its own verdict linescripts/lib/test_gate_errexit_discipline.shasserts both a static arm (no bareset -e) and a behavioural arm (full run, crashing checker, all gates still report)Mutation check
The test takes
HYDRA_GATES_RUNNER_UNDER_TESTso the fix can be checked against a known-bad copy without editing the shipped file. Against pristineorigin/mainall seven assertions go red, with the exact measured numbers.Worth recording: a single-site mutation is not enough to red the behavioural arm — the remaining
set +esites switch errexit back off a few gates later. That is precisely why this leak survived long enough to be documented twice and fixed neither time, and why the honest mutant is the whole pre-fix file.Coordination
No overlap with #247 on any errexit line (verified:
git diff origin/main...pr247touches none of them).🤖 Generated with Claude Code