fix(apphost): stop MetricsController's extends from 500-ing every route

lib/AppInfo/Application.php

@@ -732,19 +732,40 @@ static function (ContainerInterface $c) {
             }
         );
 
-        // Build the thin openconnector MetricsController subclass (URL
-        // /api/metrics, route name metrics#index — both unchanged). Admin-only
-        // posture is engine-owned and re-declared on the subclass method.
+        // Build the thin openconnector MetricsController (URL /api/metrics,
+        // route name metrics#index — both unchanged) with the engine delegate
+        // resolved from OpenRegister's app container, scoped to this app's
+        // manifest via appName. Admin-only posture is engine-owned and
+        // re-declared on the openconnector method.
         $context->registerService(
             MetricsController::class,
             static function (ContainerInterface $c) {
-                // phpcs:ignore CustomSniffs.Nextcloud.NoLegacyServerAccessors.LegacyNamedAccessor -- cross-app DI container lookup; no \OCP\Server equivalent, still used by NC34 core (OCP\AppFramework\App).
-                $orContainer = \OC::$server->getRegisteredAppContainer('openregister');
+                $appManager = $c->get(\OCP\App\IAppManager::class);
+
+                // Mirrors the HealthController guard above. When OpenRegister
+                // is absent the engine delegate cannot be built, so pass null
+                // and let MetricsController return a clean 503 instead of a
+                // bare DI 500 — getRegisteredAppContainer() throws for an app
+                // that is not registered. Building the delegate references
+                // OpenRegister classes, so it is only done when OpenRegister
+                // is enabled.
+                $delegate = null;
+                if ($appManager->isInstalled('openregister') === true) {
+                    // phpcs:ignore CustomSniffs.Nextcloud.NoLegacyServerAccessors.LegacyNamedAccessor -- cross-app DI container lookup; no \OCP\Server equivalent, still used by NC34 core (OCP\AppFramework\App).
+                    $orContainer = \OC::$server->getRegisteredAppContainer('openregister');
+                    $delegate    = new \OCA\OpenRegister\AppHost\Controller\GenericMetricsController(
+                        appName: self::APP_ID,
+                        request: $c->get(IRequest::class),
+                        manifestLoader: $orContainer->get(\OCA\OpenRegister\AppHost\Observability\ManifestLoader::class),
+                        engine: $orContainer->get(\OCA\OpenRegister\AppHost\Observability\MetricsEngine::class)
+                    );
+                }
+
                 return new MetricsController(
                     appName: self::APP_ID,
                     request: $c->get(IRequest::class),
-                    manifestLoader: $orContainer->get(\OCA\OpenRegister\AppHost\Observability\ManifestLoader::class),
-                    engine: $orContainer->get(\OCA\OpenRegister\AppHost\Observability\MetricsEngine::class)
+                    appManager: $appManager,
+                    delegate: $delegate
                 );
             }
         );
@@ -1078,6 +1099,8 @@ private function assertStorageMigrated(): void
      * @param IBootContext $context Boot context.
      *
      * @return void
+     *
+     * @spec openspec/specs/apphost-adoption/spec.md
      */
     public function boot(IBootContext $context): void
     {

lib/Controller/MetricsController.php

@@ -1,23 +1,35 @@
 <?php
 
 /**
- * OpenConnector Metrics Controller — AppHost adapter.
+ * OpenConnector Metrics Controller — AppHost adapter with OpenRegister guard.
  *
- * Thin app-namespace subclass of the OpenRegister AppHost
- * {@see \OCA\OpenRegister\AppHost\Controller\GenericMetricsController}. It
- * carries no metric logic of its own: the engine reads openconnector's
- * `src/manifest.json` `observability.metrics` block (resolved by `appName`)
- * and renders Prometheus text exposition 0.0.4. This class exists only so the
- * `metrics#index` route name (URL `/api/metrics`, unchanged) resolves in
- * openconnector's namespace and so the admin-only posture is declared on a
- * concrete openconnector method (ADR-006 / ADR-016).
+ * Thin app-namespace metrics endpoint (URL `/api/metrics`, route name
+ * `metrics#index`, both unchanged). It carries no metric logic of its own:
+ * when OpenRegister is present it delegates to the OpenRegister AppHost
+ * {@see \OCA\OpenRegister\AppHost\Controller\GenericMetricsController}, which
+ * reads openconnector's `src/manifest.json` `observability.metrics` block
+ * (resolved by `appName`) and renders Prometheus text exposition 0.0.4.
+ *
+ * This class deliberately extends {@see \OCP\AppFramework\Controller} rather
+ * than the OpenRegister generic. A parent class must be loaded before the
+ * child can be declared, and Nextcloud's router `ReflectionClass`es EVERY
+ * controller while matching a route — so extending a class from an app that
+ * may be absent turns a missing optional dependency into an HTTP 500 on every
+ * route in openconnector, not just on `/api/metrics`. Injecting the engine
+ * controller as a nullable delegate avoids that: a nullable *parameter type*
+ * is never autoloaded (only `extends`/`implements` are resolved at class
+ * declaration time), and this service is built by an explicit factory in
+ * {@see \OCA\OpenConnector\AppInfo\Application::registerAppHostObservability()},
+ * so the container never autowires — and therefore never resolves — that
+ * parameter.
+ *
+ * This mirrors the guard already used by the sibling
+ * {@see \OCA\OpenConnector\Controller\HealthController}.
  *
  * The metrics endpoint stays admin-only: the method declares no
  * `#[NoAdminRequired]`, so the Nextcloud SecurityMiddleware requires an admin
  * session — the intended ADR-006 posture, owned by the engine and preserved
- * here. The constructor + service wiring (resolving `MetricsEngine` from
- * OpenRegister's app container, with `appName = openconnector`) is registered
- * in {@see \OCA\OpenConnector\AppInfo\Application::registerAppHostObservability()}.
+ * here.
  *
  * @category Controller
  * @package  OCA\OpenConnector\Controller
@@ -36,21 +48,61 @@
 namespace OCA\OpenConnector\Controller;
 
 use OCA\OpenRegister\AppHost\Controller\GenericMetricsController;
+use OCP\App\IAppManager;
+use OCP\AppFramework\Controller;
+use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
 use OCP\AppFramework\Http\TextPlainResponse;
+use OCP\IRequest;
 
 /**
  * Admin-only declarative Prometheus metrics endpoint, delegated to the engine.
  *
  * @spec openspec/specs/apphost-adoption/spec.md
  */
-class MetricsController extends GenericMetricsController
+class MetricsController extends Controller
 {
+
+    /**
+     * The required dependency app id.
+     *
+     * @var string
+     */
+    private const REQUIRED_APP = 'openregister';
+
+    /**
+     * Prometheus text exposition content type (mirrors the engine's renderer).
+     *
+     * @var string
+     */
+    private const CONTENT_TYPE = 'text/plain; version=0.0.4; charset=utf-8';
+
+    /**
+     * Constructor.
+     *
+     * @param string                        $appName    Calling app id (openconnector).
+     * @param IRequest                      $request    HTTP request.
+     * @param IAppManager                   $appManager App-enablement query service (never touches OpenRegister classes).
+     * @param GenericMetricsController|null $delegate   Engine metrics controller, or null when OpenRegister is absent.
+     */
+    public function __construct(
+        string $appName,
+        IRequest $request,
+        private readonly IAppManager $appManager,
+        private readonly ?GenericMetricsController $delegate=null
+    ) {
+        parent::__construct(appName: $appName, request: $request);
+
+    }//end __construct()
+
     /**
      * GET /api/metrics — declarative Prometheus metrics (admin-only, ADR-006).
      *
-     * Delegates entirely to the engine. No `#[NoAdminRequired]` — admin
-     * session required (engine-owned posture).
+     * Admin-only by the deliberate absence of `#[NoAdminRequired]`.
+     *
+     * Returns HTTP 503 with a Prometheus comment line when the AppHost engine
+     * is unavailable (OpenRegister absent or disabled) — never a 500, and
+     * without referencing any OpenRegister class on that path.
      *
      * @return TextPlainResponse Prometheus text exposition 0.0.4.
      *
@@ -59,6 +111,17 @@ class MetricsController extends GenericMetricsController
     #[NoCSRFRequired]
     public function index(): TextPlainResponse
     {
-        return parent::index();
+        if ($this->appManager->isInstalled(self::REQUIRED_APP) === false || $this->delegate === null) {
+            $response = new TextPlainResponse(
+                '# metrics unavailable: OpenConnector requires the OpenRegister app — install and enable it.'."\n",
+                Http::STATUS_SERVICE_UNAVAILABLE
+            );
+            $response->addHeader('Content-Type', self::CONTENT_TYPE);
+
+            return $response;
+        }
+
+        return $this->delegate->index();
+
     }//end index()
 }//end class

tests/Unit/Observability/OpenConnectorMetricsProviderTest.php

@@ -69,6 +69,43 @@ function (string $id) use ($objectService, $schemaMapper, $registerMapper) {
     }//end buildProvider()
 
 
+    /**
+     * Every provider-backed metric NAME this provider is expected to emit.
+     *
+     * One MetricSample per name, regardless of how many points each carries.
+     * Asserting the name SET rather than a bare count means adding or losing a
+     * metric fails with a diff that names it, instead of an opaque
+     * "6 does not match 3" — which is how this assertion silently rotted when
+     * #1126 revived sources_total / calls_total / synchronization_runs_total.
+     *
+     * @var string[]
+     */
+    private const EXPECTED_METRIC_NAMES = [
+        'api_product_errors_total',
+        'api_product_latency_seconds',
+        'calls_total',
+        'circuit_breaker_state',
+        'sources_total',
+        'synchronization_runs_total',
+    ];
+
+
+    /**
+     * Assert the provider emitted exactly the expected set of metric names.
+     *
+     * @param array<int,object> $samples MetricSample objects returned by the provider.
+     *
+     * @return void
+     */
+    private function assertSameMetricNames(array $samples): void
+    {
+        $names = array_map(static fn (object $s): string => $s->name, $samples);
+        sort($names);
+
+        $this->assertSame(self::EXPECTED_METRIC_NAMES, $names);
+    }//end assertSameMetricNames()
+
+
     /**
      * TC-19 — mixed open/closed/never-evaluated sources report 1/0/0.
      *
@@ -94,11 +131,7 @@ public function searchObjects(array $query, bool $_rbac=true, bool $_multitenanc
         );
 
         $samples = $provider->metrics();
-        // api-product-gateway added two more provider-backed gauges
-        // (api_product_latency_seconds, api_product_errors_total) alongside
-        // the pre-existing circuit_breaker_state — 3 MetricSample objects
-        // (one per metric NAME) regardless of how many points each carries.
-        $this->assertCount(3, $samples);
+        $this->assertSameMetricNames($samples);
 
         $sample = $this->sampleByName($samples, 'circuit_breaker_state');
         $this->assertSame('circuit_breaker_state', $sample->name);
@@ -138,7 +171,7 @@ public function searchObjects(array $query, bool $_rbac=true, bool $_multitenanc
 
         $samples = $provider->metrics();
 
-        $this->assertCount(3, $samples);
+        $this->assertSameMetricNames($samples);
         $sample = $this->sampleByName($samples, 'circuit_breaker_state');
         $this->assertCount(1, $sample->samples);
         $this->assertSame(0, $sample->samples[0]['value']);
@@ -157,7 +190,7 @@ public function testUnavailableObjectServiceFallsBackToZeroValue(): void
 
         $samples = $provider->metrics();
 
-        $this->assertCount(3, $samples);
+        $this->assertSameMetricNames($samples);
         $circuitBreaker = $this->sampleByName($samples, 'circuit_breaker_state');
         $this->assertSame(0, $circuitBreaker->samples[0]['value']);
     }//end testUnavailableObjectServiceFallsBackToZeroValue()