Skip to content

Commit a2263de

Browse files
Protobus: updateMcpTimeout message (RooCodeInc#3085)
* updateMcpTimeout protobus conversion * changeset
1 parent e53fa83 commit a2263de

File tree

9 files changed

+325
-124
lines changed

9 files changed

+325
-124
lines changed

.changeset/clean-chicken-smash.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"claude-dev": minor
3+
---
4+
5+
Move updateMcpTimeout message to protobus

proto/mcp.proto

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,19 @@ import "common.proto";
66

77
service McpService {
88
rpc toggleMcpServer(ToggleMcpServerRequest) returns (McpServers);
9+
rpc updateMcpTimeout(UpdateMcpTimeoutRequest) returns (McpServers);
10+
}
11+
12+
message ToggleMcpServerRequest {
13+
Metadata metadata = 1;
14+
string server_name = 2;
15+
bool disabled = 3;
16+
}
17+
18+
message UpdateMcpTimeoutRequest {
19+
Metadata metadata = 1;
20+
string server_name = 2;
21+
int32 timeout = 3;
922
}
1023

1124
message McpTool {
@@ -49,12 +62,6 @@ message McpServer {
4962
optional int32 timeout = 9;
5063
}
5164

52-
message ToggleMcpServerRequest {
53-
Metadata metadata = 1;
54-
string server_name = 2;
55-
bool disabled = 3;
56-
}
57-
5865
message McpServers {
5966
repeated McpServer mcp_servers = 1;
6067
}

src/core/controller/index.ts

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -646,16 +646,6 @@ export class Controller {
646646
}
647647
break
648648
}
649-
case "updateMcpTimeout": {
650-
try {
651-
if (message.serverName && message.timeout) {
652-
await this.mcpHub?.updateServerTimeout(message.serverName, message.timeout)
653-
}
654-
} catch (error) {
655-
console.error(`Failed to update timeout for server ${message.serverName}:`, error)
656-
}
657-
break
658-
}
659649
case "openExtensionSettings": {
660650
const settingsFilter = message.text || ""
661651
await vscode.commands.executeCommand(

src/core/controller/mcp/methods.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,11 @@
44
// Import all method implementations
55
import { registerMethod } from "./index"
66
import { toggleMcpServer } from "./toggleMcpServer"
7+
import { updateMcpTimeout } from "./updateMcpTimeout"
78

89
// Register all mcp service methods
910
export function registerAllMethods(): void {
1011
// Register each method with the registry
1112
registerMethod("toggleMcpServer", toggleMcpServer)
13+
registerMethod("updateMcpTimeout", updateMcpTimeout)
1214
}
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
import { convertMcpServersToProtoMcpServers } from "@/shared/proto-conversions/mcp/mcp-server-conversion"
2+
import { Controller } from ".."
3+
import { UpdateMcpTimeoutRequest, McpServers } from "../../../shared/proto/mcp"
4+
5+
/**
6+
* Updates the timeout configuration for an MCP server.
7+
* @param controller - The Controller instance
8+
* @param request - Contains server name and timeout value
9+
* @returns Array of updated McpServer objects
10+
*/
11+
export async function updateMcpTimeout(controller: Controller, request: UpdateMcpTimeoutRequest): Promise<McpServers> {
12+
try {
13+
if (request.serverName && typeof request.serverName === "string" && typeof request.timeout === "number") {
14+
const mcpServers = await controller.mcpHub?.updateServerTimeoutRPC(request.serverName, request.timeout)
15+
console.log("mcpServers", mcpServers)
16+
const convertedMcpServers = convertMcpServersToProtoMcpServers(mcpServers)
17+
console.log("convertedMcpServers", convertedMcpServers)
18+
return { mcpServers: convertedMcpServers }
19+
} else {
20+
console.error("Server name and timeout are required")
21+
throw new Error("Server name and timeout are required")
22+
}
23+
} catch (error) {
24+
console.error(`Failed to update timeout for server ${request.serverName}:`, error)
25+
throw error
26+
}
27+
}

src/services/mcp/McpHub.ts

Lines changed: 61 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -329,10 +329,9 @@ export class McpHub {
329329
autoApprove: autoApproveConfig.includes(tool.name),
330330
}))
331331

332-
// console.log(`[MCP] Fetched tools for ${serverName}:`, tools)
333332
return tools
334333
} catch (error) {
335-
// console.error(`Failed to fetch tools for ${serverName}:`, error)
334+
console.error(`Failed to fetch tools for ${serverName}:`, error)
336335
return []
337336
}
338337
}
@@ -377,6 +376,53 @@ export class McpHub {
377376
}
378377
}
379378

379+
async updateServerConnectionsRPC(newServers: Record<string, McpServerConfig>): Promise<void> {
380+
this.isConnecting = true
381+
this.removeAllFileWatchers()
382+
const currentNames = new Set(this.connections.map((conn) => conn.server.name))
383+
const newNames = new Set(Object.keys(newServers))
384+
385+
// Delete removed servers
386+
for (const name of currentNames) {
387+
if (!newNames.has(name)) {
388+
await this.deleteConnection(name)
389+
console.log(`Deleted MCP server: ${name}`)
390+
}
391+
}
392+
393+
// Update or add servers
394+
for (const [name, config] of Object.entries(newServers)) {
395+
const currentConnection = this.connections.find((conn) => conn.server.name === name)
396+
397+
if (!currentConnection) {
398+
// New server
399+
try {
400+
if (config.transportType === "stdio") {
401+
this.setupFileWatcher(name, config)
402+
}
403+
await this.connectToServer(name, config)
404+
} catch (error) {
405+
console.error(`Failed to connect to new MCP server ${name}:`, error)
406+
}
407+
} else if (!deepEqual(JSON.parse(currentConnection.server.config), config)) {
408+
// Existing server with changed config
409+
try {
410+
if (config.transportType === "stdio") {
411+
this.setupFileWatcher(name, config)
412+
}
413+
await this.deleteConnection(name)
414+
await this.connectToServer(name, config)
415+
console.log(`Reconnected MCP server with updated config: ${name}`)
416+
} catch (error) {
417+
console.error(`Failed to reconnect MCP server ${name}:`, error)
418+
}
419+
}
420+
// If server exists with same config, do nothing
421+
}
422+
423+
this.isConnecting = false
424+
}
425+
380426
async updateServerConnections(newServers: Record<string, McpServerConfig>): Promise<void> {
381427
this.isConnecting = true
382428
this.removeAllFileWatchers()
@@ -719,7 +765,7 @@ export class McpHub {
719765
}
720766
}
721767

722-
public async updateServerTimeout(serverName: string, timeout: number): Promise<void> {
768+
public async updateServerTimeoutRPC(serverName: string, timeout: number): Promise<McpServer[]> {
723769
try {
724770
// Validate timeout against schema
725771
const setConfigResult = BaseConfigSchema.shape.timeout.safeParse(timeout)
@@ -742,7 +788,18 @@ export class McpHub {
742788

743789
await fs.writeFile(settingsPath, JSON.stringify(config, null, 2))
744790

745-
await this.updateServerConnections(config.mcpServers)
791+
await this.updateServerConnectionsRPC(config.mcpServers)
792+
793+
const serverOrder = Object.keys(config.mcpServers || {})
794+
const updatedMcpServers = [...this.connections]
795+
.sort((a, b) => {
796+
const indexA = serverOrder.indexOf(a.server.name)
797+
const indexB = serverOrder.indexOf(b.server.name)
798+
return indexA - indexB
799+
})
800+
.map((connection) => connection.server)
801+
802+
return updatedMcpServers
746803
} catch (error) {
747804
console.error("Failed to update server timeout:", error)
748805
if (error instanceof Error) {

src/shared/WebviewMessage.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@ export interface WebviewMessage {
4242
| "openExtensionSettings"
4343
| "requestVsCodeLmModels"
4444
| "toggleToolAutoApprove"
45-
| "toggleMcpServer"
4645
| "getLatestState"
4746
| "accountLoginClicked"
4847
| "accountLogoutClicked"
@@ -57,7 +56,6 @@ export interface WebviewMessage {
5756
| "fetchLatestMcpServersFromHub"
5857
| "telemetrySetting"
5958
| "openSettings"
60-
| "updateMcpTimeout"
6159
| "fetchOpenGraphData"
6260
| "checkIsImageUrl"
6361
| "invoke"

0 commit comments

Comments
 (0)