Skip to content

fix(apphost): register OpenRegister's autoloader before touching AppHost - #289

Closed
rubenvdlinde wants to merge 1 commit into
developmentfrom
fix/apphost-autoload-prelude
Closed

fix(apphost): register OpenRegister's autoloader before touching AppHost#289
rubenvdlinde wants to merge 1 commit into
developmentfrom
fix/apphost-autoload-prelude

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

What gate-64 found

lib/AppInfo/Application.php calls Bootstrap::register(...) unguarded, with no autoload prelude.

OC_App::getEnabledApps() sort()s the app list, and Coordinator::registerApps() walks that sorted list calling registerAutoloading($appId, $path) then $app->register() one app at a time. So every app's register() runs before the PSR-4 prefix of every alphabetically-later app exists.

For scholiq the ordering happens to be favourable today (scholiq sorts after openregister), so this is latent rather than live. It stops being latent the moment OpenRegister is disabled or absent — an unguarded Bootstrap::register() then throws \Error, Coordinator catches it, logs an emergency and continues, and the app stays enabled while ServiceOverrideRegistrar and EventListenerWiring below it never run. Nothing in the UI says half the wiring is missing.

Relying on your app id's alphabetical position relative to another app's is not a safety property. The prelude is idempotent and cheap.

Verified both directions, against this tree

gate-64
without the prelude rc=1 FAIL — AppHost adoption with no prelude
with the prelude rc=0 PASS

Checked under both matcher versions — the one on .github main and the named-constant fix in .github#185 — so this is not a repo change chasing a gate change.

php -l clean.

🤖 Generated with Claude Code

ADR-040's load-order hazard, caught by gate-64 once the gate package was
actually reaching this repo.

OC_App::getEnabledApps() sort()s the app list, and Coordinator::registerApps()
walks THAT sorted list calling OC_App::registerAutoloading($appId, $path) and
then $app->register() for ONE APP AT A TIME. So every app's register() runs
BEFORE the PSR-4 prefix of every alphabetically-later app exists — on a
completely healthy instance, with OpenRegister enabled.

The fix is to put OpenRegister's prefix on the autoloader ourselves, which is
exactly what Nextcloud does a few iterations later. Two properties make this
the right call: registerAutoloading() touches ONLY the autoloader and is
idempotent (it early-returns on an $alreadyRegistered key), and it does NOT
boot the app. IAppManager::loadApp('openregister') would be wrong here — it
sets loadedApps['openregister']=true and calls Coordinator::bootApp(), booting
OpenRegister BEFORE its own register() has run.

Verified both directions with the gate itself, against this tree:

  without the prelude   rc=1  FAIL — AppHost adoption with no prelude
  with the prelude      rc=0  PASS

and under BOTH matcher versions (the one on .github main and the named-constant
fix in .github#185), so this is not a repo change chasing a gate change.
@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/scholiq @ 657e42b

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
build
check-specs
composer ✅ 100/100
npm ✅ 750/750
PHPUnit
Newman ⏭️
Playwright ⏭️
Hydra gates

Quality workflow — 2026-08-06 08:06 UTC

Download the full PDF report from the workflow artifacts.

@rubenvdlinde

Copy link
Copy Markdown
Contributor Author

Superseded by #288, which landed as f757880.

Both PRs fix the same ADR-040 load-order defect. #288 is the one that follows doriath's reference implementation: the prelude lives in its own lib/AppInfo/OpenRegisterAutoloader.php class so the "this NEVER throws" contract is reachable from a unit test (tests/Unit/AppInfo/OpenRegisterAutoloaderTest.php), it carries the openspec requirement, and it declares the OC_App psalm suppression the inline version needs but does not have.

That difference is visible in CI, not just in style: this PR is red on PHP Quality (phpmd), PHP Quality (psalm), Hydra Gates and Quality Report, while #288 was green on all 32 checks.

Closing without merging — no code is lost, #288 contains the same fix.

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