Skip to content

fix: restore Nextcloud 28 support — the 32 floor breaks eight consumer repos - #2380

Merged
rubenvdlinde merged 1 commit into
developmentfrom
fix/restore-nc31-support
Aug 7, 2026
Merged

fix: restore Nextcloud 28 support — the 32 floor breaks eight consumer repos#2380
rubenvdlinde merged 1 commit into
developmentfrom
fix/restore-nc31-support

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

#2378 raised min-version 28 → 32. Its premise was correct when written and is no longer:

"the registration listener cannot prevent it, because the failure happens when PHP reads the class header, not when the provider is registered"

That was true — and #2372 is what changed it. #2372 removed every eager reference to ContentProvider (the command's constructor typehint and the listener's), so the class is now only loaded from inside interface_exists(...) guards. On a server without OCP\ContextChat the class header is never read: the feature is inert, and nothing logs.

What the floor actually costs

min-version is enforced at install time, so 32 makes occ app:enable openregister refuse on 31. Eight fleet repos install openregister as an additional-app while testing stable31 — procest, shillinq, portaliq, openbuild, decidesk, doriath, hermiq, larpingapp. Each now fails its seed with:

{"success":false,"message":"OpenRegister is not installed or enabled"}

and then a 404 from /api/configurations/import, because the fallback importer belongs to the app that did not install. Observed on procest#758, where it looked for a while like a manifest bug in that PR.

Evidence that 31 works

procest's e2e installs openregister at development and runs stable31. Its run at 2026-08-06T14:04Z — six minutes after #2372 merged at 13:58Z — passed. The app demonstrably boots, enables and serves on NC 31 with that fix in place.

The CI compose's second reason for a 32 floor does not hold either: OCP\DB\Types::DATETIME_IMMUTABLE is present in stable31 as well as 32. Checked directly against both branches.

This also corrects my own error

#2372 and its test say "NC 33" throughout. That came from a local lookup against refs this checkout does not have — which answers ABSENT for everything. It is exactly the failure a positive control exists to catch, and I did not run one at the time. Verified properly, with IUserManager.php as the control:

branch IContentProvider control
stable31 404 200
stable32 200 200

So the interfaces arrive in 32, not 33. Every "NC 33" claim in the listener, the command and the guard test is corrected.

What stays true from #2378

28-31 remain untested by this repo's own CI. That is a real coverage gap and worth closing — but it is a different thing from declaring those versions unsupported. The eight repos above are the coverage, and they were green on 31 until the floor moved.

26 ContextChat tests pass; phpcs clean.

…r repos

#2378 raised min-version 28 -> 32. Its premise was correct WHEN WRITTEN and
is no longer:

    "the registration listener cannot prevent it, because the failure happens
     when PHP reads the class header, not when the provider is registered"

That was true, and #2372 is what changed it. #2372 removed every EAGER
reference to ContentProvider — the command's constructor typehint and the
listener's — so the class is now only loaded from inside
interface_exists('OCP\ContextChat\IContentManager') guards. On a server
without OCP\ContextChat the class header is never read: the feature is inert,
and nothing logs.

WHAT THE FLOOR ACTUALLY COSTS. min-version is enforced at INSTALL time, so 32
makes `occ app:enable openregister` refuse on 31. Eight fleet repos install
openregister as an additional-app while testing stable31 — procest, shillinq,
portaliq, openbuild, decidesk, doriath, hermiq, larpingapp. Each now fails its
seed with

    {"success":false,"message":"OpenRegister is not installed or enabled"}

and then a 404 from /api/configurations/import, because the fallback importer
belongs to the app that did not install. Observed on procest#758, where it
looked for a while like a manifest bug in that PR.

EVIDENCE THAT 31 WORKS. procest's e2e installs openregister at `development`
and runs stable31. Its run at 2026-08-06T14:04Z — six minutes AFTER #2372
merged at 13:58Z — passed. The app demonstrably boots, enables and serves on
NC 31 with that fix in place.

The second reason the CI compose gave for a 32 floor does not hold either:
OCP\DB\Types::DATETIME_IMMUTABLE is present in stable31 as well as stable32.
Checked directly against both branches.

ALSO CORRECTS MY OWN ERROR. #2372 and its test say "NC 33" throughout. That
came from a local lookup against refs this checkout does not have, which
answers ABSENT for everything — the failure mode a positive control exists to
catch, and I did not run one at the time. Verified properly against
raw.githubusercontent, with IUserManager.php as the control:

    stable31  IContentProvider 404   control 200
    stable32  IContentProvider 200   control 200

So the interfaces arrive in 32, not 33. Every "NC 33" claim in the listener,
the command and the guard test is corrected to 32.

28-31 remain untested by this repo's own CI, exactly as #2378 observed. That
is a real coverage gap and worth closing, but it is a different thing from
declaring those versions unsupported — the eight repos above ARE the coverage,
and they were green on 31 until the floor moved.

26 ContextChat tests pass; phpcs clean.
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/openregister @ 1859ffb

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
build
check-specs
test-l10n
composer ✅ 173/173
npm ✅ 546/546
PHPUnit
Newman
Playwright
Hydra gates

Quality workflow — 2026-08-07 10:00 UTC

Download the full PDF report from the workflow artifacts.

@rubenvdlinde
rubenvdlinde merged commit baf4aa9 into development Aug 7, 2026
30 checks passed
@rubenvdlinde
rubenvdlinde deleted the fix/restore-nc31-support branch August 7, 2026 10:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant