fix(apphost): register OpenRegister's autoloader before referencing AppHost (gate-64 / ADR-040) - #752
Merged
Merged
Conversation
…ppHost Nextcloud registers apps in sorted order: OC_App::getEnabledApps() does sort($apps) and Coordinator::registerApps() walks that list calling OC_App::registerAutoloading($appId, $path) and then $app->register() for one app at a time, so every app registers before the PSR-4 prefix of every alphabetically-later app exists. `procest` sorts after `openregister`, so this happens to hold today — by alphabet, not by design. The class_exists() guard in AppHostRegistrar cannot tell 'OpenRegister absent' from 'OpenRegister's prefix not registered yet': both answer FALSE, and both silently skip the entire AppHost engine — health, metrics, preferences, deep links, the SPA page/catch-all, the seven dashboard widgets and the MCP provider. Fix: register OpenRegister's prefix ourselves before the guard. registerAutoloading() touches only the autoloader and is idempotent, so on the current ordering this costs nothing. IAppManager::loadApp() is deliberately NOT used: it marks OpenRegister loaded and calls Coordinator::bootApp(), booting it before its own register() has run. Caught by hydra gate-64 (apphost-autoload-prelude), ADR-040.
rubenvdlinde
requested review from
Rem-Dam,
WilcoLouwerse,
bbrands02 and
rjzondervan
as code owners
August 6, 2026 07:52
Two CI findings on the prelude, both real: 1. psalm UndefinedClass on \OC_App. It is Nextcloud's server-private legacy bootstrap class, absent from nextcloud/ocp, and there is no OCP interface for registering another app's autoloader. Declared as a suppressed referencedClass in psalm.xml, the same way doriath declares it. 2. The coverage ratchet. `return true` after the call plus `return false` in the catch gave the method a branch that NO environment can exercise both sides of — whichever runs, the other is dead in that run — so the class could never reach full line coverage. No caller ever consumed the return value either: what callers depend on is the class_exists() guard that follows the call. The method is now void with a single statement in the try and a comment-only catch, so every executable line runs in every environment. The tests now assert the two things that are actually observable: that control returns to the caller at all (a Throwable escaping would fail the test, and in production would abort the whole register()), and that a second call does not stack another autoloader. phpmd StaticAccess on the new composition-root call is documented on the calling method rather than baselined.
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 | ✅ | ✅ 550/550 | |||
| PHPUnit | ✅ | ||||
| Newman | ⏭️ | ||||
| Playwright | ❌ | ||||
| Hydra gates | ❌ |
Quality workflow — 2026-08-06 08:11 UTC
Download the full PDF report from the workflow artifacts.
The prelude requirement was appended to an existing legacy spec, which pulls every scenario in that file into gate-19's diff scope and demands e2e coverage for scenarios this change never touched. It is also not the same capability: apphost-adoption / skill-requirement-enforcement describe what the wiring DOES, this describes whether the wiring happens at all. Moved to openspec/specs/apphost-autoload-prelude/spec.md, deliberately with no scenarios: both behaviours live in the app-registration phase, which completes before the first request is dispatched, so neither is reachable from a browser, and the absent-OpenRegister path cannot be set up on an instance that needs OpenRegister to serve the app at all. They are asserted in the unit test named in the spec, so no @e2e exclusion is claimed for either.
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 | ✅ | ✅ 550/550 | |||
| PHPUnit | ❌ | ||||
| Newman | ⏭️ | ||||
| Playwright | ❌ | ||||
| Hydra gates | ❌ |
Quality workflow — 2026-08-06 08:18 UTC
Download the full PDF report from the workflow artifacts.
The coverage ratchet was right and the code was wrong. Clover for scholiq shows it exactly: line 100 (the registerAutoloading call) count=2, line 101 (the catch) count=0. The catch was never entered — because every instance this suite runs on HAS OpenRegister installed, so getAppPath() never throws. The never-rethrow branch, which is the entire reason this class exists, had never once been executed by a test. register() now takes an optional app id. Production callers pass nothing and get 'openregister'; the new test passes an id that cannot resolve, so getAppPath() throws and the catch runs. The literal stays AT the registerAutoloading call site rather than becoming a signature default, so it remains visible to a reader and to hydra gate-64, which reads that call's arguments. The new test asserts something real rather than merely not throwing: a prelude whose app cannot be resolved must leave spl_autoload_functions() untouched.
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 | ✅ | ✅ 550/550 | |||
| PHPUnit | ✅ | ||||
| Newman | ⏭️ | ||||
| Playwright | ❌ | ||||
| Hydra gates | ❌ |
Quality workflow — 2026-08-06 08:37 UTC
Download the full PDF report from the workflow artifacts.
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 | ✅ | ✅ 550/550 | |||
| PHPUnit | ✅ | ||||
| Newman | ⏭️ | ||||
| Playwright | ✅ | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-08-06 08:58 UTC
Download the full PDF report from the workflow artifacts.
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.
The defect
Nextcloud registers apps in sorted order:
OC_App::getEnabledApps()doessort($apps)andCoordinator::registerApps()walks that list callingOC_App::registerAutoloading($appId, $path)and then$app->register()one app at a time. Every app'sregister()therefore runs before the PSR-4 prefix of every alphabetically-later app exists.procestsorts afteropenregister, so this happens to hold today — by alphabet, not by design. The problem is what theclass_exists()guard inAppHostRegistraractually measures: it cannot tell "OpenRegister is absent" from "OpenRegister has not registered its prefix yet". Both answerfalse, and both take the same branch — a silentreturnthat skips the entire AppHost engine: health, metrics, preferences, deep links, the SPA page/catch-all, all seven dashboard widgets and the MCP provider. On a healthy instance, with nothing in the UI to say so.hydra gate-64
apphost-autoload-prelude(v1.5.0) flags this as LATENT: it works by alphabet alone, and renaming the app or moving this code somewhere that sorts earlier breaks it silently.The fix
lib/AppInfo/OpenRegisterAutoloader.php— register OpenRegister's PSR-4 prefix ourselves before the guard runs, so the guard'sfalsemeans only what it is supposed to mean:OC_App::registerAutoloading()touches only the autoloader and is idempotent, so on the current ordering this call costs nothing.IAppManager::loadApp('openregister')is deliberately not used: it marks OpenRegister loaded and callsCoordinator::bootApp(), booting it before its ownregister()has run.Application::register(), which Nextcloud executes on every request, so an exception escaping it would abort the whole composition root.It lives in its own class so that "never throws" contract is reachable from a unit test without a Nextcloud DI container.
Verification
OpenRegisterAutoloader.phpfrom the same tree puts it back to FAIL.tests/Unit/AppInfo/OpenRegisterAutoloaderTest.phpasserts the never-throws and idempotence contracts.phpcsclean on all touched files.Spec: new requirement in
openspec/specs/apphost-adoption/spec.md.Ref: ADR-040, hydra-gates gate-64 (ConductionNL/.github#174).