Skip to content

Commit fa805d7

Browse files
authored
feat(leaves): load a leaf's client half on consuming pages (#3404)
* feat(leaves): load a leaf's client half on consuming pages ADR-066 splits a cross-app leaf in two: a server-side LeafDescriptor, and the Vue components that render it. gate-24 enforces that both halves exist and agree. Nothing ever loaded the second one. The components live in the PROVIDING app's bundle and Nextcloud serves only the current app's scripts. Measured 2026-09-04 across three codebases: every addScript()/addEntryScripts() call in this repo names 'openregister' itself, @conduction/nextcloud-vue's dist contains no dynamic script injection at all, and humaniq and planninq register no BeforeTemplateRenderedEvent listener. So a descriptor reached OCS capability discovery and every server-side consumer, getLeaves() returned it, gate-24 went green on both halves, and the surface rendered nothing. humaniq-hours has been dark on dossiq case pages for as long as it has shipped. IntegrationGlobalScriptListener looks like it covers this and does not: it loads THIS app's integration-global bundle, which installs the registry and populates it with our built-ins. It cannot contain a sibling app's components. The two listeners are the two halves; this adds the missing one. It enqueues a dedicated `<app>-leaves` entry, never the app's main bundle. Measured on planninq: main is 13.57 MiB, leaves is 0.19 MiB. A main bundle on every page of every consuming app trades a feature for a performance regression. Scope is narrow on both axes. Only on pages of an app that ships an OpenRegister register descriptor of its own — the same lib/Settings/*register*.json signal gate-55 uses, one filesystem check, and it keeps sibling bundles off Files, Photos, Mail and Settings. Never the current app's own leaf: its bundle already registered it, and a second copy registers the id twice, which AD-13 warns about in production and throws on in development. An app shipping no built js/<app>-leaves.js is skipped, exactly as FilesSidebarListener skips its own missing bundle, so this can never enqueue a 404 into someone else's page. A throwing catalogue is logged and the page still renders. leafAppsFor() returns the app ids and handle() enqueues them, because ScriptManifestLoader is static over \OCP\Util and a listener that decides and acts in one method can only be verified by booting Nextcloud. 10 tests, weighted on the negatives; removing the "not my own app" guard turns one red. Verified on a live NC 34 instance with openregister + planninq + pipelinq: a pipelinq page enqueues planninq-leaves.js, and Files, Dashboard and planninq's own page do not. composer.lock moves hydra-gates v1.10.0 -> v1.12.0. The vendored copy was stale enough to fail gate-22 on src/manifest.json with "must be equal to one of the allowed values" for page type `flow`: its schema was 2.25.0, which predates that type. v1.12.0 carries 2.29.0, which has it. Nothing in the manifest changed. * fix(files): drop deleteFiles(), superseded by the batch handler gate-57 flagged DeleteFileHandler::deleteFiles() as an orphaned write capability. It is: zero references in lib, tests, src or docs, and FileService never exposed it. The reflex is to wire an orphan up rather than delete it, and the spec looks like it agrees — file-actions.md requires a batch endpoint for publish, depublish, delete and label. But that endpoint EXISTS and is complete: appinfo/routes.php:904 -> FilesController::batch() -> FileBatchHandler::executeBatch(), with the 100-file limit, the invalid-action message, per-file results and 207 on partial failure. Every scenario the requirement lists is implemented. And it does not call deleteFiles(). It loops FileService::deleteFile() one file at a time, which is precisely what produces the per-file success/error the spec asks for — a bulk call returning one aggregate cannot. So deleteFiles() is a second, unreferenced implementation of a loop that already exists in the path that has the route. Deleting it is therefore not a shortcut to green: it removes the duplicate and leaves the batch path the only way to delete many files, which is what the spec describes. ⚠️ `use Exception` STAYS. It reads unused after the removal and is not — deleteFile() still catches it at line 127. Verified: gate-57 exits 0, phpstan and phpcs clean on the changed file.
1 parent 88ced7d commit fa805d7

6 files changed

Lines changed: 637 additions & 33 deletions

File tree

composer.lock

Lines changed: 9 additions & 8 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

lib/AppInfo/Application.php

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2944,6 +2944,28 @@ private function registerEventListeners(IRegistrationContext $context): void {
29442944
\OCA\OpenRegister\Listener\IntegrationGlobalScriptListener::class
29452945
);
29462946

2947+
// LeafScriptListener is the OTHER half of that, and without it the
2948+
// listener above delivers an empty promise for cross-app leaves. It
2949+
// installs and populates the registry with OpenRegister's OWN built-ins
2950+
// — it cannot contain a sibling app's Vue components, because those
2951+
// live in that app's bundle and Nextcloud serves only the current app's
2952+
// scripts.
2953+
//
2954+
// Measured 2026-09-04: no code path anywhere enqueued a providing app's
2955+
// bundle on a consuming page. Every addScript() here names
2956+
// 'openregister', nc-vue injects no scripts, and the providing apps
2957+
// register no template listener. So an ADR-066 leaf reached OCS
2958+
// discovery, satisfied gate-24 on both halves, and rendered NOTHING —
2959+
// `humaniq-hours` has been dark on dossiq case pages for as long as it
2960+
// has shipped.
2961+
//
2962+
// This enqueues each providing app's dedicated `leaves` entry, scoped
2963+
// to pages of apps that ship a register descriptor of their own.
2964+
$context->registerEventListener(
2965+
\OCP\AppFramework\Http\Events\BeforeTemplateRenderedEvent::class,
2966+
\OCA\OpenRegister\Listener\LeafScriptListener::class
2967+
);
2968+
29472969
// PushClientScriptListener loads the always-on, opt-in Web Push
29482970
// subscribe client on EVERY full-page render (openregister-web-push-engine).
29492971
// The client never prompts on load — it only subscribes on a user
Lines changed: 292 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,292 @@
1+
<?php
2+
3+
/**
4+
* OpenRegister LeafScriptListener
5+
*
6+
* Loads the CLIENT half of every registered render-surface leaf onto the pages
7+
* of apps that consume OpenRegister.
8+
*
9+
* WHY THIS EXISTS — THE HALF THAT WAS NEVER WIRED
10+
* -----------------------------------------------
11+
* ADR-066 decision 1 splits a cross-app leaf in two: a server-side
12+
* `LeafDescriptor` contributed to `RegisterLeafProvidersEvent`, and a JS
13+
* `registerIntegration()` call supplying the render surface, "correlated to the
14+
* descriptor by a shared id". gate-24 enforces that both halves exist and agree.
15+
*
16+
* Nothing loaded the second half. A leaf's components live in the PROVIDING
17+
* app's bundle, Nextcloud serves only the current app's scripts, and — measured
18+
* across this repository, @conduction/nextcloud-vue and the providing apps
19+
* themselves on 2026-09-04 — no code path enqueued them:
20+
*
21+
* - every `Util::addScript()` / `addEntryScripts()` call here names
22+
* 'openregister' itself,
23+
* - nc-vue's dist contains no dynamic `<script>` injection at all,
24+
* - humaniq and planninq register no `BeforeTemplateRenderedEvent` listener.
25+
*
26+
* So a descriptor reached OCS capability discovery and every server-side
27+
* consumer, `getLeaves()` returned it, gate-24 went green on both halves — and
28+
* the surface rendered NOTHING, on every consuming page, for as long as leaves
29+
* have shipped. `humaniq-hours` has been dark on dossiq case pages the whole
30+
* time. That is the failure shape ADR-113 is about: everything reports success
31+
* and the feature is absent.
32+
*
33+
* WHAT IT LOADS, AND WHERE
34+
* ------------------------
35+
* A dedicated `leaves` webpack entry per providing app, NOT the app's main
36+
* bundle. A main bundle is megabytes and carries a whole SPA; putting one on
37+
* another app's page would be a performance regression traded for a feature.
38+
* An app that ships no `<app>-leaves` build artifact is skipped, exactly as
39+
* FilesSidebarListener skips its own missing bundle — so this can never enqueue
40+
* a 404.
41+
*
42+
* Scope is deliberately narrow on both axes:
43+
*
44+
* - ONLY on pages of an app that itself ships an OpenRegister register
45+
* descriptor. That is the same signal gate-55 uses to decide "is this
46+
* register mine", it costs one filesystem check, and it keeps leaf bundles
47+
* off Files, Photos, Mail and Settings, which host no OpenRegister objects.
48+
* - NEVER the current app's own leaf: its bundle is already on its own page,
49+
* and loading a second copy would register the id twice, which the AD-13
50+
* collision policy warns about in production and throws on in development.
51+
*
52+
* @category Listener
53+
* @package OCA\OpenRegister\Listener
54+
*
55+
* @author Conduction Development Team <dev@conduction.nl>
56+
* @copyright 2026 Conduction B.V.
57+
* @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12
58+
*
59+
* @version GIT: <git-id>
60+
*
61+
* @link https://conduction.nl
62+
*
63+
* SPDX-FileCopyrightText: 2026 Conduction B.V. <info@conduction.nl>
64+
* SPDX-License-Identifier: EUPL-1.2
65+
*/
66+
67+
declare(strict_types=1);
68+
69+
namespace OCA\OpenRegister\Listener;
70+
71+
use OCA\OpenRegister\AppInfo\Application;
72+
use OCA\OpenRegister\Service\Integration\LeafDescriptor;
73+
use OCA\OpenRegister\Service\Integration\LeafRegistry;
74+
use OCA\OpenRegister\Service\ScriptManifestLoader;
75+
use OCP\App\IAppManager;
76+
use OCP\AppFramework\Http\Events\BeforeTemplateRenderedEvent;
77+
use OCP\EventDispatcher\Event;
78+
use OCP\EventDispatcher\IEventListener;
79+
use OCP\IRequest;
80+
use Psr\Log\LoggerInterface;
81+
use Throwable;
82+
83+
/**
84+
* Enqueues each providing app's leaf bundle on consuming apps' pages.
85+
*
86+
* @template-implements IEventListener<Event>
87+
*/
88+
class LeafScriptListener implements IEventListener {
89+
90+
/**
91+
* The webpack entry name a providing app must expose.
92+
*
93+
* @var string
94+
*/
95+
public const LEAF_ENTRY = 'leaves';
96+
97+
/**
98+
* Constructor.
99+
*
100+
* @param LeafRegistry $leaves The collected leaf catalogue.
101+
* @param IAppManager $appManager App installation + path lookups.
102+
* @param IRequest $request The current request, for the app id.
103+
* @param LoggerInterface $logger PSR-3 logger.
104+
*/
105+
public function __construct(
106+
private readonly LeafRegistry $leaves,
107+
private readonly IAppManager $appManager,
108+
private readonly IRequest $request,
109+
private readonly LoggerInterface $logger,
110+
) {
111+
}//end __construct()
112+
113+
/**
114+
* Enqueue the leaf bundles this page needs.
115+
*
116+
* @param Event $event The dispatched event.
117+
*
118+
* @return void
119+
*
120+
* @SuppressWarnings(PHPMD.StaticAccess) ScriptManifestLoader is static for
121+
* the same reason \OCP\Util::addScript it wraps is: there is no
122+
* injectable DI equivalent in the AppFramework.
123+
*
124+
* @spec openspec/specs/integration-registry/spec.md#requirement-a-leafs-client-half-must-be-loaded-on-consuming-pages
125+
*/
126+
public function handle(Event $event): void {
127+
if ($event instanceof BeforeTemplateRenderedEvent === false) {
128+
return;
129+
}
130+
131+
// A public page has no OpenRegister objects to hang a leaf on, and
132+
// loading another app's bundle there would widen the anonymous surface.
133+
if ($event->isLoggedIn() === false) {
134+
return;
135+
}
136+
137+
try {
138+
$currentApp = $this->currentAppId();
139+
if ($currentApp === null || $currentApp === Application::APP_ID) {
140+
return;
141+
}
142+
143+
foreach ($this->leafAppsFor(currentApp: $currentApp) as $providingApp) {
144+
ScriptManifestLoader::addEntryScripts(
145+
appId: $providingApp,
146+
entry: self::LEAF_ENTRY,
147+
fallbackScript: $providingApp . '-' . self::LEAF_ENTRY
148+
);
149+
}
150+
} catch (Throwable $e) {
151+
// A page must render even when the catalogue cannot be read. The
152+
// leaf is then absent, which is the pre-existing behaviour.
153+
$this->logger->warning(
154+
'OpenRegister could not enqueue leaf scripts: ' . $e->getMessage(),
155+
['exception' => $e]
156+
);
157+
}//end try
158+
159+
}//end handle()
160+
161+
/**
162+
* The providing apps whose leaf bundles belong on this page.
163+
*
164+
* This is the whole rule, deliberately separated from the enqueuing so it
165+
* can be tested: `ScriptManifestLoader` is static over `\OCP\Util`, and a
166+
* listener that decides and acts in one method can only be verified by
167+
* booting Nextcloud.
168+
*
169+
* @param string $currentApp The app whose page is rendering.
170+
*
171+
* @return string[] App ids to load, in catalogue order, without duplicates.
172+
*
173+
* @spec openspec/specs/integration-registry/spec.md#requirement-a-leafs-client-half-must-be-loaded-on-consuming-pages
174+
*/
175+
public function leafAppsFor(string $currentApp): array {
176+
if ($currentApp === '' || $currentApp === Application::APP_ID) {
177+
return [];
178+
}
179+
180+
// A page with no OpenRegister objects has nothing for a leaf to attach
181+
// to. This keeps sibling bundles off Files, Photos, Mail and Settings.
182+
if ($this->shipsRegisterDescriptor(appId: $currentApp) === false) {
183+
return [];
184+
}
185+
186+
$apps = [];
187+
foreach ($this->leaves->getDescriptors() as $descriptor) {
188+
if (in_array(LeafDescriptor::KIND_RENDER_SURFACE, $descriptor->getKinds(), true) === false) {
189+
// A data-provider leaf has no client half to load.
190+
continue;
191+
}
192+
193+
$providingApp = $descriptor->getRequiredApp();
194+
if ($providingApp === null || $providingApp === $currentApp) {
195+
// Built-in leaves ride on OpenRegister's own bundle, and an
196+
// app's own leaf is already loaded by its own page. Loading a
197+
// second copy would register the id twice, which the AD-13
198+
// collision policy warns about in production and throws on in
199+
// development.
200+
continue;
201+
}
202+
203+
if (in_array($providingApp, $apps, true) === true) {
204+
// One app may contribute several leaves; its bundle carries all
205+
// of them and must be enqueued once.
206+
continue;
207+
}
208+
209+
if ($this->appManager->isEnabledForUser($providingApp) === false) {
210+
continue;
211+
}
212+
213+
if ($this->hasLeafBundle(appId: $providingApp) === false) {
214+
// Enqueuing a script that does not exist is a 404 in the page,
215+
// so an app that ships no leaf entry is simply skipped.
216+
continue;
217+
}
218+
219+
$apps[] = $providingApp;
220+
}
221+
222+
return $apps;
223+
}//end leafAppsFor()
224+
225+
/**
226+
* The app whose page is being rendered, from the request path.
227+
*
228+
* Nextcloud serves an app under BOTH `/apps/<id>/…` and
229+
* `/index.php/apps/<id>/…`; the pattern below accepts either, because a
230+
* visitor arriving on the other form is a real case and matching only one
231+
* would silently drop the leaf for them.
232+
*
233+
* @return string|null The app id, or null when this is not an app page.
234+
*/
235+
private function currentAppId(): ?string {
236+
$path = (string)$this->request->getPathInfo();
237+
if (preg_match('#(?:^|/)apps/([a-z0-9_.-]+)(?:/|$)#', $path, $m) !== 1) {
238+
return null;
239+
}
240+
return $m[1];
241+
}//end currentAppId()
242+
243+
/**
244+
* Whether an app ships an OpenRegister register descriptor of its own.
245+
*
246+
* The same signal gate-55 uses to decide which registers an app owns:
247+
* a `lib/Settings/*register*.json`. An app with one consumes OpenRegister
248+
* and may render objects; an app without one (Files, Mail, Settings) has
249+
* nothing for a leaf to attach to.
250+
*
251+
* @param string $appId The app to test.
252+
*
253+
* @return boolean Whether it ships a register descriptor.
254+
*/
255+
private function shipsRegisterDescriptor(string $appId): bool {
256+
$path = $this->appPath(appId: $appId);
257+
if ($path === null) {
258+
return false;
259+
}
260+
return glob($path . '/lib/Settings/*register*.json') !== [];
261+
}//end shipsRegisterDescriptor()
262+
263+
/**
264+
* Whether an app ships a built leaf bundle.
265+
*
266+
* @param string $appId The providing app.
267+
*
268+
* @return boolean Whether `js/<app>-leaves.js` exists.
269+
*/
270+
private function hasLeafBundle(string $appId): bool {
271+
$path = $this->appPath(appId: $appId);
272+
if ($path === null) {
273+
return false;
274+
}
275+
return file_exists($path . '/js/' . $appId . '-' . self::LEAF_ENTRY . '.js');
276+
}//end hasLeafBundle()
277+
278+
/**
279+
* An app's filesystem path, or null when it cannot be resolved.
280+
*
281+
* @param string $appId The app.
282+
*
283+
* @return string|null The path.
284+
*/
285+
private function appPath(string $appId): ?string {
286+
try {
287+
return $this->appManager->getAppPath($appId);
288+
} catch (Throwable) {
289+
return null;
290+
}
291+
}//end appPath()
292+
}//end class

0 commit comments

Comments
 (0)