diff --git a/.changeset/mcp-get-more-tools-pagination.md b/.changeset/mcp-get-more-tools-pagination.md new file mode 100644 index 0000000000..fbc0f68ed4 --- /dev/null +++ b/.changeset/mcp-get-more-tools-pagination.md @@ -0,0 +1,5 @@ +--- +'@posthog/mcp': patch +--- + +Inject the get_more_tools tool only on the first tools/list page, so a paginated catalogue's concatenated listing carries it once instead of once per page. A real first-page tool with the same name still wins (warning logged); a real tool on a later page is shadowed, with a warning when a client fetches that page — rename the SDK's tool with the `missingCapabilityToolName` option if your catalogue uses the name. diff --git a/packages/mcp/README.md b/packages/mcp/README.md index 3d1e3e44a4..f2fb3f2a2a 100644 --- a/packages/mcp/README.md +++ b/packages/mcp/README.md @@ -283,10 +283,11 @@ both advertises both tools. Like `get_more_tools`, a real tool that already uses wins: the SDK warns, skips injection, and delegates calls to the real handler. On a paginated catalogue (a `tools/list` response with a `nextCursor`), `instrument()` injects -`send_feedback` on the first page only — the page every client reads, including clients that never -follow `nextCursor` — so a compliant client's concatenated list carries it once. "First page" means -a `tools/list` request with no cursor; an empty string is a valid cursor, so `cursor: ""` is a -continuation page. Hosts using `prepareToolList()` directly own this rule themselves: pass +its virtual tools (`send_feedback` and `get_more_tools`) on the first page only — the page every +client reads, including clients that never follow `nextCursor` — so a compliant client's +concatenated list carries each once. "First page" means a `tools/list` request with no cursor; an +empty string is a valid cursor, so `cursor: ""` is a continuation page. Hosts using +`prepareToolList()` directly own this rule themselves: pass `reportMissing: true` and `collectFeedback: true` only for the first page. Name collisions are detected on the first page only. A real tool named `send_feedback` (or diff --git a/packages/mcp/src/__tests__/tools-list-envelope.test.ts b/packages/mcp/src/__tests__/tools-list-envelope.test.ts index ea78947260..f6faeff52a 100644 --- a/packages/mcp/src/__tests__/tools-list-envelope.test.ts +++ b/packages/mcp/src/__tests__/tools-list-envelope.test.ts @@ -10,6 +10,7 @@ import { import { instrument } from '../index' import { MCPAnalyticsEventType } from '../extensions/event-types' import { SEND_FEEDBACK_TOOL_NAME } from '../extensions/feedback' +import { GET_MORE_TOOLS_NAME } from '../extensions/tools' import { EventCapture, fakePostHog } from './test-utils' /** @@ -63,6 +64,16 @@ function setupPaginatedServer( } } +/** Walk the whole enumeration, returning the names each page advertised. */ +const listBothPages = async (client: Client, cursorToken = 'page-2'): Promise<[string[], string[]]> => { + const firstPage = await client.request({ method: 'tools/list', params: {} }, ListToolsResultSchema) + const secondPage = await client.request( + { method: 'tools/list', params: { cursor: cursorToken } }, + ListToolsResultSchema + ) + return [firstPage.tools.map((tool) => tool.name), secondPage.tools.map((tool) => tool.name)] +} + describe('tools/list response envelope', () => { let eventCapture: EventCapture @@ -257,16 +268,6 @@ describe('tools/list response envelope', () => { inputSchema: { type: 'object' as const }, } - /** Walk the whole enumeration, returning the names each page advertised. */ - const listBothPages = async (client: Client, cursorToken = 'page-2'): Promise<[string[], string[]]> => { - const firstPage = await client.request({ method: 'tools/list', params: {} }, ListToolsResultSchema) - const secondPage = await client.request( - { method: 'tools/list', params: { cursor: cursorToken } }, - ListToolsResultSchema - ) - return [firstPage.tools.map((tool) => tool.name), secondPage.tools.map((tool) => tool.name)] - } - it('appends the virtual tool only to the first page', async () => { const { server, client, connect, cleanup } = await setupPaginatedServer() try { @@ -373,4 +374,56 @@ describe('tools/list response envelope', () => { } }) }) + + /** The same first-page rule, on the missing-capability virtual tool. */ + describe('get_more_tools on a paginated catalogue', () => { + const REAL_MISSING_TOOL = { + name: GET_MORE_TOOLS_NAME, + description: 'A real application tool that owns the name', + inputSchema: { type: 'object' as const }, + } + + it('appends the virtual tool only to the first page', async () => { + const { server, client, connect, cleanup } = setupPaginatedServer() + try { + instrument(server, fakePostHog(), { reportMissing: true }) + await connect() + + const [pageOne, pageTwo] = await listBothPages(client) + expect(pageOne).toEqual(['page_one_tool', GET_MORE_TOOLS_NAME]) + expect(pageTwo).toEqual(['page_two_tool']) + } finally { + await cleanup() + } + }) + + it('a real owner on a later page is shadowed, with a warning when its page is served', async () => { + const { server, client, connect, cleanup } = setupPaginatedServer({ + secondPage: { tools: [REAL_MISSING_TOOL] }, + }) + const warnings: string[] = [] + try { + instrument(server, fakePostHog(), { + reportMissing: true, + logger: (message: string) => warnings.push(message), + }) + await connect() + + const [pageOne, pageTwo] = await listBothPages(client) + expect(pageOne).toEqual(['page_one_tool', GET_MORE_TOOLS_NAME]) + expect(pageTwo).toEqual([GET_MORE_TOOLS_NAME]) + expect(warnings.some((message) => message.includes('is shadowed by the SDK'))).toBe(true) + + // Calls to the name go to the SDK, not the real tool. + const result = await client.request( + { method: 'tools/call', params: { name: GET_MORE_TOOLS_NAME, arguments: { context: 'need bulk delete' } } }, + CallToolResultSchema + ) + expect((result.content as { text: string }[])[0].text).not.toBe(`called: ${GET_MORE_TOOLS_NAME}`) + expect(eventCapture.findEventByType(MCPAnalyticsEventType.mcpMissingCapability)).toBeDefined() + } finally { + await cleanup() + } + }) + }) }) diff --git a/packages/mcp/src/extensions/instrumentation.ts b/packages/mcp/src/extensions/instrumentation.ts index bb981b891b..e06e14480b 100644 --- a/packages/mcp/src/extensions/instrumentation.ts +++ b/packages/mcp/src/extensions/instrumentation.ts @@ -825,19 +825,32 @@ async function getTracedToolsList( } if (data) { + // A compliant client concatenates every page into one list, so only the + // first page — the one every client reads, including clients that never + // follow `nextCursor` — may carry a virtual tool. Presence, not + // truthiness: `cursor: ""` is a continuation page. + const isFirstPage = request.params?.cursor == null + const missingToolName = resolveMissingCapabilityToolName(data.options) if (data.options.reportMissing) { const alreadyPresent = tools.some((tool) => tool?.name === missingToolName) - if (alreadyPresent) { + if (isFirstPage && alreadyPresent) { data.logger( - `Warning: Cannot inject missing-capability tool "${missingToolName}" because a real tool already uses that name. The real tool will not be intercepted.` + `Warning: Cannot inject missing-capability tool "${missingToolName}" because a real tool already uses that name. The real tool will not be intercepted. To keep missing-capability reports, rename the SDK's tool with the missingCapabilityToolName option.` ) - } else { + } else if (isFirstPage) { const virtualTool = getReportMissingToolDescriptor(missingToolName) tools.push(virtualTool) // Cached separately because the virtual tool is added after the listing // was cached, and its calls need ownership like any other tool's. cacheToolAnalyticsParameterOwnership(data.toolAnalyticsParameterOwnership, [virtualTool]) + } else if (alreadyPresent) { + // Conflicts are only detected on the pages a client actually + // fetches; a real owner here is already shadowed by the first-page + // injection, so the host must rename the SDK's tool. + data.logger( + `Warning: A real tool "${missingToolName}" on a later tools/list page is shadowed by the SDK's missing-capability tool. Its calls will be intercepted. Rename the SDK's tool with the missingCapabilityToolName option to keep both.` + ) } } @@ -845,11 +858,6 @@ async function getTracedToolsList( if (feedbackOptions) { const feedbackToolName = feedbackOptions.toolName ?? SEND_FEEDBACK_TOOL_NAME const alreadyPresent = tools.some((tool) => tool?.name === feedbackToolName) - // A compliant client concatenates every page into one list, so only the - // first page — the one every client reads, including clients that never - // follow `nextCursor` — may carry the virtual tool. Presence, not - // truthiness: `cursor: ""` is a continuation page. - const isFirstPage = request.params?.cursor == null if (isFirstPage && alreadyPresent) { data.logger( `Warning: Cannot inject agent-feedback tool "${feedbackToolName}" because a real tool already uses that name. The real tool will not be intercepted. To collect feedback alongside it, rename the SDK's tool with collectFeedback: { toolName: "..." }.`