From f14542d61f0ac4be5ebb2cbe7f3634679e2e2588 Mon Sep 17 00:00:00 2001 From: Saoud Rizwan <7799382+saoudrizwan@users.noreply.github.com> Date: Mon, 24 Feb 2025 18:13:59 -0800 Subject: [PATCH] Revert "Everyone Gets a Timeout (#1889)" This reverts commit 87670a37b89dcd39d9ece7e3fc59a3abfbece1d2. --- .changeset/dull-planets-battle.md | 5 -- src/core/prompts/system.ts | 5 +- src/core/webview/ClineProvider.ts | 10 --- src/services/mcp/McpHub.ts | 86 +++++++---------------- src/shared/WebviewMessage.ts | 2 - src/shared/mcp.ts | 3 - src/utils/time.ts | 3 - webview-ui/src/components/mcp/McpView.tsx | 52 +------------- 8 files changed, 31 insertions(+), 135 deletions(-) delete mode 100644 .changeset/dull-planets-battle.md delete mode 100644 src/utils/time.ts diff --git a/.changeset/dull-planets-battle.md b/.changeset/dull-planets-battle.md deleted file mode 100644 index 47393fa995..0000000000 --- a/.changeset/dull-planets-battle.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -"claude-dev": minor ---- - -Added timeout configuration for individual MCP servers. diff --git a/src/core/prompts/system.ts b/src/core/prompts/system.ts index 7c10163e92..8d6d13766d 100644 --- a/src/core/prompts/system.ts +++ b/src/core/prompts/system.ts @@ -759,9 +759,8 @@ IMPORTANT: Regardless of what else you see in the MCP settings file, you must de "args": ["/path/to/weather-server/build/index.js"], "env": { "OPENWEATHER_API_KEY": "user-provided-api-key" - }, - "timeout": 60, // 60 seconds should be set as the default timeout for all MCP servers - } + } + }, } } \`\`\` diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 9987a34288..3eac593af2 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -821,16 +821,6 @@ export class ClineProvider implements vscode.WebviewViewProvider { } break } - case "updateMcpTimeout": { - try { - if (message.serverName && message.timeout) { - await this.mcpHub?.updateServerTimeout(message.serverName, message.timeout) - } - } catch (error) { - console.error(`Failed to update timeout for server ${message.serverName}:`, error) - } - break - } case "openExtensionSettings": { const settingsFilter = message.text || "" await vscode.commands.executeCommand( diff --git a/src/services/mcp/McpHub.ts b/src/services/mcp/McpHub.ts index 75332cd906..556d6d8c47 100644 --- a/src/services/mcp/McpHub.ts +++ b/src/services/mcp/McpHub.ts @@ -16,7 +16,6 @@ import * as vscode from "vscode" import { z } from "zod" import { ClineProvider, GlobalFileNames } from "../../core/webview/ClineProvider" import { - DEFAULT_MCP_TIMEOUT_SECONDS, McpMode, McpResource, McpResourceResponse, @@ -27,7 +26,7 @@ import { } from "../../shared/mcp" import { fileExistsAtPath } from "../../utils/fs" import { arePathsEqual } from "../../utils/path" -import { secondsToMs } from "../../utils/time" + export type McpConnection = { server: McpServer client: Client @@ -43,7 +42,6 @@ const StdioConfigSchema = z.object({ env: z.record(z.string()).optional(), autoApprove: AutoApproveSchema.optional(), disabled: z.boolean().optional(), - timeout: z.number().min(1).max(3600).optional().default(DEFAULT_MCP_TIMEOUT_SECONDS), }) const McpSettingsSchema = z.object({ @@ -244,6 +242,28 @@ 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" @@ -323,6 +343,10 @@ 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) { @@ -539,7 +563,6 @@ export class McpHub { if (connection.server.disabled) { throw new Error(`Server "${serverName}" is disabled`) } - return await connection.client.request( { method: "resources/read", @@ -563,17 +586,6 @@ export class McpHub { throw new Error(`Server "${serverName}" is disabled and cannot be used`) } - let timeout = secondsToMs(DEFAULT_MCP_TIMEOUT_SECONDS) // sdk expects ms - - try { - const config = JSON.parse(connection.server.config) - const parsedConfig = StdioConfigSchema.parse(config) - timeout = secondsToMs(parsedConfig.timeout) - } catch (error) { - console.error(`Failed to parse timeout configuration for server ${serverName}: ${error}`) - // Continue with default timeout - } - return await connection.client.request( { method: "tools/call", @@ -583,9 +595,6 @@ export class McpHub { }, }, CallToolResultSchema, - { - timeout, - }, ) } @@ -654,47 +663,6 @@ export class McpHub { } } - public async updateServerTimeout(serverName: string, timeout: number): Promise { - try { - // Validate timeout against schema - const setConfigResult = StdioConfigSchema.shape.timeout.safeParse(timeout) - if (!setConfigResult.success) { - throw new Error(`Invalid timeout value: ${timeout}. Must be between 1 and 3600 seconds.`) - } - - const settingsPath = await this.getMcpSettingsFilePath() - const content = await fs.readFile(settingsPath, "utf-8") - const config = JSON.parse(content) - - if (!config.mcpServers?.[serverName]) { - throw new Error(`Server "${serverName}" not found in settings`) - } - - // Update the timeout in the config - config.mcpServers[serverName] = { - ...config.mcpServers[serverName], - timeout, - } - - // Write updated config back to file - await fs.writeFile(settingsPath, JSON.stringify(config, null, 2)) - - // Update server connections to apply the new timeout - await this.updateServerConnections(config.mcpServers) - - vscode.window.showInformationMessage(`Updated timeout to ${timeout} seconds`) - } 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 dispose(): Promise { this.removeAllFileWatchers() for (const connection of this.connections) { diff --git a/src/shared/WebviewMessage.ts b/src/shared/WebviewMessage.ts index 6fd9dfe085..e42cb5860b 100644 --- a/src/shared/WebviewMessage.ts +++ b/src/shared/WebviewMessage.ts @@ -50,7 +50,6 @@ export interface WebviewMessage { | "searchCommits" | "showMcpView" | "fetchLatestMcpServersFromHub" - | "updateMcpTimeout" // | "relaunchChromeDebugMode" text?: string disabled?: boolean @@ -64,7 +63,6 @@ export interface WebviewMessage { chatSettings?: ChatSettings chatContent?: ChatContent mcpId?: string - timeout?: number // For updateMcpTimeout // For toggleToolAutoApprove serverName?: string diff --git a/src/shared/mcp.ts b/src/shared/mcp.ts index fb8816ac23..f0a09afa2d 100644 --- a/src/shared/mcp.ts +++ b/src/shared/mcp.ts @@ -1,5 +1,3 @@ -export const DEFAULT_MCP_TIMEOUT_SECONDS = 60 // matches Anthropic's default timeout in their MCP SDK - export type McpMode = "full" | "server-use-only" | "off" export type McpServer = { @@ -11,7 +9,6 @@ export type McpServer = { resources?: McpResource[] resourceTemplates?: McpResourceTemplate[] disabled?: boolean - timeout?: number } export type McpTool = { diff --git a/src/utils/time.ts b/src/utils/time.ts deleted file mode 100644 index 316ea5a949..0000000000 --- a/src/utils/time.ts +++ /dev/null @@ -1,3 +0,0 @@ -export function secondsToMs(seconds: number): number { - return seconds * 1000 -} diff --git a/webview-ui/src/components/mcp/McpView.tsx b/webview-ui/src/components/mcp/McpView.tsx index fee5322ef6..f40dec9ddd 100644 --- a/webview-ui/src/components/mcp/McpView.tsx +++ b/webview-ui/src/components/mcp/McpView.tsx @@ -1,15 +1,7 @@ -import { - VSCodeButton, - VSCodeLink, - VSCodePanels, - VSCodePanelTab, - VSCodePanelView, - VSCodeDropdown, - VSCodeOption, -} from "@vscode/webview-ui-toolkit/react" +import { VSCodeButton, VSCodeLink, VSCodePanels, VSCodePanelTab, VSCodePanelView } from "@vscode/webview-ui-toolkit/react" import { useEffect, useState } from "react" import styled from "styled-components" -import { DEFAULT_MCP_TIMEOUT_SECONDS, McpServer } from "../../../../src/shared/mcp" +import { McpServer } from "../../../../src/shared/mcp" import { useExtensionState } from "../../context/ExtensionStateContext" import { getMcpServerDisplayName } from "../../utils/mcp" import { vscode } from "../../utils/vscode" @@ -218,36 +210,6 @@ const ServerRow = ({ server }: { server: McpServer }) => { } } - const [timeout, setTimeout] = useState(() => { - try { - const config = JSON.parse(server.config) - return config.timeout?.toString() || DEFAULT_MCP_TIMEOUT_SECONDS.toString() - } catch { - return DEFAULT_MCP_TIMEOUT_SECONDS.toString() - } - }) - - const timeoutOptions = [ - { value: "30", label: "30 seconds" }, - { value: "60", label: "1 minute" }, - { value: "300", label: "5 minutes" }, - { value: "600", label: "10 minutes" }, - { value: "1800", label: "30 minutes" }, - { value: "3600", label: "1 hour" }, - ] - - const handleTimeoutChange = (e: any) => { - const select = e.target as HTMLSelectElement - const value = select.value - const num = parseInt(value) - setTimeout(value) - vscode.postMessage({ - type: "updateMcpTimeout", - serverName: server.name, - timeout: num, - }) - } - const handleRestart = () => { vscode.postMessage({ type: "restartMcpServer", @@ -448,16 +410,6 @@ const ServerRow = ({ server }: { server: McpServer }) => { -
- - - {timeoutOptions.map((option) => ( - - {option.label} - - ))} - -