From 6a0e80ee56ecede186ff6ca407107a029db47e1b Mon Sep 17 00:00:00 2001 From: jms830 Date: Tue, 16 Jun 2026 21:25:03 -0400 Subject: [PATCH] fix(mcp): treat resources/templates/list -32601 as empty, not a resource-load failure --- packages/coding-agent/src/mcp/client.ts | 51 ++++++-- .../fixtures/resources-no-templates-mcp.ts | 77 ++++++++++++ .../mcp-resource-templates-missing.test.ts | 118 ++++++++++++++++++ 3 files changed, 233 insertions(+), 13 deletions(-) create mode 100755 packages/coding-agent/test/fixtures/resources-no-templates-mcp.ts create mode 100644 packages/coding-agent/test/mcp-resource-templates-missing.test.ts diff --git a/packages/coding-agent/src/mcp/client.ts b/packages/coding-agent/src/mcp/client.ts index 739895f1e..792c6f12a 100644 --- a/packages/coding-agent/src/mcp/client.ts +++ b/packages/coding-agent/src/mcp/client.ts @@ -303,8 +303,22 @@ export async function listResources( return allResources; } +/** True when an error is a JSON-RPC "method not found" (-32601) response. */ +function isMethodNotFoundError(error: unknown): boolean { + const message = error instanceof Error ? error.message : String(error); + return message.includes("-32601") || /method not found/i.test(message); +} + /** * List resource templates from a connected server. + * + * A server MAY advertise the `resources` capability without implementing the + * optional `resources/templates/list` method (it is optional in the MCP spec). + * Such servers reject the request with JSON-RPC -32601 ("Method not found"). + * Treat that as "no templates" and return `[]` rather than throwing — otherwise + * a caller that loads resources and templates together (see `MCPManager`'s + * `Promise.all([listResources, listResourceTemplates])`) would discard the + * server's concrete resources too. Any other error still propagates. */ export async function listResourceTemplates( connection: MCPServerConnection, @@ -321,20 +335,31 @@ export async function listResourceTemplates( const allTemplates: MCPResourceTemplate[] = []; let cursor: string | undefined; - do { - const params: Record = {}; - if (cursor) { - params.cursor = cursor; - } + try { + do { + const params: Record = {}; + if (cursor) { + params.cursor = cursor; + } - const result = await connection.transport.request( - "resources/templates/list", - params, - options, - ); - allTemplates.push(...result.resourceTemplates); - cursor = result.nextCursor; - } while (cursor); + const result = await connection.transport.request( + "resources/templates/list", + params, + options, + ); + allTemplates.push(...result.resourceTemplates); + cursor = result.nextCursor; + } while (cursor); + } catch (error) { + // A server that doesn't implement the optional templates method answers + // -32601; cache an empty list so we neither retry nor let the failure + // bubble up and discard the server's concrete resources. + if (isMethodNotFoundError(error)) { + connection.resourceTemplates = []; + return []; + } + throw error; + } connection.resourceTemplates = allTemplates; return allTemplates; diff --git a/packages/coding-agent/test/fixtures/resources-no-templates-mcp.ts b/packages/coding-agent/test/fixtures/resources-no-templates-mcp.ts new file mode 100755 index 000000000..9d0c09829 --- /dev/null +++ b/packages/coding-agent/test/fixtures/resources-no-templates-mcp.ts @@ -0,0 +1,77 @@ +#!/usr/bin/env bun +/** + * Test fixture: a stdio MCP server that advertises the `resources` capability + * and serves `resources/list`, but does NOT implement the optional + * `resources/templates/list` method — it answers that request with a JSON-RPC + * -32601 ("Method not found") error, exactly like jcodemunch/jdocmunch. + * + * Used by `mcp-resource-templates-missing.test.ts` to prove that a missing + * templates method no longer discards the server's concrete resources. + * + * Speaks newline-delimited JSON-RPC 2.0 (the wire format of `StdioTransport`), + * same shape as `many-tools-mcp.ts`. Exported constants are imported by the + * test; the server only starts when run as the entry module (`import.meta.main`). + */ +import * as readline from "node:readline"; + +/** Concrete resource URIs the fixture advertises via `resources/list`. */ +export const RESOURCE_URIS = ["test://alpha", "test://beta"]; + +type JsonRpcRequest = { + jsonrpc: "2.0"; + id?: string | number; + method: string; + params?: Record; +}; + +function buildResult(method: string): Record { + switch (method) { + case "initialize": + return { + protocolVersion: "2025-03-26", + serverInfo: { name: "resources-no-templates-fixture", version: "1.0.0" }, + capabilities: { resources: {} }, + }; + case "resources/list": + return { + resources: RESOURCE_URIS.map((uri, i) => ({ uri, name: `Resource ${i}` })), + }; + default: + return {}; + } +} + +function startServer(): void { + const rl = readline.createInterface({ input: process.stdin }); + rl.on("line", line => { + const trimmed = line.trim(); + if (trimmed.length === 0) return; + let msg: JsonRpcRequest; + try { + msg = JSON.parse(trimmed) as JsonRpcRequest; + } catch { + return; + } + // Notifications (no `id`) get no response. + if (msg.id === undefined || msg.id === null) return; + + if (msg.method === "resources/templates/list") { + // Optional method this server doesn't implement. + const error = { + jsonrpc: "2.0" as const, + id: msg.id, + error: { code: -32601, message: "Method not found" }, + }; + process.stdout.write(`${JSON.stringify(error)}\n`); + return; + } + + const response = { jsonrpc: "2.0" as const, id: msg.id, result: buildResult(msg.method) }; + process.stdout.write(`${JSON.stringify(response)}\n`); + }); + rl.on("close", () => process.exit(0)); +} + +if (import.meta.main) { + startServer(); +} diff --git a/packages/coding-agent/test/mcp-resource-templates-missing.test.ts b/packages/coding-agent/test/mcp-resource-templates-missing.test.ts new file mode 100644 index 000000000..bed402f46 --- /dev/null +++ b/packages/coding-agent/test/mcp-resource-templates-missing.test.ts @@ -0,0 +1,118 @@ +/** + * Regression test: a server that declares the `resources` capability but does + * NOT implement the optional `resources/templates/list` method answers with + * JSON-RPC -32601 ("Method not found"). Before the fix, `listResourceTemplates` + * rethrew that error, which made `MCPManager`'s + * `Promise.all([listResources, listResourceTemplates])` reject and discard the + * server's concrete resources too (the jcodemunch/jdocmunch bug). + * + * Contract this test defends: a missing templates method is treated as "no + * templates" (returns []), so resources still load; any OTHER error from + * `resources/templates/list` still propagates. + */ +import { afterEach, beforeEach, describe, expect, it } from "bun:test"; +import * as fs from "node:fs"; +import * as os from "node:os"; +import * as path from "node:path"; +import { listResourceTemplates } from "../src/mcp/client"; +import { MCPManager } from "../src/mcp/manager"; +import type { MCPServerConnection, MCPTransport } from "../src/mcp/types"; +import type { MCPStdioServerConfig } from "../src/mcp/types"; +import { RESOURCE_URIS } from "./fixtures/resources-no-templates-mcp"; + +const FIXTURE_PATH = path.join(import.meta.dir, "fixtures", "resources-no-templates-mcp.ts"); +const BUN_EXEC = process.execPath; + +/** Minimal mock transport where `request` is controlled by the caller. */ +function mockTransport(requestFn: (method: string) => Promise): MCPTransport { + return { + connected: true, + request: ((method: string) => requestFn(method)) as MCPTransport["request"], + async notify() {}, + async close() {}, + }; +} + +function makeResourceConnection(transport: MCPTransport): MCPServerConnection { + return { + name: "docs", + config: { type: "stdio", command: "echo" }, + transport, + serverInfo: { name: "docs", version: "1.0" }, + capabilities: { resources: {} }, + }; +} + +describe("listResourceTemplates -32601 handling", () => { + it("returns [] when the server answers resources/templates/list with -32601", async () => { + const connection = makeResourceConnection( + mockTransport(async method => { + if (method === "resources/templates/list") { + throw new Error("MCP error -32601: Method not found"); + } + return { resourceTemplates: [] }; + }), + ); + + await expect(listResourceTemplates(connection)).resolves.toEqual([]); + // Cached as "no templates" so a second call does not re-request. + expect(connection.resourceTemplates).toEqual([]); + }); + + it("rethrows non-method-not-found errors", async () => { + const connection = makeResourceConnection( + mockTransport(async () => { + throw new Error("MCP error -32603: Internal error"); + }), + ); + + await expect(listResourceTemplates(connection)).rejects.toThrow("-32603"); + // Not cached: a transient failure must be retryable. + expect(connection.resourceTemplates).toBeUndefined(); + }); +}); + +describe("MCPManager loads resources for a templates-less server", () => { + let workDir: string; + + beforeEach(() => { + workDir = fs.mkdtempSync(path.join(os.tmpdir(), "omp-mcp-templates-")); + }); + + afterEach(() => { + fs.rmSync(workDir, { recursive: true, force: true }); + }); + + it("keeps concrete resources when resources/templates/list is unimplemented", async () => { + const manager = new MCPManager(workDir); + const config: MCPStdioServerConfig = { + type: "stdio", + command: BUN_EXEC, + args: [FIXTURE_PATH], + }; + + try { + await manager.connectServers({ docs: config }, {}); + + // Genuine integration wait: `#loadServerResourcesAndPrompts` runs + // fire-and-forget against a real spawned subprocess and exposes no + // completion promise or event to await, and fake timers cannot drive a + // child process. Poll the live manager with a generous ceiling, exiting + // the instant resources arrive (mirrors sdk-mcp-auto-discovery.test.ts). + const deadline = Date.now() + 10_000; + let resources = manager.getServerResources("docs"); + while ((resources?.resources.length ?? 0) === 0 && Date.now() < deadline) { + await Bun.sleep(25); + resources = manager.getServerResources("docs"); + } + + expect(resources).toBeDefined(); + // The -32601 from templates/list must NOT discard the concrete resources. + expect(resources?.resources.map(r => r.uri).sort()).toEqual([...RESOURCE_URIS].sort()); + // Templates are treated as empty, not an error. + expect(resources?.templates).toEqual([]); + } finally { + await manager.disconnectAll(); + } + }, 20_000); +});