fix: tool-retrieval MCP server crashes on startup (plain-object schema bug)

bin/cli.js

@@ -544,7 +544,11 @@ switch (command) {
   case 'mcp-serve': {
     // Start the tool-retrieval MCP stdio server.
     // Launched by opencode via: "command": ["opencode-workspace", "mcp-serve"]
-    require('../src/mcp/tool-retrieval-server.js');
+    const { startServer } = require('../src/mcp/tool-retrieval-server.js');
+    startServer().catch(err => {
+      process.stderr.write(`tool-retrieval-server: fatal error: ${err.message}\n`);
+      process.exit(1);
+    });
     break;
   }
 

docs/tool-retrieval-mcp.feature

@@ -57,3 +57,22 @@ Feature: On-Demand Tool Retrieval MCP Tool
     And an opencode session is active with no specific browser tools in context
     When the agent calls search_tools with query "click a button in a web page"
     Then at least one tool from the "playwright" server appears in the results
+
+  # ── Server wiring tests (the gap that let the startup crash go undetected) ──
+
+  Scenario: The MCP server wires request handlers with Zod schemas not plain objects
+    When the MCP server is configured with a mock SDK
+    Then setRequestHandler was called twice
+    And the list-tools handler schema is a valid Zod schema
+    And the call-tool handler schema is a valid Zod schema
+
+  Scenario: list-tools returns the search_tools manifest over the wire protocol
+    When the MCP server handles a list-tools request via in-memory transport
+    Then the response contains a tool named "search_tools"
+    And the search_tools tool declares a required "query" input parameter
+
+  Scenario: call-tool returns a valid CallToolResult envelope over the wire protocol
+    Given the tool corpus has not been built
+    When the MCP server handles a call-tool request for "search_tools" via in-memory transport
+    Then the response is a valid CallToolResult with a content array
+    And the content text is a non-empty string

src/mcp/tool-retrieval-server.js

@@ -22,22 +22,41 @@
  *   2. Complements the TUI first-message hook (lib/tool-retrieval.plugin.js)
  *      by giving the agent on-demand access to the retrieval pipeline at any
  *      point in the conversation.
+ *
+ * Exports:
+ *   createMcpServer(sdk) — builds and returns a configured Server instance.
+ *                          Accepts SDK dependencies explicitly so it can be
+ *                          unit-tested with a mock Server (no stdio transport).
+ *   startServer()        — loads the real SDK, calls createMcpServer, attaches
+ *                          a StdioServerTransport, and connects.  Called by
+ *                          bin/cli.js for the mcp-serve command.
  */
 
 const { handleSearchTools } = require('./search-tools-handler');
 
-async function startServer() {
-  const { Server }               = await import('@modelcontextprotocol/sdk/server/index.js');
-  const { StdioServerTransport } = await import('@modelcontextprotocol/sdk/server/stdio.js');
+// ── server factory ────────────────────────────────────────────────────────────
 
+/**
+ * Create and configure the MCP Server instance.
+ *
+ * Accepts the SDK constructors/schemas as explicit parameters so this function
+ * can be unit-tested without spawning a real stdio transport.
+ *
+ * @param {object} sdk
+ * @param {Function} sdk.Server                   - Server constructor
+ * @param {object}  sdk.ListToolsRequestSchema     - Zod schema for tools/list
+ * @param {object}  sdk.CallToolRequestSchema      - Zod schema for tools/call
+ * @returns {object} configured MCP Server instance
+ */
+function createMcpServer({ Server, ListToolsRequestSchema, CallToolRequestSchema }) {
   const server = new Server(
     { name: 'tool-retrieval', version: '1.0.0' },
     { capabilities: { tools: {} } },
   );
 
   // ── tool list ─────────────────────────────────────────────────────────────
   server.setRequestHandler(
-    { method: 'tools/list' },
+    ListToolsRequestSchema,
     async () => ({
       tools: [
         {
@@ -71,7 +90,7 @@ async function startServer() {
 
   // ── tool call ─────────────────────────────────────────────────────────────
   server.setRequestHandler(
-    { method: 'tools/call' },
+    CallToolRequestSchema,
     async (request) => {
       const { name, arguments: args } = request.params;
 
@@ -86,15 +105,31 @@ async function startServer() {
     },
   );
 
-  // ── transport ─────────────────────────────────────────────────────────────
+  return server;
+}
+
+// ── transport entry point ─────────────────────────────────────────────────────
+
+/**
+ * Load the real MCP SDK, build the server via createMcpServer(), attach a
+ * StdioServerTransport, and connect.
+ *
+ * Called explicitly by bin/cli.js for the `mcp-serve` command — NOT invoked
+ * automatically at module load time so that unit tests can safely require()
+ * this module and call createMcpServer() without triggering stdio binding.
+ */
+async function startServer() {
+  const { Server }               = await import('@modelcontextprotocol/sdk/server/index.js');
+  const { StdioServerTransport } = await import('@modelcontextprotocol/sdk/server/stdio.js');
+  const { ListToolsRequestSchema, CallToolRequestSchema } =
+    await import('@modelcontextprotocol/sdk/types.js');
+
+  const server    = createMcpServer({ Server, ListToolsRequestSchema, CallToolRequestSchema });
   const transport = new StdioServerTransport();
   await server.connect(transport);
 
   process.on('SIGTERM', () => server.close());
   process.on('SIGINT',  () => server.close());
 }
 
-startServer().catch(err => {
-  process.stderr.write(`tool-retrieval-server: fatal error: ${err.message}\n`);
-  process.exit(1);
-});
+module.exports = { createMcpServer, startServer };

unit-tests/step-definitions/tool-retrieval-mcp.steps.js

@@ -172,3 +172,98 @@ Then('at least one tool from the {string} server appears in the results', functi
     `Expected a tool from "${serverName}" in results.\nActual:\n${text}`,
   );
 });
+
+// ─── Server-wiring When ────────────────────────────────────────────────────────
+
+When('the MCP server is configured with a mock SDK', async function () {
+  await this.runStartServer();
+});
+
+When('the MCP server handles a list-tools request via in-memory transport', async function () {
+  await this.runWireListTools();
+});
+
+When('the MCP server handles a call-tool request for {string} via in-memory transport',
+  async function (toolName) {
+    await this.runWireCallTool(toolName, { query: 'test query' });
+  },
+);
+
+// ─── Server-wiring Then ────────────────────────────────────────────────────────
+
+Then('setRequestHandler was called twice', function () {
+  assert.equal(
+    this._serverCalls?.length,
+    2,
+    `Expected setRequestHandler to be called exactly 2 times, got: ${this._serverCalls?.length}`,
+  );
+});
+
+Then('the list-tools handler schema is a valid Zod schema', function () {
+  const schema = this._serverCalls?.[0]?.schema;
+  assert.ok(
+    typeof schema?.parse === 'function',
+    'Expected list-tools schema to be a Zod schema with a .parse() method; ' +
+    'got a plain object — this is the bug: setRequestHandler needs a Zod schema, not { method: "..." }',
+  );
+  const result = schema.safeParse({ method: 'tools/list', params: {} });
+  assert.ok(
+    result.success,
+    `Expected list-tools schema to accept a { method: "tools/list" } request.\n` +
+    `safeParse error: ${JSON.stringify(result.error)}`,
+  );
+});
+
+Then('the call-tool handler schema is a valid Zod schema', function () {
+  const schema = this._serverCalls?.[1]?.schema;
+  assert.ok(
+    typeof schema?.parse === 'function',
+    'Expected call-tool schema to be a Zod schema with a .parse() method; ' +
+    'got a plain object — this is the bug: setRequestHandler needs a Zod schema, not { method: "..." }',
+  );
+  const result = schema.safeParse({ method: 'tools/call', params: { name: 'search_tools', arguments: {} } });
+  assert.ok(
+    result.success,
+    `Expected call-tool schema to accept a tools/call request.\n` +
+    `safeParse error: ${JSON.stringify(result.error)}`,
+  );
+});
+
+Then('the response contains a tool named {string}', function (toolName) {
+  const tools = this._wireToolsList?.tools ?? [];
+  const found = tools.find(t => t.name === toolName);
+  assert.ok(
+    found,
+    `Expected a tool named "${toolName}" in the tools/list response.\n` +
+    `Got: ${tools.map(t => t.name).join(', ') || '(empty)'}`,
+  );
+});
+
+Then('the search_tools tool declares a required {string} input parameter', function (paramName) {
+  const tools = this._wireToolsList?.tools ?? [];
+  const tool  = tools.find(t => t.name === 'search_tools');
+  assert.ok(tool, 'Expected search_tools to be present in tools/list response');
+  const required = tool.inputSchema?.required ?? [];
+  assert.ok(
+    required.includes(paramName),
+    `Expected "${paramName}" to be in inputSchema.required.\nGot: ${JSON.stringify(required)}`,
+  );
+});
+
+Then('the response is a valid CallToolResult with a content array', function () {
+  const result = this._wireCallResult;
+  assert.ok(result, 'Expected _wireCallResult to be set (tools/call produced no response)');
+  assert.ok(
+    Array.isArray(result.content),
+    `Expected result.content to be an array.\nGot: ${JSON.stringify(result)}`,
+  );
+  assert.ok(result.content.length > 0, 'Expected result.content to be non-empty');
+});
+
+Then('the content text is a non-empty string', function () {
+  const text = this._wireCallResult?.content?.[0]?.text;
+  assert.ok(
+    typeof text === 'string' && text.trim().length > 0,
+    `Expected content[0].text to be a non-empty string.\nGot: ${JSON.stringify(text)}`,
+  );
+});

unit-tests/support/world.js

@@ -38,6 +38,9 @@ class OWWorld extends World {
     this._hookHandler        = null;   // reusable handler from createFirstMessageHandler
     this._hookLastQuery      = null;   // last text passed to _searchFn in runHook
     this._searchToolsResult  = null;   // result from runSearchTools
+    this._serverCalls        = null;   // setRequestHandler calls captured by runStartServer
+    this._wireToolsList      = null;   // listTools() result from runWireListTools
+    this._wireCallResult     = null;   // callTool() result from runWireCallTool
   }
 
   // ── helpers ─────────────────────────────────────────────────────────────────
@@ -298,7 +301,96 @@ class OWWorld extends World {
     const args = { query, ...(opts.k !== undefined ? { k: opts.k } : {}) };
     this._searchToolsResult = await handleSearchTools(args, { _searchFn: realSearch });
   }
-}
+
+  /**
+   * Invoke createMcpServer() with a mock SDK to validate request-handler
+   * registration without connecting a real stdio transport.
+   *
+   * The mock Server records every setRequestHandler() call so that Then steps
+   * can assert that proper Zod schemas — not plain objects — were passed.
+   * Captures results in this._serverCalls = [{ schema, handler }, ...].
+   */
+  async runStartServer() {
+    const { ListToolsRequestSchema, CallToolRequestSchema } =
+      await import('@modelcontextprotocol/sdk/types.js');
+
+    const calls = [];
+
+    class MockServer {
+      constructor() {}
+      setRequestHandler(schema, handler) {
+        calls.push({ schema, handler });
+      }
+    }
+
+    const { createMcpServer } = require('../../src/mcp/tool-retrieval-server');
+    createMcpServer({ Server: MockServer, ListToolsRequestSchema, CallToolRequestSchema });
+
+    this._serverCalls = calls;
+  }
+
+  /**
+   * Run a full wire-protocol tools/list round-trip using InMemoryTransport.
+   *
+   * Creates a real MCP Server (via createMcpServer) and a real Client, links
+   * them in-process, then calls client.listTools().  No stdio, no subprocess.
+   * Result stored in this._wireToolsList.
+   */
+  async runWireListTools() {
+    const { Server }  = await import('@modelcontextprotocol/sdk/server/index.js');
+    const { Client }  = await import('@modelcontextprotocol/sdk/client/index.js');
+    const { InMemoryTransport } = await import('@modelcontextprotocol/sdk/inMemory.js');
+    const { ListToolsRequestSchema, CallToolRequestSchema } =
+      await import('@modelcontextprotocol/sdk/types.js');
+
+    const { createMcpServer } = require('../../src/mcp/tool-retrieval-server');
+    const server = createMcpServer({ Server, ListToolsRequestSchema, CallToolRequestSchema });
+
+    const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair();
+    await server.connect(serverTransport);
+
+    const client = new Client({ name: 'test-client', version: '1.0.0' });
+    await client.connect(clientTransport);
+    try {
+      this._wireToolsList = await client.listTools();
+    } finally {
+      await client.close();
+    }
+  }
+
+  /**
+   * Run a full wire-protocol tools/call round-trip using InMemoryTransport.
+   *
+   * Intentionally uses an empty corpus so handleSearchTools() returns its
+   * graceful "corpus is empty" response without invoking the embedder.
+   * This makes the test self-contained and fast.
+   * Result stored in this._wireCallResult.
+   *
+   * @param {string} toolName  - tool to call (e.g. 'search_tools')
+   * @param {object} args      - arguments to pass to the tool
+   */
+  async runWireCallTool(toolName, args) {
+    const { Server }  = await import('@modelcontextprotocol/sdk/server/index.js');
+    const { Client }  = await import('@modelcontextprotocol/sdk/client/index.js');
+    const { InMemoryTransport } = await import('@modelcontextprotocol/sdk/inMemory.js');
+    const { ListToolsRequestSchema, CallToolRequestSchema } =
+      await import('@modelcontextprotocol/sdk/types.js');
+
+    const { createMcpServer } = require('../../src/mcp/tool-retrieval-server');
+    const server = createMcpServer({ Server, ListToolsRequestSchema, CallToolRequestSchema });
+
+    const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair();
+    await server.connect(serverTransport);
+
+    const client = new Client({ name: 'test-client', version: '1.0.0' });
+    await client.connect(clientTransport);
+    try {
+      this._wireCallResult = await client.callTool({ name: toolName, arguments: args });
+    } finally {
+      await client.close();
+    }
+  }
+}  // end OWWorld
 
 // ── Expose ExitError as a global so step files can catch it ──────────────────