fix(runtime,rest): advertise mcp only when it is actually serveable

.changeset/mcp-discovery-service-aware.md

@@ -0,0 +1,30 @@
+---
+"@objectstack/runtime": patch
+"@objectstack/rest": patch
+---
+
+Advertise `mcp` in `/discovery` only when it is actually serveable (#4024).
+
+Both discovery producers gated the `/mcp` route on `isMcpServerEnabled()` alone.
+The stated justification was a lockstep — `os serve` auto-loads plugin-mcp from
+the same flag, so on that path advertised did imply mounted. But the lockstep is
+a property of the CLI, not of the dispatcher: `@objectstack/rest` has no
+`@objectstack/mcp` dependency, mounts no `/mcp` route and performs no auto-load,
+so a host that embedded it without plugin-mcp advertised `/mcp` in `/discovery`
+and then answered 501 on it — the `declared ≠ enforced` failure #3369 forbids,
+and a broken contract for third-party clients that read `/discovery` to decide
+what exists.
+
+Both producers now require the flag AND a serveable MCP service. The runtime
+dispatcher gates on the handler's own predicate (`typeof
+mcp.handleHttpRequest === 'function'`), so a wrong-shaped service can't
+over-promise either. `@objectstack/rest` probes via the per-request kernel or the
+single-env `serviceExistsProvider`; when it genuinely cannot probe it keeps the
+prior flag-only answer rather than hiding a working endpoint (fail-open,
+ADR-0057 D10). The `os serve` / `os dev` path is unchanged — it loads the plugin,
+so the service resolves and `/mcp` is still advertised.
+
+Also exercises the `mcp: false` seam in `route-parity.integration.test.ts`, which
+had existed unused since the file was written: `bootServe()` was only ever called
+with no args or `{ notification: false }`. The one capability whose advertisement
+was not service-presence gated was also the one whose absence was never tested.

content/docs/ai/connect-mcp.mdx

@@ -191,6 +191,7 @@ skill and a guided `/objectstack:connect` command.
 |:---|:---|
 | `404` on `/api/v1/mcp` | The HTTP surface is disabled — unset `OS_MCP_SERVER_ENABLED` (default is on) |
 | `501 Not Implemented` | The MCP plugin isn't part of this build — check your stack's plugins |
+| `mcp` missing from `GET /api/v1/discovery` and no Connect-an-Agent card, but `OS_MCP_SERVER_ENABLED` is on | The same cause as the `501` above, seen from the other side: the surface is *enabled* but not *serveable*, so discovery declines to advertise a route that would 501 rather than over-promising it (`declared === enforced`). Load the MCP plugin — `os serve` / `os dev` do it for you; a host that embeds `@objectstack/rest` directly must add `@objectstack/mcp` itself |
 | stdio won't start / boot fails closed | `OS_MCP_STDIO_ENABLED=true` but `OS_MCP_STDIO_API_KEY` is missing, unknown, revoked, or expired — fail-closed by design (ADR-0101). Set a valid `osk_` key; there is no unscoped or `system` fallback |
 | `401` on every call | Anonymous or invalid credentials. Interactive clients: complete the browser login (the `WWW-Authenticate` header advertises the OAuth metadata). Headless: check the `osk_` key and header spelling |
 | `403 insufficient_scope` | The OAuth token lacks the scope for that tool family (e.g. writes without `data:write`) — reconnect and grant the scope |

packages/rest/src/rest-server.ts

@@ -2127,6 +2127,41 @@ export class RestServer {
  }
  }
 
+ /**
+ * Is `/mcp` actually serveable — i.e. is the MCP service registered with
+ * the shape `handleMcpRequest` needs?
+ *
+ * `true`/`false` are answers; `null` means "could not probe". The route
+ * itself is served by the runtime dispatcher (`domains/mcp.ts`), which
+ * 501s on `!mcp || typeof mcp.handleHttpRequest !== 'function'` — so this
+ * probe exists to keep our `/discovery` from advertising a route that
+ * would 501 (#4024).
+ *
+ * Same two probe paths as {@link resolveRegisteredServices} (ADR-0057
+ * D10): the per-request kernel for multi-env hosts, else the single-env
+ * `serviceExistsProvider` — which `rest-api-plugin` always wires. Via the
+ * 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).
+ */
+ 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 */ }
+ }
+ if (environmentId && environmentId !== 'platform' && this.kernelManager) {
+ const kernel: any = await this.kernelManager.getOrCreate(environmentId);
+ if (kernel && typeof kernel.getServiceAsync === 'function') {
+ const svc: any = await kernel.getServiceAsync('mcp').catch(() => undefined);
+ return typeof svc?.handleHttpRequest === 'function';
+ }
+ }
+ if (this.serviceExistsProvider) return this.serviceExistsProvider('mcp') === true;
+ } catch { /* fall through to "cannot probe" */ }
+ return null;
+ }
+
  /**
  * Register discovery endpoints
  */
@@ -2166,8 +2201,22 @@ export class RestServer {
  // project-scoped), so point at the unscoped base. This
  // `/discovery` (served by @objectstack/rest) is separate
  // from the dispatcher's getDiscoveryInfo — both must
- // advertise `mcp` (single source: isMcpServerEnabled).
- if (isMcpServerEnabled()) {
+ // advertise `mcp` on the same terms.
+ //
+ // Enabled is NOT the same as serveable (#4024). The flag
+ // alone used to gate this, on the reasoning that `os serve`
+ // auto-loads plugin-mcp from the same flag. But that
+ // lockstep belongs to the CLI: `@objectstack/rest` has no
+ // `@objectstack/mcp` dependency, mounts no /mcp route and
+ // performs no auto-load, so an embedder that skips
+ // plugin-mcp had `mcp` advertised here while the route
+ // 501'd — the `declared ≠ enforced` failure #3369 forbids.
+ // A `null` probe means we genuinely cannot tell; keep the
+ // old flag-only answer there rather than hiding a working
+ // endpoint (fail-open, ADR-0057 D10) — the dispatcher's own
+ // discovery is service-aware and stays authoritative.
+ const mcpServeable = await this.probeMcpServeable(req);
+ if (isMcpServerEnabled() && mcpServeable !== false) {
  const unscopedBase = isScoped
  ? basePath.replace(/\/(environments|projects)\/:environmentId$/, '')
  : basePath;

packages/rest/src/rest.test.ts

@@ -2423,12 +2423,33 @@ describe('mapDataError — schema/constraint envelopes', () => {
 // ---------------------------------------------------------------------------
 
 describe('discovery — routes.mcp (ADR-0036, #152)', () => {
- function discoveryHandler() {
+ /**
+ * `serviceExists` omitted → the server cannot probe whether MCP is actually
+ * serveable, so it keeps the flag-only answer (fail-open, ADR-0057 D10).
+ * Pass one to exercise the service-aware gate added for #4024.
+ */
+ function discoveryHandler(serviceExists?: (name: string) => boolean) {
  const server = createMockServer();
  const protocol = createMockProtocol();
  // protocol discovery carries a `routes` object the server augments.
  (protocol.getDiscovery as any) = vi.fn().mockResolvedValue({ routes: { data: '', metadata: '' } });
- const rest = new RestServer(server as any, protocol as any, ANON_API as any);
+ const rest = new RestServer(
+ server as any, protocol as any, ANON_API as any,
+ undefined, // kernelManager
+ undefined, // envRegistry
+ undefined, // defaultEnvironmentIdProvider
+ undefined, // authServiceProvider
+ undefined, // objectQLProvider
+ undefined, // emailServiceProvider
+ undefined, // sharingServiceProvider
+ undefined, // reportsServiceProvider
+ undefined, // approvalsServiceProvider
+ undefined, // sharingRulesServiceProvider
+ undefined, // i18nServiceProvider
+ undefined, // analyticsServiceProvider
+ undefined, // settingsServiceProvider
+ serviceExists,
+ );
  rest.registerRoutes();
  const entry = rest.getRouteManager().get('GET', '/api/v1/discovery');
  if (!entry) throw new Error('discovery route not registered');
@@ -2465,6 +2486,37 @@ describe('discovery — routes.mcp (ADR-0036, #152)', () => {
  const body = await invoke(discoveryHandler());
  expect(body.routes.mcp).toBeUndefined();
  });
+
+ // ── #4024: enabled ≠ serveable ──────────────────────────────────────────
+ // The flag alone used to decide this. But @objectstack/rest has no
+ // @objectstack/mcp dependency, mounts no /mcp route and performs no
+ // auto-load — that lockstep belongs to `os serve`. So an embedder without
+ // plugin-mcp advertised /mcp here and got 501 from the runtime dispatcher,
+ // which is the `declared ≠ enforced` failure #3369 forbids.
+
+ it('omits routes.mcp when enabled but the mcp service is absent (#4024)', async () => {
+ delete process.env.OS_MCP_SERVER_ENABLED; // default-on
+ const body = await invoke(discoveryHandler((name) => name !== 'mcp'));
+ expect(
+ body.routes.mcp,
+ 'enabled-but-unserveable must not be advertised — it would 501',
+ ).toBeUndefined();
+ });
+
+ it('advertises routes.mcp when enabled AND the mcp service is present (#4024)', async () => {
+ delete process.env.OS_MCP_SERVER_ENABLED;
+ const body = await invoke(discoveryHandler(() => true));
+ expect(body.routes.mcp).toBe('/api/v1/mcp');
+ });
+
+ it('keeps the flag-only answer when the host cannot be probed (fail-open, ADR-0057 D10)', async () => {
+ // No kernelManager and no serviceExistsProvider → "unknown", not "absent".
+ // Hiding a working endpoint here would be the opposite over-correction, and
+ // the dispatcher's own service-aware discovery stays authoritative.
+ delete process.env.OS_MCP_SERVER_ENABLED;
+ const body = await invoke(discoveryHandler());
+ expect(body.routes.mcp).toBe('/api/v1/mcp');
+ });
 });
 
 // ──────────────────────────────────────────────────────────────────────────

packages/runtime/src/http-dispatcher.ts

@@ -856,7 +856,7 @@ export class HttpDispatcher {
  const [
  authSvc, searchSvc, realtimeSvc, filesSvc,
  analyticsSvc, workflowSvc, aiSvc, notificationSvc, i18nSvc,
- uiSvc, automationSvc, cacheSvc, queueSvc, jobSvc,
+ uiSvc, automationSvc, cacheSvc, queueSvc, jobSvc, mcpSvc,
  ] = await Promise.all([
  this.resolveService(CoreServiceName.enum.auth),
  this.resolveService(CoreServiceName.enum.search),
@@ -872,6 +872,9 @@ export class HttpDispatcher {
  this.resolveService(CoreServiceName.enum.cache),
  this.resolveService(CoreServiceName.enum.queue),
  this.resolveService(CoreServiceName.enum.job),
+ // Not a CoreServiceName — plugin-mcp registers under the bare
+ // 'mcp' key, the same string handleMcpRequest resolves.
+ this.resolveService('mcp'),
  ]);
 
  const hasAuth = !!authSvc;
@@ -887,6 +890,11 @@ export class HttpDispatcher {
  const hasCache = !!cacheSvc;
  const hasQueue = !!queueSvc;
  const hasJob = !!jobSvc;
+ // Mirrors handleMcpRequest's OWN guard byte for byte (domains/mcp.ts):
+ // it 501s on `!mcp || typeof mcp.handleHttpRequest !== 'function'`, so
+ // advertising on mere service presence would still over-promise when a
+ // wrong-shaped service is registered. Same predicate ⇒ same answer.
+ const hasMcp = typeof mcpSvc?.handleHttpRequest === 'function';
 
  // Routes are only exposed when a plugin provides the service
  const routes = {
@@ -908,21 +916,22 @@ export class HttpDispatcher {
  notifications: hasNotification ? `${prefix}/notifications` : undefined,
  ai: hasAi ? `${prefix}/ai` : undefined,
  i18n: hasI18n ? `${prefix}/i18n` : undefined,
- // MCP (Streamable HTTP) is a default-on core capability —
- // advertised unless OS_MCP_SERVER_ENABLED=false opts the env
- // out. The objectui Integrations page reads this.
+ // MCP (Streamable HTTP) is a default-on core capability — but
+ // "enabled" and "serveable" are two different facts and both
+ // must hold. The objectui Integrations page reads this.
  //
- // `declared === enforced` here is guaranteed by a LOCKSTEP, not
- // by service-presence gating like the routes above (#3369 /
- // #2698): `os serve` auto-loads plugin-mcp from the SAME
- // `isMcpServerEnabled()` flag that gates this advertisement, so
- // whenever `/mcp` is advertised the handler is mounted (a key /
- // token yields 401, never a 404/501). Kept flag-based on purpose
- // — `@objectstack/rest` advertises `mcp` from the identical
- // single source (rest-server.ts), so the two discovery producers
- // stay symmetric. The route-parity gate asserts the lockstep
- // holds (advertised ⇒ reachable, never 501).
- mcp: isMcpServerEnabled() ? `${prefix}/mcp` : undefined,
+ // Service-presence gated like every other optional route above
+ // (#3369 / #2698), NOT flag-only (#4024). This used to trust a
+ // LOCKSTEP instead: `os serve` auto-loads plugin-mcp from the
+ // same `isMcpServerEnabled()` flag, so on that path advertised
+ // did imply mounted. But the lockstep is a property of the CLI,
+ // not of the dispatcher — an embedder that mounts the dispatcher
+ // (or `@objectstack/rest`) without plugin-mcp got `/mcp`
+ // advertised here and 501 from handleMcpRequest, which is
+ // exactly the `declared ≠ enforced` failure #3369 forbids.
+ // Gating on the handler's own predicate makes the invariant hold
+ // by construction on every host instead of by convention on one.
+ mcp: isMcpServerEnabled() && hasMcp ? `${prefix}/mcp` : undefined,
  };
 
  // Build per-service status map