fix(rest): route probeMcpServeable through the shared environment entry point so /discovery's mcp advertisement follows the request's kernel

.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') {