mirror of
https://github.com/Gitlawb/openclaude.git
synced 2026-08-24 02:34:15 -05:00
fix(mcp): paginate discovery list operations (#2132)
* fix(mcp): paginate discovery list operations * fix(mcp): bound paginated discovery retries
This commit is contained in:
@@ -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')
|
||||
}
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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<void> {
|
||||
for (let turn = 0; turn < 5; turn++) {
|
||||
await Promise.resolve()
|
||||
}
|
||||
}
|
||||
|
||||
async function advanceRetryTimers(...delays: number[]): Promise<void> {
|
||||
await flushMicrotasks()
|
||||
for (const delay of delays) {
|
||||
vi.advanceTimersByTime(delay)
|
||||
await flushMicrotasks()
|
||||
}
|
||||
}
|
||||
|
||||
function makePaginatedConnection(
|
||||
name: string,
|
||||
pages: Record<ListMethod, PageStep[]>,
|
||||
): { connection: ConnectedMCPServer; requests: ListRequest[] } {
|
||||
const requests: ListRequest[] = []
|
||||
const offsets = new Map<ListMethod, number>()
|
||||
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<ListMethod, () => Promise<void>>()
|
||||
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<void>,
|
||||
) => {
|
||||
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<typeof registerMcpListChangedHandlers>[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)`,
|
||||
])
|
||||
}
|
||||
}
|
||||
}
|
||||
})
|
||||
})
|
||||
+68
-49
@@ -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 => {
|
||||
|
||||
@@ -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<TMethod extends McpListMethod> =
|
||||
| { method: TMethod }
|
||||
| { method: TMethod; params: { cursor: string } }
|
||||
|
||||
type McpListPaginationOptions<TMethod extends McpListMethod, TSchema, TPage, TItem> = {
|
||||
method: TMethod
|
||||
resultSchema: TSchema
|
||||
requestPage: (
|
||||
request: McpListRequest<TMethod>,
|
||||
resultSchema: TSchema,
|
||||
) => Promise<TPage>
|
||||
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<TMethod, TSchema, TPage, TItem>): Promise<TItem[]> {
|
||||
const items: TItem[] = []
|
||||
const usedCursors = new Set<string>()
|
||||
let cursor: string | undefined
|
||||
|
||||
for (let pageNumber = 1; ; pageNumber++) {
|
||||
const request: McpListRequest<TMethod> =
|
||||
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<MCPServerConnection, { type: 'connected' }>,
|
||||
): Promise<ServerResource[]> {
|
||||
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,
|
||||
}))
|
||||
}
|
||||
@@ -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<PendingUpdate[]>([])
|
||||
const flushTimerRef = useRef<ReturnType<typeof setTimeout> | 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
|
||||
}
|
||||
|
||||
|
||||
@@ -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' } },
|
||||
])
|
||||
})
|
||||
})
|
||||
|
||||
+2
-10
@@ -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 []
|
||||
|
||||
Reference in New Issue
Block a user