diff --git a/.changeset/paged-mcp-tool-lists.md b/.changeset/paged-mcp-tool-lists.md new file mode 100644 index 00000000..526b5d3a --- /dev/null +++ b/.changeset/paged-mcp-tool-lists.md @@ -0,0 +1,5 @@ +--- +"openwiki": patch +--- + +fix: follow `nextCursor` when listing MCP tools, so tools on a paginated server past the first page are discovered and callable instead of rejected as "not returned by tools/list" diff --git a/src/connectors/mcp-client.ts b/src/connectors/mcp-client.ts index ef5b9c62..28da8846 100644 --- a/src/connectors/mcp-client.ts +++ b/src/connectors/mcp-client.ts @@ -7,6 +7,9 @@ import { import type { McpConnectorConfig, McpReadOnlyOperation } from "./types.js"; import { fetchWithResilience } from "./http.js"; +/** Upper bound on `tools/list` pages, so a bad cursor chain cannot spin forever. */ +const MAX_TOOL_LIST_PAGES = 100; + type JsonRpcRequest = { id?: number; jsonrpc: "2.0"; @@ -375,6 +378,56 @@ function extractToolValues(value: unknown): unknown[] { return []; } +function extractNextCursor(value: unknown): string | undefined { + if ( + value !== null && + typeof value === "object" && + "nextCursor" in value && + typeof value.nextCursor === "string" && + value.nextCursor.length > 0 + ) { + return value.nextCursor; + } + + return undefined; +} + +/** + * Reads every page of `tools/list`. + * + * The MCP spec paginates `tools/list` with a top-level `nextCursor`: the client + * must re-issue the request with that cursor until the field is absent. + * Requesting only the first page silently drops the rest of the tool set, and + * `callMcpConnectorTool` then rejects those tools as "not returned by + * tools/list" even though they are valid. + * + * The loop is bounded twice over so a misbehaving server cannot hang discovery: + * a repeated cursor stops it, and so does the page cap. + */ +async function collectPaginatedTools( + request: ( + method: string, + params: Record, + ) => Promise, +): Promise<{ tools: unknown[] }> { + const tools: unknown[] = []; + const seenCursors = new Set(); + let cursor: string | undefined; + + for (let page = 0; page < MAX_TOOL_LIST_PAGES; page += 1) { + const result = await request("tools/list", cursor ? { cursor } : {}); + tools.push(...extractToolValues(result)); + + cursor = extractNextCursor(result); + if (!cursor || seenCursors.has(cursor)) { + break; + } + seenCursors.add(cursor); + } + + return { tools }; +} + function normalizeMcpTool(value: unknown): McpToolDescriptor | null { if (value === null || typeof value !== "object") { return null; @@ -460,7 +513,9 @@ class StdioJsonRpcClient { } listTools(): Promise { - return this.request("tools/list", {}); + return collectPaginatedTools((method, params) => + this.request(method, params), + ); } close(): void { @@ -606,7 +661,9 @@ class HttpJsonRpcClient { } listTools(): Promise { - return this.request("tools/list", {}); + return collectPaginatedTools((method, params) => + this.request(method, params), + ); } private async notify(method: string): Promise { diff --git a/test/mcp-client.test.ts b/test/mcp-client.test.ts index 44cc55e5..07c63279 100644 --- a/test/mcp-client.test.ts +++ b/test/mcp-client.test.ts @@ -1,5 +1,5 @@ -import { afterEach, beforeEach, describe, expect, test } from "vitest"; -import { buildChildEnv } from "../src/connectors/mcp-client.ts"; +import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; +import { buildChildEnv, listMcpTools } from "../src/connectors/mcp-client.ts"; describe("buildChildEnv", () => { const SECRET_KEYS = [ @@ -68,3 +68,145 @@ describe("buildChildEnv", () => { ); }); }); + +describe("listMcpTools pagination", () => { + const MCP_URL = "https://mcp.example.com/mcp"; + + type JsonRpcCall = { + method: string; + params?: Record; + }; + + /** + * Stubs an HTTP MCP server that answers `initialize` and then serves the + * given `tools/list` pages in order, recording every JSON-RPC call it saw. + */ + function stubHttpMcpServer(pages: Record[]): JsonRpcCall[] { + const calls: JsonRpcCall[] = []; + let pageIndex = 0; + + vi.stubGlobal( + "fetch", + vi.fn((_input: unknown, init?: { body?: string }) => { + const message = JSON.parse(init?.body ?? "{}") as { + id?: number; + method: string; + params?: Record; + }; + calls.push({ method: message.method, params: message.params }); + + const result = + message.method === "tools/list" + ? (pages[Math.min(pageIndex++, pages.length - 1)] ?? { tools: [] }) + : {}; + + return Promise.resolve( + new Response( + JSON.stringify({ id: message.id, jsonrpc: "2.0", result }), + { headers: { "content-type": "application/json" }, status: 200 }, + ), + ); + }), + ); + + return calls; + } + + function listToolsCalls(calls: JsonRpcCall[]): JsonRpcCall[] { + return calls.filter((call) => call.method === "tools/list"); + } + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + test("follows nextCursor so tools past the first page are discovered", async () => { + const calls = stubHttpMcpServer([ + { nextCursor: "cursor-2", tools: [{ name: "page_one_tool" }] }, + { tools: [{ name: "page_two_tool" }] }, + ]); + + const result = await listMcpTools({ + transport: { type: "http", url: MCP_URL }, + }); + + expect(result.tools.map((tool) => tool.name)).toEqual([ + "page_one_tool", + "page_two_tool", + ]); + // The second page must be requested with the cursor the server handed back. + expect(listToolsCalls(calls).map((call) => call.params)).toEqual([ + {}, + { cursor: "cursor-2" }, + ]); + }); + + test("issues a single request when the server does not paginate", async () => { + const calls = stubHttpMcpServer([{ tools: [{ name: "only_tool" }] }]); + + const result = await listMcpTools({ + transport: { type: "http", url: MCP_URL }, + }); + + expect(result.tools.map((tool) => tool.name)).toEqual(["only_tool"]); + expect(listToolsCalls(calls)).toHaveLength(1); + }); + + test("stops instead of looping when a server repeats the same cursor", async () => { + const calls = stubHttpMcpServer([ + { nextCursor: "stuck", tools: [{ name: "first_tool" }] }, + { nextCursor: "stuck", tools: [{ name: "second_tool" }] }, + ]); + + const result = await listMcpTools({ + transport: { type: "http", url: MCP_URL }, + }); + + expect(result.tools.map((tool) => tool.name)).toEqual([ + "first_tool", + "second_tool", + ]); + expect(listToolsCalls(calls)).toHaveLength(2); + }); + + test("caps the number of pages when a server always returns a fresh cursor", async () => { + const calls: JsonRpcCall[] = []; + let cursorSeed = 0; + + vi.stubGlobal( + "fetch", + vi.fn((_input: unknown, init?: { body?: string }) => { + const message = JSON.parse(init?.body ?? "{}") as { + id?: number; + method: string; + params?: Record; + }; + calls.push({ method: message.method, params: message.params }); + + cursorSeed += 1; + const result = + message.method === "tools/list" + ? { + nextCursor: `cursor-${cursorSeed}`, + tools: [{ name: `tool_${cursorSeed}` }], + } + : {}; + + return Promise.resolve( + new Response( + JSON.stringify({ id: message.id, jsonrpc: "2.0", result }), + { headers: { "content-type": "application/json" }, status: 200 }, + ), + ); + }), + ); + + const result = await listMcpTools({ + transport: { type: "http", url: MCP_URL }, + }); + + // Terminates on the page cap rather than following cursors forever. + expect(listToolsCalls(calls)).toHaveLength(100); + expect(result.tools).toHaveLength(100); + }); +});