fix(mcp): registration-time assert — каждый не-inline спек регистрируем (#494)

Коммит 1. Спек без execute падал по-разному на двух хостах: MCP-хост
(index.ts) в цикле регистрации делает `mcpExecute` иначе `spec.execute!` —
без execute это TypeError в момент ВЫЗОВА тула (то есть в проде, только когда
модель выберет именно этот тул); in-app-хост (ai-chat-tools.service.ts) делает
`inAppExecute ?? execute`, затем `if (!run) continue` — то есть МОЛЧА роняет
тул, он просто исчезает у агента без единой ошибки.

Комментарий «mirror this» гардом не считается: закрываем зеркало настоящим
структурным assert'ом. `assertEverySpecIsRegisterable()` гоняется при загрузке
модуля tool-specs на ОБОИХ хостах (оба его импортируют) и кидает исключение,
если не-inline спек, который хост регистрирует, не несёт исполнителя для этого
хоста — латентный рантайм-TypeError / тихий дроп превращается в громкий отказ
на старте. `inlineBothHosts` освобождён (оба хоста регистрируют его inline,
execute у него намеренно нет); `inAppOnly`/`mcpOnly` проверяются только для
своего хоста.

Тест по реестру с мутационной проверкой: синтетические плохие реестры
(без execute; inAppOnly без inAppExecute) обязаны кидать, а mcpExecute-only /
inAppExecute-only / inlineBothHosts — проходить. Часть (б) — assert
объявления write-класса — уже приземлилась в #489.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

apps/server/src/core/ai-chat/tools/ai-chat-tools.service.ts

@@ -718,7 +718,11 @@ export class AiChatToolsService {
       if (spec.mcpOnly) continue;
       if (spec.inlineBothHosts) continue;
       const run = spec.inAppExecute ?? spec.execute;
-      if (!run) continue; // defensive: a shared spec always carries one of them.
+      // Guaranteed present by assertEverySpecIsRegisterable() (#494), which runs
+      // at tool-specs module load and throws if a non-inline spec the in-app host
+      // registers carries neither inAppExecute nor execute — so this can no longer
+      // silently drop a mis-declared tool. Kept as a type-narrowing guard.
+      if (!run) continue;
       tools[spec.inAppKey] = sharedTool(
         spec,
         (async (args) =>

packages/mcp/src/index.ts

@@ -304,7 +304,11 @@ export function createDocmostMcpServer(config: DocmostMcpConfig): McpServer {
           content: { type: "text"; text: string }[];
         };
       }
-      // Canonical execute returns raw data; wrap it as JSON text content.
+      // Canonical execute returns raw data; wrap it as JSON text content. The `!`
+      // is backed by assertEverySpecIsRegisterable() (#494), which runs at
+      // tool-specs module load and throws if a non-inline, non-inAppOnly spec
+      // reaches this loop without an execute/mcpExecute — so this can no longer be
+      // a call-time TypeError in production.
       const raw = await spec.execute!(docmostClient, args);
       return jsonContent(raw);
     };

packages/mcp/src/tool-specs.ts

@@ -2433,3 +2433,48 @@ export function assertEverySpecDeclaresWriteClass(): void {
 
 // Enforce at module load (registration time) on both hosts.
 assertEverySpecDeclaresWriteClass();
+
+/**
+ * Registration-time assert (#494): every spec that a host registers through the
+ * shared-registry loop MUST carry a callable execute path for THAT host. On the
+ * MCP host (index.ts) the loop runs `mcpExecute` else `spec.execute!` — a spec
+ * with neither throws a TypeError at CALL time (fires only once the model picks
+ * that tool, in production, on the MCP host). On the in-app host
+ * (ai-chat-tools.service.ts) the loop does `inAppExecute ?? execute`, then
+ * `if (!run) continue` — a spec with neither is SILENTLY dropped, so the tool
+ * simply vanishes from the agent with no error at all. A `// mirror this` comment
+ * is not a guard; this closes the mirror with a real structural check that fires
+ * at module load on BOTH hosts (both import this file), turning a latent runtime
+ * TypeError / silent-drop into a loud startup failure.
+ *
+ * `inlineBothHosts` specs are exempt: both hosts register them inline with a
+ * hand-wired handler and they deliberately carry no execute (their backing helper
+ * cannot cross into this zod-agnostic file). A spec that is `inAppOnly` need not
+ * satisfy the MCP-host arm (that host skips it) and vice-versa for `mcpOnly`.
+ */
+export function assertEverySpecIsRegisterable(
+  specs: Record<string, SharedToolSpec> = SHARED_TOOL_SPECS,
+): void {
+  for (const [key, spec] of Object.entries(specs)) {
+    if (spec.inlineBothHosts) continue;
+    // MCP host registers the spec unless it is inAppOnly; its handler calls
+    // `mcpExecute` when present, otherwise `execute!`.
+    if (!spec.inAppOnly && !spec.execute && !spec.mcpExecute) {
+      throw new Error(
+        `tool-specs: spec "${key}" is registered on the MCP host but carries ` +
+          `neither execute nor mcpExecute`,
+      );
+    }
+    // In-app host registers it unless it is mcpOnly; its loop runs
+    // `inAppExecute ?? execute`.
+    if (!spec.mcpOnly && !spec.execute && !spec.inAppExecute) {
+      throw new Error(
+        `tool-specs: spec "${key}" is registered on the in-app host but carries ` +
+          `neither execute nor inAppExecute`,
+      );
+    }
+  }
+}
+
+// Enforce at module load (registration time) on both hosts.
+assertEverySpecIsRegisterable();

packages/mcp/test/unit/tool-specs.test.mjs

@@ -7,6 +7,7 @@ import {
   SHARED_TOOL_WRITE_CLASS,
   isRetryableWriteClass,
   assertEverySpecDeclaresWriteClass,
+  assertEverySpecIsRegisterable,
 } from "../../build/tool-specs.js";
 
 // The shared registry is consumed by BOTH the zod-v3 MCP server and the zod-v4
@@ -83,6 +84,99 @@ test("#489: representative reads are readOnly and representative writes are writ
   }
 });
 
+// #494 — every non-inline spec MUST carry a callable execute path for each host
+// that registers it, or the MCP host throws a call-time TypeError (`execute!`)
+// and the in-app host silently drops the tool. A registration-time assert closes
+// this mirror. These tests REDDEN if the guard is weakened/removed.
+test("#494: assertEverySpecIsRegisterable does not throw for the shipped registry", () => {
+  assert.doesNotThrow(() => assertEverySpecIsRegisterable());
+  // Sanity: the shipped registry really does satisfy the invariant per-spec.
+  for (const [key, spec] of Object.entries(SHARED_TOOL_SPECS)) {
+    if (spec.inlineBothHosts) continue;
+    if (!spec.inAppOnly) {
+      assert.ok(
+        spec.execute || spec.mcpExecute,
+        `${key}: MCP-host spec missing execute/mcpExecute`,
+      );
+    }
+    if (!spec.mcpOnly) {
+      assert.ok(
+        spec.execute || spec.inAppExecute,
+        `${key}: in-app-host spec missing execute/inAppExecute`,
+      );
+    }
+  }
+});
+
+test("#494: a non-inline spec with no execute/mcpExecute is rejected (MCP-host arm)", () => {
+  // A shared spec (registered on BOTH hosts) with no execute at all.
+  const bad = {
+    lonely: {
+      mcpName: "lonely",
+      inAppKey: "lonely",
+      writeClass: "readOnly",
+      description: "no execute anywhere",
+      tier: "core",
+      catalogLine: "lonely — nothing",
+    },
+  };
+  assert.throws(
+    () => assertEverySpecIsRegisterable(bad),
+    /lonely.*neither execute nor mcpExecute/,
+  );
+});
+
+test("#494: an inAppOnly spec with no execute/inAppExecute is rejected (in-app-host arm)", () => {
+  const bad = {
+    lonely: {
+      mcpName: "lonely",
+      inAppKey: "lonely",
+      writeClass: "readOnly",
+      description: "in-app only, but no runner",
+      tier: "core",
+      catalogLine: "lonely — nothing",
+      inAppOnly: true,
+    },
+  };
+  assert.throws(
+    () => assertEverySpecIsRegisterable(bad),
+    /lonely.*neither execute nor inAppExecute/,
+  );
+});
+
+test("#494: an mcpExecute-only spec passes the MCP arm; an inAppExecute-only spec passes the in-app arm", () => {
+  // mcpOnly spec with only mcpExecute — the MCP arm is satisfied, the in-app arm
+  // is skipped (mcpOnly), so it must NOT throw.
+  const mcpOnly = {
+    t: {
+      mcpName: "t", inAppKey: "t", writeClass: "readOnly",
+      description: "x", tier: "core", catalogLine: "t — x",
+      mcpOnly: true, mcpExecute: async () => ({}),
+    },
+  };
+  assert.doesNotThrow(() => assertEverySpecIsRegisterable(mcpOnly));
+  // inAppOnly spec with only inAppExecute — symmetric.
+  const inAppOnly = {
+    t: {
+      mcpName: "t", inAppKey: "t", writeClass: "readOnly",
+      description: "x", tier: "core", catalogLine: "t — x",
+      inAppOnly: true, inAppExecute: async () => ({}),
+    },
+  };
+  assert.doesNotThrow(() => assertEverySpecIsRegisterable(inAppOnly));
+});
+
+test("#494: an inlineBothHosts spec is exempt from the execute requirement", () => {
+  const inline = {
+    t: {
+      mcpName: "t", inAppKey: "t", writeClass: "readOnly",
+      description: "x", tier: "core", catalogLine: "t — x",
+      inlineBothHosts: true, // no execute — registered inline by both hosts
+    },
+  };
+  assert.doesNotThrow(() => assertEverySpecIsRegisterable(inline));
+});
+
 test("buildShape (when present) returns a usable ZodRawShape with a real zod", () => {
   for (const [key, spec] of Object.entries(SHARED_TOOL_SPECS)) {
     if (!spec.buildShape) continue;