diff --git a/.changeset/flat-items-buy.md b/.changeset/flat-items-buy.md new file mode 100644 index 0000000..f36bb1d --- /dev/null +++ b/.changeset/flat-items-buy.md @@ -0,0 +1,5 @@ +--- +"roo-cline": patch +--- + +Add per-server MCP network timeout configuration diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 45368e0..808a50e 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -1177,6 +1177,16 @@ export class ClineProvider implements vscode.WebviewViewProvider { } await this.postStateToWebview() break + case "updateMcpTimeout": + if (message.serverName && typeof message.timeout === "number") { + try { + await this.mcpHub?.updateServerTimeout(message.serverName, message.timeout) + } catch (error) { + console.error(`Failed to update timeout for ${message.serverName}:`, error) + vscode.window.showErrorMessage(`Failed to update server timeout`) + } + } + break case "updateCustomMode": if (message.modeConfig) { await this.customModesManager.updateCustomMode(message.modeConfig.slug, message.modeConfig) diff --git a/src/services/mcp/McpHub.ts b/src/services/mcp/McpHub.ts index 6dd5dd4..dcbe4a9 100644 --- a/src/services/mcp/McpHub.ts +++ b/src/services/mcp/McpHub.ts @@ -35,12 +35,13 @@ export type McpConnection = { // StdioServerParameters const AlwaysAllowSchema = z.array(z.string()).default([]) -const StdioConfigSchema = z.object({ +export const StdioConfigSchema = z.object({ command: z.string(), args: z.array(z.string()).optional(), env: z.record(z.string()).optional(), alwaysAllow: AlwaysAllowSchema.optional(), disabled: z.boolean().optional(), + timeout: z.number().min(1).max(3600).optional().default(60), }) const McpSettingsSchema = z.object({ @@ -238,28 +239,6 @@ export class McpHub { } transport.start = async () => {} // No-op now, .connect() won't fail - // // Set up notification handlers - // client.setNotificationHandler( - // // @ts-ignore-next-line - // { method: "notifications/tools/list_changed" }, - // async () => { - // console.log(`Tools changed for server: ${name}`) - // connection.server.tools = await this.fetchTools(name) - // await this.notifyWebviewOfServerChanges() - // }, - // ) - - // client.setNotificationHandler( - // // @ts-ignore-next-line - // { method: "notifications/resources/list_changed" }, - // async () => { - // console.log(`Resources changed for server: ${name}`) - // connection.server.resources = await this.fetchResources(name) - // connection.server.resourceTemplates = await this.fetchResourceTemplates(name) - // await this.notifyWebviewOfServerChanges() - // }, - // ) - // Connect await client.connect(transport) connection.server.status = "connected" @@ -339,10 +318,6 @@ export class McpHub { const connection = this.connections.find((conn) => conn.server.name === name) if (connection) { try { - // connection.client.removeNotificationHandler("notifications/tools/list_changed") - // connection.client.removeNotificationHandler("notifications/resources/list_changed") - // connection.client.removeNotificationHandler("notifications/stderr") - // connection.client.removeNotificationHandler("notifications/stderr") await connection.transport.close() await connection.client.close() } catch (error) { @@ -468,8 +443,6 @@ export class McpHub { }) } - // Public methods for server management - public async toggleServerDisabled(serverName: string, disabled: boolean): Promise { let settingsPath: string try { @@ -545,6 +518,59 @@ export class McpHub { } } + public async updateServerTimeout(serverName: string, timeout: number): Promise { + let settingsPath: string + try { + settingsPath = await this.getMcpSettingsFilePath() + + // Ensure the settings file exists and is accessible + try { + await fs.access(settingsPath) + } catch (error) { + console.error("Settings file not accessible:", error) + throw new Error("Settings file not accessible") + } + const content = await fs.readFile(settingsPath, "utf-8") + const config = JSON.parse(content) + + // Validate the config structure + if (!config || typeof config !== "object") { + throw new Error("Invalid config structure") + } + + if (!config.mcpServers || typeof config.mcpServers !== "object") { + config.mcpServers = {} + } + + if (config.mcpServers[serverName]) { + // Create a new server config object to ensure clean structure + const serverConfig = { + ...config.mcpServers[serverName], + timeout, + } + + config.mcpServers[serverName] = serverConfig + + // Write the entire config back + const updatedConfig = { + mcpServers: config.mcpServers, + } + + await fs.writeFile(settingsPath, JSON.stringify(updatedConfig, null, 2)) + await this.notifyWebviewOfServerChanges() + } + } catch (error) { + console.error("Failed to update server timeout:", error) + if (error instanceof Error) { + console.error("Error details:", error.message, error.stack) + } + vscode.window.showErrorMessage( + `Failed to update server timeout: ${error instanceof Error ? error.message : String(error)}`, + ) + throw error + } + } + async readResource(serverName: string, uri: string): Promise { const connection = this.connections.find((conn) => conn.server.name === serverName) if (!connection) { @@ -579,6 +605,16 @@ export class McpHub { throw new Error(`Server "${serverName}" is disabled and cannot be used`) } + let timeout: number + try { + const parsedConfig = StdioConfigSchema.parse(JSON.parse(connection.server.config)) + timeout = (parsedConfig.timeout ?? 60) * 1000 + } catch (error) { + console.error("Failed to parse server config for timeout:", error) + // Default to 60 seconds if parsing fails + timeout = 60 * 1000 + } + return await connection.client.request( { method: "tools/call", @@ -588,6 +624,9 @@ export class McpHub { }, }, CallToolResultSchema, + { + timeout, + }, ) } diff --git a/src/services/mcp/__tests__/McpHub.test.ts b/src/services/mcp/__tests__/McpHub.test.ts index dd46183..b418bae 100644 --- a/src/services/mcp/__tests__/McpHub.test.ts +++ b/src/services/mcp/__tests__/McpHub.test.ts @@ -2,8 +2,8 @@ import type { McpHub as McpHubType } from "../McpHub" import type { ClineProvider } from "../../../core/webview/ClineProvider" import type { ExtensionContext, Uri } from "vscode" import type { McpConnection } from "../McpHub" +import { StdioConfigSchema } from "../McpHub" -const vscode = require("vscode") const fs = require("fs/promises") const { McpHub } = require("../McpHub") @@ -280,6 +280,7 @@ describe("McpHub", () => { }, }, expect.any(Object), + expect.objectContaining({ timeout: 60000 }), // Default 60 second timeout ) }) @@ -288,5 +289,192 @@ describe("McpHub", () => { "No connection found for server: non-existent-server", ) }) + + describe("timeout configuration", () => { + it("should validate timeout values", () => { + // Test valid timeout values + const validConfig = { + command: "test", + timeout: 60, + } + expect(() => StdioConfigSchema.parse(validConfig)).not.toThrow() + + // Test invalid timeout values + const invalidConfigs = [ + { command: "test", timeout: 0 }, // Too low + { command: "test", timeout: 3601 }, // Too high + { command: "test", timeout: -1 }, // Negative + ] + + invalidConfigs.forEach((config) => { + expect(() => StdioConfigSchema.parse(config)).toThrow() + }) + }) + + it("should use default timeout of 60 seconds if not specified", async () => { + const mockConnection: McpConnection = { + server: { + name: "test-server", + config: JSON.stringify({ command: "test" }), // No timeout specified + status: "connected", + }, + client: { + request: jest.fn().mockResolvedValue({ content: [] }), + } as any, + transport: {} as any, + } + + mcpHub.connections = [mockConnection] + await mcpHub.callTool("test-server", "test-tool") + + expect(mockConnection.client.request).toHaveBeenCalledWith( + expect.anything(), + expect.anything(), + expect.objectContaining({ timeout: 60000 }), // 60 seconds in milliseconds + ) + }) + + it("should apply configured timeout to tool calls", async () => { + const mockConnection: McpConnection = { + server: { + name: "test-server", + config: JSON.stringify({ command: "test", timeout: 120 }), // 2 minutes + status: "connected", + }, + client: { + request: jest.fn().mockResolvedValue({ content: [] }), + } as any, + transport: {} as any, + } + + mcpHub.connections = [mockConnection] + await mcpHub.callTool("test-server", "test-tool") + + expect(mockConnection.client.request).toHaveBeenCalledWith( + expect.anything(), + expect.anything(), + expect.objectContaining({ timeout: 120000 }), // 120 seconds in milliseconds + ) + }) + }) + + describe("updateServerTimeout", () => { + it("should update server timeout in settings file", async () => { + const mockConfig = { + mcpServers: { + "test-server": { + command: "node", + args: ["test.js"], + timeout: 60, + }, + }, + } + + // Mock reading initial config + ;(fs.readFile as jest.Mock).mockResolvedValueOnce(JSON.stringify(mockConfig)) + + await mcpHub.updateServerTimeout("test-server", 120) + + // Verify the config was updated correctly + const writeCall = (fs.writeFile as jest.Mock).mock.calls[0] + const writtenConfig = JSON.parse(writeCall[1]) + expect(writtenConfig.mcpServers["test-server"].timeout).toBe(120) + }) + + it("should fallback to default timeout when config has invalid timeout", async () => { + const mockConfig = { + mcpServers: { + "test-server": { + command: "node", + args: ["test.js"], + timeout: 60, + }, + }, + } + + // Mock initial read + ;(fs.readFile as jest.Mock).mockResolvedValueOnce(JSON.stringify(mockConfig)) + + // Update with invalid timeout + await mcpHub.updateServerTimeout("test-server", 3601) + + // Config is written + expect(fs.writeFile).toHaveBeenCalled() + + // Setup connection with invalid timeout + const mockConnection: McpConnection = { + server: { + name: "test-server", + config: JSON.stringify({ + command: "node", + args: ["test.js"], + timeout: 3601, // Invalid timeout + }), + status: "connected", + }, + client: { + request: jest.fn().mockResolvedValue({ content: [] }), + } as any, + transport: {} as any, + } + + mcpHub.connections = [mockConnection] + + // Call tool - should use default timeout + await mcpHub.callTool("test-server", "test-tool") + + // Verify default timeout was used + expect(mockConnection.client.request).toHaveBeenCalledWith( + expect.anything(), + expect.anything(), + expect.objectContaining({ timeout: 60000 }), // Default 60 seconds + ) + }) + + it("should accept valid timeout values", async () => { + const mockConfig = { + mcpServers: { + "test-server": { + command: "node", + args: ["test.js"], + timeout: 60, + }, + }, + } + + ;(fs.readFile as jest.Mock).mockResolvedValueOnce(JSON.stringify(mockConfig)) + + // Test valid timeout values + const validTimeouts = [1, 60, 3600] + for (const timeout of validTimeouts) { + await mcpHub.updateServerTimeout("test-server", timeout) + expect(fs.writeFile).toHaveBeenCalled() + jest.clearAllMocks() // Reset for next iteration + ;(fs.readFile as jest.Mock).mockResolvedValueOnce(JSON.stringify(mockConfig)) + } + }) + + it("should notify webview after updating timeout", async () => { + const mockConfig = { + mcpServers: { + "test-server": { + command: "node", + args: ["test.js"], + timeout: 60, + }, + }, + } + + ;(fs.readFile as jest.Mock).mockResolvedValueOnce(JSON.stringify(mockConfig)) + + await mcpHub.updateServerTimeout("test-server", 120) + + expect(mockProvider.postMessageToWebview).toHaveBeenCalledWith( + expect.objectContaining({ + type: "mcpServers", + }), + ) + }) + }) }) }) diff --git a/src/shared/WebviewMessage.ts b/src/shared/WebviewMessage.ts index e2830bf..113b233 100644 --- a/src/shared/WebviewMessage.ts +++ b/src/shared/WebviewMessage.ts @@ -51,6 +51,7 @@ export interface WebviewMessage { | "restartMcpServer" | "toggleToolAlwaysAllow" | "toggleMcpServer" + | "updateMcpTimeout" | "fuzzyMatchThreshold" | "preferredLanguage" | "writeDelayMs" @@ -99,6 +100,7 @@ export interface WebviewMessage { query?: string slug?: string modeConfig?: ModeConfig + timeout?: number } export type ClineAskResponse = "yesButtonClicked" | "noButtonClicked" | "messageResponse" diff --git a/src/shared/mcp.ts b/src/shared/mcp.ts index 7df1415..2bc38a1 100644 --- a/src/shared/mcp.ts +++ b/src/shared/mcp.ts @@ -7,6 +7,7 @@ export type McpServer = { resources?: McpResource[] resourceTemplates?: McpResourceTemplate[] disabled?: boolean + timeout?: number } export type McpTool = { diff --git a/webview-ui/src/components/mcp/McpView.tsx b/webview-ui/src/components/mcp/McpView.tsx index 49a3ac7..46db17e 100644 --- a/webview-ui/src/components/mcp/McpView.tsx +++ b/webview-ui/src/components/mcp/McpView.tsx @@ -19,66 +19,6 @@ type McpViewProps = { const McpView = ({ onDone }: McpViewProps) => { const { mcpServers: servers, alwaysAllowMcp, mcpEnabled } = useExtensionState() - // const [servers, setServers] = useState([ - // // Add some mock servers for testing - // { - // name: "local-tools", - // config: JSON.stringify({ - // mcpServers: { - // "local-tools": { - // command: "npx", - // args: ["-y", "@modelcontextprotocol/server-tools"], - // }, - // }, - // }), - // status: "connected", - // tools: [ - // { - // name: "execute_command", - // description: "Run a shell command on the local system", - // }, - // { - // name: "read_file", - // description: "Read contents of a file from the filesystem", - // }, - // ], - // }, - // { - // name: "postgres-db", - // config: JSON.stringify({ - // mcpServers: { - // "postgres-db": { - // command: "npx", - // args: ["-y", "@modelcontextprotocol/server-postgres", "postgresql://localhost/mydb"], - // }, - // }, - // }), - // status: "disconnected", - // error: "Failed to connect to database: Connection refused", - // }, - // { - // name: "github-tools", - // config: JSON.stringify({ - // mcpServers: { - // "github-tools": { - // command: "npx", - // args: ["-y", "@modelcontextprotocol/server-github"], - // }, - // }, - // }), - // status: "connecting", - // resources: [ - // { - // uri: "github://repo/issues", - // name: "Repository Issues", - // }, - // { - // uri: "github://repo/pulls", - // name: "Pull Requests", - // }, - // ], - // }, - // ]) return (
{ // Server Row Component const ServerRow = ({ server, alwaysAllowMcp }: { server: McpServer; alwaysAllowMcp?: boolean }) => { const [isExpanded, setIsExpanded] = useState(false) + const [timeoutValue, setTimeoutValue] = useState(() => { + const configTimeout = JSON.parse(server.config)?.timeout + return configTimeout ?? 60 // Default 1 minute (60 seconds) + }) + + const timeoutOptions = [ + { value: 15, label: "15 seconds" }, + { value: 30, label: "30 seconds" }, + { value: 60, label: "1 minute" }, + { value: 300, label: "5 minutes" }, + { value: 600, label: "10 minutes" }, + { value: 900, label: "15 minutes" }, + { value: 1800, label: "30 minutes" }, + { value: 3600, label: "60 minutes" }, + ] const getStatusColor = () => { switch (server.status) { @@ -186,6 +141,16 @@ const ServerRow = ({ server, alwaysAllowMcp }: { server: McpServer; alwaysAllowM }) } + const handleTimeoutChange = (event: React.ChangeEvent) => { + const seconds = parseInt(event.target.value) + setTimeoutValue(seconds) + vscode.postMessage({ + type: "updateMcpTimeout", + serverName: server.name, + timeout: seconds, + }) + } + return (
- + Tools ({server.tools?.length || 0}) Resources ( @@ -351,6 +316,46 @@ const ServerRow = ({ server, alwaysAllowMcp }: { server: McpServer; alwaysAllowM + {/* Network Timeout */} +
+
+ Network Timeout + +
+ + Maximum time to wait for server responses + +
+