fix(bootstrap): register OpenRegister's autoloader before probing for its classes (ADR-040) by rubenvdlinde · Pull Request #276 · ConductionNL/larpingapp

rubenvdlinde · 2026-08-06T08:01:16Z

The defect

larpingapp sorts before openregister, and 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() one app at a time. So OCA\OpenRegister\ is not autoloadable inside LarpingApp's own register() — on a perfectly healthy instance, with OpenRegister enabled.

All three class_exists('OCA\OpenRegister\Event\…') probes in register() were therefore answering false. Not "not loaded yet" — false, indistinguishable from OpenRegister being absent. LarpingApp registered no event listeners at all:

probe	listener that never registered
`DeepLinkRegistrationEvent`	`DeepLinkRegistrationListener` — unified-search deep links
`ObjectCreatingEvent`	`CharacterRequirementListener`
`ObjectUpdatingEvent`	`CharacterRequirementListener`

The last two carry the server-authoritative skill-requirement and XP-budget enforcement on character writes. That enforcement is server-side precisely because the client cannot be trusted, and it was silently not running. The app stayed enabled, served normally, and nothing in the UI reported the gap. A validation that is never invoked is indistinguishable from having no validation.

This is measured, not inferred from theory. The identical load order was observed directly on the sibling app openbuild (also sorting before openregister), whose CI logs OpenRegister AppHost\Bootstrap is not autoloadable on every occ call while OpenRegister is installed and enabled the whole time.

It is invisible on a dev instance: any alphabetically-earlier app that pulls OpenRegister's autoloader in (doriath does) registers the prefix process-wide and masks the failure for every app that registers after it — exactly where you would test for it.

The fix

lib/AppInfo/OpenRegisterAutoloader.php — put OpenRegister's PSR-4 prefix on the autoloader before the guards run:

OC_App::registerAutoloading() touches only the autoloader and is idempotent.
IAppManager::loadApp('openregister') is deliberately not used: it marks OpenRegister loaded and calls Coordinator::bootApp(), booting it before its own register() has run.
The prelude returns false rather than throwing when OpenRegister genuinely is absent, so the existing guards still do their job.

It lives in its own class so its "never throws" contract is reachable from a unit test: Application cannot be constructed without a Nextcloud DI container.

Relationship to gate-64

Same defect class as hydra gate-64 apphost-autoload-prelude (ADR-040), one namespace over. gate-64's class_exists rule only matches OCA\OpenRegister\AppHost\… names, so it does not flag this file — larpingapp reports PASS today while being live-exposed. Worth widening the gate's rule 2 to any OCA\OpenRegister\… literal.

Verification

New tests/unit/AppInfo/OpenRegisterAutoloaderTest.php asserts the never-throws and idempotence contracts.
phpcs clean on all touched files.

Spec: new requirement in openspec/specs/skill-requirement-enforcement/spec.md.

… its classes `larpingapp` sorts before `openregister`, and 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 OCA\OpenRegister\ is NOT autoloadable inside LarpingApp's own register() — on a perfectly healthy instance, with OpenRegister enabled. All three class_exists('OCA\OpenRegister\Event\…') probes in register() were therefore answering FALSE. Not "not loaded yet" — FALSE, indistinguishable from OpenRegister being absent. LarpingApp registered NO event listeners at all: - DeepLinkRegistrationEvent -> unified-search deep links, absent - ObjectCreatingEvent -> CharacterRequirementListener, absent - ObjectUpdatingEvent -> CharacterRequirementListener, absent The last two are the server-authoritative skill-requirement and XP-budget enforcement on character writes. That enforcement is server-side precisely because the client cannot be trusted, and it was silently not running — the app stayed enabled, served normally, and nothing reported the gap. The same load order was measured directly on the sibling app openbuild (also sorting before openregister), whose CI logs `OpenRegister AppHost\Bootstrap is not autoloadable` on every occ call while OpenRegister is installed and enabled. Fix: put OpenRegister's PSR-4 prefix on the autoloader before the guards. registerAutoloading() touches only the autoloader and is idempotent. IAppManager::loadApp() is deliberately NOT used: it marks OpenRegister loaded and calls Coordinator::bootApp(), booting it before its own register() has run. The prelude returns false rather than throwing when OpenRegister genuinely is absent, so the guards still do their job. Same defect class as hydra gate-64 (apphost-autoload-prelude) / ADR-040, one namespace over: gate-64 only probes for OCA\OpenRegister\AppHost\ names, so it does not see this one.

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.

github-actions · 2026-08-06T08:11:10Z

Quality Report — ConductionNL/larpingapp @ `9716a82`

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

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

Download the full PDF report from the workflow artifacts.

@e2e

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.

github-actions · 2026-08-06T08:18:05Z

Quality Report — ConductionNL/larpingapp @ `4c67969`

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

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

Download the full PDF report from the workflow artifacts.

github-actions · 2026-08-06T08:34:51Z

Quality Report — ConductionNL/larpingapp @ `81bc577`

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

Quality workflow — 2026-08-06 08:34 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.

github-actions · 2026-08-06T08:51:45Z

Quality Report — ConductionNL/larpingapp @ `a51ee30`

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

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

Download the full PDF report from the workflow artifacts.

github-actions · 2026-08-08T10:19:59Z

Quality Report — ConductionNL/larpingapp @ `eab2dbd`

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

Quality workflow — 2026-08-08 10:19 UTC

Download the full PDF report from the workflow artifacts.

rubenvdlinde · 2026-08-08T11:01:28Z

Reviewed as part of owning this repo's open-PR queue. Not merged, and the reason is one job.

Strict-subset evaluation

development at c73df92d fails, by name:

quality / E2E Tests (Playwright)
quality / Integration Tests (Newman)
quality / Quality Report          (pure aggregator — not debt)

This PR, on a fresh run after gh pr update-branch (head 7aa816fb, run 31252050355):

quality / Quality Report                  ← on development
quality / Integration Tests (Newman)      ← on development
quality / PHPUnit (PHP 8.3, NC stable32)  ← NOT on development

One name outside the base set, so this is not a strict subset and I am not landing it. (The earlier run you may have seen was from 2026-08-06 against a base of 906a3db0 and additionally showed Frontend Check (test:l10n) + a stable31 leg — both stale: #285 fixed l10n and the matrix is ["stable32"] only.)

What actually fails — PHPUnit passes; the coverage ratchet does not

Job 93089838477 step conclusions: Run PHPUnit tests success, Guard coverage baseline failure.

Coverage current:       71.67%  (1022/1426 statements)
Coverage merge base:    71.73%  (1020/1422 statements)
Committed .coverage-baseline: 67.68% — recorded only, NOT enforced on this run.
FAIL: coverage dropped by 0.06% against the merge base.
      This change adds 4 statements. Adding code without tests drops coverage.

4 statements added, 2 covered. The two that are not are structural, not an oversight:

Application.php's single added line OpenRegisterAutoloader::register(); — Application cannot be constructed without a Nextcloud DI container, which is the reason the prelude was extracted into its own class in the first place. Nothing unit-tests Application.
\OC_App::registerAutoloading($appId ?? 'openregister', $path); inside the try — in the unit environment \OCP\Server::get() throws on the line above, so the happy path is never reached. All three tests exercise only the degraded branch.

So the suite covers "never throws" and "is idempotent" but never once observes the prelude actually registering a prefix — which is the entire behaviour the PR exists to add.

Two things worth fixing while you are here

testRegisterNeverThrows asserts on the wrong object. It ends with:

$before = count(spl_autoload_functions());
OpenRegisterAutoloader::register();
$this->assertGreaterThan(expected: 0, actual: $before, ...);

$before is sampled before the call and there is always at least one autoloader registered, so that assertion is true on any tree, including one where register() is an empty method body. The comment says reaching the line is the real assertion — true, but then the honest form is expectNotToPerformAssertions() or an assertion on state that the call actually changes. As written it reads like a check and cannot fail.

The happy path is reachable in the PHPUnit environment. These tests already run inside a real Nextcloud (the job checks out the server, installs it, and enables the app before Run PHPUnit tests), so \OCP\Server::get(\OCP\App\IAppManager::class) resolves. A test that calls register() and then asserts class_exists('OCA\OpenRegister\...') — or that spl_autoload_functions() grew — would cover both remaining statements and, more importantly, would be the first test that proves the prelude does its job. Right now if the body were deleted, all three tests would still pass.

Note for the other five repos in this sweep

The coverage drop is a property of the change shape (a small, deliberately-untestable call site plus a happy path unreachable from unit tests), so the same ratchet failure will appear in every repo where this landed with the same test file. Worth fixing once in the shared test rather than six times.

Nothing here argues against the change itself — the load-order analysis is correct and the defect is real. It just needs the happy path covered before it can pass this repo's ratchet.

rubenvdlinde · 2026-08-08T11:10:11Z

Addendum — I was pointed at launchpad#58, which was closed as a no-op that also regresses psalm, and asked to verify before merging. I measured it here rather than inheriting either conclusion.

larpingapp is not launchpad. The premise holds here.

1. There IS a `register()`-time OpenRegister reference

lib/AppInfo/Application.php on development, inside register():

if (class_exists('OCA\OpenRegister\Event\DeepLinkRegistrationEvent') === true) {   // :78
if (class_exists('OCA\OpenRegister\Event\ObjectCreatingEvent') === true) {         // :89
if (class_exists('OCA\OpenRegister\Event\ObjectUpdatingEvent') === true) {         // :96

Three eager probes, resolved at register() time — not lazy registerService closures. That is precisely the distinction launchpad drew to justify closing #58, and larpingapp falls on the other side of it.

No class-header exposure, for the record: no extends/implements on an OCA\OpenRegister\* class anywhere, so the router-ReflectionClass-during-matching 500 is not in play. The exposure is only the silent-skip kind.

2. The load order is confirmed from Nextcloud's own source, not inferred

lib/private/legacy/OC_App.php:193:

sort($apps);
array_unshift($apps, 'files');

lib/private/AppFramework/Bootstrap/Coordinator.php::registerApps() — one loop, autoloader then register(), per app:

foreach ($appIds as $appId) {
    $path = $this->appManager->getAppPath($appId);
    OC_App::registerAutoloading($appId, $path);
    ...
    $application->register($this->registrationContext->for($appId));
}

larpingapp < openregister, so OCA\OpenRegister\ is genuinely not autoloadable when larpingapp's register() runs. All three probes answer FALSE on a healthy instance with OpenRegister enabled — indistinguishable from OpenRegister being absent.

The consequence is not cosmetic: ObjectCreatingEvent / ObjectUpdatingEvent carry the server-authoritative skill-requirement and XP-budget enforcement on character writes. Enforcement is server-side precisely because the client cannot be trusted.

3. "gate-64 passes" is not evidence here — and I confirmed the gate can fail

gate-64 matches only AppHost:

BOOTSTRAP_REF        = re.compile(r"OpenRegister\\{1,2}AppHost\\{1,2}Bootstrap")
CLASS_EXISTS_APPHOST = re.compile(r"class_exists\s*\(\s*["][^\"]*OpenRegister\\{1,2}AppHost\\{1,2}[^"]*[\"]")

larpingapp references OpenRegister\Event\*, never OpenRegister\AppHost\*, so gate-64 is out of scope by design — not blind, and not a clean bill of health.

Discrimination control on origin/development, check_apphost_autoload_prelude.py:

arm	tree	result
A	unmodified	`apphost-autoload-prelude: OK`, exit 0
B	one `class_exists('OCA\OpenRegister\AppHost\Bootstrap')` injected	`FAIL larpingapp: ... references OCA\OpenRegister\AppHost\Bootstrap, but nothing under lib/AppInfo/ registers OpenRegister's autoloader first`, exit 1
A′	restored	exit 0

So this repo is a genuine instance of the ADR-040 load-order defect in its Event-listener form, which gate-64 does not cover.

4. The psalm suppression

psalm.xml adds one <referencedClass name="OC_App"/> to an existing <UndefinedClass><errorLevel type="suppress"> list that already carries OCP\AppFramework\App, IBootstrap and others. It suppresses a stub-availability artifact — OC_App is server-private and absent from nextcloud/ocp — not a real finding, and it is scoped to one class rather than blanket. Consistent with the file's established pattern. PHP Quality (psalm) is SUCCESS on the fresh run.

Worth saying out loud: reaching for \OC_App is a real smell, but there is no OCP API for registering another app's autoloader, so the alternative is not available.

Verdict

The change is correct and wanted for this repo. It is still not mergeable for the reason in my previous comment — the coverage ratchet, and the fact that no test ever observes the prelude actually registering a prefix. Cover the happy path (these tests already run inside a real installed Nextcloud, so \OCP\Server::get(IAppManager::class) resolves) and this lands.

Whatever is decided for launchpad should not be generalised to here.

The coverage ratchet was the only thing red on this PR: Coverage current: 71.67% (1022/1426 statements) Coverage merge base: 71.73% (1020/1422 statements) FAIL: coverage dropped by 0.06% against the merge base. Four new statements, two of them uncovered — `OC_App::registerAutoloading()` inside the prelude's try (unreachable in a bare unit run, where `OCP\\Server::get()` throws first) and the `OpenRegisterAutoloader::register()` call at the top of `Application::register()`, which had no test at all. Rather than move the floor, this covers the thing the whole PR is about. `OpenRegisterAutoloaderTest` proves the prelude never throws; it cannot prove what actually matters — that once the prelude has done its job, the listeners are there. Two of the three carry the server-authoritative skill-requirement and XP-budget enforcement on character writes, and an unregistered listener is indistinguishable from no enforcement at all. `ApplicationRegisterTest` calls `register()` with a recording `IRegistrationContext` and asserts the exact (event, listener) pairs, in both worlds: OpenRegister resolvable (the post-prelude world, simulated by tests/unit/AppInfo/fixtures/openregister-events.php declaring the three event names — a name is the entire contract, since register() only ever passes the class name and never constructs one) and OpenRegister absent (must attach nothing and must not throw). `Application` is built with `newInstanceWithoutConstructor()`: `App::__construct()` needs the Nextcloud DI container, and `register()` neither reads nor writes instance state. Can-fail verified by deleting the ObjectUpdatingEvent registration: 1) ...ApplicationRegisterTest::testEveryListenerRegistersWhenOpenRegisterIsResolvable Application did not register CharacterRequirementListener. FAILURES! Tests: 2, Assertions: 7, Failures: 1. and restored — `git diff lib/` empty. The fixture-loaded assertion guards the other direction: if the fixture stopped making the names resolvable, the test says so instead of passing over a register() that attaches nothing.

github-actions · 2026-08-09T15:59:43Z

Quality Report — ConductionNL/larpingapp @ `bf0b332`

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

Quality workflow — 2026-08-09 15:59 UTC

Download the full PDF report from the workflow artifacts.

rubenvdlinde added 2 commits August 6, 2026 10:00

Merge branch 'development' into fix/adr-040-autoload-prelude

7aa816f

rubenvdlinde merged commit 0ac3af2 into development Aug 9, 2026
31 of 33 checks passed

rubenvdlinde deleted the fix/adr-040-autoload-prelude branch August 9, 2026 19:55

Navigation Menu

Uh oh!

fix(bootstrap): register OpenRegister's autoloader before probing for its classes (ADR-040) - #276

fix(bootstrap): register OpenRegister's autoloader before probing for its classes (ADR-040)#276
rubenvdlinde merged 6 commits into
developmentfrom
fix/adr-040-autoload-prelude

rubenvdlinde commented Aug 6, 2026

Uh oh!

github-actions Bot commented Aug 6, 2026

Uh oh!

github-actions Bot commented Aug 6, 2026

Uh oh!

github-actions Bot commented Aug 6, 2026

Uh oh!

github-actions Bot commented Aug 6, 2026

Uh oh!

github-actions Bot commented Aug 8, 2026

Uh oh!

rubenvdlinde commented Aug 8, 2026

Uh oh!

rubenvdlinde commented Aug 8, 2026

Uh oh!

github-actions Bot commented Aug 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

rubenvdlinde commented Aug 6, 2026

The defect

The fix

Relationship to gate-64

Verification

Uh oh!

github-actions Bot commented Aug 6, 2026

Quality Report — ConductionNL/larpingapp @ 9716a82

Uh oh!

github-actions Bot commented Aug 6, 2026

Quality Report — ConductionNL/larpingapp @ 4c67969

Uh oh!

github-actions Bot commented Aug 6, 2026

Quality Report — ConductionNL/larpingapp @ 81bc577

Uh oh!

github-actions Bot commented Aug 6, 2026

Quality Report — ConductionNL/larpingapp @ a51ee30

Uh oh!

github-actions Bot commented Aug 8, 2026

Quality Report — ConductionNL/larpingapp @ eab2dbd

Uh oh!

rubenvdlinde commented Aug 8, 2026

Strict-subset evaluation

What actually fails — PHPUnit passes; the coverage ratchet does not

Two things worth fixing while you are here

Note for the other five repos in this sweep

Uh oh!

rubenvdlinde commented Aug 8, 2026

1. There IS a register()-time OpenRegister reference

2. The load order is confirmed from Nextcloud's own source, not inferred

3. "gate-64 passes" is not evidence here — and I confirmed the gate can fail

4. The psalm suppression

Verdict

Uh oh!

github-actions Bot commented Aug 9, 2026

Quality Report — ConductionNL/larpingapp @ bf0b332

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Quality Report — ConductionNL/larpingapp @ `9716a82`

Quality Report — ConductionNL/larpingapp @ `4c67969`

Quality Report — ConductionNL/larpingapp @ `81bc577`

Quality Report — ConductionNL/larpingapp @ `a51ee30`

Quality Report — ConductionNL/larpingapp @ `eab2dbd`

1. There IS a `register()`-time OpenRegister reference

Quality Report — ConductionNL/larpingapp @ `bf0b332`