fix(gates 19, 25, 26, 51, 52, 54, 55): the scope decision has exactly one source (#416) - #418
Merged
Merged
Conversation
… one source (#416) #378 made whole-tree the default file scope. It did not reach `pull_request`, the event that gates a merge. Seven state gates receive their base ref through the environment, and are invoked through a shape whose `else` branch spawns the checker without clearing it: if [ "${SCOPE_TO_DIFF}" = "1" ]; then HYDRA_GATE_BASE_REF="${BASE_REF}" python3 .../check_x.py . else python3 .../check_x.py . fi That `if` guards the explicit pass — gate-19's copy of the comment cites #242 and explains this exact hazard. It cannot guard the environment. The shared quality workflow exports the variable on `pull_request` and leaves it empty on `push`, so those seven silently diff-scoped themselves on pull requests while the run announced full scope, and swept the tree only on the push that happens after the merge. Measured on a clean docudesk clone, one tree, one base, full scope in both arms, only the delivery channel different: gates 19/25/26 answered NOT APPLICABLE through the environment and FAIL 396 / PASS / FAIL 6 through --base. Coverage read 56 of 65 against 59 of 65. In both arms the preamble printed the same sentence — that the delta gates judge the change set and every other gate reads the whole tree — and in one of them it was false. The fix consumes the variable where it is read and removes it from the runner's environment, so every checker is spawned into an environment that does not contain it and can receive a base only when this script explicitly supplies one. Patching the seven `else` branches would also work; this does not, because that repair has to be remembered by each gate, and gates 25, 26, 51, 52, 54 and 55 were each written with the unguarded `else` after gate-19's author had already documented why it is wrong. The environment variable remains a supported INPUT for direct runner callers. Only the ambient inheritance is removed, and the new suite asserts that distinction, because dropping the input would silently retire gates 16/29/47/48/61 for every direct caller while leaving the parity property satisfied. scripts/lib/test_gate_base_ref_delivery_channel.sh pins the property generically: for one tree and one base at full scope, the per-gate verdict set must be identical whichever channel delivered the base. It refuses to grade until a positive control proves the fixture's uncovered scenario is findable, and it separately requires gate-19 to FAIL in BOTH arms, so two arms agreeing about nothing can never satisfy it. Reverting the one-line change turns it red naming gate-19 and printing the differing pair. Blast radius, measured on all 18 core apps at their current development tips: gate-19 5,505 findings, gate-25 241, gate-26 146. These are not new findings — they are the numbers the push run has been printing all along, now arriving where they can block. gate-19 is a coverage backlog, not a defect, and its only annotation-shaped remedy is scored as positive coverage by .github#345, so mass-excluding would turn every cell green while covering nothing.
`join` and `comm` compare in their collating order; `_verdict_set` sorts numerically so the table reads 1, 2, ..., 10 rather than 1, 10, 2. With both arms emitting the same gate set the orders match and it happens to work, which is why the revert control passed. As soon as a gate is present in one arm and absent from the other the orders diverge, and `join` drops the remaining rows SILENTLY while printing `is not sorted` into the middle of the table. Demonstrated on synthetic input: with one arm carrying an extra gate, the old form reported gate-2 and lost gates 19 and 25 entirely; the amended form reports all three. A vanished gate is the WORSE of the two symptoms this block reports, so losing it from the diagnostic was the wrong failure to have.
The repo's wrapper exits 1 on findings of ANY severity, so two info/warning
notes failed the build:
SC2154 `eval "_o=\${_out_${_arm}}"` assigns _o where the analyser cannot
see it. Replaced with indirect expansion, which is also plainer.
SC2016 a literal `$HYDRA_GATE_BASE_REF` in a printf format. The column
header now names the variable without the sigil rather than buying a
suppression directive for a character no reader needs.
Verified with shellcheck 0.10.0 locally: the file is clean, and the runner's
one pre-existing SC2015 is byte-identical to main's.
This was referenced Aug 13, 2026
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
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 #416.
What was wrong
#378made whole-tree the default file scope. It did not reachpull_request— the event that gates a merge.Seven state gates take their base ref from the environment and are invoked through a shape whose
elsebranch spawns the checker without clearing it:That
ifguards the explicit pass; gate-19's copy of the comment cites#242and explains this precise hazard. It cannot guard the environment. The shared quality workflow exports the variable onpull_requestand leaves it empty onpush— so those seven silently diff-scoped themselves on pull requests and swept the tree only on the push that happens after the merge, where nothing can block.The issue named gates 19, 25 and 26. Reading the runner, the same unguarded shape is at seven sites — 19, 25, 26, 51, 52, 54 and 55 — and all seven helpers read the variable from
os.environ.The control, before anything was changed
Fresh clone of
ConductionNL/docudeskdevelopmentat272ec7a1plus one commit touching onlyREADME.md— the shape of a one-file PR. Gate package at5e73e640. Both arms full scope, baseorigin/development, one changed file. Only the delivery channel differs.--baseDiffing the two complete 65-gate verdict sets: exactly three lines differ, and they are 19, 25 and 26. The delta gates, which read the same resolved base, are unaffected by the channel.
The sharpest form of the defect is the runner's own preamble. Both arms print "The DELTA gates (16, 29, 47, 48, 61) judge that change set. Every other gate reads the whole tree." In the environment arm that sentence is false, and the log gives the reader no way to tell. Each of the three even states a true reason — the diff really did touch no spec file — but the scope it applied is not the scope the run announced.
The fix, and why it is not the
elsebranchesOne line in
run-hydra-gates.sh, immediately after the variable is consumed intoBASE_REF:unset HYDRA_GATE_BASE_REFThe runner reads the variable, then removes it from its own environment. Every checker is spawned into an environment that does not contain it, so a checker can receive a base only when this script explicitly supplies one — which is exactly what the
SCOPE_TO_DIFFbranches already do.Patching the seven
elsebranches (the remedy the issue suggested) also works. I did not choose it because that repair has to be remembered by each gate, and it already failed six times: gate-19's author wrote the#242comment explaining the hazard, and gates 25, 26, 51, 52, 54 and 55 were each subsequently written with the same unguardedelse. Under theunset, a gate written tomorrow with the identical shape is correct by construction.The invariant this establishes: the scope decision has exactly one source — the shell variables
BASE_REFandSCOPE_TO_DIFF, resolved in one place and printed.--base Xand the environment variable become two spellings of one input rather than two channels, one of them unprinted.What it deliberately does not do: it does not stop the runner reading the variable. It stays a supported input for direct callers (the builder skill, a human at a shell); only the ambient inheritance is removed. "Stop reading it" would silently retire gates 16/29/47/48/61 for every direct caller while leaving the parity property satisfied — so the new suite asserts that distinction separately.
Post-fix, same rig, same two arms: the verdict sets are identical, both report 59 of 65, both exit 7.
Side note:
scripts/lib/test_gate16_spec_coverage_scope.sh:22already asserts in prose that "gate-19's else-branch dropsHYDRA_GATE_BASE_REFand scans the whole tree". That sentence was false when it was written and is true after this change.The acceptance arm
scripts/lib/test_gate_base_ref_delivery_channel.sh— auto-discovered bytests/run-helper-suites.sh, driving the realbin/hydra-gates.It pins the property generically: for one tree and one base at full scope, the per-gate verdict set must be identical whichever channel delivered the base. A gate added tomorrow that reads the ambient variable is caught without anyone editing the file.
Three things keep it from being vacuous, because two arms agreeing that nothing is applicable is exactly the state the defect produced:
The fixture's subject is a single scenario whose title is a token appearing nowhere else in the package, so a finding about any other subject cannot satisfy the assertion, and the file carries exactly one finding.
Revert-proven. With the one-line change commented out, the suite goes red, names gate-19, and prints the differing pair:
Blast radius — measured on all 18 core apps, and it is the point
These are not new findings. They are the numbers the
pushrun has been printing all along, now arriving on the event where they can block.Fresh clones of
development, each checker run with the variable absent — which is what the runner now does. Validated against the full runner on docudesk, where the direct-checker numbers are byte-identical to the post-fix wrapper verdicts.e36dbc40be7cbcf85d4c7d61b3f1a5373b2d30cd0bd62a04f1f17df03634876f00ab159c9d96964c5fa973b0b6d4bf54f3ce45aa950742123051aa8586393c4Totals: gate-19 5,505 · gate-25 241 · gate-26 146. portaliq and zaakafhandelapp are already clean on gate-19 and stay clean.
Four of these independently reproduce figures reported earlier in the programme — decidesk 983, docudesk 396, openconnector 332, launchpad 128 — measured here from a different rig, a different clone and a direct checker invocation.
#345). Mass-excluding would turn every cell green while covering nothing. Nobody should. Every number above is also a floor, not a ceiling: exclusion inheritance (#356) only moves scenarios intoexcluded.Verification
tests/test-hydra-gates-bin.sh: 70 passed, 0 failedtests/run-helper-suites.sh: 79 passed, 2 quarantined (pre-existing and documented), 2 failed — both disproven as failures of this change:test_gate_45_to_55_acceptance.shrefuses to run whenajvis unresolvable. Reproduced identically on a pristine clone ofmainat the same SHA. Re-run withNODE_PATHat anajvinstall it is ALL GREEN on both trees — and that is the arm covering gates 51/52/54/55.test_gate_route_registration.shaborted mid-run under my own parallel load (two full 65-gate sweeps running concurrently). Isolated re-run on this branch: 47 passed, 0 failed; the pristine control likewise 47/0.test_gate_scope_matrix.sh,test_gate16_spec_coverage_scope.sh,test_gate19_coverage_credibility.shandtest_gate_acceptance_matrix.shall pass — the fixture-coverage ratchet accepted the new bundle without an entry.main's.What this does not establish
else, all four helpers read the variable fromos.environ— and they are in scope of the fix. But their whole-tree finding count is zero in every app measured, so scoped and swept are typographically identical there. Unproven behaviourally, not immune.bin/hydra-gates. The first app pull request after this lands is the confirming observation and is worth looking at deliberately.