amico-run(#112): B5 — real MCP stdio facade for amico mcp-serve

packages/amico-run/package.json

@@ -20,6 +20,7 @@
   },
   "dependencies": {
     "@amicode/schema": "workspace:*",
+    "@modelcontextprotocol/sdk": "^1.29.0",
     "smol-toml": "^1.3.0"
   },
   "devDependencies": {

packages/amico-run/src/amico.ts

@@ -7,8 +7,10 @@
 // B1 SCOPE: `run` / `resolve` / `sandbox` delegate VERBATIM to the existing amico-run launch
 // path (src/launch.ts): `amico <verb> <args>` is exactly `amico-run <equivalent-args>`, so
 // there is no behavior fork and the amico-run test suite still covers the real bodies. The
-// spine verbs (catalog/vault/device/note) and `mcp-serve` are STUB seams (see verbs.ts,
-// mcp_serve.ts) — routing works today; real bodies land in later spine slices.
+// spine verbs (catalog/vault/device/note) are STUB seams (see verbs.ts) — routing works
+// today; real bodies land in later spine slices.
+// B5 SCOPE (issue #112): `mcp-serve` is now REAL — it stands up an MCP stdio transport that
+// exposes the same spine verbs as MCP tools (see mcp_serve.ts). One impl, two transports.
 import { launch } from "./launch.js";
 import { SPINE_VERBS } from "./verbs.js";
 import { serve } from "./mcp_serve.js";
@@ -19,7 +21,7 @@ function usage(): string {
     ["resolve --platform <p> --kind <k> --size <n>", "tier resolution → JSON (amico-run subcommand)"],
     ["sandbox <workspace-dir> --packages A,B,…", "generate a per-problem Julia env (amico-run subcommand)"],
     ...SPINE_VERBS.map((v) => [`${v.name} …`, `${v.summary} [stub → ${v.slice}]`] as [string, string]),
-    ["mcp-serve [--list]", "expose the spine verbs as MCP tools (optional facade) [stub]"],
+    ["mcp-serve [--list]", "serve the spine verbs as MCP tools over stdio (optional facade; --list = mapping only)"],
     ["--help, -h", "show this verb surface"],
   ];
   const width = Math.max(...rows.map(([u]) => u.length));

packages/amico-run/src/mcp_serve.ts

@@ -3,16 +3,25 @@
 // spine verbs as MCP tools makes them callable-by-name with typed discovery in BOTH,
 // without a second implementation.
 //
-// The mapping is the whole point: each Verb becomes one MCP tool; a tools/call would
-// dispatch to the SAME Verb.run the CLI uses. One impl, two transports.
+// The mapping is the whole point: each Verb becomes one MCP tool; a tools/call dispatches
+// to the SAME Verb.run the CLI uses. One impl, two transports.
 //
-// B1 SCOPE (issue #108): this is a STUB seam. `--list` renders the verb↔tool mapping; the
-// real transport (an MCP stdio server) is NOT wired here. Note the deliberate constraint:
-// test/s31.test.ts (S31 / spec §4) forbids the MCP server SDK inside the orchestrator src,
-// so this slice carries ZERO MCP dependency. Landing the real transport body requires an
-// explicit, reviewed S31 amendment in a later slice — exactly as spec C amended the S31
-// SolveSpec ban to name amico-run the launch gate. Either path here exits cleanly (code 0).
+// B5 SCOPE (issue #112): this lands the REAL @modelcontextprotocol/sdk stdio transport.
+// ⚠️ GOVERNANCE — this is the ONLY file in the orchestrator src/ permitted to reference the
+// MCP SDK. test/s31.test.ts (S31 / spec §4) bans `modelcontextprotocol` everywhere in src/;
+// B5 amends that ban with a single, named carve-out for THIS file only (see the amendment
+// block in s31.test.ts), mirroring spec C's single-file SolveSpec carve-out for amico-run.
+// The carve-out lifts ONLY the MCP-SDK pattern here — this file stays subject to the HTTP
+// and fetch bans (the transport is stdio, never network), and every other src file stays
+// under the full ban. Do NOT import the MCP SDK anywhere else.
 
+import { Server } from "@modelcontextprotocol/sdk/server/index.js";
+import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js";
+import {
+  CallToolRequestSchema,
+  ListToolsRequestSchema,
+  type CallToolResult,
+} from "@modelcontextprotocol/sdk/types.js";
 import { SPINE_VERBS, type Verb } from "./verbs.js";
 
 interface McpToolDescriptor {
@@ -35,30 +44,62 @@ export function listMcpTools(): McpToolDescriptor[] {
   return SPINE_VERBS.map(verbToMcpTool);
 }
 
-/** tools/call handler — dispatches to the same Verb.run the CLI uses. */
-export async function callMcpTool(name: string, argv: string[]): Promise<unknown> {
+/** tools/call handler core — dispatches to the SAME Verb.run the CLI uses (one impl, two
+ *  transports) and wraps the verb's JSON result as an MCP text block. A non-zero verb exit
+ *  code surfaces as isError; an unknown tool name is an isError result (not a throw) so the
+ *  MCP client sees a structured tool error rather than a protocol fault. */
+export async function callMcpTool(name: string, argv: string[]): Promise<CallToolResult> {
   const verb = SPINE_VERBS.find((v) => `amico_${v.name}` === name);
-  if (!verb) throw new Error(`unknown tool ${name}`);
-  const result = await verb.run(argv);
-  return result.json;
+  if (!verb) {
+    return { content: [{ type: "text", text: `amico mcp: unknown tool ${name}` }], isError: true };
+  }
+  const { json, code } = await verb.run(argv);
+  return { content: [{ type: "text", text: JSON.stringify(json) }], isError: code !== 0 };
+}
+
+/** Build the MCP server with its two request handlers wired to the shared verb spine:
+ *  list-tools → the verb↔tool mapping; call-tool → the same verb function the CLI dispatches.
+ *  No transport is attached here, so this is unit-testable over an in-memory transport pair;
+ *  serve() attaches the real stdio transport. */
+export function createMcpServer(): Server {
+  const server = new Server({ name: "amico", version: "0.1.0" }, { capabilities: { tools: {} } });
+  server.setRequestHandler(ListToolsRequestSchema, async () => ({ tools: listMcpTools() }));
+  server.setRequestHandler(CallToolRequestSchema, async (req) => {
+    const argv = (req.params.arguments?.argv as string[] | undefined) ?? [];
+    return callMcpTool(req.params.name, argv);
+  });
+  return server;
 }
 
 export async function serve(argv: string[]): Promise<number> {
   if (argv.includes("--list")) {
-    // Demonstrable path: show the verb↔tool mapping without standing up a transport.
+    // Transport-free path: render the verb↔tool mapping (used for discovery + tests).
     console.log(JSON.stringify({ tools: listMcpTools() }, null, 2));
     return 0;
   }
-  // ── the transport seam (the only net-new code MCP adds over the CLI) — lands in a later
-  //    slice. It stands up an MCP stdio server whose list-tools returns listMcpTools() and
-  //    whose call-tool dispatches to callMcpTool(name, argv). It is intentionally NOT
-  //    imported here so this slice stays free of the MCP SDK (S31; see the file header). ──
-  console.log(
-    JSON.stringify({
-      stub: true,
-      note: "B1 seam only — the MCP stdio transport is not wired here (S31 keeps the orchestrator SDK-free); use --list for the verb↔tool mapping",
-      tools: listMcpTools().map((t) => t.name),
-    }),
-  );
+  // Real facade: stand up the MCP stdio server. stdout is the MCP JSON-RPC channel now, so
+  // NOTHING may be written to it here (no console.log) — diagnostics go to stderr. The call
+  // blocks until the client disconnects, then exits cleanly.
+  const server = createMcpServer();
+  const transport = new StdioServerTransport();
+  await server.connect(transport);
+  await new Promise<void>((resolve) => {
+    let settled = false;
+    const finish = () => {
+      if (settled) return;
+      settled = true;
+      resolve();
+    };
+    // Protocol owns transport.onclose; hook the server's onclose so we don't clobber it —
+    // fires when the peer closes the session.
+    server.onclose = finish;
+    // StdioServerTransport watches stdin `data`/`error` only, NOT EOF — so a client that
+    // simply disconnects (stdin end) would otherwise hang the process. Close the server on
+    // stdin end/close so `amico mcp-serve` always exits cleanly (server.close() → onclose →
+    // finish; also drops the stdin listener so the event loop can drain).
+    const shutdown = () => void server.close().catch(finish);
+    process.stdin.once("end", shutdown);
+    process.stdin.once("close", shutdown);
+  });
   return 0;
 }

packages/amico-run/test/amico.test.ts

@@ -156,11 +156,13 @@ describe("amico router — mcp-serve facade", () => {
       expect.arrayContaining(["amico_catalog", "amico_vault", "amico_device", "amico_note"]),
     );
   });
-  it("no flag → stub note + tool list, exit 0 (cleanly)", () => {
-    const r = run(["mcp-serve"]);
-    expect(r.code).toBe(0);
-    const out = JSON.parse(r.stdout);
-    expect(out.stub).toBe(true);
-    expect(out.tools).toEqual(expect.arrayContaining(["amico_catalog"]));
+  it("no flag → stands up the real stdio server, exits 0 on stdin EOF (empty stdin)", () => {
+    // The real facade (B5) blocks on stdin serving the MCP protocol; feeding an immediate
+    // EOF (empty input) is a disconnected client → the server shuts down and exits cleanly.
+    // stdout is the MCP JSON-RPC channel, so with no client messages it stays silent. The
+    // full protocol round-trip (tools/list + tools/call over real stdio) lives in
+    // mcp_serve.test.ts. A timeout guards against a regression that would hang the server.
+    const out = execFileSync("node", [BUNDLE, "mcp-serve"], { encoding: "utf8", input: "", timeout: 20000 });
+    expect(out).toBe("");
   });
 });

packages/amico-run/test/mcp_serve.test.ts

@@ -0,0 +1,105 @@
+// B5 (issue #112): the `amico mcp-serve` MCP facade. These tests prove the two acceptance
+// criteria — (1) mcp-serve LISTS the spine verbs as MCP tools, and (2) a tools/call
+// DISPATCHES to the SAME verb function the CLI uses (one impl, two transports) — at three
+// levels: the pure verb↔tool mapping, an in-memory Client↔Server round-trip over the real
+// MCP protocol, and a real stdio round-trip against the built `amico` bundle.
+//
+// The SDK imports here live in TEST code, not src/ — the S31 grep guard (s31.test.ts) only
+// scans src/, where the MCP SDK is carved out to mcp_serve.ts alone.
+import { describe, it, expect, beforeAll } from "vitest";
+import { execFileSync } from "node:child_process";
+import { join } from "node:path";
+import { Client } from "@modelcontextprotocol/sdk/client/index.js";
+import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js";
+import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js";
+import { callMcpTool, createMcpServer, listMcpTools, verbToMcpTool } from "../src/mcp_serve.js";
+import { SPINE_VERBS } from "../src/verbs.js";
+
+const BUNDLE = join(__dirname, "..", "dist", "amico.js");
+const TOOL_NAMES = ["amico_catalog", "amico_vault", "amico_device", "amico_note"];
+
+// The distinctive JSON a verb.run() returns for given args — the fingerprint we assert the
+// tool call reproduces, proving the call reached the SAME verb function.
+async function verbJson(verbName: string, argv: string[]): Promise<unknown> {
+  const verb = SPINE_VERBS.find((v) => v.name === verbName)!;
+  return (await verb.run(argv)).json;
+}
+function toolText(res: { content: unknown }): string {
+  return (res.content as { type: string; text: string }[])[0].text;
+}
+
+describe("verb ↔ MCP-tool mapping (pure)", () => {
+  it("verbToMcpTool names the tool amico_<verb>, carries the summary + argv schema", () => {
+    const v = SPINE_VERBS.find((x) => x.name === "catalog")!;
+    expect(verbToMcpTool(v)).toEqual({
+      name: "amico_catalog",
+      description: v.summary,
+      inputSchema: { type: "object", properties: { argv: { type: "array", items: { type: "string" } } } },
+    });
+  });
+  it("listMcpTools exposes exactly the four spine verbs, one tool each", () => {
+    const tools = listMcpTools();
+    expect(tools.map((t) => t.name).sort()).toEqual([...TOOL_NAMES].sort());
+    expect(tools).toHaveLength(SPINE_VERBS.length);
+  });
+});
+
+describe("callMcpTool dispatches to the same verb function (direct)", () => {
+  it("routes amico_note → the note verb; content text is the verb's own JSON", async () => {
+    const res = await callMcpTool("amico_note", ["write", "exp-42"]);
+    expect(res.isError).toBe(false);
+    expect(JSON.parse(toolText(res))).toEqual(await verbJson("note", ["write", "exp-42"]));
+  });
+  it("unknown tool → structured isError result (not a throw)", async () => {
+    const res = await callMcpTool("amico_frobnicate", []);
+    expect(res.isError).toBe(true);
+    expect(toolText(res)).toMatch(/unknown tool amico_frobnicate/);
+  });
+});
+
+describe("MCP round-trip over an in-memory transport (real Client + Server)", () => {
+  it("tools/list returns the spine verbs; tools/call reaches the same verb function", async () => {
+    const server = createMcpServer();
+    const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair();
+    const client = new Client({ name: "amico-mcp-test", version: "0" }, { capabilities: {} });
+    await server.connect(serverTransport);
+    await client.connect(clientTransport);
+
+    const { tools } = await client.listTools();
+    expect(tools.map((t) => t.name).sort()).toEqual([...TOOL_NAMES].sort());
+    // description round-trips from the verb summary
+    const catalogTool = tools.find((t) => t.name === "amico_catalog")!;
+    expect(catalogTool.description).toBe(SPINE_VERBS.find((v) => v.name === "catalog")!.summary);
+
+    const res = await client.callTool({ name: "amico_device", arguments: { argv: ["status", "--json"] } });
+    expect(res.isError).toBeFalsy();
+    expect(JSON.parse(toolText(res as { content: unknown }))).toEqual(
+      await verbJson("device", ["status", "--json"]),
+    );
+
+    await client.close();
+  });
+});
+
+describe("MCP round-trip over the REAL stdio transport (built bundle)", () => {
+  beforeAll(() => {
+    execFileSync("node", [join(__dirname, "..", "esbuild.config.mjs")], { cwd: join(__dirname, "..") });
+  });
+
+  it("spawns `amico mcp-serve`, lists tools + dispatches a call over stdio", async () => {
+    const transport = new StdioClientTransport({ command: "node", args: [BUNDLE, "mcp-serve"] });
+    const client = new Client({ name: "amico-mcp-stdio-test", version: "0" }, { capabilities: {} });
+    await client.connect(transport);
+
+    const { tools } = await client.listTools();
+    expect(tools.map((t) => t.name).sort()).toEqual([...TOOL_NAMES].sort());
+
+    const res = await client.callTool({ name: "amico_catalog", arguments: { argv: ["lookup", "H-gate"] } });
+    expect(res.isError).toBeFalsy();
+    expect(JSON.parse(toolText(res as { content: unknown }))).toEqual(
+      await verbJson("catalog", ["lookup", "H-gate"]),
+    );
+
+    await client.close(); // terminates the spawned server
+  }, 30000);
+});

packages/amico-run/test/s31.test.ts

@@ -7,16 +7,54 @@ import { join } from "node:path";
 // named SolveSpec launch gate — it validates + gates the spec before spawning
 // Julia. The physics-flag bans below still hold: --spec is a spec-file path,
 // NOT a physics knob; all physics stays in the script.)
-const FORBIDDEN = [/--gate\b/, /--system\b/, /--pulse\b/, /modelcontextprotocol/i, /node:https?\b/, /\bfetch\s*\(/];
+//
+// ─────────────────────────────────────────────────────────────────────────────
+// S31 AMENDMENT — B5 / issue #112 (MCP facade). ⚠️ NEEDS JACK GOVERNANCE REVIEW.
+// ─────────────────────────────────────────────────────────────────────────────
+// B5 lands the real `amico mcp-serve` MCP stdio transport (the OPTIONAL facade that
+// exposes the spine verbs as MCP tools — one impl, two transports). That requires the
+// `@modelcontextprotocol/sdk` import, which the /modelcontextprotocol/ ban forbids.
+// Rather than removing the ban, we carve out EXACTLY ONE file — src/mcp_serve.ts —
+// mirroring spec C, which amended the SolveSpec ban with a single named carve-out
+// (amico-run = the launch gate) instead of a blanket lift. The carve-out is minimal:
+//   • it names ONE file (MCP_SDK_ALLOWED), never the whole src/ tree;
+//   • it lifts ONLY the /modelcontextprotocol/ pattern — mcp_serve.ts is STILL banned
+//     from HTTP and fetch (the transport is stdio, never network) and from physics flags;
+//   • every OTHER src file — orchestrator, harness, launch path — stays under the FULL
+//     ban, MCP included. No orchestrator/harness code may import the MCP SDK.
+// The two extra `it(...)` blocks below pin the carve-out so it cannot silently widen.
+const MCP_SDK_PATTERN = /modelcontextprotocol/i;
+const FORBIDDEN = [/--gate\b/, /--system\b/, /--pulse\b/, MCP_SDK_PATTERN, /node:https?\b/, /\bfetch\s*\(/];
+
+// The MCP-SDK ban (/modelcontextprotocol/i) — and ONLY that ban — is lifted for these
+// file(s). Keep this set at exactly the facade module.
+const MCP_SDK_ALLOWED = new Set(["mcp_serve.ts"]);
 
 describe("S31 grep rule", () => {
-  it("src/ contains no forbidden tool-layer patterns", () => {
+  it("src/ contains no forbidden tool-layer patterns (MCP-SDK carve-out: mcp_serve.ts only)", () => {
     const srcDir = join(__dirname, "..", "src");
     for (const f of readdirSync(srcDir)) {
       const text = readFileSync(join(srcDir, f), "utf8");
       for (const re of FORBIDDEN) {
+        // Narrow carve-out: the MCP-facade module may reference the MCP SDK (and ONLY the
+        // SDK — it stays subject to every other forbidden pattern); every other file stays
+        // subject to this one too.
+        if (re === MCP_SDK_PATTERN && MCP_SDK_ALLOWED.has(f)) continue;
         expect(text, `${f} matches forbidden ${re}`).not.toMatch(re);
       }
     }
   });
+
+  it("the MCP-SDK carve-out is exactly one file (no scope creep)", () => {
+    expect([...MCP_SDK_ALLOWED]).toEqual(["mcp_serve.ts"]);
+  });
+
+  it("every non-carved-out src file is still MCP-SDK-free", () => {
+    const srcDir = join(__dirname, "..", "src");
+    for (const f of readdirSync(srcDir)) {
+      if (MCP_SDK_ALLOWED.has(f)) continue;
+      const text = readFileSync(join(srcDir, f), "utf8");
+      expect(text, `${f} must not import the MCP SDK`).not.toMatch(MCP_SDK_PATTERN);
+    }
+  });
 });