Skip to content

Commit 80aebb0

Browse files
committed
fix(mcp): follow tools/list pagination instead of silently truncating
The SDK's listTools() returns a single page; a server that paginates via nextCursor was silently truncated to page one. Follow the cursor bounded by four independent budgets (50 pages / 1000 tools / 5 MB / 60s aggregate wall-clock) plus a repeated-cursor guard — a page cap alone can't stop a server returning a fresh cursor with no new tools. Partial results from earlier pages are kept when a later page fails; only a page-one failure throws. Matches the LibreChat capped-cursor-loop pattern; caps live in MCP_CLIENT_CONSTANTS.
1 parent a360cd4 commit 80aebb0

3 files changed

Lines changed: 135 additions & 24 deletions

File tree

apps/sim/lib/mcp/client.test.ts

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ describe('McpClient notification handler', () => {
177177
undefined,
178178
expect.objectContaining({
179179
timeout: 30_000,
180-
maxTotalTimeout: 60_000,
180+
maxTotalTimeout: expect.any(Number),
181181
resetTimeoutOnProgress: true,
182182
onprogress: expect.any(Function),
183183
})
@@ -196,10 +196,58 @@ describe('McpClient notification handler', () => {
196196

197197
expect(mockSdkListTools).toHaveBeenCalledWith(
198198
undefined,
199-
expect.objectContaining({ timeout: 60_000, maxTotalTimeout: 60_000 })
199+
expect.objectContaining({ timeout: 60_000, maxTotalTimeout: expect.any(Number) })
200200
)
201201
})
202202

203+
it('follows nextCursor pagination and aggregates all pages', async () => {
204+
mockSdkListTools
205+
.mockResolvedValueOnce({ tools: [{ name: 'a' }, { name: 'b' }], nextCursor: 'c1' })
206+
.mockResolvedValueOnce({ tools: [{ name: 'c' }], nextCursor: 'c2' })
207+
.mockResolvedValueOnce({ tools: [{ name: 'd' }] })
208+
const client = new McpClient({
209+
config: createConfig(),
210+
securityPolicy: { requireConsent: false, auditLevel: 'basic' },
211+
})
212+
213+
await client.connect()
214+
const tools = await client.listTools()
215+
216+
expect(tools.map((t) => t.name)).toEqual(['a', 'b', 'c', 'd'])
217+
expect(mockSdkListTools).toHaveBeenNthCalledWith(2, { cursor: 'c1' }, expect.anything())
218+
expect(mockSdkListTools).toHaveBeenNthCalledWith(3, { cursor: 'c2' }, expect.anything())
219+
})
220+
221+
it('stops paginating when the server repeats a cursor (loop guard)', async () => {
222+
mockSdkListTools.mockResolvedValue({ tools: [{ name: 'x' }], nextCursor: 'same' })
223+
const client = new McpClient({
224+
config: createConfig(),
225+
securityPolicy: { requireConsent: false, auditLevel: 'basic' },
226+
})
227+
228+
await client.connect()
229+
const tools = await client.listTools()
230+
231+
// Page 1 sets cursor 'same'; page 2 returns 'same' again → guard stops. Not 50 pages.
232+
expect(mockSdkListTools).toHaveBeenCalledTimes(2)
233+
expect(tools).toHaveLength(2)
234+
})
235+
236+
it('returns partial tools when a later page fails', async () => {
237+
mockSdkListTools
238+
.mockResolvedValueOnce({ tools: [{ name: 'a' }], nextCursor: 'c1' })
239+
.mockRejectedValueOnce(new Error('page 2 blew up'))
240+
const client = new McpClient({
241+
config: createConfig(),
242+
securityPolicy: { requireConsent: false, auditLevel: 'basic' },
243+
})
244+
245+
await client.connect()
246+
const tools = await client.listTools()
247+
248+
expect(tools.map((t) => t.name)).toEqual(['a'])
249+
})
250+
203251
it('logs connection diagnostics without header values', async () => {
204252
const client = new McpClient({
205253
config: {

apps/sim/lib/mcp/client.ts

Lines changed: 79 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import {
55
LATEST_PROTOCOL_VERSION,
66
type ListToolsResult,
77
SUPPORTED_PROTOCOL_VERSIONS,
8-
type Tool,
98
ToolListChangedNotificationSchema,
109
} from '@modelcontextprotocol/sdk/types.js'
1110
import { createLogger } from '@sim/logger'
@@ -275,43 +274,101 @@ export class McpClient {
275274
const maxTotalTimeoutMs = MCP_CLIENT_CONSTANTS.LIST_TOOLS_MAX_TOTAL_TIMEOUT_MS
276275
const startedAt = Date.now()
277276

277+
// The SDK's `listTools()` returns a single page; a server that paginates via
278+
// `nextCursor` would otherwise be silently truncated to page one. Follow the
279+
// cursor, bounded by four independent budgets — pages, tool count, byte size,
280+
// and aggregate wall-clock — plus a repeated-cursor guard, since a page cap
281+
// alone can't stop a server that returns a fresh cursor with no new tools.
282+
const deadline = startedAt + maxTotalTimeoutMs
283+
const tools: McpTool[] = []
284+
const seenCursors = new Set<string>()
285+
let cursor: string | undefined
286+
let bytes = 0
287+
let truncated: string | undefined
288+
278289
try {
279-
const result: ListToolsResult = await this.client.listTools(undefined, {
280-
// resetTimeoutOnProgress only takes effect when onprogress is supplied.
281-
timeout: idleTimeoutMs,
282-
maxTotalTimeout: maxTotalTimeoutMs,
283-
resetTimeoutOnProgress: true,
284-
onprogress: (progress) => {
285-
logger.debug(`Tool discovery progress from ${this.config.name}`, {
290+
for (let page = 0; page < MCP_CLIENT_CONSTANTS.LIST_TOOLS_MAX_PAGES; page++) {
291+
const remainingMs = deadline - Date.now()
292+
if (remainingMs <= 0) {
293+
truncated = 'aggregate timeout'
294+
break
295+
}
296+
const result: ListToolsResult = await this.client.listTools(
297+
cursor ? { cursor } : undefined,
298+
{
299+
// resetTimeoutOnProgress only takes effect when onprogress is supplied.
300+
timeout: Math.min(idleTimeoutMs, remainingMs),
301+
maxTotalTimeout: remainingMs,
302+
resetTimeoutOnProgress: true,
303+
onprogress: (progress) => {
304+
logger.debug(`Tool discovery progress from ${this.config.name}`, {
305+
serverId: this.config.id,
306+
progress: progress.progress,
307+
total: progress.total,
308+
})
309+
},
310+
}
311+
)
312+
313+
if (!result.tools || !Array.isArray(result.tools)) {
314+
logger.warn(`Invalid tools response from server ${this.config.name}:`, result)
315+
break
316+
}
317+
318+
for (const tool of result.tools) {
319+
if (tools.length >= MCP_CLIENT_CONSTANTS.LIST_TOOLS_MAX_TOOLS) {
320+
truncated = 'tool count'
321+
break
322+
}
323+
bytes += JSON.stringify(tool).length
324+
if (bytes > MCP_CLIENT_CONSTANTS.LIST_TOOLS_MAX_BYTES) {
325+
truncated = 'byte size'
326+
break
327+
}
328+
tools.push({
329+
name: tool.name,
330+
description: tool.description,
331+
inputSchema: tool.inputSchema as McpTool['inputSchema'],
286332
serverId: this.config.id,
287-
progress: progress.progress,
288-
total: progress.total,
333+
serverName: this.config.name,
289334
})
290-
},
291-
})
335+
}
336+
if (truncated) break
337+
338+
const next = result.nextCursor
339+
if (!next) break // missing/empty cursor = end of results (spec)
340+
if (seenCursors.has(next)) {
341+
truncated = 'repeated cursor'
342+
break
343+
}
344+
seenCursors.add(next)
345+
cursor = next
346+
}
292347

293-
if (!result.tools || !Array.isArray(result.tools)) {
294-
logger.warn(`Invalid tools response from server ${this.config.name}:`, result)
295-
return []
348+
if (truncated || seenCursors.size >= MCP_CLIENT_CONSTANTS.LIST_TOOLS_MAX_PAGES - 1) {
349+
logger.warn(`Tool discovery truncated for server ${this.config.name}`, {
350+
serverId: this.config.id,
351+
reason: truncated ?? 'page cap',
352+
toolsCollected: tools.length,
353+
pagesFetched: seenCursors.size + 1,
354+
})
296355
}
297356

298-
return result.tools.map((tool: Tool) => ({
299-
name: tool.name,
300-
description: tool.description,
301-
inputSchema: tool.inputSchema as McpTool['inputSchema'],
302-
serverId: this.config.id,
303-
serverName: this.config.name,
304-
}))
357+
return tools
305358
} catch (error) {
306359
logger.error(`Failed to list tools from server ${this.config.name}`, {
307360
serverId: this.config.id,
308361
phase: 'tools/list',
309362
durationMs: Date.now() - startedAt,
310363
idleTimeoutMs,
311364
maxTotalTimeoutMs,
365+
pagesFetched: seenCursors.size + 1,
366+
toolsCollected: tools.length,
312367
sessionIdPresent: Boolean(this.transport.sessionId),
313368
error: getMcpSafeErrorDiagnostics(error),
314369
})
370+
// Partial results from earlier pages are still useful; only fail if page one failed.
371+
if (tools.length > 0) return tools
315372
throw error
316373
}
317374
}

apps/sim/lib/mcp/utils.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,12 @@ export const MCP_CLIENT_CONSTANTS = {
5555
LIST_TOOLS_TIMEOUT_MS: 30_000,
5656
/** Hard ceiling for tools/list regardless of progress (SDK maxTotalTimeout safeguard). */
5757
LIST_TOOLS_MAX_TOTAL_TIMEOUT_MS: 60_000,
58+
/** Max `tools/list` pages followed via `nextCursor` before truncating (see fetch loop). */
59+
LIST_TOOLS_MAX_PAGES: 50,
60+
/** Max tools aggregated across all pages before truncating. */
61+
LIST_TOOLS_MAX_TOOLS: 1000,
62+
/** Max total tool-payload bytes aggregated across all pages before truncating. */
63+
LIST_TOOLS_MAX_BYTES: 5 * 1024 * 1024,
5864
FAILURE_CACHE_TTL_MS: 120_000,
5965
} as const
6066

0 commit comments

Comments
 (0)