fix(rest): route probeMcpServeable through the shared environment entry point (#9120) (#9291)

`resolveRequestEnvironmentId` is, by its own doc-comment, "THE single entry
point for every unscoped-route environment decision ... so they can never
disagree about which kernel a request belongs to". Eight consumers go through
it; `probeMcpServeable` -- the ninth site needing the request's environment,
and the one deciding whether /discovery advertises `routes.mcp` -- re-derived
its own from `params.environmentId` else `defaultEnvironmentIdProvider`.

That is the shared chain minus the host's ADR-0006 `kernel-resolver` seam and
the legacy hostname / X-Environment-Id steps. Single-environment boots agreed
anyway (the default provider is step 3 of the shared chain), so the defect was
multi-tenant-only: on a hostname-routed host an unscoped /discovery request has
no `params.environmentId` and no default provider is registered, so the probe
fell through to `serviceExistsProvider` -- the HOST kernel's answer, not the
request environment's. Both misadvertisement directions were reachable and are
now pinned.

The `'platform'` guard and the `serviceExistsProvider` fallback are unchanged.
The unsubstituted `':environmentId'` route pattern is normalised to "no id"
before the call, since the entry point short-circuits on any truthy explicit
value.


Claude-Session: https://claude.ai/code/session_01Y26DJEHSBhhAQ6wwfsHNza

Co-authored-by: Claude <noreply@anthropic.com>

Author: os-project-manager

.changeset/probe-mcp-serveable-shared-entry-point.md

@@ -0,0 +1,50 @@
+---
+"@objectstack/rest": patch
+---
+
+fix(rest): `/discovery`'s `mcp` advertisement follows the request's environment — `probeMcpServeable` routes through the shared resolution entry point (#9120)
+
+`RestServer.resolveRequestEnvironmentId` calls itself, in its own doc-comment,
+"THE single entry point for every unscoped-route environment decision (protocol,
+i18n, exec-ctx, analytics, …) so they can never disagree about which kernel a
+request belongs to." Eight consumers go through it. `probeMcpServeable` — the
+ninth site that needs the request's environment, and the one whose answer decides
+whether `/discovery` advertises `routes.mcp` — re-derived its own:
+
+```ts
+let environmentId: string | undefined = req?.params?.environmentId;
+if ((!environmentId || environmentId === ':environmentId') && this.defaultEnvironmentIdProvider) {
+    try { environmentId = this.defaultEnvironmentIdProvider() || undefined; } catch { /* ignore */ }
+}
+```
+
+That is the shared chain minus its first and middle steps: the host's ADR-0006
+`kernel-resolver` seam (wired through `RestRequestEnvResolver`), and the legacy
+hostname / `X-Environment-Id` chain beneath it.
+
+**Single-environment boots were correct throughout** — there
+`defaultEnvironmentIdProvider` is registered, and it is also step 3 of the shared
+chain, so both spellings agreed. The defect is multi-tenant-only: on a
+hostname-routed host an unscoped `/discovery` request carries no
+`params.environmentId`, and no default provider is registered (that is
+`createSingleEnvironmentPlugin`'s wiring). Neither input the probe read was
+present, so it fell through to `serviceExistsProvider` — which answers for the
+**host** kernel, not the request's environment. Both misadvertisement directions
+were reachable, and are now pinned as regression tests:
+
+- the host kernel has `mcp` and the request's environment does not ⇒ `/discovery`
+  advertised `routes.mcp` for an environment whose `/mcp` answers 501 — the
+  `declared ≠ enforced` shape the probe was added to close;
+- the host kernel lacks it and the environment has it ⇒ the route was withheld
+  from an environment that would have served it (`mcpServeable !== false` fails
+  open only for a `null` probe, never for a confident `false` computed against
+  the wrong kernel).
+
+The probe now calls `resolveRequestEnvironmentId` like its eight siblings. The
+`'platform'` guard and the `serviceExistsProvider` fallback are unchanged, and
+the unsubstituted `':environmentId'` route pattern is normalised to "no id"
+before the call — the entry point short-circuits on any truthy explicit value,
+so passing the pattern through would have sent it to `getOrCreate`. This also
+makes good the parity the probe's doc-comment already claimed with
+`resolveRegisteredServices`, whose kernel arrives as `ctx.__kernel` — set
+downstream of the same entry point.

packages/rest/src/rest-env-resolution.test.ts

@@ -58,6 +58,8 @@ type RestServerArgs = {
   defaultEnvironmentIdProvider?: () => string | undefined;
   requestEnvResolver?: RestRequestEnvResolver;
   kernelManager?: { getOrCreate: (id: string) => Promise<any> };
+  /** Single-env service-existence probe — answers for the HOST kernel. */
+  serviceExistsProvider?: (name: string) => boolean;
 };
 
 /** Build a RestServer with only the seams under test wired. */
@@ -88,7 +90,7 @@ function buildRest(args: RestServerArgs = {}) {
     undefined, // i18nServiceProvider
     undefined, // analyticsServiceProvider
     undefined, // settingsServiceProvider
-    undefined, // serviceExistsProvider
+    args.serviceExistsProvider,
     undefined, // securityServiceProvider
     args.requestEnvResolver,
   );
@@ -203,6 +205,204 @@ describe('resolveRequestEnvironmentId (D11④ seam)', () => {
   });
 });
 
+// ---------------------------------------------------------------------------
+// probeMcpServeable — the NINTH consumer of the shared entry point (#9120)
+//
+// `/discovery`'s `mcp` advertisement is computed from this probe. It used to
+// re-derive the environment itself (`req.params.environmentId`, else
+// `defaultEnvironmentIdProvider`), so it saw neither the host's ADR-0006
+// `kernel-resolver` seam nor the legacy hostname / `X-Environment-Id` chain.
+// On a hostname-routed multi-tenant host neither of its two inputs is present
+// — the route is unscoped and the default provider is `createSingleEnvironment
+// Plugin`'s wiring — so it fell through to `serviceExistsProvider`, which
+// answers for the HOST kernel. Both misadvertisement directions were reachable
+// from there; single-environment boots were correct throughout, which is why
+// every pin below is written on the multi-tenant shape.
+// ---------------------------------------------------------------------------
+
+/** A kernel whose `mcp` slot holds a service of the shape `/mcp` needs. */
+function kernelWithMcp() {
+  return { getServiceAsync: vi.fn().mockResolvedValue({ handleHttpRequest: () => undefined }) };
+}
+/** A kernel with no `mcp` service at all — `/mcp` would 501 here. */
+function kernelWithoutMcp() {
+  return { getServiceAsync: vi.fn().mockResolvedValue(undefined) };
+}
+
+describe('probeMcpServeable (D11④ seam, ninth consumer)', () => {
+  /**
+   * A hostname-routed multi-tenant host: the injected resolver answers for the
+   * request, and NO `defaultEnvironmentIdProvider` is registered — that is
+   * single-environment wiring, and its absence is what left the old derivation
+   * with nothing but the host-wide `serviceExistsProvider`.
+   */
+  function multiTenantHost(envKernel: any, hostHasMcp: boolean) {
+    const getOrCreate = vi.fn().mockResolvedValue(envKernel);
+    const { rest } = buildRest({
+      requestEnvResolver: { resolveRequestEnvironmentId: vi.fn().mockResolvedValue('tenant-b') },
+      kernelManager: { getOrCreate },
+      serviceExistsProvider: () => hostHasMcp,
+    });
+    return { getOrCreate, probe: (req: any) => (rest as any).probeMcpServeable(req) };
+  }
+
+  it('answers for the REQUEST environment when the host kernel serves mcp and that environment does not', async () => {
+    const { probe, getOrCreate } = multiTenantHost(kernelWithoutMcp(), true);
+
+    // The over-advertisement direction #4024 was filed to close: the host says
+    // yes, the request's own kernel would 501. `false` withholds `routes.mcp`.
+    await expect(probe(mockReq())).resolves.toBe(false);
+    expect(getOrCreate).toHaveBeenCalledWith('tenant-b');
+  });
+
+  it('answers for the REQUEST environment when it serves mcp and the host kernel does not', async () => {
+    const { probe, getOrCreate } = multiTenantHost(kernelWithMcp(), false);
+
+    // The other direction: a confident `false` computed against the wrong
+    // kernel withholds a route that would have served (`mcpServeable !== false`
+    // fails open only for `null`).
+    await expect(probe(mockReq())).resolves.toBe(true);
+    expect(getOrCreate).toHaveBeenCalledWith('tenant-b');
+  });
+
+  it('reaches the request environment through the legacy hostname chain when no resolver is injected', async () => {
+    const getOrCreate = vi.fn().mockResolvedValue(kernelWithMcp());
+    const { rest } = buildRest({
+      envRegistry: legacyRegistry(),
+      kernelManager: { getOrCreate },
+      serviceExistsProvider: () => false,
+    });
+
+    await expect((rest as any).probeMcpServeable(mockReq())).resolves.toBe(true);
+    expect(getOrCreate).toHaveBeenCalledWith('legacy-env');
+  });
+
+  it('follows X-Environment-Id, which the hand-rolled derivation never read', async () => {
+    const getOrCreate = vi.fn().mockResolvedValue(kernelWithMcp());
+    const { rest } = buildRest({
+      envRegistry: {
+        resolveByHostname: vi.fn().mockResolvedValue(null),
+        resolveById: vi.fn().mockImplementation(async (id: string) => (id === 'header-env' ? {} : null)),
+      },
+      kernelManager: { getOrCreate },
+      serviceExistsProvider: () => false,
+    });
+
+    await expect(
+      (rest as any).probeMcpServeable(mockReq({ 'x-environment-id': 'header-env' })),
+    ).resolves.toBe(true);
+    expect(getOrCreate).toHaveBeenCalledWith('header-env');
+  });
+
+  it('keeps the single-environment answer unchanged (default provider → that kernel)', async () => {
+    const getOrCreate = vi.fn().mockResolvedValue(kernelWithoutMcp());
+    const { rest } = buildRest({
+      defaultEnvironmentIdProvider: () => 'default-env',
+      kernelManager: { getOrCreate },
+      serviceExistsProvider: () => true,
+    });
+
+    // Correct before this change and correct after: the shared entry point's
+    // step 3 IS the default provider, so single-env boots keep their answer.
+    await expect((rest as any).probeMcpServeable(mockReq())).resolves.toBe(false);
+    expect(getOrCreate).toHaveBeenCalledWith('default-env');
+  });
+
+  it("keeps the 'platform' guard — the reserved id is never handed to getOrCreate", async () => {
+    const getOrCreate = vi.fn().mockResolvedValue(kernelWithMcp());
+    const { rest } = buildRest({
+      kernelManager: { getOrCreate },
+      serviceExistsProvider: (name: string) => name === 'mcp',
+    });
+
+    // `platform` is a virtual id, not a row in the environments table.
+    await expect(
+      (rest as any).probeMcpServeable({ ...mockReq(), params: { environmentId: 'platform' } }),
+    ).resolves.toBe(true);
+    expect(getOrCreate).not.toHaveBeenCalled();
+  });
+
+  it('never mistakes the literal ":environmentId" placeholder for an environment', async () => {
+    const getOrCreate = vi.fn().mockResolvedValue(kernelWithMcp());
+    const { rest } = buildRest({
+      requestEnvResolver: { resolveRequestEnvironmentId: vi.fn().mockResolvedValue('tenant-b') },
+      kernelManager: { getOrCreate },
+      serviceExistsProvider: () => false,
+    });
+
+    // An unsubstituted route pattern is the absence of an id, not an id — the
+    // shared entry point short-circuits on any truthy explicit value, so the
+    // placeholder must be normalised away before it is passed in.
+    await expect(
+      (rest as any).probeMcpServeable({ ...mockReq(), params: { environmentId: ':environmentId' } }),
+    ).resolves.toBe(true);
+    expect(getOrCreate).toHaveBeenCalledWith('tenant-b');
+    expect(getOrCreate).not.toHaveBeenCalledWith(':environmentId');
+  });
+
+  it('keeps the serviceExistsProvider fallback when no environment resolves at all', async () => {
+    const getOrCreate = vi.fn().mockResolvedValue(kernelWithMcp());
+    const { rest } = buildRest({
+      kernelManager: { getOrCreate },
+      serviceExistsProvider: (name: string) => name === 'mcp',
+    });
+
+    await expect((rest as any).probeMcpServeable(mockReq())).resolves.toBe(true);
+    expect(getOrCreate).not.toHaveBeenCalled();
+  });
+
+  it('still reports null ("cannot probe") when nothing in either path can answer', async () => {
+    const { rest } = buildRest({ kernelManager: { getOrCreate: vi.fn() } });
+    await expect((rest as any).probeMcpServeable(mockReq())).resolves.toBeNull();
+  });
+});
+
+// ---------------------------------------------------------------------------
+// /discovery end to end — the advertisement the probe feeds, driven through
+// the real handler so the `req` it receives is the one the route was given.
+// ---------------------------------------------------------------------------
+
+describe('/discovery mcp advertisement (#9120)', () => {
+  function driveDiscovery(args: RestServerArgs) {
+    const { rest, server } = buildRest(args);
+    rest.registerRoutes();
+    const route = server.get.mock.calls.find((c: any[]) => c[0] === '/api/v1/discovery');
+    expect(route, 'GET /api/v1/discovery must be registered').toBeDefined();
+    const res = {
+      json: vi.fn(),
+      status: vi.fn().mockReturnThis(),
+      send: vi.fn(),
+      setHeader: vi.fn(),
+      headersSent: false,
+    };
+    return async (req: any) => {
+      await route![1](req, res);
+      expect(res.json).toHaveBeenCalledTimes(1);
+      return res.json.mock.calls[0][0];
+    };
+  }
+
+  it('withholds routes.mcp when the request environment cannot serve it, though the host kernel can', async () => {
+    const discovery = await driveDiscovery({
+      requestEnvResolver: { resolveRequestEnvironmentId: vi.fn().mockResolvedValue('tenant-b') },
+      kernelManager: { getOrCreate: vi.fn().mockResolvedValue(kernelWithoutMcp()) },
+      serviceExistsProvider: () => true,
+    })(mockReq());
+
+    expect(discovery.routes.mcp).toBeUndefined();
+  });
+
+  it('advertises routes.mcp when the request environment serves it, though the host kernel does not', async () => {
+    const discovery = await driveDiscovery({
+      requestEnvResolver: { resolveRequestEnvironmentId: vi.fn().mockResolvedValue('tenant-b') },
+      kernelManager: { getOrCreate: vi.fn().mockResolvedValue(kernelWithMcp()) },
+      serviceExistsProvider: () => false,
+    })(mockReq());
+
+    expect(discovery.routes.mcp).toBe('/api/v1/mcp');
+  });
+});
+
 // ---------------------------------------------------------------------------
 // RestApiPlugin adapter — binds the host's `kernel-resolver` service
 // ---------------------------------------------------------------------------

packages/rest/src/rest-server.ts

@@ -2862,13 +2862,36 @@ export class RestServer {
      * kernel we can check the SHAPE; the single-env provider answers existence
      * only, which is the dominant case (the dispatcher's own service-aware
      * discovery covers the wrong-shape case).
+     *
+     * [#9120] The first of those two paths goes through
+     * {@link resolveRequestEnvironmentId} — THE shared entry point, like every
+     * other consumer that needs the request's environment. It used to re-derive
+     * one here (`params.environmentId`, else `defaultEnvironmentIdProvider`),
+     * which is the same chain minus the host's ADR-0006 `kernel-resolver` seam
+     * and the legacy hostname / `X-Environment-Id` steps. On a hostname-routed
+     * multi-tenant host neither of the two inputs it read is present — the
+     * `/discovery` route is unscoped, and the default provider is
+     * `createSingleEnvironmentPlugin`'s wiring — so the probe fell through to
+     * `serviceExistsProvider` and answered for the HOST kernel: `routes.mcp`
+     * advertised for an environment whose route 501s, or withheld from one that
+     * would have served it. `resolveRegisteredServices` was never exposed to
+     * this because its kernel arrives as `ctx.__kernel`, set downstream of the
+     * shared entry point — so routing through it is what makes the parity this
+     * doc-comment claims actually hold. Single-environment boots are unaffected:
+     * the default provider is step 3 of the shared chain.
      */
     private async probeMcpServeable(req: any): Promise<boolean | null> {
         try {
-            let environmentId: string | undefined = req?.params?.environmentId;
-            if ((!environmentId || environmentId === ':environmentId') && this.defaultEnvironmentIdProvider) {
-                try { environmentId = this.defaultEnvironmentIdProvider() || undefined; } catch { /* ignore */ }
-            }
+            // An unsubstituted route pattern is the ABSENCE of an id, not an id.
+            // The shared entry point short-circuits on any truthy explicit
+            // value, so the placeholder must be normalised away before it — or
+            // `getOrCreate(':environmentId')` would go looking for a kernel
+            // named after the pattern.
+            const routeParam: string | undefined = req?.params?.environmentId;
+            const environmentId = await this.resolveRequestEnvironmentId(
+                routeParam === ':environmentId' ? undefined : routeParam,
+                req,
+            );
             if (environmentId && environmentId !== 'platform' && this.kernelManager) {
                 const kernel: any = await this.kernelManager.getOrCreate(environmentId);
                 if (kernel && typeof kernel.getServiceAsync === 'function') {