Skip to content

feat(gates)!: scan the whole tree by default — ADR-020 superseded - #378

Merged
rubenvdlinde merged 7 commits into
mainfrom
feat/gates-full-scope-by-default
Aug 12, 2026
Merged

feat(gates)!: scan the whole tree by default — ADR-020 superseded#378
rubenvdlinde merged 7 commits into
mainfrom
feat/gates-full-scope-by-default

Conversation

@rubenvdlinde

@rubenvdlinde rubenvdlinde commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

The gates scan the whole tree by default. ADR-020 is superseded.

bin/hydra-gates defaulted to diff scoping and --full was the opt-in. That is reversed: the gates scan the entire tracked tree unless the caller asks for a diff with --scope-to-diff / --diff / HYDRA_GATE_SCOPE=diff.

Ruben's rationale, in his words:

if a gate changes or is added we want the next push to beta to fail unless the old code is fixed to the new standard. This will force developers to take updates to the gates along in their new releases.

Recorded as a decision in hydra-gates/ADR-020-SUPERSEDED.md — the numbered ADR registry lives in hydra/openspec/, so this is the gate package's own record and it is what the runner and the wrapper cite.

What it costs, stated up front

The first development → beta run after this lands surfaces the whole fleet backlog at once. The last fleet-wide wide-scope measurement was around three thousand nine hundred findings, and that number is a floor — it predates several gate tightenings, and several gates still have no fixture. Every finding it surfaces was already true; nothing about the code changed. This is a sequencing problem, not a correctness one, and it is why the PR is open rather than merged.

The scope is two inputs now, not one

SCOPE_TO_DIFF decided two unrelated things at once — which files the state gates open, and what the delta gates compare against. That conflation is the shared root cause of the two known empty-scope defects: one gate swept the whole tree while another fell back to a hardcoded origin/development and reported a pass over nothing, and a third declined citing a diff the run had never computed. One package, one question, two answers.

input controls default
file scope which files the state gates open full
delta base what the delta gates compare against resolved whenever possible

Five gates ask what a change did and cannot be answered by a checkout: spec-coverage, gitignore-then-commit, security-change-has-tests, csrf-cochange and listener-work-placement. They are keyed on whether a base was resolved, not on the file scope — so a pull request keeps every delta gate and gains whole-tree state coverage. A dispatch has no base and those five decline by name, with a reason, and are never counted as passing.

BASE_REF no longer carries a hardcoded default. An unresolvable base stays fatal on a diff-scoped run and is non-fatal at full scope, where it costs five gates rather than all sixty-four. An unrecognised HYDRA_GATE_SCOPE is refused rather than guessed at.

Eighteen gates could report a pass over a scope they never opened

The linked issue enumerates sixteen. Sweeping the whole table found two more — route-reachability and or-objectservice-api — both of which count findings where they needed to count inspections. That is the search worth reusing: a findings counter standing in for an inspection counter is this defect's signature.

Full scope removes the empty set in the common case but leaves the fall-through latent, so it is fixed at the fall-through, through one shared helper, rather than routed around. The form-label-association gate carried an inline note arguing against exactly this change; both of its claims were refuted by the same run and it is deleted with the fix. The custom-widget ratchet's uncomputed half and the effective-manifest gate's uncomputed orphan-promotion half now say so on stdout instead of hiding behind a pass.

A second scope defect, in a different mechanism

APP_DIR was never absolutised before the runner's cd, and one gate hands ${APP_DIR} to its checker afterwards — so the scan root resolved a second time, against the app dir itself. On a fixture holding exactly one pass-through wrapper, the absolute path found it and the relative path did not. CI was never exposed, because the wrapper absolutises; the invocation the runner's own header documents for humans was. The path is now resolved once, early, and the resolved absolute value is printed.

Tests

The acceptance suite enforced the empty-scope property for seven gates by name, which is how eighteen others carried it for months. It now asserts the property itself, gate-agnostically, over a tree carrying planted defects — with an allowlist that may only shrink and whose every entry states what that gate actually computed. It also gains a relative-path arm, which the standing acceptance driver is structurally unable to express because it always builds absolute paths from the package root.

Both new arms were driven against deliberately broken code first, with the breakage verified to have landed before the run, and both went red naming the right gate.

Three existing suites encoded the old default. None was weakened: the gate-16 scope suite gained a second arm proving the delta contract survives a wide file scope; the 56–61 suite's reason check was tightened with a length floor alongside the token list; the entry-point suite declares the diff contract once at the top and gains a new final section that unsets it and asserts the default.

Helper suites: 74 passed, 2 quarantined, 1 failed — before and after. The failure is the pre-existing unresolvable-ajv needs-external skip, which fails closed locally and is green in CI's package-invariants job. Entry-point tests: 61 passed before, 69 passed after, zero failures either side. ShellCheck clean on every changed file.

Do not merge this alongside anything else

It changes the verdict for eighteen apps simultaneously, and the next fleet measurement has to be attributable to exactly one cause. Capture a wide-scope baseline per app first — the standing development → beta PR already runs a wide-scope job and its log carries the full table, so no dispatch is needed, and dispatching one cancels that very run.

Expect the coverage line, not the findings, to be the loudest change: a pass that was really an unopened scope now reports as not-applicable, so COVERAGE: N of 64 drops in some repos. That is the line getting more honest.

Refs: #374, #361, #347, #364, #371


⚠️ This PR changes a REQUIRED check — read this before merging

.github/workflows/hydra-gates-package.yml is edited here, and that file backs the
required Package invariants check. The step "Unresolvable base ref must exit 99 with
no green"
encoded ADR-020's rule as the only acceptable behaviour, so this PR could
not merge: under the new default an unresolvable base no longer aborts, it falls back to
full scope.

The old assertion is kept byte-for-byte and simply run in the mode it was always
about — --scope-to-diff. Every clause of it still holds there: with an explicit diff
request and no usable base the runner must still exit 99, print NOTHING WAS CHECKED, and emit no gate lines. Nothing was weakened; the assertion was moved to
the mode whose contract it states.

A second step covers the new default, because otherwise the full-scope path would
have no assertion at all:

  • the run must say the base was unresolvable
  • it must still emit gate lines
  • none of gates 16 / 29 / 47 / 48 / 61 may print PASS over a scope never read

Positive-controlled four ways against this runner's real captured output: injecting
[gate-16] … PASS fails it, deleting a [gate-47] line fails it, and the diff-mode arm
fails if the 99 / NOTHING WAS CHECKED behaviour regresses.

Evidence for the scope change itself

measurement result
test_gate_scope_matrix.sh on this runner PASS
the same suite on main's runner 9 failures — it discriminates
real before/after on one fixture, only the package varying 14 gates move PASSNOT APPLICABLE (17, 18, 20, 21, 34–40, 42, 43, 44)

⚠️ 14, 22, 41 and 52 are NOT refuted by that fixture — it has no lib/Controller, no
manifest and no templates/, so those decline for a repo-level reason on both arms.
Their evidence comes from the scope-matrix suites, not from the table above.
gate-15 survives deliberately — that is not a regression.

The gates now scan the ENTIRE tracked codebase. Diff scoping becomes opt-in
via --scope-to-diff / --diff / HYDRA_GATE_SCOPE=diff.

Ruben's rationale: when a gate changes or is added, the next push to beta
should fail unless the old code is brought up to the new standard, so gate
updates travel with releases instead of applying only to code written after
them. Recorded in hydra-gates/ADR-020-SUPERSEDED.md, including what it costs:
the first development-to-beta run after this lands surfaces the fleet backlog
at once, and the last wide-scope measurement was roughly 3,900 findings.

The scope is now two independent, named inputs rather than one. File scope
controls which files the state gates open and defaults to full; the delta base
is resolved separately and is what the five delta gates compare against. Gates
16, 29, 47, 48 and 61 are keyed on whether a base was resolved, not on the file
scope, so a pull request keeps every delta gate and gains whole-tree state
coverage. Without a base they report NOT APPLICABLE by name, never PASS. An
unresolvable base stays fatal on a diff-scoped run and is non-fatal at full
scope, where it costs five gates rather than all sixty-four. BASE_REF no longer
carries a hardcoded origin/development default, which was the shared root cause
of the gate-16 and gate-61 empty-scope defects.

Eighteen gates could report PASS over a scope they never opened. Full scope
removes the empty set in the common case but leaves the fall-through latent, so
it is fixed at the fall-through through one shared helper: gates 14, 17, 18,
20, 21, 22, 34 through 44 and 52. Gates 14 and 20 were found by sweeping the
table and are not in the issue's list of sixteen. Gate 40's inline note arguing
against this was false on both of its claims and is deleted with the fix; gate
52's uncomputed ratchet half and gate 53's uncomputed orphan-promotion half now
say so on stdout instead of hiding behind a PASS.

APP_DIR is absolutised before the cd and the resolved absolute path is printed.
Gate 17 was the only gate handing APP_DIR to its checker afterwards, so a
relative app-dir made the scan root resolve a second time against the app dir
itself: same tree, absolute path FAIL 1, relative path PASS. CI was never
exposed because bin/hydra-gates absolutises; the documented human invocation
was.

The acceptance suite now enforces the empty-scope property across the package
instead of for seven named gates, and carries a relative-path arm the standing
driver is structurally unable to express because it always builds absolute
paths. Both new arms were proven able to fail by deliberate breakage.

Helper suites 74 passed / 2 quarantined / 1 failed before and after, the
failure being the pre-existing unresolvable-ajv needs-external skip that fails
closed locally and is green in CI. Entry-point tests 61 passed / 0 failed
before, 69 passed / 0 failed after.

Refs: .github#374, #361, #347, #364, #371
The Options block still said --scope-to-diff was the mode and named
origin/development as the base default. Both are gone: full scope is the
default, and BASE_REF starts empty because that hardcoded default is the
shared root cause of .github#347 and #361.
…ented a count

Two repairs, both from the same rule: a crashed checker must never read as an
empty scope, and an empty scope must never read as a pass.

gate-61 — the first draft of the delta-base change skipped the checker
invocation entirely when no base was resolved and short-circuited to the
empty-scope status. Nothing ran, so nothing could crash, and
test_gate_crashed_checker_is_not_a_finding.sh caught it immediately: with a
python3 that cannot run, gate-61 reported NOT APPLICABLE where it must report
SKIPPED (wiring). That is the same regression #364's first draft made in
gate-16, caught by the same suite, and the repair is the same one: always run
the checker, evaluate wiring first and scope second. With no base the helper is
now invoked in its whole-tree mode purely to learn whether it can run, and its
findings are discarded — sweeping on a baseless run was tried before and
reverted because the builder runs that way.

gate-24 — the parity wrapper is app-owned and declares no terminal marker, so a
wrapper that exited non-zero having written nothing parseable was reported as
one parity violation: a fabricated number with a plausible message and nothing
behind it, the same shape as gates 22, 25 and 28. The verdict deliberately
stays FAIL, because nothing here can tell a crash from a differently-phrased
finding and guessing wiring would turn a real parity failure into a green hole.
It simply stops claiming a count nobody measured.

gate-61 carries the same clamp idiom and it is left alone with the reason
written down: its wiring branch sits immediately above, so a crash cannot reach
it, and zero counted lines with a non-zero exit is format drift rather than a
dead checker.

Two suites updated, neither weakened. The crashed-checker suite now accepts
gate-17's NOT APPLICABLE on its E2BIG arm only when the terminal count marker
proves the interpreter ran over the oversized scope list, which is a stronger
assertion than the two it sits beside; its verdict regex also gained the
multi-word form that made NOT APPLICABLE parse as NOT. The route-auth suite's
gate-14 expectation moves from PASS to NOT APPLICABLE for the same reason its
gate-5 neighbour moved in August: a pass here is scoping, not absence, and that
reclassification was only half done.

Refs: .github#374, #330, #364, #379
…ther a diff existed

The gate-agnostic property — a NOT APPLICABLE may not blame a diff on a run
that computed none — is exactly right and stays. Its TRIGGER was wrong: it
keyed on --full, when the property is about whether a diff was COMPUTED. Those
are two independent inputs now, so the suite was reading the wrong one. That is
the same conflation .github#347 was, one layer up, inside the test written to
catch it. It fired on this branch and it fired correctly.

Arm 3 now removes the BASE rather than the file scope, so every assertion it
already carried holds verbatim. Arm 3b covers what used to be unreachable and
is now what every PR in the fleet runs: full file scope with a real base. There,
naming the diff is a true statement, and what has to hold instead is the
falsifiability condition whose absence made #347 unreadable for weeks — a reason
may name a diff only on a run that PRINTED the base it diffed against, so a
reader can check the claim rather than trust it. Asserted from both sides.

The advisory whole-tree sweep now runs in BOTH empty-scope branches. It ran only
on the no-base path, because that used to be the only way to reach a state where
the size of the unread backlog mattered. Full scope is the default now, so "a
base resolved and excluded every registration" is the common case, and the
ambiguity the sweep was added to remove had simply moved to the path everyone
runs: 0 of 1 and 0 of 45 still printed identically there. The whole point of the
reversal is that inherited debt stops being invisible, and leaving this gate's
backlog unstated on the common path would have exempted it from exactly that.

Refs: .github#374, #347
…supersedes

The required "Unresolvable base ref must exit 99 with no green" step encoded
ADR-020's rule as the ONLY acceptable behaviour, so this PR could not merge:
under the new default an unresolvable base no longer aborts, it falls back to
full scope.

The old assertion is kept BYTE-FOR-BYTE and simply run in the mode it was
always about (--scope-to-diff): with an explicit diff request and no usable
base, the runner must still exit 99, print NOTHING WAS CHECKED, and emit no
gate lines. Every clause of it still holds.

A second step covers the new default: the run must say the base was
unresolvable, must still emit gate lines, and none of gates 16/29/47/48/61 may
print PASS over a scope that was never read.

Positive-controlled against this runner's real captured output four ways —
injecting "[gate-16] ... PASS" fails it, deleting a gate line fails it, and the
diff-mode arm fails if the 99/NOTHING WAS CHECKED behaviour regresses.
rubenvdlinde added a commit that referenced this pull request Aug 12, 2026
… and disable the dead app:check-code (#389)

TWO DEFECTS, plus a third found on the way in.

1. quality.yml produced ZERO JOBS fleet-wide since 11:26:16Z. #385 merged from a
branch written before #383; its hunks anchored on what had become info-xml's
Checkout step, and the clean textual merge spliced two jobs into a third —
app-check-code lost its steps:, reuse swallowed info-xml's body, a stray
sparse-checkout landed inside a run: block, and the final upload-artifact ended
up with two name: keys. Every core app consumes this file at @main, so both apps
that pushed after 11:26 got a run with no jobs at all (openconnector 31592994170,
larpingapp 31592027242). An unresolvable reusable workflow never goes red on its
own; only this repo's resolve probe noticed. Note for future linting: PyYAML's
safe_load parses the broken file happily and reports twenty jobs, so a generic
YAML lint would have called it clean.

2. None of the three Nextcloud legs was in the Quality Report's needs:, which is
the only meaningful required check across the fleet. Now 16 -> 19. That is three
jobs, not five: multi-database PHPUnit is a matrix dimension of phpunit and was
already covered, and integrity:sign-app lives in release.yml, which has no
Quality Report to gate.

3. occ app:check-code WAS REMOVED FROM NEXTCLOUD AFTER v20, so that job cannot
pass anywhere here. Extracting registered App-namespace commands from
core/register_command.php, same expression, only the tag varying: v20 yields
CheckCode alongside the others, v34 yields the others without it — v20 is the
control proving the extraction can produce the token. Corroborated on a second
artefact: core/Command/App/CheckCode.php is 200 at v20/v21 and 404 at v25 through
v34, while ListApps.php is 200 at every one of those tags. Every app here
declares min-version 31+, and the job's positive control carries an unconditional
exit 1 that sits above the check-code-blocking branch — so merging it into needs:
would have hard-failed all 18 PHP apps on an impossible check. enable-check-code
now defaults false, with the job left wired and still in needs: (a skipped result
is skipped, not failure) so the wiring is right when someone rebuilds the check.
The positive control was correct throughout: it refused to certify a run that
inspected nothing.

BLAST RADIUS, re-measured rather than quoted. info-xml has no blocking flag and
gates on arrival. Against every repo's live development tip: 6 pass, 12 fail —
not #383's 5/13. The difference is larpingapp, reordered by efa6aead at
11:28:48Z; that one file validates FAIL at the old ref and PASS at the new one.
Eleven of the twelve are xs:sequence element order; the twelfth is an over-length
summary. All are a manifest edit, not a code change. That work is not filed
anywhere yet.

reuse and info-xml are both proven to work — reuse passes in this PR's own
resolve probe, and info-xml produced a real schema error on openregister run
31591669849 that a local validator reproduces byte for byte. Only app-check-code
was dead. From needs: all three looked identical.

Also corrected: the queue was not held by a red guard. Six of the seven open PRs
carry a Shared-workflow guard pass from BEFORE the break — stale greens that
would flip red on any re-run. Only #378 is measurably blocked, by both defects.

check-code-blocking and reuse-blocking are untouched. Neither was asked to move.
@rubenvdlinde
rubenvdlinde merged commit 4c37371 into main Aug 12, 2026
34 checks passed
@rubenvdlinde
rubenvdlinde deleted the feat/gates-full-scope-by-default branch August 12, 2026 13:14
rubenvdlinde added a commit that referenced this pull request Aug 12, 2026
…erdict

This PR merges into #378 textually clean, and that was the hazard. Three
corrections, none of which the merge could have made:

1. THE PROVENANCE WORDING IS NOW FALSE. The note said the base was "the
   runner's own default — --full forwards no base". Under #378 there is no
   runner default: full scope IS the default, and a delta base is NAMED by
   --base or $HYDRA_GATE_BASE_REF (or resolved from the remote's default
   branch). Both sentences reworded to say what actually happened. The
   mechanism was right; only its explanation had gone stale.

2. gf_verdict READ THE NOTE INSTEAD OF THE VERDICT. It was
   `grep "^\[gate-N\] " | head -1`, and this PR's NOTE prints BEFORE the
   verdict with the same prefix. Every gf_verdict-based assertion on gate-61
   therefore read an advisory line where it expected PASS/FAIL.

   ⚠️ gate-61 was NOT quiet — it emitted
   `[gate-61] listener-work-placement: FAIL — 1 post-event listener(s) ...`
   on the same run. Verified by dumping every `[gate-61]` line the runner
   produced. The gate was correct and the INSTRUMENT was blind, which is the
   more dangerous of the two because the suite reports it as the gate's
   silence. Advisory forms (NOTE/WARN/INFO) are now skipped.

3. test_gate_diff_provenance.sh's CONTROL asserted full scope by grepping
   'Base ref: n/a'. #378 reworded that preamble AND split the two facts apart:
   a run is now full-scope independently of whether a delta base resolved, so
   "reports no base" no longer means "is full-scope". It reads SCOPE-MODE now,
   which is what the control always meant.

Attribution measured, not assumed: both suites pass on this PR's own head
(pre-merge, exit 0) and fail on the merge result — an interaction, not a defect
in either side. gf_verdict positive-controlled three ways: verdict-after-NOTE
returns the verdict, verdict-alone is unchanged, NOTE-alone returns empty
rather than a false verdict.

Full helper suite on this branch: 80 discovered, 78 passed, 2 quarantined,
0 failed, "ALL discovered helper suites PASSED".
rubenvdlinde added a commit that referenced this pull request Aug 12, 2026
…osition is not its owner (#377)

* fix(gates 22/25/28/49/54/61): a crash is not a finding, and a tag's position is not its owner

Six defects across six gates, each reproduced with a positive control before
any line was changed, and each pinned by an acceptance arm that is proven to
fail against the previous package.

A CRASHED CHECKER THAT INVENTS A FINDING (gates 22, 25, 28)

Measured on one fixture, two runs, only the interpreter varying. All three
turned an environment failure into a claim about the source:

  gate-22 clamped a measured count of zero UP to one, so a validator that died
  on import was reported as a schema violation in a manifest that validates
  clean. node indents its stack trace, so the diagnostic count was zero.
  gate-25 could not read a count out of its log, substituted the words "an
  unreported number of", and blocked the build on it.
  gate-28 ran its helper inside a pipeline whose value was sed's, discarding
  both the exit status and the traceback, and fell through to a branch that
  told the repository not one of its files carried a licence header — while
  both files in scope declared EUPL-1.2.

gate-28's own header was written to prevent exactly that sentence. It guards a
MISSING helper and not a CRASHING one, and that gap is the bug.

The guard is the one already in this package: require the checker's own
measured output before believing a finding count, and report a wiring skip
otherwise. gate-28 additionally keeps its traceback on disk.

A TAG BOUND TO SOMETHING IT DOES NOT DESCRIBE (gates 49, 25)

gate-49's METHOD_RE opened with an unbounded lazy docblock group, so for the
first method in every file it swallowed the span from the file header down to
that method's own docblock. One @throws written once at the top of a file
silenced the first method in it, invisibly and fleet-wide; the tell was a
finding reporting line 2 for a method on line 22. The group is now tempered so
it cannot cross a comment terminator. A docblock separated from its
declaration by an attribute did not bind before and still does not — that is a
separate decision, in the opposite direction, and is not smuggled in here.

gate-25 asked the same question with a twenty-line window and was wrong in
both directions in one file in one run: it flagged an administrator-only method
by reading the previous method's attribute across a closing brace, and stayed
silent about a genuinely public untested one whose own attribute sat twenty-one
lines up behind its docblock. The silent half is the dangerous half. The auth
lookup now walks the declaration's own preamble structurally, the way the
@contract lookup beside it already did.

A VERDICT THAT DOES NOT SAY WHICH DIFF IT CAME FROM (gates 61, 54)

gate-61 judges against a base on runs whose preamble reports none: on --full
the runner keeps its own default and the gate uses it, so a head commit
touching a listener produces a real FAIL under a header denying the diff
exists. The verdict does not change; it now names the base it judged against,
whichever way the full-scope decision goes.

gate-54's file-scoped checks stay file-scoped and stay blocking — a banned
dialect in a register you edited is yours to fix. What changes is that a
one-line retitle inheriting every finding in the file now says so: each
finding is labelled INHERITED or INTRODUCED against the base, and the verdict
carries the split.

WHAT PROVES IT

Four planted/clean bundles (25, 28, 49, 54), removing their UNCOVERED.md rows.
The gate-49 pair differs only in WHICH docblock holds the tag and the gate-25
pair only in the test file, so a gate that merely got quieter cannot satisfy
them. A new test_gate_diff_provenance.sh covers what needs real history, and
the crashed-checker suite grows an arm for 22/25/28 asserting, by text as well
as by verdict, that gate-28 makes no claim about files it could not read.

Against the previous package these arms produce 3 acceptance failures and 3
provenance failures. Against this one, all pass.

* style(test): declare and assign separately in the provenance suite (SC2155)

`local x=$(...)` returns local's status, not the substitution's, so a failing
basename would have been invisible. Caught by the package's own ShellCheck job
on the first push of this branch.

* test(gate-49): pin the WORSE half — an attribute-bearing method was never inspected at all

Measuring the fix on real apps turned up a mechanism bigger than the one this
branch set out to fix, and the fixture did not reproduce it.

The unbounded docblock group could not END a match at a declaration carrying a
PHP attribute — `\s*` cannot cross `#` — so the regex kept expanding to the
next declaration it COULD end at, and because re.finditer returns
non-overlapping matches, every method in between was consumed. Not credited
with the wrong tag: never inspected.

Measured on portaliq's ContributionController: one match spans lines 167-334
and absorbs index(), inbox() and markRead(). The old regex saw 13 of that
file's 24 methods, and the eleven it could not see included every routed
#[PublicPage] endpoint in it. Across 8 apps and 413 controller files: 2,754
methods of 3,120 were visible, so 366 (11.7%) were not, 33.9% in one app. The
methods that carry attributes are exactly the network-reachable ones, so the
gate was blindest precisely where it matters.

The fixture now carries that shape, and needs a third method to do it: without
an attribute-free declaration BELOW the plant there is no later end for the
expansion to reach, the engine falls back to matching the plant directly, and
the arm passes against the broken gate. That was the first version of this
fixture and it is recorded in the file so a future edit does not undo it.

Verified in both directions: planted PASS -> FAIL — 2 naming purge(), which is
a method the old regex could not see at all; clean stays PASS, so seeing it did
not turn into a false positive.

* fix(gate-61): a NOTE that shares the verdict's prefix displaces the verdict

This PR merges into #378 textually clean, and that was the hazard. Three
corrections, none of which the merge could have made:

1. THE PROVENANCE WORDING IS NOW FALSE. The note said the base was "the
   runner's own default — --full forwards no base". Under #378 there is no
   runner default: full scope IS the default, and a delta base is NAMED by
   --base or $HYDRA_GATE_BASE_REF (or resolved from the remote's default
   branch). Both sentences reworded to say what actually happened. The
   mechanism was right; only its explanation had gone stale.

2. gf_verdict READ THE NOTE INSTEAD OF THE VERDICT. It was
   `grep "^\[gate-N\] " | head -1`, and this PR's NOTE prints BEFORE the
   verdict with the same prefix. Every gf_verdict-based assertion on gate-61
   therefore read an advisory line where it expected PASS/FAIL.

   ⚠️ gate-61 was NOT quiet — it emitted
   `[gate-61] listener-work-placement: FAIL — 1 post-event listener(s) ...`
   on the same run. Verified by dumping every `[gate-61]` line the runner
   produced. The gate was correct and the INSTRUMENT was blind, which is the
   more dangerous of the two because the suite reports it as the gate's
   silence. Advisory forms (NOTE/WARN/INFO) are now skipped.

3. test_gate_diff_provenance.sh's CONTROL asserted full scope by grepping
   'Base ref: n/a'. #378 reworded that preamble AND split the two facts apart:
   a run is now full-scope independently of whether a delta base resolved, so
   "reports no base" no longer means "is full-scope". It reads SCOPE-MODE now,
   which is what the control always meant.

Attribution measured, not assumed: both suites pass on this PR's own head
(pre-merge, exit 0) and fail on the merge result — an interaction, not a defect
in either side. gf_verdict positive-controlled three ways: verdict-after-NOTE
returns the verdict, verdict-alone is unchanged, NOTE-alone returns empty
rather than a false verdict.

Full helper suite on this branch: 80 discovered, 78 passed, 2 quarantined,
0 failed, "ALL discovered helper suites PASSED".
rubenvdlinde added a commit that referenced this pull request Aug 12, 2026
…th sides

Three hunks in run-hydra-gates.sh (gates 35 and 36) and one in UNCOVERED.md.
Both sides were fixing the same class from opposite ends, so neither side is
discarded:

gates 35 + 36 — this branch replaces the raw greps with check_markup_a11y.py,
because `grep -rnE` read the COMMENT documenting a rule as a violation of it.
main meanwhile added `.github#374`'s "count what was opened" invariant, using
`_iae_inspected` / `_tp_inspected` counters precisely because a grep hands back
no file list. The helper takes an explicit file array, so that array IS the
corpus and the count: #374's invariant is kept structurally, and the counters
it needed are gone with the greps that needed them. Noted at both sites so the
next reader does not restore a counter that no longer has a job.

The empty-scope path now goes through main's shared `_skip_empty_scope` rather
than the reason string this branch hand-wrote. That string named ADR-020 as the
rule in force ("the diff touches none under ADR-020"), and #378 made diff
scoping OPT-IN — so on every full-scope run it would have stated the wrong
cause for the skip. `_skip_empty_scope` distinguishes "narrowed with
--scope-to-diff" from "the tree contains no such file", which is the whole point
of the distinction. Keeping this branch's wording would have shipped a correct
skip carrying a false explanation — the same defect as #377's stale provenance
note, one merge later.

For gate-36 the subject string is main's "frontend source file
(...vue|js|ts|php|html)": .js/.ts are in this gate's corpus and NOT in
`_a11y_markup_files`, a distinction #374 was explicit about preserving.

UNCOVERED.md — both sides correct the same stale claim about `auth-guards/`.
Combined: main's superseding record (auth-guards is real, authn-vs-authz joins
it, and the two assert gate-7 from opposite directions) plus this branch's
general principle, which is why the old text is struck through rather than
deleted — a bundle with no expect.conf is not coverage, and the driver hard-fails
it.

Full helper suite after resolution: 80 discovered, 78 passed, 2 quarantined,
0 failed, "ALL discovered helper suites PASSED".
rubenvdlinde added a commit that referenced this pull request Aug 13, 2026
… one source (#416) (#418)

* fix(gates 19, 25, 26, 51, 52, 54, 55): the scope decision has exactly 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.

* fix(test): the failure diagnostic dropped the gates it exists to name

`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.

* style(test): make the new suite ShellCheck-clean

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.

---------

Co-authored-by: Ruben van der Linde <juan.claude@conduction.nl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant