Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ jest.mock('~/auth', () => ({
}));

jest.mock('~/mcp/mcpConfig', () => ({
mcpConfig: { CONNECTION_CHECK_TTL: 0 },
mcpConfig: { CONNECTION_CHECK_TTL: 0, TOOLS_LIST_MAX_PAGES: 50 },
}));

const mockLogger = logger as jest.Mocked<typeof logger>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,11 @@ jest.mock('~/auth', () => ({
}));

jest.mock('~/mcp/mcpConfig', () => ({
mcpConfig: { CONNECTION_CHECK_TTL: 0, USER_CONNECTION_IDLE_TIMEOUT: 30 * 60 * 1000 },
mcpConfig: {
CONNECTION_CHECK_TTL: 0,
USER_CONNECTION_IDLE_TIMEOUT: 30 * 60 * 1000,
TOOLS_LIST_MAX_PAGES: 50,
},
}));

const SERVER_NAME = 'sdk-oauth-server';
Expand Down
183 changes: 183 additions & 0 deletions packages/api/src/mcp/__tests__/MCPConnectionFetchTools.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,183 @@
/**
* Unit tests for MCPConnection.fetchTools pagination.
*
* MCP `tools/list` is a paginated method: the server may return a page of tools
* plus a `nextCursor` that the client must follow to retrieve the rest. These
* tests verify that fetchTools walks every page, passes the cursor back
* unchanged, and is bounded against misbehaving servers (page cap + repeated
* cursor guard) while preserving the original single-page and error behavior.
*/

import { logger } from '@librechat/data-schemas';
import { MCPConnection } from '~/mcp/connection';

jest.mock('@librechat/data-schemas', () => ({
logger: {
info: jest.fn(),
warn: jest.fn(),
error: jest.fn(),
debug: jest.fn(),
},
}));

jest.mock('~/auth', () => ({
createSSRFSafeUndiciConnect: jest.fn(() => undefined),
isOAuthUrlAllowed: jest.fn(() => false),
isSSRFTarget: jest.fn(() => false),
resolveHostnameSSRF: jest.fn(async () => false),
}));

/** Pin the page cap to a small value so the cap path is cheap to exercise. */
jest.mock('~/mcp/mcpConfig', () => ({
mcpConfig: { TOOLS_LIST_MAX_PAGES: 3, CONNECTION_CHECK_TTL: 0 },
}));

const mockLogger = logger as jest.Mocked<typeof logger>;

const makeTool = (name: string) => ({
name,
description: `${name} description`,
inputSchema: { type: 'object' as const, properties: {} },
});

/** Build a bare MCPConnection (no real transport) with an injected, controllable client. */
function createConnectionWithListTools(listTools: jest.Mock): MCPConnection {
const conn = new MCPConnection({
serverName: 'pagination-test',
serverConfig: { type: 'streamable-http', url: 'http://localhost/mcp' },
useSSRFProtection: false,
});
conn.client = { listTools } as unknown as MCPConnection['client'];
return conn;
}

describe('MCPConnection.fetchTools pagination', () => {
beforeEach(() => {
jest.clearAllMocks();
});

it('returns the tools from a single page and makes one request when there is no nextCursor', async () => {
const listTools = jest.fn().mockResolvedValue({ tools: [makeTool('a'), makeTool('b')] });
const conn = createConnectionWithListTools(listTools);

const tools = await conn.fetchTools();

expect(tools.map((t) => t.name)).toEqual(['a', 'b']);
expect(listTools).toHaveBeenCalledTimes(1);
expect(listTools).toHaveBeenNthCalledWith(1, undefined);
expect(mockLogger.warn).not.toHaveBeenCalled();
});

it('follows nextCursor across pages, concatenating every tool and passing the cursor back', async () => {
const listTools = jest.fn(async (params?: { cursor?: string }) => {
switch (params?.cursor) {
case undefined:
return { tools: [makeTool('a'), makeTool('b')], nextCursor: 'c1' };
case 'c1':
return { tools: [makeTool('c'), makeTool('d')], nextCursor: 'c2' };
case 'c2':
return { tools: [makeTool('e')] };
default:
throw new Error(`unexpected cursor: ${params?.cursor}`);
}
});
const conn = createConnectionWithListTools(listTools);

const tools = await conn.fetchTools();

expect(tools.map((t) => t.name)).toEqual(['a', 'b', 'c', 'd', 'e']);
expect(listTools).toHaveBeenCalledTimes(3);
expect(listTools).toHaveBeenNthCalledWith(1, undefined);
expect(listTools).toHaveBeenNthCalledWith(2, { cursor: 'c1' });
expect(listTools).toHaveBeenNthCalledWith(3, { cursor: 'c2' });
expect(mockLogger.warn).not.toHaveBeenCalled();
});

it('stops at the page cap and warns when a server keeps returning new cursors', async () => {
let page = 0;
const listTools = jest.fn(async () => {
page += 1;
return { tools: [makeTool(`t${page}`)], nextCursor: `cursor-${page}` };
});
const conn = createConnectionWithListTools(listTools);

const tools = await conn.fetchTools();

// mcpConfig.TOOLS_LIST_MAX_PAGES is mocked to 3.
expect(listTools).toHaveBeenCalledTimes(3);
expect(tools.map((t) => t.name)).toEqual(['t1', 't2', 't3']);
expect(mockLogger.warn).toHaveBeenCalledWith(expect.stringContaining('pagination limit'));
});

it('stops and warns when the server repeats a cursor instead of looping forever', async () => {
const listTools = jest.fn().mockResolvedValue({ tools: [makeTool('x')], nextCursor: 'same' });
const conn = createConnectionWithListTools(listTools);

const tools = await conn.fetchTools();

expect(listTools).toHaveBeenCalledTimes(2);
// The second page's tools are collected before the repeated cursor is detected, hence two copies.
expect(tools.map((t) => t.name)).toEqual(['x', 'x']);
expect(mockLogger.warn).toHaveBeenCalledWith(
expect.stringContaining('repeated tools/list cursor'),
);
});

it('continues paginating across an empty intermediate page', async () => {
const listTools = jest.fn(async (params?: { cursor?: string }) => {
if (params?.cursor == null) {
return { tools: [], nextCursor: 'c1' };
}
return { tools: [makeTool('a')] };
});
const conn = createConnectionWithListTools(listTools);

const tools = await conn.fetchTools();

expect(tools.map((t) => t.name)).toEqual(['a']);
expect(listTools).toHaveBeenCalledTimes(2);
});

it('treats an empty-string nextCursor as a valid cursor, not end-of-list', async () => {
const listTools = jest.fn(async (params?: { cursor?: string }) => {
if (params?.cursor == null) {
return { tools: [makeTool('a')], nextCursor: '' };
}
return { tools: [makeTool('b')] };
});
const conn = createConnectionWithListTools(listTools);

const tools = await conn.fetchTools();

expect(tools.map((t) => t.name)).toEqual(['a', 'b']);
expect(listTools).toHaveBeenCalledTimes(2);
expect(listTools).toHaveBeenNthCalledWith(2, { cursor: '' });
});

it('returns the pages already fetched when a later page fails, without throwing', async () => {
const listTools = jest.fn(async (params?: { cursor?: string }) => {
if (params?.cursor == null) {
return { tools: [makeTool('a'), makeTool('b')], nextCursor: 'c1' };
}
throw new Error('page 2 boom');
});
const conn = createConnectionWithListTools(listTools);

const tools = await conn.fetchTools();

expect(tools.map((t) => t.name)).toEqual(['a', 'b']);
expect(listTools).toHaveBeenCalledTimes(2);
expect(mockLogger.error).toHaveBeenCalledWith(expect.stringContaining('Failed to fetch tools'));
});

it('returns an empty array when the first page request rejects', async () => {
const listTools = jest.fn().mockRejectedValue(new Error('boom'));
const conn = createConnectionWithListTools(listTools);

const tools = await conn.fetchTools();

expect(tools).toEqual([]);
expect(listTools).toHaveBeenCalledTimes(1);
expect(mockLogger.error).toHaveBeenCalledWith(expect.stringContaining('Failed to fetch tools'));
});
});
2 changes: 1 addition & 1 deletion packages/api/src/mcp/__tests__/MCPConnectionSSRF.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ jest.mock('~/auth', () => ({
}));

jest.mock('~/mcp/mcpConfig', () => ({
mcpConfig: { CONNECTION_CHECK_TTL: 0 },
mcpConfig: { CONNECTION_CHECK_TTL: 0, TOOLS_LIST_MAX_PAGES: 50 },
}));

const mockedResolveHostnameSSRF = resolveHostnameSSRF as jest.MockedFunction<
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ jest.mock('~/auth', () => ({
}));

jest.mock('~/mcp/mcpConfig', () => ({
mcpConfig: { CONNECTION_CHECK_TTL: 0 },
mcpConfig: { CONNECTION_CHECK_TTL: 0, TOOLS_LIST_MAX_PAGES: 50 },
}));

/** Track all Agents for cleanup */
Expand Down
99 changes: 52 additions & 47 deletions packages/api/src/mcp/connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1095,6 +1095,9 @@ interface MCPConnectionParams {
ephemeralConnection?: boolean;
}

/** Result of an MCP `tools/list` request: one page of tools plus an optional pagination cursor. */
type MCPListToolsResult = Awaited<ReturnType<Client['listTools']>>;

export class MCPConnection extends EventEmitter {
public client: Client;
private options: t.MCPOptions;
Expand Down Expand Up @@ -2191,56 +2194,58 @@ export class MCPConnection extends EventEmitter {
}
}

async fetchTools(): Promise<
{
inputSchema: {
[x: string]: unknown;
type: 'object';
properties?: Record<string, object> | undefined;
required?: string[] | undefined;
};
name: string;
description?: string | undefined;
outputSchema?:
| {
[x: string]: unknown;
type: 'object';
properties?: Record<string, object> | undefined;
required?: string[] | undefined;
}
| undefined;
annotations?:
| {
title?: string | undefined;
readOnlyHint?: boolean | undefined;
destructiveHint?: boolean | undefined;
idempotentHint?: boolean | undefined;
openWorldHint?: boolean | undefined;
}
| undefined;
execution?:
| {
taskSupport?: 'optional' | 'required' | 'forbidden' | undefined;
}
| undefined;
_meta?: Record<string, unknown> | undefined;
icons?:
| {
src: string;
mimeType?: string | undefined;
sizes?: string[] | undefined;
theme?: 'light' | 'dark' | undefined;
}[]
| undefined;
title?: string | undefined;
}[]
> {
/**
* Fetches the server's tools, following MCP `tools/list` cursor pagination so a
* server that spans multiple pages (e.g. an aggregating gateway exposing many
* tools) is loaded in full instead of being truncated to the first page.
*
* Pagination is bounded by {@link mcpConfig.TOOLS_LIST_MAX_PAGES} and a
* repeated-cursor guard. On error, the tools already fetched are returned rather
* than discarded, and the method never throws.
*/
async fetchTools(): Promise<MCPListToolsResult['tools']> {
const maxPages = mcpConfig.TOOLS_LIST_MAX_PAGES;
const allTools: MCPListToolsResult['tools'] = [];
const seenCursors = new Set<string>();
let cursor: string | undefined;

for (let page = 1; page <= maxPages; page++) {
const result = await this.listToolsPage(cursor);
if (result == null) {
/** Request failed mid-pagination: return the pages already fetched instead of discarding them. */
return allTools;
}

allTools.push(...result.tools);

const { nextCursor } = result;
if (nextCursor == null) {
return allTools;
}
if (seenCursors.has(nextCursor)) {
logger.warn(
`${this.getLogPrefix()} MCP server returned a repeated tools/list cursor; stopping pagination after ${page} page(s).`,
);
return allTools;
}

seenCursors.add(nextCursor);
cursor = nextCursor;
}

logger.warn(
`${this.getLogPrefix()} Reached the tools/list pagination limit of ${maxPages} page(s); some tools may be omitted. Set MCP_TOOLS_LIST_MAX_PAGES higher if this server legitimately exposes more.`,
);
return allTools;
}

/** Fetches a single `tools/list` page, returning null (and logging) on failure so pagination can stop gracefully. */
private async listToolsPage(cursor: string | undefined): Promise<MCPListToolsResult | null> {
try {
const { tools } = await this.client.listTools();
return tools;
return await this.client.listTools(cursor != null ? { cursor } : undefined);
} catch (error) {
this.emitError(error, 'Failed to fetch tools');
return [];
return null;
}
}

Expand Down
5 changes: 5 additions & 0 deletions packages/api/src/mcp/mcpConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ export const mcpConfig: {
/** TTL (ms) for OAuth flow state. Must outlive OAUTH_HANDLING_TIMEOUT so the state survives the wait. Default: 15 minutes */
OAUTH_FLOW_TTL: number;
CONNECTION_CHECK_TTL: number;
/** Max number of `tools/list` pages to request when an MCP server paginates its tool list.
* Bounds the pagination loop so a misbehaving server cannot stall tool discovery. Default: 50 */
TOOLS_LIST_MAX_PAGES: number;
/** Idle timeout (ms) after which user connections are disconnected. Default: 15 minutes */
USER_CONNECTION_IDLE_TIMEOUT: number;
/** Max connect/disconnect cycles before the circuit breaker trips. Default: 7 */
Expand All @@ -47,6 +50,8 @@ export const mcpConfig: {
/** TTL (ms) for OAuth flow state. Clamped to never fall below OAUTH_HANDLING_TIMEOUT. Default: 15 minutes */
OAUTH_FLOW_TTL: oauthFlowTtl,
CONNECTION_CHECK_TTL: math(process.env.MCP_CONNECTION_CHECK_TTL ?? 60000),
/** Max number of `tools/list` pages to request when an MCP server paginates its tool list. Clamped to >= 1. Default: 50 */
TOOLS_LIST_MAX_PAGES: Math.max(1, math(process.env.MCP_TOOLS_LIST_MAX_PAGES ?? 50)),
/** Idle timeout (ms) after which user connections are disconnected. Default: 15 minutes */
USER_CONNECTION_IDLE_TIMEOUT: math(
process.env.MCP_USER_CONNECTION_IDLE_TIMEOUT ?? 15 * 60 * 1000,
Expand Down
6 changes: 3 additions & 3 deletions packages/api/src/mcp/registry/MCPServerInspector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,8 @@ export class MCPServerInspector {
private async fetchServerCapabilities(): Promise<void> {
const capabilities = this.connection!.client.getServerCapabilities();
this.config.capabilities = JSON.stringify(capabilities);
const tools = await this.connection!.client.listTools();
this.config.tools = tools.tools.map((tool) => tool.name).join(', ');
const tools = await this.connection!.fetchTools();
this.config.tools = tools.map((tool) => tool.name).join(', ');
}

private async fetchToolFunctions(): Promise<void> {
Expand All @@ -171,7 +171,7 @@ export class MCPServerInspector {
serverName: string,
connection: MCPConnection,
): Promise<t.LCAvailableTools> {
const { tools }: t.MCPToolListResponse = await connection.client.listTools();
const tools = await connection.fetchTools();

const toolFunctions: t.LCAvailableTools = {};
tools.forEach((tool) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ jest.mock('~/cluster', () => ({
}));

jest.mock('~/mcp/mcpConfig', () => ({
mcpConfig: { CONNECTION_CHECK_TTL: 0 },
mcpConfig: { CONNECTION_CHECK_TTL: 0, TOOLS_LIST_MAX_PAGES: 50 },
}));

jest.mock('~/mcp/registry/db/ServerConfigsDB', () => ({
Expand Down
Loading
Loading