diff --git a/src/__tests__/security-hardening.test.ts b/src/__tests__/security-hardening.test.ts index c32cb62ad..d9aa7c121 100644 --- a/src/__tests__/security-hardening.test.ts +++ b/src/__tests__/security-hardening.test.ts @@ -21,8 +21,8 @@ const file = (relative: string) => Bun.file(resolve(SRC, relative)) describe('MCP tool result sanitization', () => { test('transformResultContent sanitizes text content', async () => { const content = await file('services/mcp/client.ts').text() - // Tool definitions are already sanitized (line ~1798) - expect(content).toContain('recursivelySanitizeUnicode(result.tools)') + // The complete, aggregated tool list is sanitized before conversion. + expect(content).toContain('recursivelySanitizeUnicode(tools)') // Tool results must also be sanitized expect(content).toMatch( /case 'text':[\s\S]*?recursivelySanitizeUnicode\(resultContent\.text\)/, @@ -188,4 +188,4 @@ describe('Swarm permission file polling removed', () => { expect(preceding).not.toContain('@deprecated') } }) -}) \ No newline at end of file +}) diff --git a/src/services/mcp/client.pagination.test.ts b/src/services/mcp/client.pagination.test.ts new file mode 100644 index 000000000..63136a3b6 --- /dev/null +++ b/src/services/mcp/client.pagination.test.ts @@ -0,0 +1,950 @@ +import { afterEach, describe, expect, test, vi } from 'vitest' +import { feature } from 'bun:bundle' +import { + PromptListChangedNotificationSchema, + ResourceListChangedNotificationSchema, + ToolListChangedNotificationSchema, +} from '@modelcontextprotocol/sdk/types.js' +import type { ConnectedMCPServer } from './types.js' +import { + fetchCommandsForClient, + fetchResourcesForClient, + fetchToolsForClient, +} from './client.js' + +type ListMethod = 'tools/list' | 'resources/list' | 'prompts/list' +type ListRequest = { + method: ListMethod + params?: { cursor?: string } +} +type PageStep = unknown | Error + +async function flushMicrotasks(): Promise { + for (let turn = 0; turn < 5; turn++) { + await Promise.resolve() + } +} + +async function advanceRetryTimers(...delays: number[]): Promise { + await flushMicrotasks() + for (const delay of delays) { + vi.advanceTimersByTime(delay) + await flushMicrotasks() + } +} + +function makePaginatedConnection( + name: string, + pages: Record, +): { connection: ConnectedMCPServer; requests: ListRequest[] } { + const requests: ListRequest[] = [] + const offsets = new Map() + const client = { + request: async ( + request: ListRequest, + resultSchema: { parse: (value: unknown) => unknown }, + ) => { + requests.push(request) + const offset = offsets.get(request.method) ?? 0 + offsets.set(request.method, offset + 1) + const page = pages[request.method][offset] + if (page === undefined) { + throw new Error(`unexpected ${request.method} page ${offset + 1}`) + } + if (page instanceof Error) throw page + return resultSchema.parse(page) + }, + } + + return { + connection: { + type: 'connected', + name, + config: { type: 'sdk', scope: 'local' }, + capabilities: { tools: {}, resources: {}, prompts: {} }, + client, + cleanup: async () => {}, + } as unknown as ConnectedMCPServer, + requests, + } +} + +describe('MCP list cursor pagination', () => { + afterEach(() => { + vi.useRealTimers() + vi.restoreAllMocks() + }) + + test('legacy single-page responses omit params for all list methods', async () => { + const { connection, requests } = makePaginatedConnection('legacy-pages', { + 'tools/list': [ + { tools: [{ name: 'only-tool', inputSchema: { type: 'object' } }] }, + ], + 'resources/list': [ + { resources: [{ uri: 'file:///only-resource', name: 'only-resource' }] }, + ], + 'prompts/list': [{ prompts: [{ name: 'only-prompt' }] }], + }) + + const [tools, resources, prompts] = await Promise.all([ + fetchToolsForClient(connection), + fetchResourcesForClient(connection), + fetchCommandsForClient(connection), + ]) + + expect(tools.map(tool => tool.mcpInfo?.toolName)).toEqual(['only-tool']) + expect(resources.map(resource => resource.name)).toEqual([ + 'only-resource', + ]) + expect(prompts.map(command => command.userFacingName?.())).toEqual([ + 'legacy-pages:only-prompt (MCP)', + ]) + expect(requests).toEqual([ + { method: 'tools/list' }, + { method: 'resources/list' }, + { method: 'prompts/list' }, + ]) + }) + + test('tools/list follows nextCursor and preserves page order', async () => { + const { connection, requests } = makePaginatedConnection('pages-tools', { + 'tools/list': [ + { + tools: [{ name: 'first', inputSchema: { type: 'object' } }], + nextCursor: 'tools-page-2', + }, + { tools: [{ name: 'second', inputSchema: { type: 'object' } }] }, + ], + 'resources/list': [], + 'prompts/list': [], + }) + + const tools = await fetchToolsForClient(connection) + + expect(tools.map(tool => tool.mcpInfo?.toolName)).toEqual([ + 'first', + 'second', + ]) + expect(requests).toEqual([ + { method: 'tools/list' }, + { method: 'tools/list', params: { cursor: 'tools-page-2' } }, + ]) + }) + + test('resources/list follows nextCursor and preserves page order', async () => { + const { connection, requests } = makePaginatedConnection( + 'pages-resources', + { + 'tools/list': [], + 'resources/list': [ + { + resources: [{ uri: 'file:///first', name: 'first' }], + nextCursor: 'resources-page-2', + }, + { resources: [{ uri: 'file:///second', name: 'second' }] }, + ], + 'prompts/list': [], + }, + ) + + const resources = await fetchResourcesForClient(connection) + + expect(resources.map(resource => resource.name)).toEqual([ + 'first', + 'second', + ]) + expect(requests).toEqual([ + { method: 'resources/list' }, + { method: 'resources/list', params: { cursor: 'resources-page-2' } }, + ]) + }) + + test('prompts/list follows nextCursor and preserves page order', async () => { + const { connection, requests } = makePaginatedConnection('pages-prompts', { + 'tools/list': [], + 'resources/list': [], + 'prompts/list': [ + { prompts: [{ name: 'first' }], nextCursor: 'prompts-page-2' }, + { prompts: [{ name: 'second' }] }, + ], + }) + + const commands = await fetchCommandsForClient(connection) + + expect(commands.map(command => command.userFacingName?.())).toEqual([ + 'pages-prompts:first (MCP)', + 'pages-prompts:second (MCP)', + ]) + expect(requests).toEqual([ + { method: 'prompts/list' }, + { method: 'prompts/list', params: { cursor: 'prompts-page-2' } }, + ]) + }) + + test('all list methods traverse several pages in exact server order', async () => { + const { connection } = makePaginatedConnection('several-pages', { + 'tools/list': [ + { + tools: [{ name: 'tool-1', inputSchema: { type: 'object' } }], + nextCursor: 'tool-2', + }, + { + tools: [{ name: 'tool-2', inputSchema: { type: 'object' } }], + nextCursor: 'tool-3', + }, + { tools: [{ name: 'tool-3', inputSchema: { type: 'object' } }] }, + ], + 'resources/list': [ + { + resources: [{ uri: 'file:///resource-1', name: 'resource-1' }], + nextCursor: 'resource-2', + }, + { + resources: [{ uri: 'file:///resource-2', name: 'resource-2' }], + nextCursor: 'resource-3', + }, + { + resources: [{ uri: 'file:///resource-3', name: 'resource-3' }], + }, + ], + 'prompts/list': [ + { prompts: [{ name: 'prompt-1' }], nextCursor: 'prompt-2' }, + { prompts: [{ name: 'prompt-2' }], nextCursor: 'prompt-3' }, + { prompts: [{ name: 'prompt-3' }] }, + ], + }) + + const [tools, resources, prompts] = await Promise.all([ + fetchToolsForClient(connection), + fetchResourcesForClient(connection), + fetchCommandsForClient(connection), + ]) + + expect(tools.map(tool => tool.mcpInfo?.toolName)).toEqual([ + 'tool-1', + 'tool-2', + 'tool-3', + ]) + expect(resources.map(resource => resource.name)).toEqual([ + 'resource-1', + 'resource-2', + 'resource-3', + ]) + expect(prompts.map(command => command.userFacingName?.())).toEqual([ + 'several-pages:prompt-1 (MCP)', + 'several-pages:prompt-2 (MCP)', + 'several-pages:prompt-3 (MCP)', + ]) + }) + + test('an empty intermediate page still advances to the next cursor', async () => { + const { connection, requests } = makePaginatedConnection( + 'empty-intermediate', + { + 'tools/list': [], + 'resources/list': [ + { + resources: [{ uri: 'file:///first', name: 'first' }], + nextCursor: 'empty-page', + }, + { resources: [], nextCursor: 'final-page' }, + { resources: [{ uri: 'file:///last', name: 'last' }] }, + ], + 'prompts/list': [], + }, + ) + + const resources = await fetchResourcesForClient(connection) + + expect(resources.map(resource => resource.name)).toEqual(['first', 'last']) + expect(requests).toEqual([ + { method: 'resources/list' }, + { method: 'resources/list', params: { cursor: 'empty-page' } }, + { method: 'resources/list', params: { cursor: 'final-page' } }, + ]) + }) + + test('an empty-string nextCursor is sent as an opaque cursor', async () => { + const { connection, requests } = makePaginatedConnection( + 'empty-string-cursor', + { + 'tools/list': [ + { + tools: [{ name: 'before', inputSchema: { type: 'object' } }], + nextCursor: '', + }, + { tools: [{ name: 'after', inputSchema: { type: 'object' } }] }, + ], + 'resources/list': [], + 'prompts/list': [], + }, + ) + + const tools = await fetchToolsForClient(connection) + + expect(tools.map(tool => tool.mcpInfo?.toolName)).toEqual([ + 'before', + 'after', + ]) + expect(requests[1]).toEqual({ + method: 'tools/list', + params: { cursor: '' }, + }) + }) + + test.each(['opaque-secret-123', ''])( + 'rejects a repeated opaque cursor %j without exposing it in the error', + async repeatedCursor => { + const { paginateMcpList } = await import('./pagination.js') + const promise = paginateMcpList({ + method: 'resources/list', + resultSchema: null, + requestPage: async () => ({ + items: [], + nextCursor: repeatedCursor, + }), + getItems: page => page.items, + getNextCursor: page => page.nextCursor, + }) + + try { + await promise + throw new Error('expected repeated cursor rejection') + } catch (error) { + const message = error instanceof Error ? error.message : String(error) + expect(message).toBe( + 'MCP resources/list pagination repeated a cursor', + ) + if (repeatedCursor !== '') { + expect(message).not.toContain(repeatedCursor) + } + } + }, + ) + + test('rejects a malformed nextCursor instead of coercing it', async () => { + const { paginateMcpList } = await import('./pagination.js') + await expect( + paginateMcpList({ + method: 'prompts/list', + resultSchema: null, + requestPage: async () => ({ items: [], nextCursor: 42 }), + getItems: page => page.items, + getNextCursor: page => page.nextCursor, + }), + ).rejects.toThrow( + 'MCP prompts/list pagination returned malformed nextCursor', + ) + }) + + test('fails rather than truncating when the page ceiling is exceeded', async () => { + const { paginateMcpList } = await import('./pagination.js') + let page = 0 + await expect( + paginateMcpList({ + method: 'tools/list', + resultSchema: null, + requestPage: async () => ({ + items: [`item-${++page}`], + nextCursor: `cursor-${page}`, + }), + getItems: result => result.items, + getNextCursor: result => result.nextCursor, + pageLimit: 2, + }), + ).rejects.toThrow('MCP tools/list pagination exceeded page limit (2)') + expect(page).toBe(2) + }) + + test('fails rather than truncating when the aggregate item ceiling is exceeded', async () => { + const { paginateMcpList } = await import('./pagination.js') + let page = 0 + await expect( + paginateMcpList({ + method: 'resources/list', + resultSchema: null, + requestPage: async () => + ++page === 1 + ? { items: ['first', 'second'], nextCursor: 'more' } + : { items: ['third'] }, + getItems: result => result.items, + getNextCursor: result => result.nextCursor, + itemLimit: 2, + }), + ).rejects.toThrow( + 'MCP resources/list pagination exceeded item limit (2)', + ) + }) + + test('production page and item ceilings fail fetchers without caching a prefix', async () => { + const { MCP_LIST_ITEM_LIMIT, MCP_LIST_PAGE_LIMIT } = await import( + './pagination.js' + ) + expect(MCP_LIST_PAGE_LIMIT).toBe(100) + expect(MCP_LIST_ITEM_LIMIT).toBe(10_000) + + const toolRequests: ListRequest[] = [] + const pageLimitedConnection = { + type: 'connected', + name: 'production-page-limit', + config: { type: 'sdk', scope: 'local' }, + capabilities: { tools: {} }, + client: { + request: async ( + request: ListRequest, + resultSchema: { parse: (value: unknown) => unknown }, + ) => { + toolRequests.push(request) + const pageNumber = toolRequests.length + return resultSchema.parse({ + tools: [ + { + name: `tool-${pageNumber}`, + inputSchema: { type: 'object' }, + }, + ], + nextCursor: `cursor-${pageNumber}`, + }) + }, + }, + cleanup: async () => {}, + } as unknown as ConnectedMCPServer + + expect(await fetchToolsForClient(pageLimitedConnection)).toEqual([]) + expect(toolRequests).toHaveLength(MCP_LIST_PAGE_LIMIT) + + const oversizedResources = Array.from( + { length: MCP_LIST_ITEM_LIMIT + 1 }, + (_, index) => ({ + uri: `test://resource-${index}`, + name: `resource-${index}`, + }), + ) + const itemLimitedConnection = { + type: 'connected', + name: 'production-item-limit', + config: { type: 'sdk', scope: 'local' }, + capabilities: { resources: {} }, + client: { + request: async ( + _request: ListRequest, + resultSchema: { parse: (value: unknown) => unknown }, + ) => resultSchema.parse({ resources: oversizedResources }), + }, + cleanup: async () => {}, + } as unknown as ConnectedMCPServer + + expect(await fetchResourcesForClient(itemLimitedConnection)).toEqual([]) + }) + + test('a rejected resources page returns and caches no partial prefix', async () => { + const { connection, requests } = makePaginatedConnection( + 'atomic-resource-rejection', + { + 'tools/list': [], + 'resources/list': [ + { + resources: [{ uri: 'file:///prefix', name: 'prefix' }], + nextCursor: 'fails', + }, + new Error('later page rejected'), + ], + 'prompts/list': [], + }, + ) + + expect(await fetchResourcesForClient(connection)).toEqual([]) + expect(await fetchResourcesForClient(connection)).toEqual([]) + expect(requests).toHaveLength(2) + }) + + test('a schema-invalid prompts page returns no partial prefix and is not retried', async () => { + const { connection, requests } = makePaginatedConnection( + 'atomic-prompt-schema', + { + 'tools/list': [], + 'resources/list': [], + 'prompts/list': [ + { prompts: [{ name: 'prefix' }], nextCursor: 'invalid' }, + { prompts: [{ description: 'missing required name' }] }, + ], + }, + ) + + expect(await fetchCommandsForClient(connection)).toEqual([]) + expect(requests).toHaveLength(2) + }) + + test('tools retry only the transient later page and never duplicate page one', async () => { + vi.useFakeTimers() + const requests: ListRequest[] = [] + let secondPageAttempts = 0 + const client = { + request: async ( + request: ListRequest, + resultSchema: { parse: (value: unknown) => unknown }, + ) => { + requests.push(request) + if (!('params' in request)) { + return resultSchema.parse({ + tools: [{ name: 'first', inputSchema: { type: 'object' } }], + nextCursor: 'second-page', + }) + } + secondPageAttempts++ + if (secondPageAttempts === 1) { + throw new Error('transient later-page failure') + } + return resultSchema.parse({ + tools: [{ name: 'second', inputSchema: { type: 'object' } }], + }) + }, + } + const connection = { + type: 'connected', + name: 'later-page-tools-retry', + config: { type: 'sdk', scope: 'local' }, + capabilities: { tools: {} }, + client, + cleanup: async () => {}, + } as unknown as ConnectedMCPServer + + const toolsPromise = fetchToolsForClient(connection) + await advanceRetryTimers(1_000) + const tools = await toolsPromise + + expect(tools.map(tool => tool.mcpInfo?.toolName)).toEqual([ + 'first', + 'second', + ]) + expect(requests).toEqual([ + { method: 'tools/list' }, + { method: 'tools/list', params: { cursor: 'second-page' } }, + { method: 'tools/list', params: { cursor: 'second-page' } }, + ]) + }) + + test('a terminal later tools page retries that cursor three times and returns no prefix', async () => { + vi.useFakeTimers() + const requests: ListRequest[] = [] + const connection = { + type: 'connected', + name: 'terminal-later-page-tools-retry', + config: { type: 'sdk', scope: 'local' }, + capabilities: { tools: {} }, + client: { + request: async ( + request: ListRequest, + resultSchema: { parse: (value: unknown) => unknown }, + ) => { + requests.push(request) + if (!('params' in request)) { + return resultSchema.parse({ + tools: [{ name: 'prefix', inputSchema: { type: 'object' } }], + nextCursor: 'terminal-page', + }) + } + throw new Error('terminal later-page failure') + }, + }, + cleanup: async () => {}, + } as unknown as ConnectedMCPServer + + const toolsPromise = fetchToolsForClient(connection) + await advanceRetryTimers(1_000, 2_000) + + expect(await toolsPromise).toEqual([]) + expect(requests).toEqual([ + { method: 'tools/list' }, + { method: 'tools/list', params: { cursor: 'terminal-page' } }, + { method: 'tools/list', params: { cursor: 'terminal-page' } }, + { method: 'tools/list', params: { cursor: 'terminal-page' } }, + ]) + }) + + test('tools share one retry deadline across all paginated pages', async () => { + vi.useFakeTimers() + const requests: ListRequest[] = [] + let firstPageAttempts = 0 + const connection = { + type: 'connected', + name: 'tools-traversal-retry-deadline', + config: { type: 'sdk', scope: 'local' }, + capabilities: { tools: {} }, + client: { + request: async ( + request: ListRequest, + resultSchema: { parse: (value: unknown) => unknown }, + ) => { + requests.push(request) + if (!('params' in request)) { + firstPageAttempts++ + if (firstPageAttempts === 1) { + throw new Error('transient first-page failure') + } + return resultSchema.parse({ + tools: [{ name: 'prefix', inputSchema: { type: 'object' } }], + nextCursor: 'terminal-page', + }) + } + throw new Error('terminal later-page failure') + }, + }, + cleanup: async () => {}, + } as unknown as ConnectedMCPServer + + const toolsPromise = fetchToolsForClient(connection) + await advanceRetryTimers(1_000, 1_000, 1_000) + const requestsAtTraversalDeadline = [...requests] + // Let the base implementation's per-page 2s backoff finish too, so the + // red/green proof never leaves a pending promise or timer behind. + await advanceRetryTimers(1_000) + + expect(requestsAtTraversalDeadline).toHaveLength(5) + expect(await toolsPromise).toEqual([]) + expect(requests).toEqual([ + { method: 'tools/list' }, + { method: 'tools/list' }, + { method: 'tools/list', params: { cursor: 'terminal-page' } }, + { method: 'tools/list', params: { cursor: 'terminal-page' } }, + { method: 'tools/list', params: { cursor: 'terminal-page' } }, + ]) + }) + + test('an overdue retry timer cannot start a post-deadline request', async () => { + vi.useFakeTimers() + let nowMs = 0 + vi.spyOn(performance, 'now').mockImplementation(() => nowMs) + const requests: ListRequest[] = [] + const connection = { + type: 'connected', + name: 'tools-overdue-retry-deadline', + config: { type: 'sdk', scope: 'local' }, + capabilities: { tools: {} }, + client: { + request: async ( + request: ListRequest, + resultSchema: { parse: (value: unknown) => unknown }, + ) => { + requests.push(request) + if (!('params' in request)) { + return resultSchema.parse({ + tools: [{ name: 'prefix', inputSchema: { type: 'object' } }], + nextCursor: 'late-page', + }) + } + throw new Error('terminal later-page failure') + }, + }, + cleanup: async () => {}, + } as unknown as ConnectedMCPServer + + const toolsPromise = fetchToolsForClient(connection) + await flushMicrotasks() + nowMs = 1_100 + vi.advanceTimersByTime(1_000) + await flushMicrotasks() + nowMs = 3_200 + vi.advanceTimersByTime(1_900) + await flushMicrotasks() + // Drain the final 100ms that the base implementation still has pending; + // the corrected implementation has already rejected the traversal. + await advanceRetryTimers(100) + + expect(await toolsPromise).toEqual([]) + expect(requests).toEqual([ + { method: 'tools/list' }, + { method: 'tools/list', params: { cursor: 'late-page' } }, + { method: 'tools/list', params: { cursor: 'late-page' } }, + ]) + }) + + test('reconnect cache invalidation starts a complete fresh traversal', async () => { + let generation = 1 + const requests: ListRequest[] = [] + const client = { + request: async ( + request: ListRequest, + resultSchema: { parse: (value: unknown) => unknown }, + ) => { + requests.push(request) + const suffix = 'params' in request ? 'second' : 'first' + const nextCursor = 'params' in request ? undefined : 'next' + const result = + request.method === 'tools/list' + ? { + tools: [ + { + name: `tool-${generation}-${suffix}`, + inputSchema: { type: 'object' }, + }, + ], + nextCursor, + } + : request.method === 'resources/list' + ? { + resources: [ + { + uri: `file:///resource-${generation}-${suffix}`, + name: `resource-${generation}-${suffix}`, + }, + ], + nextCursor, + } + : { + prompts: [{ name: `prompt-${generation}-${suffix}` }], + nextCursor, + } + return resultSchema.parse(result) + }, + } + const connection = { + type: 'connected', + name: 'fresh-after-reconnect', + config: { type: 'sdk', scope: 'local' }, + capabilities: { tools: {}, resources: {}, prompts: {} }, + client, + cleanup: async () => {}, + } as unknown as ConnectedMCPServer + + await Promise.all([ + fetchToolsForClient(connection), + fetchResourcesForClient(connection), + fetchCommandsForClient(connection), + ]) + generation = 2 + fetchToolsForClient.cache.delete(connection.name) + fetchResourcesForClient.cache.delete(connection.name) + fetchCommandsForClient.cache.delete(connection.name) + requests.length = 0 + + const [tools, resources, prompts] = await Promise.all([ + fetchToolsForClient(connection), + fetchResourcesForClient(connection), + fetchCommandsForClient(connection), + ]) + + expect(tools.map(tool => tool.mcpInfo?.toolName)).toEqual([ + 'tool-2-first', + 'tool-2-second', + ]) + expect(resources.map(resource => resource.name)).toEqual([ + 'resource-2-first', + 'resource-2-second', + ]) + expect(prompts.map(command => command.userFacingName?.())).toEqual([ + 'fresh-after-reconnect:prompt-2-first (MCP)', + 'fresh-after-reconnect:prompt-2-second (MCP)', + ]) + expect(requests).toHaveLength(6) + }) + + test('each production list-changed handler refetches every page', async () => { + const { registerMcpListChangedHandlers } = await import( + './useManageMCPConnections.js' + ) + const mcpSkillsEnabled = feature('MCP_SKILLS') ? true : false + if (mcpSkillsEnabled) { + // Production eagerly loads this module before MCP connections start; it + // registers the builders used when a skill:// resource is read. + await import('../../skills/loadSkillsDir.js') + } + const fetchMcpSkillsForClient = mcpSkillsEnabled + ? (await import('../../skills/mcpSkills.js')).fetchMcpSkillsForClient + : null + const requests: ListRequest[] = [] + const resourceReads: string[] = [] + const handlers = new Map Promise>() + let generation = 1 + const client = { + request: async ( + request: + | ListRequest + | { method: 'resources/read'; params: { uri: string } }, + resultSchema: { parse: (value: unknown) => unknown }, + ) => { + if (request.method === 'resources/read') { + resourceReads.push(request.params.uri) + return resultSchema.parse({ + contents: [ + { + uri: request.params.uri, + mimeType: 'text/markdown', + text: [ + '---', + `name: skill-${generation}`, + 'description: Paginated notification skill', + '---', + `# Skill ${generation}`, + ].join('\n'), + }, + ], + }) + } + requests.push(request) + const suffix = 'params' in request ? 'second' : 'first' + const nextCursor = 'params' in request ? undefined : 'next' + const page = + request.method === 'tools/list' + ? { + tools: [ + { + name: `tool-${generation}-${suffix}`, + inputSchema: { type: 'object' }, + }, + ], + nextCursor, + } + : request.method === 'resources/list' + ? { + resources: [ + { + uri: `file:///resource-${generation}-${suffix}`, + name: `resource-${generation}-${suffix}`, + }, + ...(suffix === 'second' + ? [ + { + uri: `skill://skill-${generation}`, + name: `skill-${generation}`, + }, + ] + : []), + ], + nextCursor, + } + : { + prompts: [{ name: `prompt-${generation}-${suffix}` }], + nextCursor, + } + return resultSchema.parse(page) + }, + setNotificationHandler: ( + schema: unknown, + handler: () => Promise, + ) => { + const method = + schema === ToolListChangedNotificationSchema + ? 'tools/list' + : schema === ResourceListChangedNotificationSchema + ? 'resources/list' + : schema === PromptListChangedNotificationSchema + ? 'prompts/list' + : undefined + if (method) handlers.set(method, handler) + }, + } + const connection = { + type: 'connected', + name: 'list-changed-pagination', + config: { type: 'sdk', scope: 'local' }, + capabilities: { + tools: { listChanged: true }, + resources: { listChanged: true }, + prompts: { listChanged: true }, + }, + client, + cleanup: async () => {}, + } as unknown as ConnectedMCPServer + + await Promise.all([ + fetchToolsForClient(connection), + fetchResourcesForClient(connection), + fetchCommandsForClient(connection), + fetchMcpSkillsForClient?.(connection) ?? Promise.resolve([]), + ]) + type ListChangedUpdate = Parameters< + Parameters[1] + >[0] + const updates: ListChangedUpdate[] = [] + registerMcpListChangedHandlers(connection, update => updates.push(update)) + expect([...handlers.keys()].sort()).toEqual([ + 'prompts/list', + 'resources/list', + 'tools/list', + ]) + + let cachedSkillGeneration = generation + for (const method of [ + 'tools/list', + 'resources/list', + 'prompts/list', + ] as const) { + generation++ + requests.length = 0 + resourceReads.length = 0 + updates.length = 0 + await handlers.get(method)?.() + + const expectedTraversal: ListRequest[] = [ + { method }, + { method, params: { cursor: 'next' } }, + ] + if (method === 'resources/list' && mcpSkillsEnabled) { + // The production branch refreshes ordinary resources, prompts, and + // resource-backed skills concurrently. Avoid asserting Promise.all + // interleaving, but require both resource traversals and every page. + expect( + requests.filter(request => request.method === 'resources/list'), + ).toEqual([ + { method: 'resources/list' }, + { method: 'resources/list' }, + { method: 'resources/list', params: { cursor: 'next' } }, + { method: 'resources/list', params: { cursor: 'next' } }, + ]) + expect( + requests.filter(request => request.method === 'prompts/list'), + ).toEqual([ + { method: 'prompts/list' }, + { method: 'prompts/list', params: { cursor: 'next' } }, + ]) + expect( + requests.filter(request => request.method === 'tools/list'), + ).toEqual([]) + } else { + expect(requests).toEqual(expectedTraversal) + } + const update = updates[0] + if (method === 'tools/list') { + expect(update?.tools?.map(tool => tool.mcpInfo?.toolName)).toEqual([ + `tool-${generation}-first`, + `tool-${generation}-second`, + ]) + } else if (method === 'resources/list') { + expect(update?.resources?.map(resource => resource.name)).toEqual([ + `resource-${generation}-first`, + `resource-${generation}-second`, + `skill-${generation}`, + ]) + if (mcpSkillsEnabled) { + expect(resourceReads).toEqual([`skill://skill-${generation}`]) + expect(update?.commands?.map(command => command.name)).toEqual([ + `mcp__list-changed-pagination__prompt-${generation}-first`, + `mcp__list-changed-pagination__prompt-${generation}-second`, + `mcp__list-changed-pagination__skill-${generation}`, + ]) + cachedSkillGeneration = generation + } + } else { + expect(resourceReads).toEqual([]) + expect(update?.commands?.map(command => command.name)).toEqual([ + `mcp__list-changed-pagination__prompt-${generation}-first`, + `mcp__list-changed-pagination__prompt-${generation}-second`, + ...(mcpSkillsEnabled + ? [ + `mcp__list-changed-pagination__skill-${cachedSkillGeneration}`, + ] + : []), + ]) + if (!mcpSkillsEnabled) { + expect( + update?.commands?.map(command => command.userFacingName?.()), + ).toEqual([ + `list-changed-pagination:prompt-${generation}-first (MCP)`, + `list-changed-pagination:prompt-${generation}-second (MCP)`, + ]) + } + } + } + }) +}) diff --git a/src/services/mcp/client.ts b/src/services/mcp/client.ts index e9772c66a..c60c85065 100644 --- a/src/services/mcp/client.ts +++ b/src/services/mcp/client.ts @@ -28,7 +28,6 @@ import { type JSONRPCMessage, type ListPromptsResult, ListPromptsResultSchema, - ListResourcesResultSchema, ListRootsRequestSchema, type ListToolsResult, ListToolsResultSchema, @@ -112,6 +111,7 @@ import { } from './elicitationHandler.js' import { buildMcpToolName } from './mcpStringUtils.js' import { normalizeNameForMCP } from './normalization.js' +import { listAllMcpResources, paginateMcpList } from './pagination.js' import { getLoggingSafeMcpBaseUrl } from './utils.js' /* eslint-disable @typescript-eslint/no-require-imports */ @@ -1822,6 +1822,7 @@ export function areMcpConfigsEqual( // Max cache size for fetch* caches. Keyed by server name (stable across // reconnects), bounded to prevent unbounded growth with many MCP servers. const MCP_FETCH_CACHE_SIZE = 20 +export const MCP_TOOLS_LIST_RETRY_BUDGET_MS = 3_000 /** * Encode MCP tool input for the auto-mode security classifier. @@ -1847,35 +1848,60 @@ export const fetchToolsForClient = memoizeWithLRU( return [] } - // Retry tool list fetch up to 2 times on transient failures. - // Without retry, a single timeout during tools/list makes all MCP tools - // silently disappear from the model's context until the next reconnect. - let result: ListToolsResult | undefined - let lastError: unknown - for (let attempt = 0; attempt < 3; attempt++) { - try { - result = (await client.client.request( - { method: 'tools/list' }, - ListToolsResultSchema, - )) as ListToolsResult - break - } catch (err) { - lastError = err - if (attempt < 2) { - logMCPDebug( - client.name, - `tools/list failed (attempt ${attempt + 1}/3): ${errorMessage(err)}. Retrying...`, - ) - await sleep(1000 * (attempt + 1)) + const retryDeadline = + performance.now() + MCP_TOOLS_LIST_RETRY_BUDGET_MS + const tools = await paginateMcpList({ + method: 'tools/list', + resultSchema: ListToolsResultSchema, + // Preserve the existing three-attempt 1s/2s policy for each page while + // one traversal-wide deadline has retry budget remaining. A page-two + // retry must not replay the successful first page. + requestPage: async (request, resultSchema) => { + let result: ListToolsResult | undefined + let lastError: unknown + for (let attempt = 0; attempt < 3; attempt++) { + try { + result = (await client.client.request( + request, + resultSchema, + )) as ListToolsResult + break + } catch (err) { + lastError = err + if (attempt < 2) { + const remainingRetryBudget = + retryDeadline - performance.now() + if (remainingRetryBudget <= 0) { + break + } + logMCPDebug( + client.name, + `tools/list failed (attempt ${attempt + 1}/3): ${errorMessage(err)}. Retrying...`, + ) + await sleep( + Math.min(1000 * (attempt + 1), remainingRetryBudget), + ) + // A timer can wake after its scheduled deadline when the event + // loop is busy. Do not turn that overdue wake-up into another + // request; a wake exactly at the deadline preserves the legacy + // third attempt after the full 1s/2s backoff. + if (performance.now() > retryDeadline) { + break + } + } + } } - } - } - if (!result) { - throw lastError ?? new Error('tools/list failed after 3 attempts') - } + if (!result) { + throw lastError ?? new Error('tools/list failed after 3 attempts') + } + return result + }, + getItems: result => result.tools, + getNextCursor: result => result.nextCursor, + }) - // Sanitize tool data from MCP server - const toolsToProcess = recursivelySanitizeUnicode(result.tools) + // Sanitize tool data from MCP server after the complete list succeeds. + const toolsToProcess = recursivelySanitizeUnicode(tools) // Check if we should skip the mcp__ prefix for SDK MCP servers const skipPrefix = @@ -2221,18 +2247,7 @@ export const fetchResourcesForClient = memoizeWithLRU( return [] } - const result = await client.client.request( - { method: 'resources/list' }, - ListResourcesResultSchema, - ) - - if (!result.resources) return [] - - // Add server name to each resource - return result.resources.map(resource => ({ - ...resource, - server: client.name, - })) + return await listAllMcpResources(client) } catch (error) { logMCPError( client.name, @@ -2254,16 +2269,20 @@ export const fetchCommandsForClient = memoizeWithLRU( return [] } - // Request prompts list from client - const result = (await client.client.request( - { method: 'prompts/list' }, - ListPromptsResultSchema, - )) as ListPromptsResult + const prompts = await paginateMcpList({ + method: 'prompts/list', + resultSchema: ListPromptsResultSchema, + requestPage: async (request, resultSchema) => + (await client.client.request( + request, + resultSchema, + )) as ListPromptsResult, + getItems: result => result.prompts, + getNextCursor: result => result.nextCursor, + }) - if (!result.prompts) return [] - - // Sanitize prompt data from MCP server - const promptsToProcess = recursivelySanitizeUnicode(result.prompts) + // Sanitize prompt data from MCP server after the complete list succeeds. + const promptsToProcess = recursivelySanitizeUnicode(prompts) // Convert MCP prompts to our Command format return promptsToProcess.map(prompt => { diff --git a/src/services/mcp/pagination.ts b/src/services/mcp/pagination.ts new file mode 100644 index 000000000..bae4bf199 --- /dev/null +++ b/src/services/mcp/pagination.ts @@ -0,0 +1,115 @@ +import { + type ListResourcesResult, + ListResourcesResultSchema, +} from '@modelcontextprotocol/sdk/types.js' +import type { MCPServerConnection, ServerResource } from './types.js' + +/** + * MCP discovery responses are controlled by external servers. One hundred + * pages matches the repository's other bounded cursor traversal, while 10,000 + * aggregate items prevents an unbounded server from retaining unlimited tool + * schemas/resource metadata/prompt definitions in a single cached fetch. + */ +export const MCP_LIST_PAGE_LIMIT = 100 +export const MCP_LIST_ITEM_LIMIT = 10_000 + +type McpListMethod = 'tools/list' | 'resources/list' | 'prompts/list' + +export type McpListRequest = + | { method: TMethod } + | { method: TMethod; params: { cursor: string } } + +type McpListPaginationOptions = { + method: TMethod + resultSchema: TSchema + requestPage: ( + request: McpListRequest, + resultSchema: TSchema, + ) => Promise + getItems: (page: TPage) => readonly TItem[] + getNextCursor: (page: TPage) => unknown + pageLimit?: number + itemLimit?: number +} + +/** + * Traverse one MCP list operation atomically. The caller owns the page request + * policy (tools/list retries each page; resources/list and prompts/list do not), + * while this function owns cursor validation, ordering, and safety bounds. + */ +export async function paginateMcpList< + TMethod extends McpListMethod, + TSchema, + TPage, + TItem, +>({ + method, + resultSchema, + requestPage, + getItems, + getNextCursor, + pageLimit = MCP_LIST_PAGE_LIMIT, + itemLimit = MCP_LIST_ITEM_LIMIT, +}: McpListPaginationOptions): Promise { + const items: TItem[] = [] + const usedCursors = new Set() + let cursor: string | undefined + + for (let pageNumber = 1; ; pageNumber++) { + const request: McpListRequest = + cursor === undefined + ? { method } + : { method, params: { cursor } } + const page = await requestPage(request, resultSchema) + const pageItems = getItems(page) + + if (items.length + pageItems.length > itemLimit) { + throw new Error( + `MCP ${method} pagination exceeded item limit (${itemLimit})`, + ) + } + items.push(...pageItems) + + const nextCursor = getNextCursor(page) + if (nextCursor === undefined) { + return items + } + if (typeof nextCursor !== 'string') { + throw new Error(`MCP ${method} pagination returned malformed nextCursor`) + } + if (usedCursors.has(nextCursor)) { + throw new Error(`MCP ${method} pagination repeated a cursor`) + } + if (pageNumber >= pageLimit) { + throw new Error( + `MCP ${method} pagination exceeded page limit (${pageLimit})`, + ) + } + + usedCursors.add(nextCursor) + cursor = nextCursor + } +} + +/** + * List and annotate every resource exposed by one connected MCP server. Both + * ordinary resource discovery and resource-backed skills share this exact + * cursor traversal so their protocol behavior cannot drift. + */ +export async function listAllMcpResources( + client: Extract, +): Promise { + const resources = await paginateMcpList({ + method: 'resources/list', + resultSchema: ListResourcesResultSchema, + requestPage: async (request, resultSchema) => + (await client.client.request(request, resultSchema)) as ListResourcesResult, + getItems: result => result.resources, + getNextCursor: result => result.nextCursor, + }) + + return resources.map(resource => ({ + ...resource, + server: client.name, + })) +} diff --git a/src/services/mcp/useManageMCPConnections.ts b/src/services/mcp/useManageMCPConnections.ts index f2d918bd4..beb991ba1 100644 --- a/src/services/mcp/useManageMCPConnections.ts +++ b/src/services/mcp/useManageMCPConnections.ts @@ -13,6 +13,7 @@ import { reconnectMcpServerImpl, } from './client.js' import type { + ConnectedMCPServer, MCPServerConnection, ScopedMcpServerConfig, ServerResource, @@ -89,6 +90,159 @@ const MAX_RECONNECT_ATTEMPTS = 5 const INITIAL_BACKOFF_MS = 1000 const MAX_BACKOFF_MS = 30000 +type PendingUpdate = MCPServerConnection & { + tools?: Tool[] + commands?: Command[] + resources?: ServerResource[] +} +type ConnectedListUpdate = ConnectedMCPServer & { + tools?: Tool[] + commands?: Command[] + resources?: ServerResource[] +} + +/** + * Register the production list-changed handlers at one testable seam. Keeping + * cache invalidation and refetch here ensures tests exercise the same closures + * the hook installs rather than reimplementing notification behavior. + */ +export function registerMcpListChangedHandlers( + client: ConnectedMCPServer, + updateServer: (update: ConnectedListUpdate) => void, +): void { + if (client.capabilities?.tools?.listChanged) { + client.client.setNotificationHandler( + ToolListChangedNotificationSchema, + async () => { + logMCPDebug( + client.name, + `Received tools/list_changed notification, refreshing tools`, + ) + try { + // Grab cached promise before invalidating to log previous count + const previousToolsPromise = fetchToolsForClient.cache.get(client.name) + fetchToolsForClient.cache.delete(client.name) + const newTools = await fetchToolsForClient(client) + const newCount = newTools.length + if (previousToolsPromise) { + previousToolsPromise.then( + (previousTools: Tool[]) => { + logEvent('tengu_mcp_list_changed', { + type: 'tools' as AnalyticsMetadata_I_VERIFIED_THIS_IS_NOT_CODE_OR_FILEPATHS, + previousCount: previousTools.length, + newCount, + }) + }, + () => { + logEvent('tengu_mcp_list_changed', { + type: 'tools' as AnalyticsMetadata_I_VERIFIED_THIS_IS_NOT_CODE_OR_FILEPATHS, + newCount, + }) + }, + ) + } else { + logEvent('tengu_mcp_list_changed', { + type: 'tools' as AnalyticsMetadata_I_VERIFIED_THIS_IS_NOT_CODE_OR_FILEPATHS, + newCount, + }) + } + updateServer({ ...client, tools: newTools }) + } catch (error) { + logMCPError( + client.name, + `Failed to refresh tools after list_changed notification: ${errorMessage(error)}`, + ) + } + }, + ) + } + + if (client.capabilities?.prompts?.listChanged) { + client.client.setNotificationHandler( + PromptListChangedNotificationSchema, + async () => { + logMCPDebug( + client.name, + `Received prompts/list_changed notification, refreshing prompts`, + ) + logEvent('tengu_mcp_list_changed', { + type: 'prompts' as AnalyticsMetadata_I_VERIFIED_THIS_IS_NOT_CODE_OR_FILEPATHS, + }) + try { + // Skills come from resources, not prompts — don't invalidate their + // cache here. fetchMcpSkillsForClient returns the cached result. + fetchCommandsForClient.cache.delete(client.name) + const [mcpPrompts, mcpSkills] = await Promise.all([ + fetchCommandsForClient(client), + feature('MCP_SKILLS') + ? fetchMcpSkillsForClient!(client) + : Promise.resolve([]), + ]) + updateServer({ + ...client, + commands: [...mcpPrompts, ...mcpSkills], + }) + // MCP skills changed — invalidate skill-search index so next + // discovery rebuilds with the new set. + clearSkillIndexCache?.() + } catch (error) { + logMCPError( + client.name, + `Failed to refresh prompts after list_changed notification: ${errorMessage(error)}`, + ) + } + }, + ) + } + + if (client.capabilities?.resources?.listChanged) { + client.client.setNotificationHandler( + ResourceListChangedNotificationSchema, + async () => { + logMCPDebug( + client.name, + `Received resources/list_changed notification, refreshing resources`, + ) + logEvent('tengu_mcp_list_changed', { + type: 'resources' as AnalyticsMetadata_I_VERIFIED_THIS_IS_NOT_CODE_OR_FILEPATHS, + }) + try { + fetchResourcesForClient.cache.delete(client.name) + if (feature('MCP_SKILLS')) { + // Skills are discovered from resources, so refresh them too. + // Invalidate prompts cache as well: we write commands here, and a + // concurrent prompts/list_changed could otherwise have us stomp + // its fresh result with our cached stale one. + fetchMcpSkillsForClient!.cache.delete(client.name) + fetchCommandsForClient.cache.delete(client.name) + const [newResources, mcpPrompts, mcpSkills] = await Promise.all([ + fetchResourcesForClient(client), + fetchCommandsForClient(client), + fetchMcpSkillsForClient!(client), + ]) + updateServer({ + ...client, + resources: newResources, + commands: [...mcpPrompts, ...mcpSkills], + }) + // MCP skills changed — invalidate skill-search index so next + // discovery rebuilds with the new set. + clearSkillIndexCache?.() + } else { + const newResources = await fetchResourcesForClient(client) + updateServer({ ...client, resources: newResources }) + } + } catch (error) { + logMCPError( + client.name, + `Failed to refresh resources after list_changed notification: ${errorMessage(error)}`, + ) + } + }, + ) + } +} + /** * Create a unique key for a plugin error to enable deduplication */ @@ -205,11 +359,6 @@ export function useManageMCPConnections( // (instead of queueMicrotask) ensures updates are batched even when // connection callbacks arrive at different times due to network I/O. const MCP_BATCH_FLUSH_MS = 100 - type PendingUpdate = MCPServerConnection & { - tools?: Tool[] - commands?: Command[] - resources?: ServerResource[] - } const pendingUpdatesRef = useRef([]) const flushTimerRef = useRef | null>(null) @@ -610,142 +759,9 @@ export function useManageMCPConnections( } } - // Register notification handlers for list_changed notifications - // These allow the server to notify us when tools, prompts, or resources change - if (client.capabilities?.tools?.listChanged) { - client.client.setNotificationHandler( - ToolListChangedNotificationSchema, - async () => { - logMCPDebug( - client.name, - `Received tools/list_changed notification, refreshing tools`, - ) - try { - // Grab cached promise before invalidating to log previous count - const previousToolsPromise = fetchToolsForClient.cache.get( - client.name, - ) - fetchToolsForClient.cache.delete(client.name) - const newTools = await fetchToolsForClient(client) - const newCount = newTools.length - if (previousToolsPromise) { - previousToolsPromise.then( - (previousTools: Tool[]) => { - logEvent('tengu_mcp_list_changed', { - type: 'tools' as AnalyticsMetadata_I_VERIFIED_THIS_IS_NOT_CODE_OR_FILEPATHS, - previousCount: previousTools.length, - newCount, - }) - }, - () => { - logEvent('tengu_mcp_list_changed', { - type: 'tools' as AnalyticsMetadata_I_VERIFIED_THIS_IS_NOT_CODE_OR_FILEPATHS, - newCount, - }) - }, - ) - } else { - logEvent('tengu_mcp_list_changed', { - type: 'tools' as AnalyticsMetadata_I_VERIFIED_THIS_IS_NOT_CODE_OR_FILEPATHS, - newCount, - }) - } - updateServer({ ...client, tools: newTools }) - } catch (error) { - logMCPError( - client.name, - `Failed to refresh tools after list_changed notification: ${errorMessage(error)}`, - ) - } - }, - ) - } - - if (client.capabilities?.prompts?.listChanged) { - client.client.setNotificationHandler( - PromptListChangedNotificationSchema, - async () => { - logMCPDebug( - client.name, - `Received prompts/list_changed notification, refreshing prompts`, - ) - logEvent('tengu_mcp_list_changed', { - type: 'prompts' as AnalyticsMetadata_I_VERIFIED_THIS_IS_NOT_CODE_OR_FILEPATHS, - }) - try { - // Skills come from resources, not prompts — don't invalidate their - // cache here. fetchMcpSkillsForClient returns the cached result. - fetchCommandsForClient.cache.delete(client.name) - const [mcpPrompts, mcpSkills] = await Promise.all([ - fetchCommandsForClient(client), - feature('MCP_SKILLS') - ? fetchMcpSkillsForClient!(client) - : Promise.resolve([]), - ]) - updateServer({ - ...client, - commands: [...mcpPrompts, ...mcpSkills], - }) - // MCP skills changed — invalidate skill-search index so - // next discovery rebuilds with the new set. - clearSkillIndexCache?.() - } catch (error) { - logMCPError( - client.name, - `Failed to refresh prompts after list_changed notification: ${errorMessage(error)}`, - ) - } - }, - ) - } - - if (client.capabilities?.resources?.listChanged) { - client.client.setNotificationHandler( - ResourceListChangedNotificationSchema, - async () => { - logMCPDebug( - client.name, - `Received resources/list_changed notification, refreshing resources`, - ) - logEvent('tengu_mcp_list_changed', { - type: 'resources' as AnalyticsMetadata_I_VERIFIED_THIS_IS_NOT_CODE_OR_FILEPATHS, - }) - try { - fetchResourcesForClient.cache.delete(client.name) - if (feature('MCP_SKILLS')) { - // Skills are discovered from resources, so refresh them too. - // Invalidate prompts cache as well: we write commands here, - // and a concurrent prompts/list_changed could otherwise have - // us stomp its fresh result with our cached stale one. - fetchMcpSkillsForClient!.cache.delete(client.name) - fetchCommandsForClient.cache.delete(client.name) - const [newResources, mcpPrompts, mcpSkills] = - await Promise.all([ - fetchResourcesForClient(client), - fetchCommandsForClient(client), - fetchMcpSkillsForClient!(client), - ]) - updateServer({ - ...client, - resources: newResources, - commands: [...mcpPrompts, ...mcpSkills], - }) - // MCP skills changed — invalidate skill-search index so - // next discovery rebuilds with the new set. - clearSkillIndexCache?.() - } else { - const newResources = await fetchResourcesForClient(client) - updateServer({ ...client, resources: newResources }) - } - } catch (error) { - logMCPError( - client.name, - `Failed to refresh resources after list_changed notification: ${errorMessage(error)}`, - ) - } - }, - ) - } + // These handlers invalidate their list cache and run a complete + // pagination traversal before updating server state. + registerMcpListChangedHandlers(client, updateServer) break } diff --git a/src/skills/mcpSkills.test.ts b/src/skills/mcpSkills.test.ts index 615c4cc9a..22af97499 100644 --- a/src/skills/mcpSkills.test.ts +++ b/src/skills/mcpSkills.test.ts @@ -111,4 +111,63 @@ describe('fetchMcpSkillsForClient privilege stripping', () => { } expect(command.allowedTools).toEqual([]) }) + + test('discovers and reads a skill resource that appears only on page two', async () => { + const listRequests: Array<{ + method: string + params?: { cursor?: string } + }> = [] + const client = { + type: 'connected', + name: 'page-two-skill-server', + config: { type: 'sdk', scope: 'local' }, + capabilities: { resources: {} }, + client: { + request: async (request: { + method: string + params?: { cursor?: string; uri?: string } + }) => { + if (request.method === 'resources/list') { + listRequests.push(request) + return request.params?.cursor === undefined + ? { + resources: [{ uri: 'file:///ordinary', name: 'ordinary' }], + nextCursor: 'skill-page', + } + : { + resources: [{ uri: 'skill://page-two', name: 'page-two' }], + } + } + if (request.method === 'resources/read') { + return { + contents: [ + { + uri: 'skill://page-two', + mimeType: 'text/markdown', + text: [ + '---', + 'name: page-two', + 'description: Paginated skill', + '---', + '# Page two', + ].join('\n'), + }, + ], + } + } + throw new Error(`unexpected method ${request.method}`) + }, + }, + cleanup: async () => {}, + } as unknown as MCPServerConnection + + const commands = await fetchMcpSkillsForClient(client) + + expect(commands).toHaveLength(1) + expect(commands[0]?.name).toBe('mcp__page-two-skill-server__page-two') + expect(listRequests).toEqual([ + { method: 'resources/list' }, + { method: 'resources/list', params: { cursor: 'skill-page' } }, + ]) + }) }) diff --git a/src/skills/mcpSkills.ts b/src/skills/mcpSkills.ts index c021e6928..485641de3 100644 --- a/src/skills/mcpSkills.ts +++ b/src/skills/mcpSkills.ts @@ -3,11 +3,11 @@ import { parseFrontmatter } from '../utils/frontmatterParser.js' import { memoizeWithLRU } from '../utils/memoize.js' import { recursivelySanitizeUnicode } from '../utils/sanitization.js' import { normalizeNameForMCP } from '../services/mcp/normalization.js' +import { listAllMcpResources } from '../services/mcp/pagination.js' import type { MCPServerConnection, ServerResource } from '../services/mcp/types.js' import { getMCPSkillBuilders } from './mcpSkillBuilders.js' import { logForDebugging } from '../utils/debug.js' import { - ListResourcesResultSchema, type ReadResourceResult, ReadResourceResultSchema, } from '@modelcontextprotocol/sdk/types.js' @@ -94,15 +94,7 @@ export const fetchMcpSkillsForClient = memoizeWithLRU( if (!client.capabilities?.resources) return [] try { - const result = await client.client.request( - { method: 'resources/list' }, - ListResourcesResultSchema, - ) - - const resources = (result.resources ?? []).map(r => ({ - ...r, - server: client.name, - })) as ServerResource[] + const resources = await listAllMcpResources(client) const skillResources = resources.filter(isSkillResource) if (skillResources.length === 0) return []