diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index 670211af74..5b7c1e25ee 100755 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -1951,6 +1951,7 @@ export async function loadCliConfig( bareMode || safeMode ? undefined : settings.tools?.callCommand, mcpServerCommand: bareMode || safeMode ? undefined : settings.mcp?.serverCommand, + mcpToolIdleTimeoutMs: settings.mcp?.toolIdleTimeoutMs, mcpServers, topTierMcpServers, pendingMcpServers, diff --git a/packages/cli/src/config/settingsSchema.ts b/packages/cli/src/config/settingsSchema.ts index 288627e57f..b742c5e0b3 100644 --- a/packages/cli/src/config/settingsSchema.ts +++ b/packages/cli/src/config/settingsSchema.ts @@ -2458,6 +2458,18 @@ const SETTINGS_SCHEMA = { showInDialog: false, mergeStrategy: MergeStrategy.CONCAT, }, + toolIdleTimeoutMs: { + type: 'number', + label: 'MCP Tool Idle Timeout (ms)', + category: 'MCP', + requiresRestart: false, + default: 300000, + minimum: 10000, + maximum: 3600000, + description: + 'Idle timeout in milliseconds for MCP tool calls. If the MCP server does not produce any response or progress update within this time, the call is aborted. Default: 300000 (5 minutes). Can be overridden via QWEN_CODE_MCP_TOOL_IDLE_TIMEOUT_MS environment variable.', + showInDialog: false, + }, }, }, security: { diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 870570ff33..3a5ef0c86e 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -962,6 +962,13 @@ export interface ConfigParameters { */ cliAllowedMcpServerNames?: string[]; excludedMcpServers?: string[]; + /** + * Idle timeout in milliseconds for MCP tool calls. If the MCP server does + * not produce any response or progress update within this time, the call + * is aborted. Default: 300000 (5 minutes). Can be overridden via + * QWEN_CODE_MCP_TOOL_IDLE_TIMEOUT_MS environment variable. + */ + mcpToolIdleTimeoutMs?: number; /** * Names of project-scoped (`.mcp.json`) servers that are NOT yet approved * (pending or rejected). These are loaded so they can be listed, but the @@ -1392,6 +1399,7 @@ export class Config { private readonly cliAllowedMcpServerNames?: string[]; private excludedMcpServers?: string[]; private pendingMcpServers?: string[]; + private readonly mcpToolIdleTimeoutMs: number; /** * Guards against concurrent MCP reconcile passes (hot-reload watcher vs. * `/reload`). `SettingsWatcher` serializes its own listeners, but `/reload` @@ -1617,6 +1625,11 @@ export class Config { this.cliAllowedMcpServerNames = params.cliAllowedMcpServerNames; this.excludedMcpServers = params.excludedMcpServers; this.pendingMcpServers = params.pendingMcpServers; + const envTimeout = process.env['QWEN_CODE_MCP_TOOL_IDLE_TIMEOUT_MS']; + const parsedEnv = envTimeout !== undefined ? Number(envTimeout) : NaN; + this.mcpToolIdleTimeoutMs = + params.mcpToolIdleTimeoutMs ?? + (Number.isFinite(parsedEnv) && parsedEnv >= 0 ? parsedEnv : 300000); // 5 minutes default this.sessionSubagents = params.sessionSubagents ?? []; this.sdkMode = params.sdkMode ?? false; this.userMemory = params.userMemory ?? ''; @@ -3830,6 +3843,10 @@ export class Config { this.excludedMcpServers = excluded; } + getMcpToolIdleTimeoutMs(): number { + return this.mcpToolIdleTimeoutMs; + } + isMcpServerDisabled(serverName: string): boolean { if (matchesAnyServerPattern(serverName, this.excludedMcpServers)) return true; diff --git a/packages/core/src/tools/mcp-client.ts b/packages/core/src/tools/mcp-client.ts index ef05d2aa86..bad571d1ea 100644 --- a/packages/core/src/tools/mcp-client.ts +++ b/packages/core/src/tools/mcp-client.ts @@ -1031,6 +1031,7 @@ export async function discoverTools( cliConfig, mcpClient, // raw MCP Client for direct callTool with progress mcpTimeout, + cliConfig?.getMcpToolIdleTimeoutMs(), annotationsMap.get(funcDecl.name!), ), ); @@ -1725,7 +1726,8 @@ export async function createTransport( ) { const provider = new ServiceAccountImpersonationProvider(mcpServerConfig); const transportOptions: - StreamableHTTPClientTransportOptions | SSEClientTransportOptions = { + | StreamableHTTPClientTransportOptions + | SSEClientTransportOptions = { authProvider: provider, }; @@ -1753,7 +1755,8 @@ export async function createTransport( ) { const provider = new GoogleCredentialProvider(mcpServerConfig); const transportOptions: - StreamableHTTPClientTransportOptions | SSEClientTransportOptions = { + | StreamableHTTPClientTransportOptions + | SSEClientTransportOptions = { authProvider: provider, }; if (mcpServerConfig.httpUrl) { diff --git a/packages/core/src/tools/mcp-tool.test.ts b/packages/core/src/tools/mcp-tool.test.ts index f162cb0aa8..a695654a24 100644 --- a/packages/core/src/tools/mcp-tool.test.ts +++ b/packages/core/src/tools/mcp-tool.test.ts @@ -1610,4 +1610,150 @@ describe('DiscoveredMCPTool', () => { expect(discoverToolsForServer).toHaveBeenCalled(); }); }); + + describe('MCP Tool Idle Timeout', () => { + it('should abort when MCP server does not respond within idle timeout', async () => { + vi.useFakeTimers(); + + const idleTimeoutMs = 1000; // 1 second for testing + const mockMcpClient: McpDirectClient = { + callTool: vi.fn().mockImplementation( + (_params, _schema, options) => + new Promise((_resolve, reject) => { + // Simulate SDK behavior: reject when signal is aborted + options?.signal?.addEventListener('abort', () => { + const error = new Error( + (options?.signal as AbortSignal & { reason?: Error })?.reason + ?.message ?? 'The operation was aborted', + ); + error.name = 'AbortError'; + reject(error); + }); + }), + ), + }; + + const tool = new DiscoveredMCPTool( + mockCallableToolInstance, + serverName, + serverToolName, + baseDescription, + inputSchema, + true, + undefined, + undefined, + mockMcpClient, + undefined, + idleTimeoutMs, + ); + + const invocation = tool.build({ param: 'test' }); + const abortController = new AbortController(); + const executePromise = invocation.execute(abortController.signal); + + // Advance time to trigger the idle timeout + vi.advanceTimersByTime(idleTimeoutMs + 100); + + await expect(executePromise).rejects.toThrow( + /did not respond within.*idle timeout/, + ); + // The external abort signal should not have been triggered + expect(abortController.signal.aborted).toBe(false); + + vi.useRealTimers(); + }); + + it('should reset idle timeout on progress updates', async () => { + vi.useFakeTimers(); + + const idleTimeoutMs = 1000; + let onProgressCallback: ((progress: any) => void) | undefined; + + const mockMcpClient: McpDirectClient = { + callTool: vi.fn().mockImplementation((_params, _schema, options) => { + onProgressCallback = options?.onprogress; + return new Promise((resolve, reject) => { + // Listen for abort signal to properly reject when timeout fires + options?.signal?.addEventListener('abort', () => { + reject(options.signal!.reason); + }); + // Resolve after 2.5 seconds (would timeout without progress) + setTimeout(() => { + resolve({ content: [{ type: 'text', text: 'Success' }] }); + }, 2500); + }); + }), + }; + + const tool = new DiscoveredMCPTool( + mockCallableToolInstance, + serverName, + serverToolName, + baseDescription, + inputSchema, + true, + undefined, + undefined, + mockMcpClient, + undefined, + idleTimeoutMs, + ); + + const invocation = tool.build({ param: 'test' }); + const executePromise = invocation.execute(new AbortController().signal); + + // Send progress at 500ms, 1400ms, 2300ms to reset the timeout + // Each progress must arrive BEFORE the 1000ms idle timeout fires + vi.advanceTimersByTime(500); + onProgressCallback?.({ progress: 0.25 }); + + vi.advanceTimersByTime(900); + onProgressCallback?.({ progress: 0.5 }); + + vi.advanceTimersByTime(900); + onProgressCallback?.({ progress: 0.75 }); + + // Advance past the mock's 2500ms resolve time + vi.advanceTimersByTime(200); + + const result = await executePromise; + + expect(result.error).toBeUndefined(); + expect(result.llmContent).toBeDefined(); + + vi.useRealTimers(); + }); + + it('should not apply idle timeout when set to 0 or undefined', async () => { + vi.useFakeTimers(); + + const mockMcpClient: McpDirectClient = { + callTool: vi.fn().mockResolvedValue({ + content: [{ type: 'text', text: 'Success' }], + }), + }; + + const tool = new DiscoveredMCPTool( + mockCallableToolInstance, + serverName, + serverToolName, + baseDescription, + inputSchema, + true, + undefined, + undefined, + mockMcpClient, + undefined, + undefined, // No idle timeout + ); + + const invocation = tool.build({ param: 'test' }); + const result = await invocation.execute(new AbortController().signal); + + expect(result.error).toBeUndefined(); + expect(result.llmContent).toBeDefined(); + + vi.useRealTimers(); + }); + }); }); diff --git a/packages/core/src/tools/mcp-tool.ts b/packages/core/src/tools/mcp-tool.ts index 44796cfd86..2f5942def3 100644 --- a/packages/core/src/tools/mcp-tool.ts +++ b/packages/core/src/tools/mcp-tool.ts @@ -136,6 +136,7 @@ class DiscoveredMCPToolInvocation extends BaseToolInvocation< private readonly cliConfig?: Config, private readonly mcpClient?: McpDirectClient, private readonly mcpTimeout?: number, + private readonly mcpToolIdleTimeoutMs?: number, private readonly annotations?: McpToolAnnotations, private readonly retryCount: number = 0, ) { @@ -264,6 +265,7 @@ class DiscoveredMCPToolInvocation extends BaseToolInvocation< this.cliConfig, newTool['mcpClient'], this.mcpTimeout, + this.mcpToolIdleTimeoutMs, this.annotations, this.retryCount + 1, ); @@ -317,7 +319,38 @@ class DiscoveredMCPToolInvocation extends BaseToolInvocation< signal: AbortSignal, updateOutput?: (output: ToolResultDisplay) => void, ): Promise { + // Create an AbortController for idle timeout + const idleTimeoutController = new AbortController(); + let idleTimeoutId: ReturnType | undefined; + + // Combine the external signal with our idle timeout controller + const combinedSignal = AbortSignal.any([ + signal, + idleTimeoutController.signal, + ]); + + const resetIdleTimeout = () => { + if (idleTimeoutId) { + clearTimeout(idleTimeoutId); + } + if (this.mcpToolIdleTimeoutMs && this.mcpToolIdleTimeoutMs > 0) { + const timer = setTimeout(() => { + const error = new Error( + `MCP tool '${this.serverToolName}' on server '${this.serverName}' ` + + `did not respond within ${this.mcpToolIdleTimeoutMs}ms idle timeout`, + ); + error.name = 'AbortError'; + idleTimeoutController.abort(error); + }, this.mcpToolIdleTimeoutMs); + timer.unref(); + idleTimeoutId = timer; + } + }; + try { + // Start the idle timeout + resetIdleTimeout(); + const callToolResult = await this.mcpClient!.callTool( { name: this.serverToolName, @@ -326,6 +359,9 @@ class DiscoveredMCPToolInvocation extends BaseToolInvocation< undefined, { onprogress: (progress) => { + // Reset idle timeout on progress + resetIdleTimeout(); + if (updateOutput) { const progressData: McpToolProgressData = { type: 'mcp_tool_progress', @@ -337,7 +373,7 @@ class DiscoveredMCPToolInvocation extends BaseToolInvocation< } }, timeout: this.mcpTimeout, - signal, + signal: combinedSignal, }, ); @@ -374,6 +410,11 @@ class DiscoveredMCPToolInvocation extends BaseToolInvocation< }; } catch (error) { return this.handleReconnectOnError(error, signal, updateOutput); + } finally { + // Clear the idle timeout in all cases + if (idleTimeoutId) { + clearTimeout(idleTimeoutId); + } } } @@ -510,6 +551,7 @@ export class DiscoveredMCPTool extends BaseDeclarativeTool< private readonly cliConfig?: Config, private readonly mcpClient?: McpDirectClient, private readonly mcpTimeout?: number, + private readonly mcpToolIdleTimeoutMs?: number, readonly annotations?: McpToolAnnotations, ) { super( @@ -542,6 +584,7 @@ export class DiscoveredMCPTool extends BaseDeclarativeTool< this.cliConfig, this.mcpClient, this.mcpTimeout, + this.mcpToolIdleTimeoutMs, this.annotations, ); } @@ -577,6 +620,7 @@ export class DiscoveredMCPTool extends BaseDeclarativeTool< this.cliConfig, this.mcpClient, this.mcpTimeout, + this.mcpToolIdleTimeoutMs, this.annotations, ); } @@ -594,6 +638,7 @@ export class DiscoveredMCPTool extends BaseDeclarativeTool< this.cliConfig, this.mcpClient, this.mcpTimeout, + this.mcpToolIdleTimeoutMs, this.annotations, ); }