diff --git a/src/client.ts b/src/client.ts index 50731cf..81310b7 100644 --- a/src/client.ts +++ b/src/client.ts @@ -329,15 +329,38 @@ function createStdioTransport(config: StdioServerConfig): StdioClientTransport { /** * List all tools from a connected client with retry logic + * Follows MCP pagination cursors until all tool pages are fetched. */ export async function listTools(client: Client): Promise { return withRetry(async () => { - const result = await client.listTools(); - return result.tools.map((tool: Tool) => ({ - name: tool.name, - description: tool.description, - inputSchema: tool.inputSchema as Record, - })); + const tools: ToolInfo[] = []; + const seenCursors = new Set(); + let cursor: string | undefined; + + while (true) { + const result = await client.listTools(cursor ? { cursor } : undefined); + + tools.push( + ...result.tools.map((tool: Tool) => ({ + name: tool.name, + description: tool.description, + inputSchema: tool.inputSchema as Record, + })), + ); + + if (!result.nextCursor) { + return tools; + } + + if (seenCursors.has(result.nextCursor)) { + throw new Error( + `listTools pagination loop detected at cursor: ${result.nextCursor}`, + ); + } + + seenCursors.add(result.nextCursor); + cursor = result.nextCursor; + } }, 'list tools'); } diff --git a/src/config.ts b/src/config.ts index 99a4e25..59e2366 100644 --- a/src/config.ts +++ b/src/config.ts @@ -496,9 +496,6 @@ export async function loadConfig( } } - // Substitute environment variables - config = substituteEnvVarsInObject(config); - return config; } @@ -514,7 +511,7 @@ export function getServerConfig( const available = Object.keys(config.mcpServers); throw new Error(formatCliError(serverNotFoundError(serverName, available))); } - return server; + return substituteEnvVarsInObject(server); } /** diff --git a/tests/client.test.ts b/tests/client.test.ts index 9c53200..643c000 100644 --- a/tests/client.test.ts +++ b/tests/client.test.ts @@ -15,6 +15,7 @@ import { isTransientError, getTimeoutMs, getConcurrencyLimit, + listTools, } from '../src/client'; describe('client', () => { @@ -240,6 +241,61 @@ describe('client', () => { }); }); + describe('listTools', () => { + test('collects all tool pages when server returns nextCursor', async () => { + const calls: Array = []; + const client = { + listTools: async (params?: { cursor?: string }) => { + calls.push(params?.cursor); + + if (!params?.cursor) { + return { + tools: [ + { + name: 'tool-a', + inputSchema: { type: 'object' }, + }, + ], + nextCursor: 'page-2', + }; + } + + return { + tools: [ + { + name: 'tool-b', + inputSchema: { type: 'object' }, + }, + ], + }; + }, + } as any; + + const tools = await listTools(client); + + expect(calls).toEqual([undefined, 'page-2']); + expect(tools.map((tool) => tool.name)).toEqual(['tool-a', 'tool-b']); + }); + + test('throws when paginated listTools repeats the same cursor', async () => { + const client = { + listTools: async () => ({ + tools: [ + { + name: 'tool-a', + inputSchema: { type: 'object' }, + }, + ], + nextCursor: 'stuck', + }), + } as any; + + await expect(listTools(client)).rejects.toThrow( + 'pagination loop detected', + ); + }); + }); + // Note: Actually testing connectToServer requires a real MCP server // Those tests are in the integration test suite }); diff --git a/tests/config.test.ts b/tests/config.test.ts index a48602c..0165f0d 100644 --- a/tests/config.test.ts +++ b/tests/config.test.ts @@ -61,7 +61,7 @@ describe('config', () => { await expect(loadConfig(configPath)).rejects.toThrow('mcpServers'); }); - test('substitutes environment variables', async () => { + test('loads config without eagerly substituting environment variables', async () => { process.env.TEST_MCP_TOKEN = 'secret123'; const configPath = join(tempDir, 'env_config.json'); @@ -79,11 +79,35 @@ describe('config', () => { const config = await loadConfig(configPath); const server = config.mcpServers.test as any; - expect(server.headers.Authorization).toBe('Bearer secret123'); + expect(server.headers.Authorization).toBe('Bearer ${TEST_MCP_TOKEN}'); delete process.env.TEST_MCP_TOKEN; }); + test('does not throw for unrelated servers with missing env vars', async () => { + delete process.env.UNRELATED_MISSING_VAR; + + const configPath = join(tempDir, 'scoped_env_config.json'); + await writeFile( + configPath, + JSON.stringify({ + mcpServers: { + safe: { + command: 'echo', + }, + noisy: { + command: 'echo', + env: { TOKEN: '${UNRELATED_MISSING_VAR}' }, + }, + }, + }) + ); + + const config = await loadConfig(configPath); + expect(() => getServerConfig(config, 'safe')).not.toThrow(); + expect(() => getServerConfig(config, 'noisy')).toThrow('MISSING_ENV_VAR'); + }); + test('handles missing env vars gracefully with MCP_STRICT_ENV=false', async () => { // Set non-strict mode to allow missing env vars with warning process.env.MCP_STRICT_ENV = 'false'; @@ -102,12 +126,35 @@ describe('config', () => { ); const config = await loadConfig(configPath); - const server = config.mcpServers.test as any; + const server = getServerConfig(config, 'test') as any; expect(server.env.TOKEN).toBe(''); delete process.env.MCP_STRICT_ENV; }); + + test('treats MCP_STRICT_ENV with surrounding-CRLF trailing-tab true as strict mode', async () => { + process.env.MCP_STRICT_ENV = '\r\ntrue\t\r\n'; + + const configPath = join(tempDir, 'missing_env_trailing_tab_true_surrounded_crlf.json'); + await writeFile( + configPath, + JSON.stringify({ + mcpServers: { + test: { + command: 'echo', + env: { TOKEN: '${NONEXISTENT_VAR}' }, + }, + }, + }) + ); + + const config = await loadConfig(configPath); + expect(() => getServerConfig(config, 'test')).toThrow('Missing environment variable'); + + delete process.env.MCP_STRICT_ENV; + }); + test('throws error on missing env vars in strict mode (default)', async () => { // Ensure strict mode is enabled (default) delete process.env.MCP_STRICT_ENV; @@ -125,7 +172,8 @@ describe('config', () => { }) ); - await expect(loadConfig(configPath)).rejects.toThrow('MISSING_ENV_VAR'); + const config = await loadConfig(configPath); + expect(() => getServerConfig(config, 'test')).toThrow('MISSING_ENV_VAR'); }); test('throws error on empty server config', async () => {