fix(mcp): treat resources/templates/list -32601 as empty, not a resource-load failure
This commit is contained in:
@@ -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<string, unknown> = {};
|
||||
if (cursor) {
|
||||
params.cursor = cursor;
|
||||
}
|
||||
try {
|
||||
do {
|
||||
const params: Record<string, unknown> = {};
|
||||
if (cursor) {
|
||||
params.cursor = cursor;
|
||||
}
|
||||
|
||||
const result = await connection.transport.request<MCPResourceTemplatesListResult>(
|
||||
"resources/templates/list",
|
||||
params,
|
||||
options,
|
||||
);
|
||||
allTemplates.push(...result.resourceTemplates);
|
||||
cursor = result.nextCursor;
|
||||
} while (cursor);
|
||||
const result = await connection.transport.request<MCPResourceTemplatesListResult>(
|
||||
"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;
|
||||
|
||||
+77
@@ -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<string, unknown>;
|
||||
};
|
||||
|
||||
function buildResult(method: string): Record<string, unknown> {
|
||||
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();
|
||||
}
|
||||
@@ -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<unknown>): 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);
|
||||
});
|
||||
Reference in New Issue
Block a user