feat(quality): derive the Nextcloud test matrix from appinfo/info.xml - #448
Merged
Merged
Conversation
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.
…rency 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.
…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.
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.
What this changes
quality.ymlno longer takes the Nextcloud test matrix from a hand-writteninput. It derives it from the app's own
appinfo/info.xml, usingicewind1991/nextcloud-version-matrix— the same action Nextcloud'sown app CI uses.
The point is not tidiness. The fleet has already broken the info.xml ↔ matrix
agreement in both directions inside one programme: first all 18 apps declared
NC 32–34 while testing
stable31/stable32, then the migration that fixed thatreplaced the list with
'["stable34"]'and 16 apps stopped testing their owndeclared floor. Both were detectable; neither was detected, because they were two
files that could be edited independently. Deriving makes the disagreement
unrepresentable rather than merely detected.
Supply chain
The action is pinned by commit SHA:
8a7bac6300b2f0f3100088b297995a229558ddba,which is tag v1.3.2 — the newest release. The tag is written in a trailing
comment so a human can read what the SHA means; the SHA is what the runner
resolves, so a moved tag cannot change what executes.
Deliberately not the action's tip:
maincarries an unreleased ES-module /node24 rewrite (
src/action.mjs,dist/index.mjs). v1.3.2 is node20 anddist/index.js. We take releases, not commits-since-a-release. 23 repositoriesconsume this workflow at
@main, so this is a supply-chain decision, not a styleone.
The ordering trap, and how it is handled
branchescomes out oldest-first — verified in the action's source, whererange(min,max)is reversed for the branched-off check and reversed back, andverified again by executing the pinned
dist/index.js:Six sites read
fromJSON(inputs.nextcloud-test-refs)[0]as THE single serverfor newman, playwright, journeydoc-capture and the coverage guard, and
newest-first was deliberate there —
stable34at[0]is what surfaced eightdead NC32/33-only selectors on portaliq. Consuming
branchesverbatim would havemoved all four onto stable32, silently.
The array is not reversed. All six sites now read
needs.nextcloud-matrix.outputs.single-server, which on the derived path is theaction's own
branches-maxoutput. A positional convention is invisible in adiff and cannot be checked; an output named
single-servercan be, and isprinted on every run.
quality-resolve-probe.ymladditionally asserts the oldest-first ordering, soa future release that flipped it surfaces as a red check rather than as four jobs
quietly testing the wrong server.
Compatibility — no flag day
nextcloud-test-refsis kept as an explicit override; derivation happens onlywhen it is unset. Chosen over a staged opt-in input because the override is
the staging mechanism: every caller that passes one keeps byte-identical
behaviour, and adopting derivation is a one-line deletion per repo, on that
repo's own schedule.
Measured across all 23 callers rather than assumed:
["stable34","stable32","stable33"], info.xml 32–34["stable34","stable32","stable33"]; info.xml 32–34["stable32","stable33","stable34"]— same set, same 6 legs, samestable34single server (derivation executed against each app's realinfo.xml)appinfo/info.xmlsource=fallbackprinted["stable31","stable32","stable33"], info.xml 28–34stable31→stable33["stable34"], info.xml 32–34code-quality.ymlcode-quality.ymlis a hand-rolled Python jobThe one behavioural change is petstore, and it is the change from
single-server = refs[0]tosingle-server = max(refs).stable31is a versionthis workflow's own comments record as producing meaningless results — openregister
declares
min-version="32", soocc app:enable openregisterfailed with only a::warning::, the run continued with no data layer, and every/apps/openregister/...call returned Nextcloud's HTML 404 page. Moving off it isa fix, but it is a change and it is called out rather than buried.
A disagreeing override emits a
::warning::naming both sets. It is deliberatelynot an error here: three callers would go red on merge. Enforcement is
gate-65's job, per app.
Derived vs declared, on two real apps
Executed the pinned
dist/index.jsagainstlarpingappandopenregister's realappinfo/info.xmlfromdevelopment:min-version="32" max-version="34"min-version="32" max-version="34"["stable34","stable32","stable33"]["stable34","stable32","stable33"]branches["stable32","stable33","stable34"]["stable32","stable33","stable34"]branches-maxbranches-max-list["stable34"]["stable34"]Both apps also pass gate-65 rule 11 as shipped (hand-declared) and with the
nextcloud-test-refsline deleted (derived), so adopting derivation is aone-line deletion that does not trade one finding for another.
Positive control — the matrix tracks info.xml
"The derived value matches what the app declares" is worth nothing on its own:
all 18 apps declare 32–34, so an action returning that array unconditionally
would satisfy it forever.
Two fixture manifests differing in one attribute are therefore checked
against each other on every PR to
main(
quality-resolve-probe.yml→ The test matrix tracks info.xml):stable34drops out. The job fails if the two derivations are equal, ifstable34survives amax-version="33", or ifbranches-maxstops trackingmax-version.Leg count — a matrix that collapses to one leg looks like a pass
quality-selftest.ymlcannot measure this: it disables every leg, and amatrix job whose
if:is false does not expand. Measured on run31692599960, where the skipped PHPUnit job renders as one row still carrying theliteral
${{ matrix.nextcloud-ref }}in its name — counting there would havereported "1 PHPUnit job" whether the matrix worked or not.
So it was measured on real Actions, on a
fixture/**branch callingquality.ymltwice with
enable-phpunit: true. Run31734347530,total_count: 62:6 and 4, from two manifests differing in one attribute, with
stable34absent from the second. The legs themselves fail —
.githubis not a Nextcloudapp — which is expected and is not the measurement. The job list is.
That fixture workflow is NOT in this PR, and that is a finding
It was, and CodeQL scored it 12 new HIGH alerts,
actions/cache-poisoning/poisonable-step, every one of them insidequality.ymlat lines this PR never touched.
mainscores 0 forlanguage:actionsand PRs#444/#446/#447 were clean, so this was the new workflow, not a pre-existing
pattern newly surfaced.
The mechanism:
quality.ymlneedscontents: write+actions: writeatrun-creation time, so a
fixture/**caller hands its ~12actions/cachesteps an entry point reachable from a non-default branch — and Actions caches are
shared with the default branch that restores them. In a repo 23 others consume at
@main, that is not worth a permanent leg counter.The recipe for repeating the measurement, and the two ways to get it wrong, are
documented on the
matrix-derivationjob inquality-resolve-probe.yml. Thesecond way is worth repeating here:
quality.ymldeclaresconcurrency: quality-${{ github.ref }}withcancel-in-progress, so two callson one ref cancel each other. Measured on run
31734110346: every job under thecontrol came back
cancelled, its PHPUnit row still carrying the literal${{ matrix.nextcloud-ref }}— the control measured nothing while looking like ithad run.
Also in this PR
gate-65 rule 11 (
check_coding_standard_adoption.py) —test-matrix-not-declaredis retired. Its premise ("a default cannot know this app's declared range")
is obsolete: a derived matrix is not a default. Three states now:
test-matrix-neither-derived-nor-declaredStates 1 and 3 differ by a single
uses:line, so a commented-out call isasserted separately (the gate-64 defect: a grep matches its string inside a
comment).
nextcloud-info-pathinput — the path the matrix derives from. This is howthe shared repo proves its own derivation:
.githubhas noappinfo/info.xml,so without it
quality-selftest.ymlcould only ever exercise the no-manifestfallback, and a derivation never executed in the repo that ships it is a
derivation nobody has measured.
nextcloud-matrixis in Quality Report'sneeds:. It is the one entry whosefailure does not describe the app's code — it fails when the promised range
cannot be turned into servers to test on. The legs still run, on the fallback
range, and still go green. That is exactly why it needs its own gate.
gate-65 rule 11, measured across all 25 callers (old checker vs new)
Real
code-quality.yml+appinfo/info.xmlfrom each repo's live branch:test-matrix-not-declaredtest-matrix-not-declaredtest-matrix-neither-derived-nor-declaredtest-matrix-misses-declared-versionsThe n8n-nextcloud row is the rule doing its job rather than a new failure: its
code-quality.ymlcontains nouses:for this workflow at all — it is ahand-rolled
setup-pythonjob — so nothing derives a range from its manifest andnothing ever did. The old rule reported it under a description ("the matrix is
whatever the shared workflow defaults to") that was not true of that repo.
Checks
quality.yml resolves (job count > 0)— pass, run31734189155,total_count: 7(not a0parse failure).The test matrix tracks info.xml— pass, and its log carries the numbers:32-34 -> branches=['stable32','stable33','stable34'] branches-max=stable34/32-33 -> branches=['stable32','stable33'] branches-max=stable33.Resolve quality.yml / Nextcloud test matrix— pass, log readssource = derived,branches = ["stable32","stable33","stable34"],single-server = stable34.count did not drop). Mutation control: run against the pre-change checker,
the four new/changed arms all go RED, so they are reachable and not vacuous.
run-helper-suites.sh: 60 PASS. One unrelated environmental refusal(
test_gate_45_to_55_acceptance.sh, exit 2,ajv is not resolvable— the suitedeclines to run rather than emit a false verdict; the file is untouched by this PR).
Do not merge
Reported for review, per the brief.
A branch
fixture/nextcloud-matrix-legsis left in place holding the withdrawnfixture workflow and the runs that produced the leg counts. Delete it once the
numbers above have been read.