fix(apphost): register OpenRegister's autoloader before referencing AppHost (gate-64 / ADR-040) by rubenvdlinde · Pull Request #138 · ConductionNL/openbuild

rubenvdlinde · 2026-08-06T07:51:16Z

Closes #140.

The defect

lib/AppInfo/Application.php referenced OCA\OpenRegister\AppHost\Bootstrap behind a class_exists() guard, with no autoload prelude anywhere in lib/ — zero registerAutoloading calls in the repo.

OC_App::getEnabledApps() sorts the app list, and Coordinator::registerApps() walks that sorted list calling OC_App::registerAutoloading($appId, $path) and 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 — and openbuild sorts before openregister.

What was actually measured (and what was not)

Baseline = the last pull_request run without the prelude, run 31081906401 (a development push run is not a baseline: the gates runner auto-detects base origin/development, which is HEAD, and exits 99 "COULD NOT RUN").

	before (31081906401)	after (this branch)
hydra gate-64 `apphost-autoload-prelude`	FAIL — "1 AppHost adoption with no OpenRegister autoload prelude"	PASS
`OpenBuild: … Bootstrap is not autoloadable` in the E2E log	3× — once per `occ` call in `tests/e2e/ci-seed.sh`	0×
`GET /api/health`, `GET /api/metrics`	200 / 200	200 / 200

So under the CLI SAPI the guard really was answering false on a healthy instance with OpenRegister installed and enabled throughout: every occ command, background job and repair step lost the whole generic AppHost plumbing, silently.

⚠️ The "every request" framing in #140 does not survive measurement. In that same pre-prelude run, /api/health returned 200 with status: ok and /api/metrics rendered the manifest's gauges. OpenBuild ships no concrete HealthController/MetricsController; health#index exists only as a Bootstrap::register() DI alias — so under the web SAPI the guard was answering true and Bootstrap::register() did run. The CLI/web divergence is not explained, and no claim is made here that web requests were degraded. The prelude removes the dependence on whatever causes the difference; that is the argument, not "it was 500ing".

Every overclaim has been walked back in the code and the spec accordingly, including in tests/e2e/apphost-observability.spec.ts, which is relabelled as a regression guard on the aliases rather than before/after evidence — it did not fail before the prelude.

The fix

lib/AppInfo/OpenRegisterAutoloader.php, called from Application::register() before the class_exists() guard. Same shape already merged in procest (#752) and scholiq (#288).

OC_App::registerAutoloading() touches only the autoloader and is idempotent (early-returns on an $alreadyRegistered key).
Deliberately not IAppManager::loadApp('openregister') — that marks OpenRegister loaded and calls Coordinator::bootApp(), booting it before its own register() has run.
It never throws. An exception escaping it would abort the whole register() — strictly worse than the silent skip it replaces.
It is a separate class because Application cannot be constructed without a Nextcloud DI container, so an inline prelude has no reachable unit test. (That is why #139, the inline variant, was closed in favour of this one.)

Proof, both directions

gate-64 positive control, run against this exact tree with the .github@main matcher: prelude present → rc=0 apphost-autoload-prelude: OK; lib/AppInfo/OpenRegisterAutoloader.php moved away → rc=1 FAIL openbuild: … nothing under lib/AppInfo/ registers OpenRegister's autoloader first. The check can still fail.
Unit negative control: tests/Unit/AppInfo/OpenRegisterAutoloaderTest.php → OK (3 tests, 3 assertions). Delete the try/catch from the prelude and all three become errors — Error: Class "OC" not found — so the never-throws contract is an assertion that can fail, not one that always passes. The third test (testRegisterSwallowsAnAppThatCannotResolve) exists specifically to reach the catch branch, which is otherwise dead on any instance that has OpenRegister.

Also in this PR, because touching `register()` pulled them into diff scope

gate-2 forbidden-patterns: the else { error_log(…) } branch is removed, not suppressed. It has been in the file all along and gate-2 is diff-scoped, which is why it passed at baseline. With the prelude, a false class_exists() no longer means "not autoloadable yet" — it means OpenRegister genuinely is absent, a whole-instance condition an app cannot usefully narrate from its composition root with no PSR logger resolvable. /api/health is where that state is readable. Same choice doriath made.
gate-46 spec-anchor-existence: two @spec tags pointed at openspec/changes/archive/2026-05-12-openbuild-rbac/tasks.md. An archived change dir is never a valid @spec target; they now point at openspec/specs/openbuild-rbac/spec.md.

Spec

New REQ-OBS-006 in openspec/specs/settings-and-observability/spec.md, with the measurement above recorded in its Notes including the unexplained CLI/web divergence. REQ-OBS-005 corrected: it claimed the lazy factories "survive the app load order in which OpenRegister is not yet autoloadable", but those factories are registered by Bootstrap::register().

Known-unrelated CI state

E2E carries pre-existing failures on development: 3 × docudesk-document-templates and 1 × page-editor-coverage wiki page, plus a flaky builder-host. Baseline was 5 failed / 168 passed; this branch 3 failed / 172 passed. None of them touch this change.

Ref: ADR-040, hydra gate-64 (ConductionNL/.github#174, false-positive fix #184/#185).

Final CI state (run `31101513785`, head `29cac98a3`)

Hydra Gates: ALL 59 APPLICABLE GATES PASSED — and all 59 of them ran. gate-2, gate-46 and gate-64 all PASS.
PHPUnit: all 6 matrix cells (PHP 8.3/8.4 × NC stable31/32/33) success.
phpcs, psalm, phpstan, phpmd, phpmetrics, Security (composer + npm), licences, frontend build/unit/l10n/manifest, Features Check, Coverage Baseline Protection: success.
E2E: 3 failed / 172 passed. All three are pre-existing on development and fail identically on the pre-fix baseline run 31081906401 (which was 5 failed / 168 passed): 2 × docudesk-document-templates and 1 × page-editor-coverage wiki page. None touches this change.
Quality Report fails purely as the aggregate of that E2E result.
OpenBuild: … Bootstrap is not autoloadable in the E2E log: 0 (baseline 3).

Two commits in this PR are not the ADR-040 fix and are labelled as such: the phpcs inline-comment style fix that removing the else-branch made necessary, and squizlabs/php_codesniffer 3.13.5 → 3.13.6 for CVE-2026-67434 (OS command injection), which turned Security (composer) red mid-PR when the advisory reached Packagist. That is a version bump, not an ignore entry; composer audit now reports no advisories.

…ppHost `openbuild` 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\ was NOT autoloadable inside OpenBuild's own register() — on a perfectly healthy instance, with OpenRegister enabled. Measured, not theorised: `OpenBuild: OpenRegister AppHost\Bootstrap is not autoloadable` was logged 3x in the E2E job (every occ call in ci-seed.sh) while lib/AppHost/Bootstrap.php existed on OpenRegister the whole time. Because the reference sits behind a class_exists() guard, this degraded SILENTLY: Bootstrap::register() apparently never ran, so the generic dashboard/settings/preferences controllers, the observability controllers, the install repair steps and the manifest-driven deep-link listener were absent, with nothing in the UI to say so. Fix: put OpenRegister's PSR-4 prefix on the autoloader ourselves before the guard. 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 lives in its own class so its 'never throws' contract is reachable from a unit test — Application cannot be constructed without a NC DI container. Caught by hydra gate-64 (apphost-autoload-prelude), ADR-040.

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:10:52Z

Quality Report — ConductionNL/openbuild @ `09529f0`

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

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

Download the full PDF report from the workflow artifacts.

@e2e

…p the exclusions The spec scenarios for the ADR-040 prelude carried '@e2e exclude composition-root load order'. That was wrong twice over: an exclusion is not evidence, and the load order HAS a user-visible symptom that Playwright can reach. OpenBuild ships no concrete HealthController or MetricsController. Their route targets health#index and metrics#index — declared by \OCA\OpenRegister\AppHost\Routes::standard() in appinfo/routes.php — exist ONLY as DI aliases created by Bootstrap::register(). Without the prelude, class_exists(Bootstrap::class) answered false, Bootstrap::register() never ran, those aliases were never created, and the routes resolved to a class with no binding: HTTP 500, not 404. tests/e2e/apphost-observability.spec.ts asserts /api/health returns 200 with the engine's {status, app, version, checks} shape and app = 'openbuild', and that /api/metrics does not 5xx. It FAILS on the code before the prelude and passes after it — which is exactly what an exclusion cannot do. Verified locally: gate-19 PASSES with the @e2e annotation on the scenario, and FAILS with the annotation removed, so the traceability is real and not a tag that would be counted whatever happened. The absent-OpenRegister scenario is removed rather than excluded: it is not reachable from an HTTP client at all, so it is stated in the requirement prose and asserted in tests/Unit/AppInfo/OpenRegisterAutoloaderTest.php.

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

Quality Report — ConductionNL/openbuild @ `7bd7c2e`

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	✅
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		✅
stylelint		✅
build		✅
check-manifest		✅
test-l10n		✅
composer			✅	✅ 101/101
npm			✅	✅ 654/654
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.

github-actions · 2026-08-06T08:37:35Z

Quality Report — ConductionNL/openbuild @ `7319e27`

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

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

Download the full PDF report from the workflow artifacts.

github-actions · 2026-08-06T09:02:44Z

Quality Report — ConductionNL/openbuild @ `ac96cf2`

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

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

Download the full PDF report from the workflow artifacts.

… at specs, and correct the blast-radius claim Three things, all forced by measurement rather than taste. 1. gate-2 forbidden-patterns FAILS on this branch and PASSED on the baseline run (31081906401). Nothing new was added: `error_log()` has been in Application.php all along, and gate-2 is diff-scoped, so touching the surrounding lines is what put it in scope. The else-branch is removed rather than suppressed. With the prelude above it, a false `class_exists()` no longer means "not autoloadable YET" — it means OpenRegister genuinely is absent, which is a whole-instance condition an app cannot usefully narrate from its composition root with no PSR logger resolvable. /api/health, bound by `observability => true`, is where that state is readable. Same choice doriath made. 2. gate-46 spec-anchor-existence FAILS on two `@spec` tags pointing at `openspec/changes/archive/2026-05-12-openbuild-rbac/tasks.md`. An archived change dir is never a valid @SPEC target; the canonical home is `openspec/specs/openbuild-rbac/spec.md`, which exists. 3. The prose overclaimed, and the overclaim is the interesting part. Comparing the pre-prelude run 31081906401 with this branch's run 31085597692: - `OpenBuild: OpenRegister AppHost\Bootstrap is not autoloadable` appeared 3x before and 0x after — once per `occ` call in ci-seed.sh. So under the CLI SAPI the guard really was answering false on a healthy instance, and every occ command, background job and repair step lost the whole generic AppHost plumbing. - But in that SAME pre-prelude run, `/api/health` returned 200 with `status: ok` and `/api/metrics` rendered the manifest gauges. OpenBuild ships no concrete HealthController/MetricsController and `health#index` exists ONLY as a `Bootstrap::register()` DI alias — so under the web SAPI the guard was answering TRUE and Bootstrap::register() did run. The CLI/web divergence is not explained. So the claims "on every single request" and "had apparently never run" are withdrawn from Application.php, OpenRegisterAutoloader.php, REQ-OBS-006 and the e2e docblock, and replaced with what was actually measured. In particular tests/e2e/apphost-observability.spec.ts is relabelled: it did NOT fail before the prelude (both routes already answered 200), so it is a regression guard on the aliases, not the before/after evidence. The before/after evidence for the prelude's own contract is the unit test. Verified locally against this tree: - tests/Unit/AppInfo/OpenRegisterAutoloaderTest.php: OK (3 tests). - Negative control: deleting the try/catch from the prelude turns all 3 into errors ("Class \"OC\" not found"), so the never-throws contract is a check that can fail, not a check that always passes.

…ment that follows Removing the else branch left an inline comment block followed by a blank line, which Squiz.Commenting.InlineComment rejects: phpcs went 0 ERRORS -> 1 ERROR at Application.php:191 while every other job held. Verified locally against this tree — full-tree phpcs now exits 0 with 0 errors. Also drops 'on every single request' from that same note. The error_log line was measured on occ calls (3 per E2E run); whether web requests reached it was never established, and the comment should not claim more than the measurement does.

github-actions · 2026-08-06T11:40:58Z

Quality Report — ConductionNL/openbuild @ `3bb9ffb`

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

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

Download the full PDF report from the workflow artifacts.

github-actions · 2026-08-06T11:47:14Z

Quality Report — ConductionNL/openbuild @ `8ae3453`

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

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

Download the full PDF report from the workflow artifacts.

quality / Security (composer) went success -> failure between run 31095490811 (11:11Z) and run 31097941416 (11:42Z) on this branch. Nothing in the branch changed in between: the advisory was published 2026-08-05T23:53Z and reached the Packagist advisory database in that window. Package: squizlabs/php_codesniffer CVE-2026-67434 — OS Command injection Affected: <3.13.6 | >=4.0.0,<4.0.2 The composer.json constraint (^3.9) already permitted the patched release, so this is a lock-only change: one package, 5 lines. Verified afterwards with 'composer audit' -> 'No security vulnerability advisories found', full-tree phpcs exit 0 / 0 errors, and the prelude unit suite still OK (3 tests). This is a dev dependency and unrelated to the ADR-040 fix, but it blocks the merge and the fix is a version bump rather than a suppression, so it lands here instead of behind an ignore entry.

github-actions · 2026-08-06T12:56:48Z

Quality Report — ConductionNL/openbuild @ `155aa8b`

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

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

Download the full PDF report from the workflow artifacts.

juanclaude-conduction added 2 commits August 6, 2026 09:50

juanclaude-conduction added 2 commits August 6, 2026 12:58

Merge remote-tracking branch 'origin/development' into HEAD

ba7b2a1

rubenvdlinde mentioned this pull request Aug 6, 2026

fix(apphost): the class_exists guard has been answering false — Bootstrap::register() has never run #139

Closed

rubenvdlinde merged commit e079377 into development Aug 6, 2026
31 of 33 checks passed

Navigation Menu

Uh oh!

fix(apphost): register OpenRegister's autoloader before referencing AppHost (gate-64 / ADR-040) - #138

fix(apphost): register OpenRegister's autoloader before referencing AppHost (gate-64 / ADR-040)#138
rubenvdlinde merged 8 commits into
developmentfrom
fix/adr-040-autoload-prelude

rubenvdlinde commented Aug 6, 2026 •

edited

Loading

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 6, 2026

Uh oh!

github-actions Bot commented Aug 6, 2026

Uh oh!

github-actions Bot commented Aug 6, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

rubenvdlinde commented Aug 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The defect

What was actually measured (and what was not)

The fix

Proof, both directions

Also in this PR, because touching register() pulled them into diff scope

Spec

Known-unrelated CI state

Final CI state (run 31101513785, head 29cac98a3)

Uh oh!

github-actions Bot commented Aug 6, 2026

Quality Report — ConductionNL/openbuild @ 09529f0

Uh oh!

github-actions Bot commented Aug 6, 2026

Quality Report — ConductionNL/openbuild @ 7bd7c2e

Uh oh!

github-actions Bot commented Aug 6, 2026

Quality Report — ConductionNL/openbuild @ 7319e27

Uh oh!

github-actions Bot commented Aug 6, 2026

Quality Report — ConductionNL/openbuild @ ac96cf2

Uh oh!

github-actions Bot commented Aug 6, 2026

Quality Report — ConductionNL/openbuild @ 3bb9ffb

Uh oh!

github-actions Bot commented Aug 6, 2026

Quality Report — ConductionNL/openbuild @ 8ae3453

Uh oh!

github-actions Bot commented Aug 6, 2026

Quality Report — ConductionNL/openbuild @ 155aa8b

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

rubenvdlinde commented Aug 6, 2026 •

edited

Loading

Also in this PR, because touching `register()` pulled them into diff scope

Final CI state (run `31101513785`, head `29cac98a3`)

Quality Report — ConductionNL/openbuild @ `09529f0`

Quality Report — ConductionNL/openbuild @ `7bd7c2e`

Quality Report — ConductionNL/openbuild @ `7319e27`

Quality Report — ConductionNL/openbuild @ `ac96cf2`

Quality Report — ConductionNL/openbuild @ `3bb9ffb`

Quality Report — ConductionNL/openbuild @ `8ae3453`

Quality Report — ConductionNL/openbuild @ `155aa8b`