fix(gates 3, 14): a comment is not a route, and a TODO is not a use (#422) - #445
Merged
Conversation
…422) Two more #415-class false negatives, both in the RUNNER's own inline greps. Each was reproduced on origin/main before it was touched, through the gate, with a fixture whose only evidence is a comment. gate-14 route-reachability, invariant 1 ThingController::orphanReport(): JSONResponse, no route entry -> FAIL — rule=missing-route (correct) the same tree with routes.php gaining ONE LINE `// TODO: wire up 'thing#orphanReport' once the export lands.` -> PASS <- the defect gate-3 stub-scan, caller-identity arm public function authorize(string $uid, string $id): bool { … } with $uid never referenced -> FAIL — rule=caller-identity-ignored (correct) the same method with ONE LINE `// TODO: verify $uid actually owns this object before returning true.` -> PASS <- the defect gate-14's in-code note at that grep says comment hits are "vanishingly rare". It anticipated the false POSITIVE and missed the false NEGATIVE — the direction in which the endpoint 404s at runtime and the gate says the route exists. gate-3's arm exists BECAUSE the builder's fix-mode wrote methods that accept a caller identity and ignore it (decidesk#45); a stub that DOCUMENTS what it does not do was being reported finished. A THIRD one, found while writing the fixture and not in the survey: a commented-out `'resources' => [...]` block exempted a live controller's whole CRUD quintet from invariant 1. Written as a control, the revert says it FLIPS. MEASURED ON REAL REPOS, BOTH DIRECTIONS. Full runs over procest, opencatalogi, openregister, softwarecatalog, docudesk and larpingapp, before and after: EVERY COUNT IDENTICAL. procest g3=3 g14=0 docudesk g3=0 g14=12 opencatalogi g3=0 g14=0 larpingapp g3=0 g14=0 openregister g3=0 g14=0 softwarecatalog g3=0 g14=0 Agreement is what a dead rig looks like, so both gates were given a POSITIVE CONTROL on a real repository — a COPY of docudesk, the one repo with a non-zero gate-14 baseline, whose 12 genuine findings the copy reproduces: gate-14 one comment line added to appinfo/routes.php naming 'portalSigningReceiver#viewDocument' — one of its three real missing-route findings on a live controller origin/main 12 findings -> 11 (a real one hidden) fixed 12 -> 12 gate-3 a stub service method taking $uid and ignoring it origin/main FAIL — 1 fixed FAIL — 1 (both see it) + `// TODO: verify $uid actually owns this report…` origin/main PASS (blind) fixed FAIL — 1⚠️ The gate-3 control is reported in TWO steps on purpose. Step (a) — both arms FAIL — is not the measurement; it is what makes step (b) one. Read alone, "1 = 1" looks like "the change did nothing", which is exactly what an arm that never tested anything also looks like.⚠️ STRING CONTENTS SURVIVE IN BOTH, AND IN BOTH THEY ARE THE EVIDENCE. `php_mask` keeps literals by default and that is not an oversight here: gate-14 a route name IS the literal 'thing#index' and nothing else. Blanking literals would not widen this gate, it would delete every route in the file. gate-3 PHP INTERPOLATES `"no such user: $uid"` — that is a genuine use of the parameter. Blanking literals would report correct methods as unfinished stubs. The single-quoted '$uid' that is NOT a use stays a residual false negative and is #424's; it is the fail-safe direction for a gate that accuses a method of being unfinished. Arms 3 and 6 of the new suite go red the moment somebody generalises "strings are not evidence" across this file. A MASK THAT CANNOT BE PRODUCED IS NOT A LICENCE TO GRADE RAW TEXT. Both gates now decline — SKIPPED (wiring) — when source_scope.py is missing or fails its positive control, rather than falling back to the raw file. A silent fallback is precisely the false negative being closed and it leaves no log to notice; this package has found that shape four times (#147, #245, #276, #374). gate-3's guard fires on the ARM, not the file: without it the gate would print PASS with one of its four arms switched off. Arm 7 removes source_scope.py from a copy of the package and asserts both gates decline. The shared `_php_code_copy` helper carries its own positive control, once per run: of `// $uid` and `$uid`, exactly ONE must survive. A helper that echoed its input back would put every caller straight back into the false negative, which is gate-5's rule (#147, #245) applied to the two gates it had not reached. It is NOT a second copy of gate-5's `_ra_masked_copy` — folding gate-5 into it means re-proving gate-5's suite and belongs in its own change. Invariant 2 still reads routes.php RAW. Its exposure is the OPPOSITE direction (a commented-out route name manufacturing a phantom route) and belongs with the false-positive half, #423. Changing both here would put two independent verdict changes behind one measurement. 🔴 REPORTED, NOT FIXED: the runner's `_php_code_only` (~L919) is a line-PREFIX filter, so an unprefixed interior line of a `/* */` block survives it. Reproduced: a removal note reading "this app used to return \OCA\OpenRegister\AppHost\Routes::standard($extra) … It no longer does" sets _HYDRA_APPHOST_ROUTE_TABLE=1 and injects ten canonical route names, exempting all of them from invariant 1. The comment directly above that helper already records this class — it says the first cut was raw greps and that a fixture "was EXEMPTED BY ITS OWN EXPLANATION" — and it was then repaired with a filter that only handles the comment shape that fixture used. It gates exemptions read by gates 5, 14, 30 and 56, so it needs its own change and its own before/after. TESTS. New suite test_gate_3_14_comment_evidence.sh, 9 arms; discovered by tests/run-helper-suites.sh with no workflow edit. Reverted against origin/main — via a separate `git worktree` of origin/main, so SCRIPT_DIR resolves to that tree's own lib/ — three FLIP (arms 2, 4, 5) and arms 1, 3 and 6 pass either way and are labelled CONTROLS. Arm 7 is neither: on origin/main neither gate consults the helper at all, so it exists because the fix creates the dependency. Refs #422, #415. Sibling PRs cover the PHP call-site gates and the registry/config gates; gate-38 is BLOCKED on #424 (its fix needs markup masking, and routing it into source_scope.markup_mask today would close its comment half and leave the delimiter half armed). gate-30 and gate-1 are deliberately NOT in this sweep — see the findings note.
rubenvdlinde
added a commit
that referenced
this pull request
Aug 13, 2026
…reads must stay split (#447) #445 (#422) and #434 (#339) both changed the SAME arm of gate-3, three lines apart, so they merged cleanly and the interaction never appeared in a diff. Both are now on main and the behaviour there is CORRECT — but nothing asserts it, and this is a shape that breaks silently. The arm reads two texts in one coordinate system, and only `php_mask` being line- and offset-preserving makes that legal: the body questions -> the MASK _sig / _sig_region / _body / _count ("is $uid referenced in CODE?") the exemption -> the ORIGINAL --file "$f" --line "${_line_no}" (#434's `@param … (unused)` marker, which lives in a COMMENT BY DESIGN) Repoint the exemption at the mask — a one-word edit, and the obvious "consistency" cleanup for anyone who notices the arm reading two different files — and the marker vanishes into blanks. The exemption silently stops working and procest's three GuardEvaluatorInterface implementors go red WITH NO CLOSING ACTION AVAILABLE: deleting the parameter breaks the interface and every call site, referencing it pointlessly is dead code written to satisfy a stub detector. #434's own note says neither is a fix, which is why it added the exemption in the first place. That is gate-17's `@spec exclude` trap, which #444 hit for real in the sibling gate of the same sweep: masking the body slid the reported line into the blanked docblock, `_method_docblock` walked from the wrong place, and a reason-bearing exclusion silently stopped exempting. Caught there by an anti-widening arm. This is the same arm for gate-3. CONTROL 6b — a supertype-declared `authorize(string $uid, …)` whose docblock reads `@param string $uid Current user UID (unused by this implementor).` must still be EXEMPT with the mask in place. MUTATION-CHECKED, so it is a coupling and not a comment: with `--file` pointed at `${_stub_code}` the arm reports `FAIL — 1` (9 passed, 1 failed); restored, 10 passed, 0 failed. Measured against main at 43ecb0c. No production code changes — one arm, in a suite tests/run-helper-suites.sh already discovers. Refs #422, #339, #445, #434. 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
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.
fix(gates 3, 14): a comment is not a route, and a TODO is not a use (#422)
Two more #415-class false negatives, both in the RUNNER's own inline greps.
Each was reproduced on origin/main before it was touched, through the gate,
with a fixture whose only evidence is a comment.
gate-14 route-reachability, invariant 1
ThingController::orphanReport(): JSONResponse, no route entry
-> FAIL — rule=missing-route (correct)
the same tree with routes.php gaining ONE LINE
// TODO: wire up 'thing#orphanReport' once the export lands.-> PASS <- the defect
gate-3 stub-scan, caller-identity arm
public function authorize(string $uid, string $id): bool { … }
with $uid never referenced
-> FAIL — rule=caller-identity-ignored (correct)
the same method with ONE LINE
// TODO: verify $uid actually owns this object before returning true.-> PASS <- the defect
gate-14's in-code note at that grep says comment hits are "vanishingly rare".
It anticipated the false POSITIVE and missed the false NEGATIVE — the direction
in which the endpoint 404s at runtime and the gate says the route exists.
gate-3's arm exists BECAUSE the builder's fix-mode wrote methods that accept a
caller identity and ignore it (decidesk#45); a stub that DOCUMENTS what it does
not do was being reported finished.
A THIRD one, found while writing the fixture and not in the survey: a
commented-out
'resources' => [...]block exempted a live controller's wholeCRUD quintet from invariant 1. Written as a control, the revert says it FLIPS.
MEASURED ON REAL REPOS, BOTH DIRECTIONS.
Full runs over procest, opencatalogi, openregister, softwarecatalog,
docudesk and larpingapp, before and after: EVERY COUNT IDENTICAL.
Agreement is what a dead rig looks like, so both gates were given a POSITIVE
CONTROL on a real repository — a COPY of docudesk, the one repo with a
non-zero gate-14 baseline, whose 12 genuine findings the copy reproduces:
arms FAIL — is not the measurement; it is what makes step (b) one. Read
alone, "1 = 1" looks like "the change did nothing", which is exactly what an
arm that never tested anything also looks like.
php_maskkeeps literals by default and that is not an oversight here:gate-14 a route name IS the literal 'thing#index' and nothing else.
Blanking literals would not widen this gate, it would delete every
route in the file.
gate-3 PHP INTERPOLATES
"no such user: $uid"— that is a genuine use ofthe parameter. Blanking literals would report correct methods as
unfinished stubs. The single-quoted '$uid' that is NOT a use stays
a residual false negative and is #424's; it is the fail-safe
direction for a gate that accuses a method of being unfinished.
Arms 3 and 6 of the new suite go red the moment somebody generalises "strings
are not evidence" across this file.
A MASK THAT CANNOT BE PRODUCED IS NOT A LICENCE TO GRADE RAW TEXT.
Both gates now decline — SKIPPED (wiring) — when source_scope.py is missing or
fails its positive control, rather than falling back to the raw file. A silent
fallback is precisely the false negative being closed and it leaves no log to
notice; this package has found that shape four times (#147, #245, #276, #374).
gate-3's guard fires on the ARM, not the file: without it the gate would print
PASS with one of its four arms switched off. Arm 7 removes source_scope.py from
a copy of the package and asserts both gates decline.
The shared
_php_code_copyhelper carries its own positive control, once perrun: of
// $uidand$uid, exactly ONE must survive. A helper that echoedits input back would put every caller straight back into the false negative,
which is gate-5's rule (#147, #245) applied to the two gates it had not reached.
It is NOT a second copy of gate-5's
_ra_masked_copy— folding gate-5 into itmeans re-proving gate-5's suite and belongs in its own change.
Invariant 2 still reads routes.php RAW. Its exposure is the OPPOSITE direction
(a commented-out route name manufacturing a phantom route) and belongs with the
false-positive half, #423. Changing both here would put two independent verdict
changes behind one measurement.
🔴 REPORTED, NOT FIXED: the runner's
_php_code_only(~L919) is a line-PREFIXfilter, so an unprefixed interior line of a
/* */block survives it.Reproduced: a removal note reading "this app used to return
\OCA\OpenRegister\AppHost\Routes::standard($extra) … It no longer does" sets
_HYDRA_APPHOST_ROUTE_TABLE=1 and injects ten canonical route names, exempting
all of them from invariant 1. The comment directly above that helper already
records this class — it says the first cut was raw greps and that a fixture
"was EXEMPTED BY ITS OWN EXPLANATION" — and it was then repaired with a filter
that only handles the comment shape that fixture used. It gates exemptions read
by gates 5, 14, 30 and 56, so it needs its own change and its own before/after.
TESTS. New suite test_gate_3_14_comment_evidence.sh, 9 arms; discovered by
tests/run-helper-suites.sh with no workflow edit. Reverted against origin/main
— via a separate
git worktreeof origin/main, so SCRIPT_DIR resolves to thattree's own lib/ — three FLIP (arms 2, 4, 5) and arms 1, 3 and 6 pass either way
and are labelled CONTROLS. Arm 7 is neither: on origin/main neither gate
consults the helper at all, so it exists because the fix creates the dependency.
Refs #422, #415. Sibling PRs cover the PHP call-site gates and the
registry/config gates; gate-38 is BLOCKED on #424 (its fix needs markup
masking, and routing it into source_scope.markup_mask today would close its
comment half and leave the delimiter half armed). gate-30 and gate-1 are
deliberately NOT in this sweep — see the findings note.