fix(gate-19): the finding count was returned as an exit status, and a byte only holds 255 - #209
Merged
Merged
Conversation
… byte only holds 255 `run_gate` returned `count`, and the bash gate reported `$?`. An exit status is one byte, so: 266 findings -> exits 10 -> "10 scenario(s) missing @e2e" 256 findings -> exits 0 -> PASS Measured on openconnector: the helper printed "266 scenario(s) without a running e2e test" on the same run the gate summarised as 10. Every count above 255 was under-reported by a factor nobody could see, and any multiple of 256 was a silent green — the failure mode this whole package exists to prevent. Two halves: * the helper clamps its status into the byte and never lets it wrap to zero while findings exist; * the bash gate reports the number the helper PRINTED, falling back to the status only when the summary line is missing. Two assertions, one of them the control: 256 findings must exit non-zero AND still say 256 in the summary; a clean spec must still exit 0, so the clamp cannot invent a failure.
rubenvdlinde
added a commit
that referenced
this pull request
Aug 8, 2026
…ation (#200) (#215) gate-57 reported `pipelinq LeadService::createLead` — a curated, spec'd MCP write tool — as an orphaned write capability. Acting on that verdict would have DELETED live code, which is the failure mode this module's own docstring already records from hydra#106 and the reason it says false positives are the primary risk of this gate. `createLead` has no syntactic `->createLead(` call site and never will. It is reached by attribute reflection (ADR-063): an `#[McpTool]` attribute on the method, the class listed by an `IMcpScannableServices` implementation, and a `registerServiceAlias('OCA\OpenRegister\Mcp\IMcpScannableServices::pipelinq', PipelinqScannableServices::class)` binding in Application.php. OpenRegister's AttributeToolScanner reflects the class and invokes the method. The caller index cannot see any of that. THE SEAM REQUIRES ALL THREE PIECES, mirroring what the register.d-handler and event-listener seams already demand, and it is applied PER METHOD: * a bare `#[McpTool]` on a class nobody registers stays RED * a registered class's write methods that carry no attribute stay RED * an alias naming a class that does not implement the interface grants nothing * an implementation nobody aliases grants nothing `LeadService::createInvoice` in the fixture is precisely that control: same class, same scannable registration, no attribute, still reported. THE gate-64 SHAPE, CHECKED AS ASKED, AND PRESENT gate-64's `has_prelude()` grepped a quoted string literal, so it missed every constant and matched inside comments — failing in both directions at once. The attribute seam would have had the comment half of that for free: pipelinq's real LeadService.php opens with * Both public entry points are annotated `#[McpTool]` (OpenRegister ADR-063 on line 7, seven lines above anything executable. A raw-text search matches that sentence. So the seam reads a comment-blanked copy of the file, and the walk-up from the method declaration only ever sees executable lines. `_blank_php_comments()` preserves length and line numbers, is string-aware, and knows that `#` opens a line comment in PHP while `#[` opens an ATTRIBUTE — blanking those would delete the thing being looked for. The two PRE-EXISTING seams had the same latent shape: a commented-out `registerEventListener(...)` or a `<!-- <job>…</job> -->` exempts a WHOLE class, which is a false GREEN. Both now strip comments first. MEASURED before changing: a sweep of the eight repos under this gate found ZERO comment-only matches, so this changes no verdict today — it closes the hole rather than reacting to it. Both directions are tested: a commented registration must not exempt, a live one must still exempt. THE COUNT-AS-EXIT-STATUS BUG (#209) IS NOT PRESENT HERE This helper prints one line per finding and always returns 0; the bash gate counts the printed lines. There is no count anywhere near the exit status. Asserted explicitly now so it cannot drift into one. MEASURED, OLD vs NEW, over lib/Service/** in the eight repos at origin/development: openconnector 1 -> 1 scholiq 0 -> 0 pipelinq 5 -> 4 nldesign 0 -> 0 shillinq 19 -> 19 softwarecatalog 0 -> 0 launchpad 2 -> 2 portaliq 0 -> 0 Exactly one finding removed — the documented false positive — and every other line byte-identical. The gate did not go quiet. 22 new tests alongside the suite's existing 21 (43 total, all green). Ten mutations of the changed predicates were each killed: the seam exempting the whole class, the seam removed, comment blanking disabled, the implements-interface requirement dropped, the attribute boundary reverted to the word-based form that `#[McpTool(handler: Foo::class)]` defeats, the alias requirement dropped, the attribute searched file-wide, the interface name blanked so any alias counts, the XML comment strip removed, and `#[` treated as a comment. Also records in the module docstring what the issue's second point asks for: a zero-caller write capability has TWO remedies, and the finding text only suggests one. softwarecatalog's `publishEntryForFederation()` had zero callers while `PublicationService::publish()` served the same capability through a live route — wiring the orphan would have duplicated a live endpoint and widened the auth surface. Deletion was correct (softwarecatalog#447). Closes #200 Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
This was referenced Aug 8, 2026
Merged
rubenvdlinde
added a commit
that referenced
this pull request
Aug 8, 2026
…, #239, #244) (#249) Gate-19 is the highest-volume gate in the fleet. Its three open false-positive issues were three symptoms of one decision — reading JavaScript with regular expressions — and all three surfaced as the same sentence, "referenced only by a test that never runs", about tests that ran and PASSED in the same CI run. #234 A TRAILING COMMA before the closing paren. The body was located by stepping back from `)` over whitespace and requiring a `}`. Prettier's default and ESLint's `comma-dangle: always-multiline` put a `,` at exactly that index, so the body read as "" and the empty-body rule fired on a real, asserting test. #239 A CONDITIONAL `test.skip(true, reason)` inside an `if` guard. The discriminator was the ARGUMENT alone, but `true` is just Playwright's "skip from this point" shape — the CALL SITE carries the condition. 111 guarded call sites in the fleet against 4 genuinely unconditional ones. Worse, the remedy the gate prints is "replace the tag with @e2e exclude", so complying DELETED a true coverage claim. #244 A TAG WRITTEN INSIDE THE `test(` ARGUMENT LIST. Tag resolution only ever searched FORWARD, so a tag between the open paren and the title bound to the NEXT test in the file. On nldesign that mis-binding then met #234 on whichever test it landed on, and 34 of 190 findings came out. Two defects, one symptom — which is why the fixture asserts the BINDING and not only the count. So the file is tokenised once (comments, string contents, template contents and regex literals blanked; string delimiters kept, because "is the first argument a string literal" is the whole difference between `test.skip('t', fn)` and `test.skip(cond, 'reason')`), and a real tree of test/describe calls is built with header and body ranges. Structure questions are answered from that tree. Everything the old regexes had earned is kept and re-asserted: `rx.test(` is not Playwright, `latest(` merely ends in a name, `test.describe.skip(` must match (#212 — NOT undone), `.only`/`.serial` are not switched-off markers, and `test.beforeEach(`/`test.use(`/`test.step(`/ `test.describe.configure(` are not declarations at all. SIGNALLING. This gate returned its finding COUNT as an exit status — a byte — so 266 findings left as 10 and 256 would have left as 0, i.e. PASS (#209). The clamp that fixed the wrap made the byte carry NEITHER: a 404-finding run exited 255 while stdout said 404 (#242). The byte is now a status and nothing else — 0 pass, 1 fail, 2 error — and the count is on stdout, where the runner already reads it. A crash now reports SKIPPED (wiring), visible to --require-full-coverage, instead of a fabricated verdict; the runner also stops discarding the helper's stderr. NOT TOUCHED: the empty-diff `_pass` branch, which is #242's subject and is being fixed separately. MEASURED, root-commit-scoped, across 24 local checkouts: 8790 -> 8698 findings, -92, and every one of the 92 is a false positive removed. Not one finding was added anywhere. nldesign 190 -> 156 (exactly the 34 in #244); decidesk 991 -> 984; procest 1181 -> 1166; softwarecatalog 312 -> 291; openregister 799 -> 794; opencatalogi 51 -> 46; shillinq 284 -> 279. Unchanged where the dead findings are genuine: openconnector 412 (6 real `test.describe.skip`), openbuild 187 (36 real `test.skip('title', …)`), larpingapp 101 (23 real `test.fixme`), scholiq 102. PLANTED TRUE POSITIVES, against nldesign's real spec + real e2e suite after the fix: a scenario with no test at all, a scenario tagged only by a skipped test, and a scenario tagged only by an empty-bodied test are all still caught (156 -> 159), while a fourth planted scenario tagged by a real test in the nldesign trailing-comma layout is correctly not flagged. TESTS: 75 -> 107, all green, plus the 27 other helper suites and the 59 entry-point tests. Mutation-checked: reinstating the trailing-comma bug, the argument-only skip rule, the forward-only tag resolution, the header branch, and the count-as-exit-status each turn the right tests red — and a mutant that calls every ref live turns 25 tests red, which is the control that this fix did not simply widen the gate. One earlier mutant SURVIVED (deleting the header branch), proving that fixture could not see the branch it was meant to cover; a describe-header case was added that kills it. Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
This was referenced Aug 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
run_gatereturnedcount, and the bash gate reported$?. An exit status is one byte:Measured on openconnector: the helper printed "266 scenario(s) without a running e2e test" on the same run the gate summarised as 10.
Every count above 255 was under-reported by a factor nobody could see, and any multiple of 256 was a silent green — the failure mode this whole package exists to prevent.
Two halves
Tests
Two assertions, one of them the control:
256in the summarySuite: 50 passed. Full helper-suite run: 27 passed, 2 quarantined as documented, 0 failed.