fix(gates 16, 60, 65): a comment about the artefact is not the artefact (#422) - #446
Merged
Conversation
…ct (#422) Three more #415-class false negatives, in the three checkers that read a REGISTRY, a CONFIG and a TAG rather than a call site. Each was reproduced first with a fixture whose only evidence lives in a comment. gate-60 icon-vocabulary manifest uses ViewDashboardOutline, icons.js registers only Cog FAIL — 1 icon name(s) NOT registered + "// TODO: the dashboard nav row still // needs this — not wired up yet. // import ViewDashboardOutline from // 'vue-material-design-icons/…'" PASS <- the defect + registerIcons(icons) named only in an unprefixed /* */ interior line PASS <- the defect gate-65 coding-standard-adoption a fixer config with no autoloader FAIL — fixer-config-missing-autoloader + "// TODO: we still need to require // __DIR__ . '/vendor/autoload.php' // here. Not done yet." PASS <- the defect gate-16 spec-coverage an untagged public method uncovered (a finding) + "* TODO: nobody has written @SPEC openspec/specs/thing/spec.md for this yet" covered <- the defect A COMMENTED-OUT IMPORT VOUCHES FOR AN ICON THAT RENDERS BLANK. gate-60 exists for exactly one failure — CnAppNav resolves an MDI name only through the registry `registerIcons()` populates, WITH NO FALLBACK, so an unregistered name renders nothing at all: no glyph, no console error. The comment saying the import is "not wired up yet" was the whole evidence that it was. gate-65's silenced rule exists PRECISELY because a php-cs-fixer fatal reads exactly like a clean tree: with no autoloader the run dies "Class not found", and --format=json reports that as ZERO FILES NEEDING CHANGES. The comment bought a green on the check whose job is to stop a green being bought. The XML paths in that same module have been protected by strip_xml_comments() from the start — "a commented-out rule is not a rule" is its own docstring. The fixer config was the one file it read raw. MEASURED ON REAL REPOS, BOTH DIRECTIONS. after/before across procest, opencatalogi, openregister, softwarecatalog, docudesk and larpingapp: IDENTICAL — no count moves on any of the three. Agreement is what a dead rig looks like, so each gate was also given a POSITIVE CONTROL on a real repository, by planting the comment in a copy: gate-60 larpingapp, +2 comment lines in src/main.js origin/main 4 unregistered icons -> 3 (a real one hidden) fixed 4 -> 4 gate-65 procest's OWN .php-cs-fixer.dist.php, real require line deleted and a TODO naming it added origin/main 1 finding -> 0 (blind) fixed 1 -> 1 gate-16 docudesk lib/AppInfo/Application.php, one TODO line added origin/main 2 findings -> 1 (register() hidden) fixed 2 -> 2⚠️ THE MASK THAT CLOSES gate-60 WOULD, ONE KEYWORD ARGUMENT FURTHER, CLOSE THE GATE. The evidence there IS a string: the icon name lives in the module specifier 'vue-material-design-icons/ViewDashboardOutline.vue'. So it uses `js_comment_mask` and NOT `js_mask(blank_strings=True)`, and arm 4 of the suite is what makes a future "strings are not evidence" generalisation go red instead of reporting every icon in the fleet unregistered. gate-65 keeps literals for the same reason — `require __DIR__ . '/vendor/autoload.php'`.⚠️ gate-16 IS THE BORDERLINE ONE, AND MEASURING CHANGED THE FIX. `@spec` is a docblock marker: unlike every other gate in #422 the evidence legitimately lives in a comment and a mask would delete it. What was missing is POSITION — the anchoring gates 47 and 48 have and gate 46 still lacks. The first cut was a plain start-anchor. Swept exhaustively over every in-scope method in the six repos (46,187 judgements, diff scope bypassed so every line counts as changed) it produced FIVE new findings, all in decidesk's VotingRoundPanel.vue, all of this shape: /** Rule enum option lists for the open-round dialog. @SPEC openspec/specs/voting-system/spec.md */ — a REAL tag in the ordinary PHPDoc order, whose only remedy would have been to reflow correct documentation. A gate that reddens documented code teaches authors to stop documenting it, which is the false-positive half of this same class and the half the survey calls corrosive. The pattern now also admits a tag FOLLOWING A COMPLETED SENTENCE, which is what the real form is and none of the prose forms are, and the sweep re-run says ZERO new findings in all six. The residual hole is stated in the code rather than hidden: a debt sentence ENDING in a full stop immediately before the tag still counts. The lead character class was measured too, not guessed: `[ \t>*#-]` — the package's markdown `standalone` lead — would have uncovered 1,264 correctly tagged methods (626 procest + 638 opencatalogi) written `/** @SPEC … */`, so `/` is in the class. And the 109 `// @SPEC openspec/…` lines in the fleet turn out to be unaffected either way, because `_docblock_block` skips `//` lines — the expectation going in was the opposite, and arm 5 records the correction. TESTS. 14 new arms. Reverted against origin/main — not `git checkout --`, which restores the COMMITTED file — six FLIP: gate-60 arms 2, 3 gate-65 the TODO arm and the removal-note arm gate-16 arms 1, 2 and eight pass either way and are labelled CONTROLS in their own docstrings, including gate-16 arm 8, which is the arm that caught the first cut being wrong, and gate-65's "a real require of a STRING path, below a block comment, still counts". Refs #422, #415. Sibling PRs cover the PHP call-site gates and the runner-inline gates; gate-38 is BLOCKED on #424.
rubenvdlinde
added a commit
that referenced
this pull request
Aug 13, 2026
…isoning regression CodeQL scored 12 NEW HIGH alerts against this PR, `actions/cache-poisoning/poisonable-step`, all inside quality.yml and none of them in code this PR wrote. main scores 0 for `language:actions` and PRs #444, #446 and #447 were clean, so this was the new workflow and not a pre-existing pattern newly surfaced. The mechanism: quality.yml needs `contents: write` + `actions: write` at run-CREATION time, so a `fixture/**` caller hands its ~12 `actions/cache` steps an entry point reachable from a non-default branch — and Actions caches are shared with the default branch that later restores them. In a repository 23 others consume at @main, that is not worth a permanent leg counter. The measurement it existed to produce is already taken, on run 31734347530: 6 legs from info-32-34.xml, 4 from info-32-33.xml, stable34 absent from the second. The recipe for repeating it — and the two ways to get it wrong — is now documented on the `matrix-derivation` job in quality-resolve-probe.yml.
rubenvdlinde
added a commit
that referenced
this pull request
Aug 13, 2026
…#448) * feat(quality): derive the Nextcloud test matrix from appinfo/info.xml The tested range and the declared range were two hand-maintained lists, and the fleet broke the agreement between them in BOTH directions inside a single programme: 18 apps declared NC 32-34 while testing stable31/32, and then the migration that fixed it replaced the list with '["stable34"]' and 16 apps stopped testing their own declared floor. Both were detectable. Neither was detected, because the two halves lived in two files that could be edited independently. quality.yml now derives the range from appinfo/info.xml with icewind1991/nextcloud-version-matrix, pinned by SHA (8a7bac6 = v1.3.2), in a new `nextcloud-matrix` job. `nextcloud-test-refs` survives as an explicit override; leaving it unset — the new default — derives. The action's `branches` output is OLDEST-FIRST. Six sites read `fromJSON(inputs.nextcloud-test-refs)[0]` as THE single server for newman, playwright, journeydoc-capture and the coverage guard, and newest-first was deliberate there. Consuming `branches` positionally would have moved all four onto stable32 silently. Those six sites now read `needs.nextcloud-matrix.outputs.single-server`, which is the action's own `branches-max` on the derived path — explicit, not positional. gate-65 rule 11 changes in step. `test-matrix-not-declared` is retired: an app that passes no override now derives, and a derived matrix is strictly better than a declared one. It is replaced by `test-matrix-neither-derived-nor-declared`, which fires only when an app neither declares a matrix nor calls the shared workflow. An override that disagrees with info.xml still fails, both directions, exactly as before. * fix(fixture): serialise the two matrix callers — quality.yml's concurrency group cancelled the control Measured on run 31734110346: quality.yml declares `concurrency: quality-${{ github.ref }}` with cancel-in-progress, so two calls on one ref share the group and the second cancels the first. Every job under `4 legs` came back cancelled and its PHPUnit row still carried the literal ${{ matrix.nextcloud-ref }} — the control measured nothing while looking like it had run. * fix: withdraw the leg-count fixture workflow — it was a real cache-poisoning regression CodeQL scored 12 NEW HIGH alerts against this PR, `actions/cache-poisoning/poisonable-step`, all inside quality.yml and none of them in code this PR wrote. main scores 0 for `language:actions` and PRs #444, #446 and #447 were clean, so this was the new workflow and not a pre-existing pattern newly surfaced. The mechanism: quality.yml needs `contents: write` + `actions: write` at run-CREATION time, so a `fixture/**` caller hands its ~12 `actions/cache` steps an entry point reachable from a non-default branch — and Actions caches are shared with the default branch that later restores them. In a repository 23 others consume at @main, that is not worth a permanent leg counter. The measurement it existed to produce is already taken, on run 31734347530: 6 legs from info-32-34.xml, 4 from info-32-33.xml, stable34 absent from the second. The recipe for repeating it — and the two ways to get it wrong — is now documented on the `matrix-derivation` job in quality-resolve-probe.yml.
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 16, 60, 65): a comment about the artefact is not the artefact (#422)
Three more #415-class false negatives, in the three checkers that read a
REGISTRY, a CONFIG and a TAG rather than a call site. Each was reproduced
first with a fixture whose only evidence lives in a comment.
gate-60 icon-vocabulary
manifest uses ViewDashboardOutline,
icons.js registers only Cog FAIL — 1 icon name(s) NOT registered
+ "// TODO: the dashboard nav row still
// needs this — not wired up yet.
// import ViewDashboardOutline from
// 'vue-material-design-icons/…'" PASS <- the defect
+ registerIcons(icons) named only in
an unprefixed /* */ interior line PASS <- the defect
gate-65 coding-standard-adoption
a fixer config with no autoloader FAIL — fixer-config-missing-autoloader
+ "// TODO: we still need to require
// DIR . '/vendor/autoload.php'
// here. Not done yet." PASS <- the defect
gate-16 spec-coverage
an untagged public method uncovered (a finding)
+ "* TODO: nobody has written @SPEC
openspec/specs/thing/spec.md
for this yet" covered <- the defect
A COMMENTED-OUT IMPORT VOUCHES FOR AN ICON THAT RENDERS BLANK. gate-60 exists
for exactly one failure — CnAppNav resolves an MDI name only through the
registry
registerIcons()populates, WITH NO FALLBACK, so an unregistered namerenders nothing at all: no glyph, no console error. The comment saying the
import is "not wired up yet" was the whole evidence that it was.
gate-65's silenced rule exists PRECISELY because a php-cs-fixer fatal reads
exactly like a clean tree: with no autoloader the run dies "Class not found",
and --format=json reports that as ZERO FILES NEEDING CHANGES. The comment
bought a green on the check whose job is to stop a green being bought. The XML
paths in that same module have been protected by strip_xml_comments() from the
start — "a commented-out rule is not a rule" is its own docstring. The fixer
config was the one file it read raw.
MEASURED ON REAL REPOS, BOTH DIRECTIONS.
after/before across procest, opencatalogi, openregister, softwarecatalog,
docudesk and larpingapp: IDENTICAL — no count moves on any of the three.
Agreement is what a dead rig looks like, so each gate was also given a
POSITIVE CONTROL on a real repository, by planting the comment in a copy:
GATE. The evidence there IS a string: the icon name lives in the module
specifier 'vue-material-design-icons/ViewDashboardOutline.vue'. So it uses
js_comment_maskand NOTjs_mask(blank_strings=True), and arm 4 of the suiteis what makes a future "strings are not evidence" generalisation go red instead
of reporting every icon in the fleet unregistered. gate-65 keeps literals for
the same reason —
require __DIR__ . '/vendor/autoload.php'.@specis a docblock marker: unlike every other gate in #422 the evidencelegitimately lives in a comment and a mask would delete it. What was missing is
POSITION — the anchoring gates 47 and 48 have and gate 46 still lacks. The
first cut was a plain start-anchor. Swept exhaustively over every in-scope
method in the six repos (46,187 judgements, diff scope bypassed so every line
counts as changed) it produced FIVE new findings, all in decidesk's
VotingRoundPanel.vue, all of this shape:
— a REAL tag in the ordinary PHPDoc order, whose only remedy would have been to
reflow correct documentation. A gate that reddens documented code teaches
authors to stop documenting it, which is the false-positive half of this same
class and the half the survey calls corrosive. The pattern now also admits a
tag FOLLOWING A COMPLETED SENTENCE, which is what the real form is and none of
the prose forms are, and the sweep re-run says ZERO new findings in all six.
The residual hole is stated in the code rather than hidden: a debt sentence
ENDING in a full stop immediately before the tag still counts.
The lead character class was measured too, not guessed:
[ \t>*#-]— thepackage's markdown
standalonelead — would have uncovered 1,264 correctlytagged methods (626 procest + 638 opencatalogi) written
/** @spec … */, so/is in the class. And the 109// @spec openspec/…lines in the fleet turnout to be unaffected either way, because
_docblock_blockskips//lines —the expectation going in was the opposite, and arm 5 records the correction.
TESTS. 14 new arms. Reverted against origin/main — not
git checkout --, whichrestores the COMMITTED file — six FLIP:
gate-60 arms 2, 3
gate-65 the TODO arm and the removal-note arm
gate-16 arms 1, 2
and eight pass either way and are labelled CONTROLS in their own docstrings,
including gate-16 arm 8, which is the arm that caught the first cut being
wrong, and gate-65's "a real require of a STRING path, below a block comment,
still counts".
Refs #422, #415. Sibling PRs cover the PHP call-site gates and the
runner-inline gates; gate-38 is BLOCKED on #424.