fix(gates): a checker that could not run must not judge the code (#245, #233) - #262
Merged
Merged
Conversation
#233) Two gates turned an ENVIRONMENT failure into a statement about the source. gate-17 (#245) passed the scope list as ONE argv string, `--changed-files=<404 KB>`. A single argument is capped at MAX_ARG_STRLEN — 128 KiB on Linux — regardless of ARG_MAX being 2 MB. The exec raised E2BIG, python3 never started, and the log held only the shell's "Argument list too long". `grep -c '^lib/'` then counted zero findings in that error text and the gate reported `FAIL — 0 pass-through method(s)`: a crashed checker wearing a finding count, and a count of zero at that. Measured on openregister: the root-scoped file list is 404,828 bytes across 7,224 files, over 3x the limit, and the pre-fix baseline prints exactly that `FAIL — 0` line. After the fix the same run reports a real PASS having inspected all 7,224 files. - the scope list now travels in a FILE (`--changed-files-file`), which has no size ceiling - a run that never prints its terminal `# count=` marker is reported as SKIPPED (wiring), never as a finding count it did not measure gate-60 (#233) reported 43 confident FAILs ("Calendar does not exist") when node_modules was absent. That was fixed by guarding the existence check on `available is not None` — which swapped it for the OPPOSITE error: the rule silently stops running and the helper returns 0, so the runner prints PASS over a check that never executed. Neither reading is honest. - the helper returns a distinct status when vue-material-design-icons is missing; the runner maps it to SKIPPED (wiring) and names the dependency - every rule that CAN run offline still runs — a Tier-A concept on a non-canonical icon is still caught, asserted as a control - the no-manifest branch reports `na` instead of PASS Mutation-checked against the pre-fix runner: gate-17 reproduces `FAIL — 0 pass-through method(s)` verbatim, and no scope file is written.
rubenvdlinde
added a commit
that referenced
this pull request
Aug 9, 2026
…ver a dead interpreter, and three could not see the defect they exist for (#280) * fix(gates 45-55): eleven gates passed over an unopened scope, eight over a dead interpreter, and three could not see the defect they exist for Every gate in this band was given ONE textbook true positive of exactly what it exists to catch, planted in a real fleet repo, then removed again. Where a gate could not fail, it was repaired; where it could, the plant is now a regression test. Measured at package sha 34370f6. ## 1. All eleven reported PASS over a scope they never opened (#242/#240/#258/#268) On a README-only diff against larpingapp, gates 45-55 printed eleven PASS lines and the summary read "53 of 53 applicable gates ran". Not one of them had opened a file. Gates 4/6/7/19/25/28/62/63 have answered the identical situation with NOT APPLICABLE since #268; this band never adopted it. Gates 47 and 48 are the sharper case: they can only answer a question about a CHANGE SET, so on every builder full-repo run in the fleet — no base ref at all — they printed a co-change verdict they had not formed. ## 2. Eight reported PASS over a crashed interpreter (#147/#249/#262) A planted defect only fires when the gate runs, so no plant can see this. With a `python3` on PATH that exits 1 on every call, on a tree carrying real findings: gate-46 PASS — over the 277 unresolved @SPEC findings, across 104 distinct targets, it had reported one run earlier on the same files gate-47 PASS — on the same diff where it had just reported FAIL gate-45/49/50 PASS (`2>/dev/null` discarded status and traceback) gate-51/54/55 PASS (`|| true` discarded the status) gate-52 FAIL — "1 custom-widget finding(s)", a fabricated finding: the helper returned its COUNT as its exit status, the same channel Python uses for a traceback (#209). The count was also clamped to 99 to fit in a byte. It now prints `findings=N` on stdout and exits boolean; no `findings=` line means the helper died. gate-54 was the quietest: its advisory WARN half reads the same log, so a dead helper silenced both halves at once. ## 3. gate-45 was the residue of #272's fix (.github#274) #272 migrated gates 35/40/42/44 off `[ -d src ]` onto `_a11y_has_markup_dir` and left the twelfth member of the family behind. On a templates-only app gate-45 reported NOT APPLICABLE — "this repo ships no frontend" — over a `<style>` block with `transition:` and no reduced-motion fallback, in the same file gate-43 FAILED on in the same run. `na` is the one verdict that removes a gate from coverage accounting. The regression test was already written and gate-45 was excluded from it by name, with a comment explaining why. Removing the name from ARM 4's skip list in test_gate_a11y_markup_scope.sh IS the test; it fails against 34370f6. ## 4. gate-47: prose satisfied it, and a qualified attribute did not `_ANNOTATION_RE` was an unanchored alternation of string literals, and it was wrong in both directions from that one regex — the pairing #269 found in gate-48 and never carried to its sibling. FALSE POSITIVE rewording ONE docblock sentence that merely NAMES the annotation ("becomes `@NoAdminRequired` again, paired with a real ownership check") made the gate demand a test co-change. A gate satisfiable by prose manufactures the appearance of a security review (#191). FALSE NEGATIVE `#[\OCP\AppFramework\Http\Attribute\NoAdminRequired]` was invisible. A commit adding exactly that to a controller — opening an admin-only endpoint to every authenticated user — with no test in the diff reported PASS. Now position-anchored, by the same rule check_csrf_removal.py already used. ## 5. gate-50: a false positive and a false negative in the same regex FALSE NEGATIVE the app-id argument had to be a QUOTED STRING, so every read written the fleet-standard way — `getValueString( Application::APP_ID, 'listing_register', '')` — was invisible. Identical code with `'larpingapp'` FAILED. Same family as #184. 7 security-relevant reads across 5 repos sit behind a constant today. FALSE POSITIVE the empty-compare guard required a closing paren immediately after the empty string, so the correct compound guard `if ($reg === '' || $sch === '')` was reported as unguarded — twice, on code the gate was asking for. A guard that is a boolean `return` rather than an `if` was rejected too. Both directions are now asserted, including the opencatalogi#86 shape that mixes them: one read guarded, the next unguarded two lines later. ## 6. gate-53 did not block the PR that creates larpingapp#286 Reintroducing #286 exactly — the check-in tab deleted from src/manifest.json, `EventRoster` left registered in src/registry.js — reported PASS. Direction 1 of the registry cross-reference stays advisory for LEGACY orphans, correctly: the gate cannot tell "wire it" from "delete it". But when the DIFF ITSELF removed the last reference it can, and that finding now blocks. Pre-existing orphans are untouched (larpingapp carries one today), so this is prevention, not a burn-down list nobody can close. ## Verified working, repaired nothing gate-46 (dangling file, dangling fragment, valid anchor), gate-48 (short and fully-qualified attribute removal; a comment reword correctly stays green), gate-49, gate-51 (title, description and nested items.properties independently), gate-52's ratchet (growth fails, shrink passes), gate-54 (flat, nested and $ref-carrying), gate-55. ## Deliberately NOT enforced `title == key` on a schema property is a real gate-51 defect — the renderer uses `prop.title || key`, so the user sees the raw technical key. Measured across 10 repos: 148 occurrences, ALL of them in softwarecatalog, where they are VNG-standardised element names (`identifier`, `type`, `name`) that must not be renamed. Enforcing it would produce 148 findings with no legitimate end state in the one repo that has them. Reported rather than gated (#252). ## Divergence to reconcile gate-45 now answers an empty in-scope set with `na`; gate-40 answers it with PASS, by a deliberate choice in #272 that cited the invariant test this PR reworks. The invariant now discriminates on the REASON — the applicability table's own phrasing must not appear once its prerequisite holds — so both behaviours are expressible. The family should pick one. ## Testing New: hydra-gates/scripts/lib/test_gate_45_to_55_acceptance.sh — 31 arms across six families, discovered by run-helper-suites.sh. Against the package as merged on main it fails 20 of 31; the 11 that pass are exactly the anti-widening and no-regression controls. Every mutation asserts its anchor is present before it plants. Repos used, chosen for different shapes: larpingapp (register-owning, manifest-driven, ships registry.js), nldesign (PHP templates, no .vue, no register), doriath (ships no phpcs SpecTagSniff — the #246 control, held at 81 findings across 46 targets before and after the plant), openconnector (41 register files). Full package suite: 52 discovered suites pass, 2 quarantined as documented; 60/60 entry-point invariants. * fix(gate-50): the fail-mode window started where the call began, not where it ended The constant-app-id fix in the parent commit made procest's config reads visible for the first time and immediately produced 3 findings on lib/Service/AiService.php — all three false positives, and both causes are ordinary code the window could never have seen: multi-line call PHPCS formats each read across five lines. Two of them plus a blank line put the guard on the ELEVENTH line, one outside a window counted from the line the match BEGAN on. The guard being missed is a textbook `if (empty($registerId) === true || empty($schemaId) === true) { $this->logger->warning(...); return; }` (AiService.php:580, :967). same-line guard `'ai_api_key_set' => ...getValueString(APP_ID, 'ai_api_key', '') !== ''` handles the empty default ON the match line, and the window started after it (AiService.php:710). The window now anchors to the END of the call expression — parentheses balanced forward from the `(` — and includes the remainder of that line. A single-line read keeps exactly the ten lines it always had. Caught by a before/after sweep of 12 fleet repos: 26 of 121 verdicts changed, 25 of them PASS -> NOT APPLICABLE (the truthfulness correction), and this was the only one that changed to FAIL. procest is PASS again, correctly. Three arms added: the multi-line shape, the same-line shape, and the reverse control — the same multi-line shape with the guard DELETED must still FAIL, so the window cannot have been widened until the gate finds nothing. Also: shellcheck SC2181 in gate-45's new status check, and a file-scoped SC2016 suppression for the acceptance suite, whose PHP fixtures are single-quoted on purpose.
rubenvdlinde
added a commit
that referenced
this pull request
Aug 10, 2026
… 19, 13, 20 (#328) * fix(gate-45): read stylesheets — the gate had never opened a .css file Gate-45 (prefers-reduced-motion) scanned <style> blocks inside markup and nothing else, so every green it has ever produced is a statement about markup, not about CSS. In a Nextcloud app the app-wide motion lives in css/, because that is what Util::addStyle() loads. Measured before this commit: nldesign 3 stylesheets with motion, 0 guards -> gate-45 PASS openregister css/main.css, 7 motion decls, 0 guards -> gate-45 PASS The un-blinding is paired with four false-positive controls, because widening a gate fleet-wide is exactly the change that turns it into a noise generator: * the guard regex now recognises a full media prelude (@media screen and (prefers-reduced-motion: reduce)), which the old 'immediately followed by (' pattern could not have matched * comments are masked (#294's lesson), // only in SCSS dialects and never the // of a url(https://...) * transition/animation: none is how a fallback is WRITTEN, not motion * a repo-wide UNIVERSAL reset in one file guards every other file, so the gate accepts the fix people will actually write rather than reporting every other stylesheet the day that reset lands * generated output is skipped by CONTENT (a >500-char line), which catches webpack's css/main-<hash>.chunk.css that has no .min in its name Fleet-wide yield measured over 40 repos: 37 new findings, 14 of them in the GitHub fleet (nextcloud-vue 8, nldesign 3, opencatalogi 1, openconnector 1, openregister 1). The existing markup arm is unchanged at 143 findings, so nothing was widened by accident. test_gate_45_stylesheet_scope.sh proves both directions and goes 3 red against the pre-fix runner. Closes #287 * fix(gate-46): resolve @SPEC targets in tests/ — the enumerator skipped them Gate-46's scope was `find lib src`. It had never opened a test file, and tests/ is where a large share of the fleet's @SPEC tags live, because a test is the natural place to name the requirement it proves. Measured over the 21 apps carrying an openspec/: 272 unresolved targets under tests/, in 16 repos, that no run has ever reported. The textbook case is procest — tests/Unit/BackgroundJob/DsoDeadlineJobTest.php annotates openspec/changes/dso-omgevingsloket/tasks.md#T14 against a tasks.md numbering T01-T08 and V01-V10. There is no T14 and never was. The identical tag in lib/ would have failed this gate since #246. Landing as a HARD FAIL, matching the existing lib/src arm. The gate is diff-scoped under ADR-020, so pre-existing debt in an untouched test file never blocks a PR — the finding surfaces on the PR that edits that file, which is exactly when the tag should be re-checked. A named WARN would reproduce the failure mode this band exists to remove: a gate emitting something other than FAIL over a real defect. openspec/ is DELIBERATELY NOT added. Measured: 292 findings, dominated by documentation TEMPLATES that quote the syntax rather than use it (openspec/changes/{name}/tasks.md#task-N, <slug>, ...) in context-briefs and proposals across shillinq, pipelinq and others. Auditing them would bury real findings under placeholders. Correction recorded in the docblock: #322 as filed reports that tasks.md targets are never existence-checked (353 on doriath). That premise does not hold on this package. A planted openspec/changes/does-not-exist-at-all/tasks.md#task-1 IS reported as 'target file not found', and #task-99999 against a real tasks.md IS reported as 'anchor not found'. The 353 all resolve through build_archive_index, which exists for the archived-under-a-date-prefix case the issue describes, and the archived file exists at the very commit the issue measured. What produced PASS there was ADR-020 diff scoping. test_gate_46_tests_scope.sh proves both directions and goes 5 red against the pre-fix runner while its lib/ arm stays green. Refs #322 * fix(gate-19): a file no Playwright project runs cannot prove a scenario The gate counted every *.spec.ts under tests/e2e/** as a running test. Playwright does not: a file excluded by testIgnore, living outside testDir, or matched by no project's testMatch is never executed, and a scenario referenced only from such a file has no automated proof at all. The gate could already see describe.skip (#239) but not the config that does the same thing to a whole directory. Reproduced by planting an @e2e anchor in a CI-ignored directory: the uncovered count dropped 271 -> 270 with the scenario reported COVERED. The live fleet shape is openregister's tests/e2e/api-direct/search-views-presentation.spec.ts, under a **/api-direct/** that is excluded at top level AND repeated in every project, because a project-level testIgnore REPLACES the top-level one rather than merging with it. A GLOB LIST WOULD HAVE BEEN THE BUG. **/visual/** and **/docs-screenshots.spec.ts sit in a testIgnore in fourteen fleet configs and are pulled BACK IN by the visual / docs-capture projects via testMatch. Treating 'named in some testIgnore' as dead would have stripped coverage credit from every visual and docs spec in the fleet. A file is dead only when NO project would run it. Validated against all 21 real fleet configs: every one parses, and the only dead files anywhere are the api-direct trees excluded on purpose (openregister 25, openconnector 6). Visual and docs specs: 0 dead. Fleet-wide finding count: ZERO. Those api-direct files carry only prose mentions of @e2e plus one whole-spec tag with no slug, so no ref is currently claimed from an unrun file. The hole is real and proven by a planted true positive; nothing in the fleet is exploiting it today, so this lands with no churn. Conservative by construction: no config, an unparsable config, an extglob/brace pattern, a non-literal testMatch or testDir all resolve to LIVE, i.e. to the pre-existing behaviour. Comments are masked first — every fleet config explains the replace-not-merge rule in prose containing 'testIgnore:'. 11 new unit tests; the 4 that assert the fix go red against the pre-fix helper while the 7 anti-widening arms stay green. Suite: 105 -> 116. Closes #308 * fix(gate-13): see a dialog tag opened across several lines The test was grep -qE '<NcModal[ \t>/]|<NcDialog[ \t>/]'. grep matches line by line, so a tag with its props on following lines — which is how Vue components with more than a prop or two are actually written, and what every formatter produces — <NcDialog :open="showConfirm" name="Delete lead"> has nothing after <NcDialog on its own line. The character class cannot match end-of-line, so the tag was invisible. Measured on pipelinq: 0 of 9 real violations seen, while the gate passed its own planted true positive the whole time — a plant is written on one line and a real dialog is not. That is the trap: a minimal plant and a real defect differing in precisely the feature the regex depends on. The delimiter is WIDENED to include end-of-line, not dropped, so <NcDialogHeader> and <NcModalFooter> still do not match. Comments are masked first, which also removes a FALSE POSITIVE the old pattern had: it reported a <NcModal> written inside a /* */ block comment. Fleet-wide finding count: 8 -> 92 files (+84), across 10 repos — nextcloud-vue 50, procest 13, pipelinq 9, doriath 6, docudesk 4, openregister 3, softwarecatalog 3, decidesk 2, app-versions 1, hermiq 1. nextcloud-vue is the shared component library and accounts for over half; its findings are dialog components sitting outside src/dialogs/ rather than modals inlined in a parent. Gate-13 is diff-scoped under ADR-020, so none of this blocks a PR that does not touch the file. Landing as a hard FAIL, unchanged from what gate-13 already is. Comment masking suppresses zero findings across the fleet today; it is here so a <!-- <NcDialog … --> in a TODO cannot become one later, which is exactly how gate-20 acquired its commented-out call (#294). The checker now reports a crashed interpreter as wiring instead of leaving an empty log this gate would call clean (#147/#249/#262). test_gate_13_multiline_dialog.sh proves both directions and goes 3 red against the pre-fix runner while 4 anti-widening arms stay green. Closes #321 * fix(gate-20): mask comments before searching — a commented-out call is not a call The first thing the un-blinded gate reported in the fleet was not a call. It was openconnector lib/Service/SearchService.php:189: // $directory = $this->objectService->findObjects(filters: [...]); grep has no idea what a comment is, and a gate whose first live finding is false is a gate people learn to ignore. Applies the pass gate-5 received in #196: source_scope.py --mask php, which blanks //, # and /* */ while PRESERVING offsets and newlines, so the reported line number still addresses the real file. #[ is left alone — it opens a PHP 8 attribute, not a comment, and swallowing it would delete #[NoAdminRequired], the line these calls sit directly under. The log now prints the ORIGINAL source line rather than the blanked mask, so a reader sees the code that is actually at that line. The mask inherits this gate's own rule: if it cannot run, the gate reports wiring and NOT a pass. Falling back to raw text would silently restore the false positive; treating empty mask output as clean would make gate-20 green everywhere — the 2026-08-08 failure mode in a new costume. Fleet-wide finding count: 2 -> 1. The one removed is openconnector's commented-out line; the one kept is shillinq lib/Controller/BookingNotificationController.php calling findObject() on a container-resolved OCA\OpenRegister\Service\ObjectService, which is the real yield #271 identified. test_gate_20_comment_masking.sh proves both directions and goes 2 red against the pre-fix runner while all four true-positive arms — including #271's receiver anchoring and the #[Attribute] non-swallow — stay green. Closes #294 * style(test): fixed-string grep so the $ needs no escape (SC2016) * style(test): escape $ inside double quotes — SC2016 fires on single quotes --------- Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
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 #245. Closes #233.
Two gates turned an environment failure into a statement about the source code — in opposite directions.
gate-17 (#245) — a crash wearing a finding count
The scope list was passed as one argv string,
--changed-files=<404 KB>. A single argument is capped atMAX_ARG_STRLEN— 128 KiB on Linux — regardless ofARG_MAXbeing 2 MB. The exec raisedE2BIG,python3never started, and the log held only the shell's "Argument list too long".grep -c '^lib/'then counted zero findings in that error text, and the gate reported:A crashed checker wearing a finding count — and a count of zero at that.
Measured on openregister: the root-scoped file list is 404,828 bytes across 7,224 files, over 3× the limit. The pre-fix baseline prints exactly that
FAIL — 0line. After the fix, the same run reports a real PASS having actually inspected all 7,224 files.--changed-files-file), which has no size ceiling# count=marker is reported asSKIPPED (wiring), never as a finding count it did not measuregate-60 (#233) — the same mistake, inverted
The gate reported 43 confident FAILs ("Calendar does not exist") when
node_moduleswas absent. That was fixed onmainby guarding the existence check onavailable is not None— which swapped it for the opposite error: the rule silently stops running, the helper returns0, and the runner prints PASS over a check that never executed.Neither reading is honest. "I could not look" is a third state.
vue-material-design-iconsis missing; the runner maps it toSKIPPED (wiring)and names the dependencynainstead of PASSTests
test_gate_crashed_checker_is_not_a_finding.sh, including a direct assertion of the mechanism itself (a 200 KiB single argv string raisesE2BIG) — because the fact the fix rests on is not obvious:ARG_MAXsays 2 MB, and the limit that bites is a different, per-argument one.Mutation-checked against the pre-fix runner: gate-17 reproduces
FAIL — 0 pass-through method(s)verbatim, and no scope file is written.One honest caveat on that mutant: it pairs the pre-fix runner with the post-fix helper, so the gate-60 arm goes red as
FAILrather thanPASS. The original 43-FAIL symptom was already fixed onmainby theavailable is not Noneguard; what this PR removes is the silent PASS that replaced it.🤖 Generated with Claude Code