fix(gates 22/25/28/49/54/61): a crash is not a finding, and a tag's position is not its owner - #377
Conversation
…osition 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.
…C2155) `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.
…ever 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.
Follow-up: measuring this on real apps found a bigger mechanism under gate-49, and my first fixture missed itThe description above says the file-header An unbounded lazy group cannot end a match at a declaration carrying a PHP attribute, because On portaliq's The fixture in the first push did not reproduce this. The plant needs an attribute-free declaration below it, or 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. The bundle now carries a third method whose only job is to be that end, and its docblock says so, so a later tidy-up does not delete the assertion. Before/after on four real appsBoth arms run back to back on the same tree against frozen package snapshots, with the tree SHA and dirty count re-read after both arms. Full scope, no base, 57–58 of 64 gates reporting. Portaliq goes Decidesk also moves on gate-25, Nothing dropped to zero anywhere. Why gate-25 does not move on openregister or opencatalogiThe sets are identical, not merely the counts, so I counted the two divergent shapes directly rather than inferring their absence — instrument positive-controlled on the fixture that has them. Across eight apps there are fourteen false-positive shapes and eight false-negative shapes. Thirteen of the fourteen are constructors, which are never routed and were never reported; the one that mattered is decidesk's. All eight false-negative shapes are openregister, seven of them routed. They are now seen as public endpoints, and every one is judged covered — which is why the count holds at 66. Traced individually, two of them have neither a Postman URL match nor a direct call, and are matched only by gate-25's looser PHPUnit fallback, where a test file naming the controller class and mentioning the method anywhere counts as coverage. So "gate-25 unchanged on openregister" is a fact about that coverage matcher, not about the binding fix: the fix widened what the gate looks at there by seven routed endpoints and the matcher absorbed all seven. I have not touched the matcher — it is a separate weakness and a separate decision. One measurement I am not quotingOpenregister's shared checkout went from four to six dirty files between my two arms — another agent working in it — so the guard I built for exactly that case fired and its runner-level row is not comparable. Its per-gate lines are identical either way, and the checker-level numbers above were single-pass reads at a pinned SHA, but I would not quote the openregister runner row as evidence. |
…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".
…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".
What this is
Six defects across six gates (22, 25, 28, 49, 54, 61). Each was reproduced with a positive control before any line changed, and each is pinned by an acceptance arm that is demonstrably able to fail against the previous package.
Three of the six are the same disease: a checker that could not run produced a statement about the source code. A gate that sends someone to fix a defect that does not exist spends the same credit as one that hides a defect that does — and it spends it on the reader, who looks, finds nothing, and learns that this gate can be ignored.
The crash class — gates 22, 25, 28
Measured on one fixture, two runs, only the interpreter varying. Every subject in that fixture is deliberately clean or deliberately singular, so the crash arm cannot hide behind a tree that was going to be red anyway.
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 genuinely zero — and zero was read as "at least one".
gate-25 could not read a count out of its log, substituted the words "an unreported number of", and blocked the build on it. The gate said, in its own verdict line, that it had not measured the thing it was failing you for.
gate-28 is the worst of the three because it names the repository. Its helper ran inside a command substitution whose pipeline value was sed's, discarding the exit status, while sed ate the traceback so the reason was never written anywhere. Zero compared files then fell through to a branch telling the repository that not one of its files carried a licence header — with both files in scope declaring EUPL-1.2.
The guard is the one this package already documents in
_helper_finished: a finding count must be measured, and the evidence that it was measured is the checker's own output. The tell differs per gate (anat <pointer>diagnostic, aFAIL — Nsummary, adeclared_files=Nline), so each gate reads its own. gate-28 additionally keeps its traceback on disk.Worth flagging for whoever picks up the rest: gate-28's own header comment was written specifically to stop that sentence being printed, and it guards the missing-helper case only. A helper that is present and crashes walks straight past it. A guard written against one way of not-running does not cover the others, and prose describing a hazard is not the same as code testing for it.
A tag bound to something it does not describe — gates 49 and 25
gate-49's method regex opened with an unbounded lazy docblock group. Lazy is not bounded: it can expand through a comment terminator and stop at a later one, and the leftmost match in a normal controller starts at the file header. So for the first method in every file the group swallowed the span from the header down to that method's own docblock, header text included. One
@throwswritten 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 is exactly one comment. Deliberately not widened: a docblock separated from its declaration by a PHP attribute did not bind before and still does not. That change would remove findings, which is the opposite direction, and it does not belong behind a fix that adds them.
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, since detecting a newly-exposed endpoint is the entire purpose of the gate. The auth lookup now walks the declaration's own preamble structurally, the way the
@contractlookup twenty lines below it in the same file already did.The finding count on that fixture stayed at one. That is worth stating plainly: this is a substitution, not a narrowing. A fix that had only quieted the false positive would have dropped it to zero and looked like progress.
A verdict that does not say which diff it came from — gates 61 and 54
gate-61 passes a base unconditionally, which is deliberate. But on a full-scope run the wrapper forwards no base, so the runner's own default is used while the preamble has already announced there is none. A head commit that touches a listener therefore produces a real blocking failure under a header denying the diff exists. Controlled on one tree, full-scope both times, only the head commit varying: touching the listener gives a failure, touching only a docs file gives not-applicable. Two answers from one tree prove the verdict is diff-derived.
The verdict does not change. The gate now names the base it judged against, on unscoped runs only — a diff-scoped run already prints the base in its preamble, and the suite asserts the note is absent there. That keeps this neutral to the in-flight full-scope-by-default decision: whichever way gate-61 is classified, its output stops being readable as a whole-tree result.
gate-54's file-scoped checks stay file-scoped and stay blocking. A banned dialect in a register you edited is yours to fix, and narrowing those checks to touched lines would be making the gate green by weakening it. What was wrong was never the verdict — it was that a one-line retitle inherits every finding in the file and the output said nothing about it, so a sweep saw base-and-head findings that were byte-identical with no way to rank them. Each finding is now labelled inherited or introduced against the base, and the verdict carries the split. When no base is known nothing is labelled and the verdict says so, rather than letting the absence of labels read as "these are all yours".
What proves it
Four new planted/clean bundles, with their UNCOVERED.md rows deleted as that ratchet requires; fixtured gates go from thirteen to seventeen. Two of them are built so a merely-quieter gate cannot pass: the exception-translation pair's two arms contain the same tag and differ only in which docblock holds it, and the contract-coverage pair's controller is byte-identical across arms so the planted arm demands more and the clean arm demands less, graded on the same file.
A new provenance suite covers what needs real git history, and the crashed-checker suite grows an arm for the three gates above — including an assertion by text, not only by verdict, that gate-28 makes no claim about files it could not read.
Both directions were measured by swapping only the package under the same fixtures. Against the previous package the new arms produce three acceptance failures and three provenance failures; against this branch they all pass. Ten existing suites were re-run green.
Notes for review
Gates 22, 25, 28, 49, 54 and 61 are implemented inside
run-hydra-gates.sh, which is being edited concurrently by others. This branch touches only those six gate blocks — no shared helper, no preamble, no coverage accounting, and notbin/hydra-gates.Two fixture errors of mine are recorded in the findings file and in the fixtures themselves, because both produced the answer I was expecting: a fixture that was not a git repository at all, and an explanatory comment that would have satisfied the very regex it was describing.
Please do not merge without a coordinator decision — this moves the verdict for eighteen apps at once.