Skip to content

feat(gates): gate-64 apphost-autoload-prelude — AppHost adoption without OpenRegister's autoloader - #174

Merged
rubenvdlinde merged 2 commits into
mainfrom
feat/gate-64-apphost-autoload-prelude
Aug 5, 2026
Merged

feat(gates): gate-64 apphost-autoload-prelude — AppHost adoption without OpenRegister's autoloader#174
rubenvdlinde merged 2 commits into
mainfrom
feat/gate-64-apphost-autoload-prelude

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

What this gate catches

OC_App::getEnabledApps() does sort($apps), and Coordinator::registerApps() walks that sorted list calling OC_App::registerAutoloading($appId, $path) and then $application->register() for one app at a time.

So every app's register() runs before the PSR-4 prefix of every alphabetically-later app exists. Any leaf whose app id sorts before openregister reaches register() while OCA\OpenRegister\ is not autoloadable — on a healthy instance, with OpenRegister enabled.

Two shapes, both quiet:

  • Guardedclass_exists('OCA\OpenRegister\AppHost\...') answers FALSE, the generic plumbing is skipped, and classes that exist ONLY as Bootstrap DI aliases (Controller\HealthControllerAppHost\Controller\GenericHealthController) fail to resolve. Those endpoints return 500, not 404.
  • Unguarded — the \Error aborts the entire register(). Every registerEventListener below it never runs. Coordinator catches the Throwable, logs an emergency and continues, so the app stays enabled and keeps serving.

Measured twice, independently: doriath (the audit listener recorded ZERO dispatched events) and openconnector, whose own source records class_exists at register(): false on a clean install — removing that guard took its flow-node registry from 10 nodes to 12.

Why a gate and not a test

The failure depends on which apps happen to be installed. Any app that pulls OpenRegister's autoloader in registers the prefix process-wide, masking the defect for every app that registers after it. On a dev instance with such an app present everything resolves; in CI with a minimal app set it does not. A masking app makes the failure vanish exactly where you would test for it.

Proof it goes RED

Not shipped green-only. Verified at both levels.

Helper, against REAL fleet codedoriath at origin/development (the pre-fix include_once ../../../openregister/vendor/autoload.php workaround, no registerAutoloading):

FAIL doriath-before: lib/AppInfo/Application.php references OCA\OpenRegister\AppHost\Bootstrap,
but nothing under lib/AppInfo/ registers OpenRegister's autoloader first. ...
1 AppHost adoption(s) without the autoload prelude.
rc=1

The same app with the prelude applied (ConductionNL/doriath#162) → apphost-autoload-prelude: OK, rc=0.

Through the actual runner (run-hydra-gates.sh, not just the helper — a working helper and a reporting gate are different things):

[gate-64] apphost-autoload-prelude: FAIL — 1 AppHost adoption(s) with no OpenRegister autoload prelude (ADR-040)
[hydra-gates] COVERAGE: 34 of 64 declared gates reported a result
[gate-64] apphost-autoload-prelude: PASS                     # fixed app
[gate-64] apphost-autoload-prelude: NOT APPLICABLE — no lib/AppInfo/ ...   # no composition root

The declared inventory moved 63 → 64 on its own, so the gate is counted by the coverage accounting rather than being invisible.

Unit suitescripts/lib/test_check_apphost_autoload_prelude.py, 11 tests, auto-discovered by tests/run-helper-suites.sh (19 passed / 0 failed / 2 documented quarantines with this branch applied). It writes a known-bad app, asserts RED, then adds the prelude to the same app and asserts GREEN.

One of those tests caught a real bug in the helper before it shipped: apphost-prelude\s+exclude\s+(?P<reason>\S.*) used \s, which matches a newline — so a bare // apphost-prelude exclude swallowed the next line of source as its "reason" and bought a free pass. The separator is now [ \t], and a reasonless annotation fails like any other unguarded adoption.

False-positive check across the fleet

Ran against the real lib/AppInfo/ of 18 apps on their development branches. Only pre-fix doriath flags. openregister is exempt by namespace (it owns AppHost), not by directory name. Lazy container closures that merely mention an AppHost class are deliberately not flagged — their bodies run long after every app has registered.

Heads-up for consumers

The fleet pins hydra-gates-ref: v1.3.0 in each consumer's code-quality.yml, so this gate does nothing until a consumer-ref sweep moves them. That sweep is deliberately not part of this PR — four agents are mid-flight on app repos tonight and changing their CI underneath them would be reckless.

When the sweep happens, a read-only dry run says openbuild and scholiq will go RED (both call Bootstrap::register with no prelude); decidesk and shillinq are green. Those four repos were left untouched here.

Also not done here: no git tag is cut, and no hydra-gate-apphost-autoload-prelude skill was added to the hydra repo alongside the other gate skills — both belong with the sweep.

…out OpenRegister's autoloader

OC_App::getEnabledApps() sort()s the app list and Coordinator::registerApps()
walks that sorted list calling registerAutoloading() then register() one app at
a time. So every app's register() runs before the PSR-4 prefix of every
alphabetically-later app exists, and any leaf sorting before 'openregister'
reaches register() while OCA\OpenRegister\ is not autoloadable.

Guarded, it degrades silently (class_exists() answers FALSE, Bootstrap DI
aliases stop resolving and their endpoints 500). Unguarded, the \Error aborts
the whole register() — Coordinator logs an emergency and continues, so the app
stays enabled with half its wiring missing. Measured on doriath (audit listener
saw ZERO events) and independently on openconnector.

It needs a gate rather than a test because the failure depends on which apps are
installed: any app that pulls OpenRegister's autoloader in registers the prefix
process-wide and masks it for everything registering after it.

Lazy service closures that merely mention an AppHost class are not flagged.
…claiming

The failure message asserted the defect for every flagged app. Only apps whose
id sorts BEFORE 'openregister' are actually broken on a healthy instance; the
rest work today by alphabet alone. Saying otherwise makes the gate look like it
is crying wolf on the majority of repos where the prelude is defensive.

The verdict stays FAIL in both cases — depending on your own app id for
correctness is a landmine, not a design — but the message now says which one it
is, and reads the id from appinfo/info.xml rather than the checkout directory,
since the id is what Nextcloud sorts on.
@rubenvdlinde
rubenvdlinde merged commit 3a51d04 into main Aug 5, 2026
7 checks passed
rubenvdlinde pushed a commit that referenced this pull request Aug 5, 2026
Three reconciliations, each caught by an existing assertion rather than by
reading the diff:

* gate-28 — #172 gave this gate a four-way skip taxonomy (`na` for no lib/,
  `na` for no composer.json, `structural` for a composer.json without a
  `license`, `structural` for nothing carrying a tag) and made PASS
  conditional on having actually compared a file. Moving the read into a
  helper collapsed all of that into one `na`, undoing it. The taxonomy is
  restored verbatim; the helper now returns the compared-file count on
  stderr so `_lt_checked` still gates the PASS. The one state that
  short-circuits it is a MISSING HELPER — which #172's chain would have
  called `structural`, i.e. a claim about the REPOSITORY, when the
  repository is fine and the gate is broken. Those must not wear the same
  words.

* gates 40 and 46 — an empty in-scope file set is ordinary ADR-020 diff
  scoping, not "not applicable". tests/test-hydra-gates-bin.sh asserts that
  no src-guarded gate reports NOT APPLICABLE while src/ exists, and gate-40
  was tripping it. Answering differently from every sibling gate drifts the
  applicability table away from the guards it mirrors — the one way that
  table could hide a live gate.

* quality.yml — #173 added a preflight for the FLAG direction of the
  floating-caller / pinned-callee desync (does the pinned runner understand
  the coverage flag?). This branch adds one for the PATH direction (does the
  pinned package contain the files the workflow executes by name?). Both are
  kept: they are different halves of the same interface, and #168 broke the
  half #173 does not cover.

And one contract change, made explicit rather than incidental:

  test-hydra-gates-bin.sh's "empty diff exits 0" fixture set BASE_SHA to
  HEAD, so it was really asserting "scoping a commit against itself exits 0"
  — the shillinq shape. Those are different facts and only one is
  legitimate. The original assertion now runs against a real
  base-behind-HEAD empty diff and still expects 0; a new assertion covers
  base == HEAD, expects 99, and checks that NO gate printed PASS.

Verified on the merged tree:

  hydra-gates entry-point tests: 36 passed, 0 failed  (was 31 passed, 3 failed)
  helper suites:                 23 passed, 0 failed
  gate-46 fleet: 1,995 -> 918    gate-40 fleet: 1,211 -> 517  (unchanged by the merge)
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