mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-08-28 05:27:24 +00:00
Everyone Gets a Timeout (#2018)
* re-open clean PR; formatting changes * remove unnecessary comments * Revert system prompt changes * Remove vscode message after timeout * Create tidy-kings-ring.md --------- Co-authored-by: Saoud Rizwan <7799382+saoudrizwan@users.noreply.github.com>
This commit is contained in:
parent
dcd275480f
commit
3d34de4908
7 changed files with 126 additions and 33 deletions
5
.changeset/tidy-kings-ring.md
Normal file
5
.changeset/tidy-kings-ring.md
Normal file
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
"claude-dev": patch
|
||||
---
|
||||
|
||||
Add timeout option to MCP servers
|
||||
|
|
@ -541,7 +541,6 @@ export class ClineProvider implements vscode.WebviewViewProvider {
|
|||
const isOptedIn = telemetrySetting === "enabled"
|
||||
telemetryService.updateTelemetryState(isOptedIn)
|
||||
})
|
||||
|
||||
break
|
||||
case "newTask":
|
||||
// Code that should run in response to the hello message command
|
||||
|
|
@ -945,6 +944,16 @@ 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(
|
||||
|
|
|
|||
|
|
@ -16,6 +16,7 @@ import * as vscode from "vscode"
|
|||
import { z } from "zod"
|
||||
import { ClineProvider, GlobalFileNames } from "../../core/webview/ClineProvider"
|
||||
import {
|
||||
DEFAULT_MCP_TIMEOUT_SECONDS,
|
||||
McpMode,
|
||||
McpResource,
|
||||
McpResourceResponse,
|
||||
|
|
@ -23,10 +24,11 @@ import {
|
|||
McpServer,
|
||||
McpTool,
|
||||
McpToolCallResponse,
|
||||
MIN_MCP_TIMEOUT_SECONDS,
|
||||
} 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
|
||||
|
|
@ -35,13 +37,13 @@ export type McpConnection = {
|
|||
|
||||
const AutoApproveSchema = z.array(z.string()).default([])
|
||||
|
||||
// StdioServerParameters
|
||||
const StdioConfigSchema = z.object({
|
||||
command: z.string(),
|
||||
args: z.array(z.string()).optional(),
|
||||
env: z.record(z.string()).optional(),
|
||||
autoApprove: AutoApproveSchema.optional(),
|
||||
disabled: z.boolean().optional(),
|
||||
timeout: z.number().min(MIN_MCP_TIMEOUT_SECONDS).optional().default(DEFAULT_MCP_TIMEOUT_SECONDS),
|
||||
})
|
||||
|
||||
const McpSettingsSchema = z.object({
|
||||
|
|
@ -242,28 +244,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"
|
||||
|
|
@ -343,10 +323,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) {
|
||||
|
|
@ -563,6 +539,7 @@ export class McpHub {
|
|||
if (connection.server.disabled) {
|
||||
throw new Error(`Server "${serverName}" is disabled`)
|
||||
}
|
||||
|
||||
return await connection.client.request(
|
||||
{
|
||||
method: "resources/read",
|
||||
|
|
@ -586,6 +563,16 @@ 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}`)
|
||||
}
|
||||
|
||||
return await connection.client.request(
|
||||
{
|
||||
method: "tools/call",
|
||||
|
|
@ -595,6 +582,9 @@ export class McpHub {
|
|||
},
|
||||
},
|
||||
CallToolResultSchema,
|
||||
{
|
||||
timeout,
|
||||
},
|
||||
)
|
||||
}
|
||||
|
||||
|
|
@ -620,7 +610,6 @@ export class McpHub {
|
|||
autoApprove.splice(toolIndex, 1)
|
||||
}
|
||||
|
||||
// Write updated config back to file
|
||||
await fs.writeFile(settingsPath, JSON.stringify(config, null, 2))
|
||||
|
||||
// Update the tools list to reflect the change
|
||||
|
|
@ -663,6 +652,42 @@ export class McpHub {
|
|||
}
|
||||
}
|
||||
|
||||
public async updateServerTimeout(serverName: string, timeout: number): Promise<void> {
|
||||
try {
|
||||
// Validate timeout against schema
|
||||
const setConfigResult = StdioConfigSchema.shape.timeout.safeParse(timeout)
|
||||
if (!setConfigResult.success) {
|
||||
throw new Error(`Invalid timeout value: ${timeout}. Must be at minimum ${MIN_MCP_TIMEOUT_SECONDS} 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`)
|
||||
}
|
||||
|
||||
config.mcpServers[serverName] = {
|
||||
...config.mcpServers[serverName],
|
||||
timeout,
|
||||
}
|
||||
|
||||
await fs.writeFile(settingsPath, JSON.stringify(config, null, 2))
|
||||
|
||||
await this.updateServerConnections(config.mcpServers)
|
||||
} 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<void> {
|
||||
this.removeAllFileWatchers()
|
||||
for (const connection of this.connections) {
|
||||
|
|
|
|||
|
|
@ -69,7 +69,7 @@ export interface WebviewMessage {
|
|||
chatSettings?: ChatSettings
|
||||
chatContent?: ChatContent
|
||||
mcpId?: string
|
||||
|
||||
timeout?: number
|
||||
// For toggleToolAutoApprove
|
||||
serverName?: string
|
||||
toolName?: string
|
||||
|
|
|
|||
|
|
@ -1,3 +1,5 @@
|
|||
export const DEFAULT_MCP_TIMEOUT_SECONDS = 60 // matches Anthropic's default timeout in their MCP SDK
|
||||
export const MIN_MCP_TIMEOUT_SECONDS = 1
|
||||
export type McpMode = "full" | "server-use-only" | "off"
|
||||
|
||||
export type McpServer = {
|
||||
|
|
@ -9,6 +11,7 @@ export type McpServer = {
|
|||
resources?: McpResource[]
|
||||
resourceTemplates?: McpResourceTemplate[]
|
||||
disabled?: boolean
|
||||
timeout?: number
|
||||
}
|
||||
|
||||
export type McpTool = {
|
||||
|
|
|
|||
3
src/utils/time.ts
Normal file
3
src/utils/time.ts
Normal file
|
|
@ -0,0 +1,3 @@
|
|||
export function secondsToMs(seconds: number): number {
|
||||
return seconds * 1000
|
||||
}
|
||||
|
|
@ -1,7 +1,15 @@
|
|||
import { VSCodeButton, VSCodeLink, VSCodePanels, VSCodePanelTab, VSCodePanelView } from "@vscode/webview-ui-toolkit/react"
|
||||
import {
|
||||
VSCodeButton,
|
||||
VSCodeLink,
|
||||
VSCodePanels,
|
||||
VSCodePanelTab,
|
||||
VSCodePanelView,
|
||||
VSCodeDropdown,
|
||||
VSCodeOption,
|
||||
} from "@vscode/webview-ui-toolkit/react"
|
||||
import { useEffect, useState } from "react"
|
||||
import styled from "styled-components"
|
||||
import { McpServer } from "../../../../src/shared/mcp"
|
||||
import { DEFAULT_MCP_TIMEOUT_SECONDS, McpServer } from "../../../../src/shared/mcp"
|
||||
import { useExtensionState } from "../../context/ExtensionStateContext"
|
||||
import { getMcpServerDisplayName } from "../../utils/mcp"
|
||||
import { vscode } from "../../utils/vscode"
|
||||
|
|
@ -210,6 +218,36 @@ const ServerRow = ({ server }: { server: McpServer }) => {
|
|||
}
|
||||
}
|
||||
|
||||
const [timeoutValue, setTimeoutValue] = useState<string>(() => {
|
||||
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)
|
||||
setTimeoutValue(value)
|
||||
vscode.postMessage({
|
||||
type: "updateMcpTimeout",
|
||||
serverName: server.name,
|
||||
timeout: num,
|
||||
})
|
||||
}
|
||||
|
||||
const handleRestart = () => {
|
||||
vscode.postMessage({
|
||||
type: "restartMcpServer",
|
||||
|
|
@ -410,6 +448,16 @@ const ServerRow = ({ server }: { server: McpServer }) => {
|
|||
</VSCodePanelView>
|
||||
</VSCodePanels>
|
||||
|
||||
<div style={{ margin: "10px 7px" }}>
|
||||
<label style={{ display: "block", marginBottom: "4px", fontSize: "13px" }}>Request Timeout</label>
|
||||
<VSCodeDropdown style={{ width: "100%" }} value={timeoutValue} onChange={handleTimeoutChange}>
|
||||
{timeoutOptions.map((option) => (
|
||||
<VSCodeOption key={option.value} value={option.value}>
|
||||
{option.label}
|
||||
</VSCodeOption>
|
||||
))}
|
||||
</VSCodeDropdown>
|
||||
</div>
|
||||
<VSCodeButton
|
||||
appearance="secondary"
|
||||
onClick={handleRestart}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue