fix: declare NC 32 as the floor — openregister cannot install below it - #759
Merged
Conversation
rubenvdlinde
requested review from
Rem-Dam,
SudoThijn,
WilcoLouwerse,
bbrands02,
remko48 and
rjzondervan
as code owners
August 7, 2026 09:54
Contributor
Quality Report — ConductionNL/procest @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| check-vue3-compile | ✅ | ||||
| test-l10n | ✅ | ||||
| composer | ✅ | ✅ 100/100 | |||
| npm | ✅ | ✅ 552/552 | |||
| PHPUnit | ✅ | ||||
| Newman | ⏭️ | ||||
| Playwright | ✅ | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-08-07 10:29 UTC
Download the full PDF report from the workflow artifacts.
rubenvdlinde
added a commit
that referenced
this pull request
Aug 8, 2026
…ed matrix (#767) The floor has been flipped twice this week (#759 raised it, #761 was closed, #762 reverted it) because nothing tied the declared range to the tested one. This raises it to 32 per the fleet-wide alignment on PHP 8.3, and adds the test that makes the next flip impossible to land silently. WHAT I CHECKED RATHER THAN ASSUMED The rationale already sitting in info.xml was stale, and #762 reverted the value while leaving that rationale in place — so the file declared 28 and explained 32. Both the info.xml comment and code-quality.yml claimed "openregister declares min-version=32". Measured today against the canonical repo, ConductionNL/openregister@development declares: <nextcloud min-version="28" max-version="34"/> openregister#2380 undid it. So the openconnector#1172/#1173 rule — min-version must be >= the max of every <app> dependency's floor — imposes NO constraint here: procest declares no <app> dependencies at all, and the app it depends on in practice has a floor of 28. Both stale comments are corrected rather than repeated. #762's own stated premise is also false at this tip. It reverted the floor because "this repo tests stable31"; code-quality.yml pins nextcloud-test-refs to exactly ["stable32"], and the stable31 leg was REMOVED. WHY 32 IS RIGHT ON TODAY'S EVIDENCE 1. Nothing below 32 is tested. stable32 is the only leg, so 28-31 was an advertised App Store range with zero exercise behind it. 2. info.xml declares <php min-version="8.3"/> two lines above. Nextcloud 28 does not support PHP 8.3, so the pair the two lines jointly advertise is not a configuration that can exist. CI MATRIX: unchanged, and now checked. No leg targets NC < 32 — stable32 is the only entry — so there is nothing to drop. max-version stays 34, the fleet-wide value everywhere except openconnector (35). CAN-FAIL PROOF for NextcloudFloorMatrixTest (3 mutations, measured): - floor 32 -> 33: 2 failures — "runs against stable32" and "no CI leg runs at or above it". - matrix ["stable32"] -> ["stable31"] at floor 32: 2 failures, naming stable31. This is literally the state #762 claimed to be in, so the test would have caught that PR. - both restored: OK (3 tests, 25 assertions). It asserts on every individual ref, not on the matrix merely being non-empty, and a separate positive control fails if either scan matches nothing — an unparsed matrix would otherwise make every assertion pass vacuously.
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 changed
1.
appinfo/info.xmlmin-version28 → 32.src/manifest.jsonlists"openregister"as a required dependency, and the existing<dependencies>comment already records OpenRegister >= 0.2.16 as a hard requirement.2.
nextcloud-test-refs["stable31","stable32"]→["stable32"].The defect
openregister raised its own floor this morning —
<nextcloud min-version="32" max-version="34"/>, commit8d5181f7a(openregister#2378), "require Nextcloud 32, which is where the ContextChat interfaces exist". Itslib/ContextChat/ContentProvider.phpimplementsOCP\ContextChat\IContentProvider, and core does not have that interface before NC32. That raise is correct and is not being reverted here.The consequence lands on every app that depends on openregister but still tests on
stable31:The enable failure is only a
::warning::(sharedquality.yml, "Install Nextcloud" step), so the run continues without its data layer and every/apps/openregister/...call returns Nextcloud's HTML 404 page.This was measured, not inferred
development's own Newman on yesterday's commits: decidesk 93 → 191 failures, larpingapp 10 → 22.stable32only.stable31is removed, not "dropped for coverage"A
stable31leg was exercising a configuration that cannot exist: the app under test declares openregister as a hard dependency, and openregister cannot install there. Nothing was being covered on that leg — its red said nothing about this app's code. Removing it corrects an impossible configuration. It does not reduce coverage.The matrix is not widened either: no
stable33is added where it was not already present.The ordering bug in the same family
Membership was not the only problem. In the shared
quality.yml, three jobs pin the first entry rather than iterating the list:newmanfromJSON(inputs.nextcloud-test-refs)[0]playwrightfromJSON(inputs.nextcloud-test-refs)[0]journeydoc-capturefromJSON(inputs.nextcloud-test-refs)[0](
phpunititerates the whole list, so it was broken by membership; the coverage-guard and coverage-artifact steps at 1644/1671/1684 are also[0]-pinned.)So wherever
stable31sat first, those three jobs ran entirely on the version openregister cannot load. doriath had already reordered for exactly this reason.Precedent
This is the same defect as openconnector#1173, which declared
min-version="28"while its own<app>openregister</app>dependency required 32 — advertising, in the app store, a range it could not deliver. This PR follows that shape.Scope
No app version bump — only the supported-server range and the CI matrix change, so there are no
occ upgradeimplications.