test(gate-3): pin the #434 exemption against the #422 mask — the two reads must stay split - #447
Merged
Merged
Conversation
…reads must stay split #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.
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.
test(gate-3): pin the #434 exemption against the #422 mask — the two reads must stay split
#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_maskbeingline- and offset-preserving makes that legal:
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 excludetrap, which #444 hit for real in the siblinggate of the same sweep: masking the body slid the reported line into the
blanked docblock,
_method_docblockwalked from the wrong place, and areason-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 docblockreads
@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
--filepointedat
${_stub_code}the arm reportsFAIL — 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.