feat(spec)!: 声明 routes.mcp,并把 discovery 一致性闸门下沉到 routes 一层 (#5679)

.changeset/discovery-routes-mcp-declared.md

@@ -0,0 +1,54 @@
+---
+"@objectstack/spec": minor
+"@objectstack/rest": patch
+"@objectstack/client": patch
+---
+
+feat(spec): declare `routes.mcp` on `ApiRoutesSchema`, and extend the discovery conformance gate one level down (#5679)
+
+`/discovery` advertises `routes.mcp`, `objectui` reads it, and
+`ApiRoutesSchema` never declared it. This is #4828's defect one level down —
+with the opposite disposition: `endpoints` was retired because a census found
+no reader, while `mcp` has two real ones (`ConnectAgentWidget.tsx` and
+`AgentConnectSection.tsx` both gate the Integrations connect card on it), and
+it is in fact the only `routes.*` key anything in `objectui` reads. So it is
+declared, not removed.
+
+Why it was a defect and not tidiness: `ApiRoutesSchema` is a plain `z.object`,
+which **strips** unknown keys. Any consumer parsing `/discovery` through the
+spec dropped `routes.mcp` silently — the connect card would blank with no
+error. Nothing broke yet only because those two readers happen to read raw
+JSON.
+
+- **`ApiRoutesSchema` declares `mcp: z.string().optional()`**, as measured off
+  both producers rather than guessed: a path string (`/api/v1/mcp`), always the
+  **unscoped** base — `/mcp` is mounted bare, so a scoped mount advertising
+  `/api/v1/environments/env_alpha/data` still advertises `/api/v1/mcp` — and
+  `optional`, not `nullable`: the key is absent (rest-server `delete`s it, the
+  dispatcher leaves it `undefined`) when MCP is disabled or unserveable.
+  Neither producer ever emits `null`.
+- **`@objectstack/rest` drops the two `as any` casts** at the emit site. That is
+  type-only — the emitted body is byte-identical — but the cast's disappearance
+  is the structural proof: with the key undeclared, removing it produced two
+  `TS2339 Property 'mcp' does not exist`; with it declared, `tsc --noEmit`
+  returns to its ratcheted baseline.
+- **The #4828 conformance gates now cover `routes` keys**, not just top-level
+  ones, in all three producer packages, deriving the allowance from
+  `ApiRoutesSchema` the same way the top-level check derives it from the
+  protocol schema. Extended one level, not recursed — full recursion stays out
+  of scope, and `capabilities` / `services` are `z.record`s whose keys are open
+  by design.
+
+- **`@objectstack/client`'s conventional route table gains an `mcp` row.** That
+  table is `Record<keyof ApiRoutes, string>` — total by design — so a newly
+  declared route owes a convention, and the public `ApiRouteType` (`keyof
+  ApiRoutes`) widens by one member. The path is `/api/v1/mcp`, which is what
+  both producers emit, so the fallback agrees with the discovered value instead
+  of competing with it. Resolution behaviour is unchanged: `getRoute()` still
+  prefers the discovered route, and the pre-existing catch-all already produced
+  the same string.
+
+Corrects one detail of the issue's premise: the runtime dispatcher's
+`getDiscoveryInfo()` **does** also emit `routes.mcp` (its routes literal always
+carries the key, holding the path or `undefined`), so both producers were
+affected, not just REST — and the new gate went red on both before the fix.

content/docs/references/api/discovery.mdx

@@ -57,6 +57,7 @@ const result = ApiRoutesSchema.parse(data);
 | **notifications** | `string` | optional | e.g. /api/v1/notifications |
 | **ai** | `string` | optional | e.g. /api/v1/ai |
 | **i18n** | `string` | optional | e.g. /api/v1/i18n |
+| **mcp** | `string` | optional | e.g. /api/v1/mcp — always the unscoped base; absent when MCP is disabled or unserveable |
 
 
 ---

packages/client/src/index.ts

@@ -4584,6 +4584,16 @@ export class ObjectStackClient {
       notifications: '/api/v1/notifications',
       ai: '/api/v1/ai',
       i18n: '/api/v1/i18n',
+      // [#5679] `mcp` became a declared `ApiRoutes` key, and this map is
+      // TOTAL over them by design — a new declared route owes a convention.
+      // `/api/v1/mcp` is not a guess: it is what both discovery producers
+      // actually emit, so the fallback agrees with the discovered value
+      // rather than competing with it.
+      //
+      // Note this table is the UNSCOPED convention (every row is `/api/v1/…`),
+      // which suits `mcp` exactly: `/mcp` is mounted bare, so even a
+      // project-scoped discovery response advertises the unscoped path.
+      mcp: '/api/v1/mcp',
     };
 
     return routeMap[type] || `/api/v1/${type}`;

packages/metadata-protocol/src/discovery-schema-conformance.test.ts

@@ -30,14 +30,29 @@
 //     from becoming a third dialect of the contract.
 
 import { describe, it, expect } from 'vitest';
-import { DiscoverySchema, GetDiscoveryResponseSchema } from '@objectstack/spec/api';
+import { ApiRoutesSchema, DiscoverySchema, GetDiscoveryResponseSchema } from '@objectstack/spec/api';
 import { ObjectStackProtocolImplementation } from './index.js';
 
 /** The keys the protocol declares for a discovery response (canonical + declared alias). */
 function declaredResponseKeys(): Set<string> {
   return new Set(Object.keys((GetDiscoveryResponseSchema as any).shape));
 }
 
+/**
+ * [#5679] The keys `ApiRoutesSchema` declares INSIDE `routes` — the #4828 gate
+ * extended one level down, where `routes.mcp` had been living undeclared.
+ *
+ * Unlike the REST and dispatcher gates, this one was ALREADY green before
+ * #5679: this builder's `routes` is annotated `const routes: ApiRoutes`, so
+ * the compiler kept it inside the declared key set and it never grew an `mcp`.
+ * It is added anyway so all three producers carry the same gate — this is the
+ * producer the OTHER two compose over, and the one place where an undeclared
+ * routes key would today be caught by `tsc` rather than by a test.
+ */
+function declaredRouteKeys(): Set<string> {
+  return new Set(Object.keys((ApiRoutesSchema as any).shape));
+}
+
 /**
  * A protocol impl over a minimal engine. `getDiscovery()` reads
  * `engine.registry` (for `sys_comment`), `engine.transaction` (for
@@ -72,6 +87,27 @@ describe('[#4828] getDiscovery() conforms to DiscoverySchema', () => {
     expect(undeclared, 'undeclared top-level keys on the getDiscovery() shape').toEqual([]);
   });
 
+  it('[#5679] emits NO `routes` key the schema does not declare', async () => {
+    const discovery: any = await makeImpl().getDiscovery();
+
+    const declared = declaredRouteKeys();
+    const undeclared = Object.keys(discovery.routes).filter(k => !declared.has(k));
+    expect(undeclared, 'undeclared keys inside `routes` on the getDiscovery() shape').toEqual([]);
+  });
+
+  it('[#5679] does NOT advertise `mcp` — this builder knows nothing about the /mcp mount', async () => {
+    const discovery: any = await makeImpl().getDiscovery();
+
+    // The negative half of the same fact: `mcp` is now DECLARED (optional), and
+    // this producer legitimately leaves it empty — the /mcp route is mounted by
+    // the host (rest-server) or gated on the kernel's mcp service (dispatcher),
+    // neither of which this builder can see. Declaring the key does not oblige
+    // every producer to fill it; it obliges every producer that fills it to
+    // spell it this way.
+    expect(Object.prototype.hasOwnProperty.call(discovery.routes, 'mcp')).toBe(false);
+    expect(declaredRouteKeys().has('mcp')).toBe(true);
+  });
+
   it('carries the canonical `name`, and keeps `apiName` for its deprecation window', async () => {
     const discovery: any = await makeImpl().getDiscovery();
 

packages/rest/src/discovery-schema-conformance.test.ts

@@ -16,7 +16,7 @@
 // undeclared.
 
 import { describe, it, expect, vi } from 'vitest';
-import { DiscoverySchema, GetDiscoveryResponseSchema } from '@objectstack/spec/api';
+import { ApiRoutesSchema, DiscoverySchema, GetDiscoveryResponseSchema } from '@objectstack/spec/api';
 import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol';
 import { RestServer } from './rest-server.js';
 
@@ -25,6 +25,24 @@ function declaredResponseKeys(): Set<string> {
   return new Set(Object.keys((GetDiscoveryResponseSchema as any).shape));
 }
 
+/**
+ * [#5679] The keys `ApiRoutesSchema` declares INSIDE `routes`.
+ *
+ * The #4828 gate was deliberately pinned at the top level, and `routes.mcp`
+ * lived in exactly the blind spot that left: emitted here, read by objectui,
+ * declared nowhere. Note which assertion catches it — `DiscoverySchema
+ * .safeParse()` above stays GREEN on an undeclared `routes.mcp`, because
+ * `ApiRoutesSchema` is a plain `z.object` and zod strips unknown keys. Only a
+ * key-set check can see it, one level down exactly as at the top.
+ *
+ * Extended one level, not recursed: this is the level with a measured
+ * producer/consumer pair. Full recursion is still out of scope (#4828), and
+ * `capabilities` / `services` are `z.record`s whose keys are open by design.
+ */
+function declaredRouteKeys(): Set<string> {
+  return new Set(Object.keys((ApiRoutesSchema as any).shape));
+}
+
 function createMockServer() {
   return {
     get: vi.fn(), post: vi.fn(), put: vi.fn(), delete: vi.fn(), patch: vi.fn(),
@@ -95,6 +113,52 @@ describe('[#4828] the REST /discovery live shape conforms to DiscoverySchema', (
     expect(undeclared, 'undeclared top-level keys on the REST /discovery body').toEqual([]);
   });
 
+  it('[#5679] emits NO `routes` key the schema does not declare', async () => {
+    const body = await invoke(discoveryHandler());
+
+    const declared = declaredRouteKeys();
+    const undeclared = Object.keys(body.routes).filter(k => !declared.has(k));
+    expect(undeclared, 'undeclared keys inside `routes` on the REST /discovery body').toEqual([]);
+  });
+
+  it('[#5679] anti-vacuity: this mount really does advertise `routes.mcp`', async () => {
+    const body = await invoke(discoveryHandler());
+
+    // Without this the gate above would pass for the empty reason. MCP is
+    // default-on and the probe returns `null` (cannot probe) with no kernel
+    // manager and no serviceExistsProvider, so the fail-open branch advertises.
+    expect(body.routes.mcp).toBe('/api/v1/mcp');
+    expect(declaredRouteKeys().has('mcp')).toBe(true);
+  });
+
+  it('[#5679] advertises the UNSCOPED /mcp even from the scoped mount', async () => {
+    const body = await invoke(
+      discoveryHandler({ scoped: true }),
+      { environmentId: 'env_alpha' },
+    );
+
+    // The /mcp route is mounted bare, so the advertised path must NOT pick up
+    // the environment segment the sibling routes carry. Measured, then declared.
+    expect(body.routes.data).toBe('/api/v1/environments/env_alpha/data');
+    expect(body.routes.mcp).toBe('/api/v1/mcp');
+  });
+
+  it('[#5679] omits the key entirely when the env opts out of MCP', async () => {
+    const old = process.env.OS_MCP_SERVER_ENABLED;
+    process.env.OS_MCP_SERVER_ENABLED = 'false';
+    try {
+      const body = await invoke(discoveryHandler());
+
+      // `optional`, not `nullable`: the emit site DELETES the key, so a
+      // consumer sees an absent key rather than an explicit null.
+      expect(Object.prototype.hasOwnProperty.call(body.routes, 'mcp')).toBe(false);
+      expect(DiscoverySchema.safeParse(body).success).toBe(true);
+    } finally {
+      if (old === undefined) delete process.env.OS_MCP_SERVER_ENABLED;
+      else process.env.OS_MCP_SERVER_ENABLED = old;
+    }
+  });
+
   it('fills the three required identity keys the schema declares', async () => {
     const body = await invoke(discoveryHandler());
 

packages/rest/src/rest-server.ts

@@ -3074,9 +3074,9 @@ export class RestServer {
                             const unscopedBase = isScoped
                                 ? basePath.replace(/\/(environments|projects)\/:environmentId$/, '')
                                 : basePath;
-                            (discovery.routes as any).mcp = `${unscopedBase}/mcp`;
+                            discovery.routes.mcp = `${unscopedBase}/mcp`;
                         } else {
-                            delete (discovery.routes as any).mcp;
+                            delete discovery.routes.mcp;
                         }
 
                         // Align auth route with the versioned base path if present.

packages/runtime/src/discovery-schema-conformance.test.ts

@@ -15,14 +15,29 @@
 // contrived fixture needed.
 
 import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
-import { DiscoverySchema, GetDiscoveryResponseSchema } from '@objectstack/spec/api';
+import { ApiRoutesSchema, DiscoverySchema, GetDiscoveryResponseSchema } from '@objectstack/spec/api';
 import { HttpDispatcher } from './http-dispatcher.js';
 
 /** The keys the protocol declares for a discovery response (canonical + declared alias). */
 function declaredResponseKeys(): Set<string> {
   return new Set(Object.keys((GetDiscoveryResponseSchema as any).shape));
 }
 
+/**
+ * [#5679] The keys `ApiRoutesSchema` declares INSIDE `routes` — the same gate
+ * one level down, where `routes.mcp` had been hiding.
+ *
+ * This producer emits `mcp` too, contrary to what #5679's issue body assumed:
+ * the routes literal below carries `mcp: isMcpServerEnabled() && hasMcp ? … :
+ * undefined`, so the KEY is always present (value `undefined` when the service
+ * is absent — `JSON.stringify` drops it on the wire, which is why it read as
+ * "not emitted"). `Object.keys()` sees it either way, so this gate covers this
+ * producer as squarely as the REST one.
+ */
+function declaredRouteKeys(): Set<string> {
+  return new Set(Object.keys((ApiRoutesSchema as any).shape));
+}
+
 describe('[#4828] getDiscoveryInfo() conforms to DiscoverySchema', () => {
   let dispatcher: HttpDispatcher;
 
@@ -64,6 +79,49 @@ describe('[#4828] getDiscoveryInfo() conforms to DiscoverySchema', () => {
     expect(undeclared, 'undeclared top-level keys on the getDiscoveryInfo() shape').toEqual([]);
   });
 
+  it('[#5679] emits NO `routes` key the schema does not declare', async () => {
+    const info: any = await dispatcher.getDiscoveryInfo('/api/v1');
+
+    const declared = declaredRouteKeys();
+    const undeclared = Object.keys(info.routes).filter(k => !declared.has(k));
+    expect(undeclared, 'undeclared keys inside `routes` on the getDiscoveryInfo() shape').toEqual([]);
+  });
+
+  it('[#5679] advertises `routes.mcp` when the mcp service is registered and shaped', async () => {
+    // Anti-vacuity for the gate above, and the fact the issue body got wrong:
+    // this producer DOES emit `routes.mcp`. Gated on the handler's own
+    // predicate (`typeof mcp.handleHttpRequest === 'function'`, #4024) — the
+    // key is present either way, carrying the path or `undefined`.
+    const kernel = {
+      context: {
+        getService: (name: string) => {
+          if (name === 'objectql') {
+            return {
+              registry: {
+                getObject: vi.fn().mockReturnValue({ name: 'test_obj' }),
+                getRegisteredTypes: vi.fn().mockReturnValue([]),
+                getAllPackages: vi.fn().mockReturnValue([]),
+              },
+            };
+          }
+          if (name === 'mcp') return { handleHttpRequest: () => undefined };
+          return null;
+        },
+      },
+    } as any;
+    const info: any = await new HttpDispatcher(kernel).getDiscoveryInfo('/api/v1');
+
+    expect(info.routes.mcp).toBe('/api/v1/mcp');
+    expect(declaredRouteKeys().has('mcp')).toBe(true);
+    expect(DiscoverySchema.safeParse(info).success).toBe(true);
+
+    // …and with no mcp service the key stays present but empty, which is the
+    // shape `optional` (not `nullable`) declares.
+    const withoutMcp: any = await dispatcher.getDiscoveryInfo('/api/v1');
+    expect(Object.prototype.hasOwnProperty.call(withoutMcp.routes, 'mcp')).toBe(true);
+    expect(withoutMcp.routes.mcp).toBeUndefined();
+  });
+
   it('has retired `features` and `endpoints` (ADR-0049 enforce-or-remove)', async () => {
     const info: any = await dispatcher.getDiscoveryInfo('/api/v1');
 

packages/spec/authorable-surface.json

@@ -528,6 +528,7 @@
     "api/ApiRoutes:data",
     "api/ApiRoutes:discovery",
     "api/ApiRoutes:i18n",
+    "api/ApiRoutes:mcp",
     "api/ApiRoutes:metadata",
     "api/ApiRoutes:notifications",
     "api/ApiRoutes:packages",

packages/spec/src/api/discovery.test.ts

@@ -983,6 +983,53 @@ describe('[#4828] DiscoverySchema ↔ GetDiscoveryResponseSchema', () => {
   });
 });
 
+// ===========================================================================
+// [#5679] The same contract, one level down: `routes`
+// ===========================================================================
+//
+// #4828 pinned its gate at the TOP level, deliberately. `routes.mcp` lived in
+// exactly the gap that left: emitted by `@objectstack/rest` (behind an `as any`)
+// and by the runtime dispatcher, read by objectui's Integrations connect card,
+// declared by nothing. `ApiRoutesSchema` is a plain `z.object`, so a
+// spec-strict consumer stripped it silently — the connect card would blank with
+// no error. The producer-side conformance tests now check `routes` keys the same
+// way they check top-level keys, and derive their allowance from
+// `ApiRoutesSchema`; these two pin that the allowance is the real one.
+describe('[#5679] routes: the declared key set the producer gates check against', () => {
+  it('is the very schema `DiscoverySchema` nests, not a second copy of it', () => {
+    // The three producer gates read `ApiRoutesSchema.shape`. That is only a
+    // faithful allowance while it IS what `DiscoverySchema.routes` declares —
+    // if the two ever diverge the gates would be policing a schema nothing
+    // parses with, which is how #4828's blind spot was built in the first place.
+    const nested = (DiscoverySchema as any).shape.routes;
+    expect(new Set(Object.keys(nested.shape)))
+      .toEqual(new Set(Object.keys((ApiRoutesSchema as any).shape)));
+  });
+
+  it('declares `mcp` — optional, string, and stripped-not-rejected before it was declared', () => {
+    const declared = Object.keys((ApiRoutesSchema as any).shape);
+    expect(declared).toContain('mcp');
+
+    const base = { data: '/api/v1/data', metadata: '/api/v1/meta' };
+
+    // Present: survives the parse. This is the assertion that was impossible
+    // before — `mcp` used to be dropped here, which is the whole defect.
+    expect(ApiRoutesSchema.parse({ ...base, mcp: '/api/v1/mcp' }).mcp).toBe('/api/v1/mcp');
+
+    // Absent: optional, so a producer that cannot answer omits it.
+    expect(ApiRoutesSchema.parse(base).mcp).toBeUndefined();
+
+    // A non-string is now rejected rather than silently dropped — the value
+    // half of the contract, which a key-set gate alone cannot judge.
+    expect(() => ApiRoutesSchema.parse({ ...base, mcp: 123 })).toThrow();
+
+    // And a still-undeclared key is still stripped, which is why the producer
+    // gates need their key-set check in addition to this parse.
+    expect(ApiRoutesSchema.parse({ ...base, notAThing: '/x' } as any))
+      .not.toHaveProperty('notAThing');
+  });
+});
+
 describe('[#4828] scoping (decision 3 — declare what REST actually emits)', () => {
   const base = {
     name: 'ObjectStack',