From 4dce299563021d2e17293911d6709458bbd503d9 Mon Sep 17 00:00:00 2001 From: seedlord Date: Tue, 20 May 2025 01:24:12 +0000 Subject: [PATCH 01/11] feat: add toggle functionality for enabling/disabling all global MCP servers with configuration reload + Tests --- src/services/mcp/McpHub.ts | 47 +++++++++++++-- src/services/mcp/__tests__/McpHub.test.ts | 72 +++++++++++++++++++++++ 2 files changed, 113 insertions(+), 6 deletions(-) diff --git a/src/services/mcp/McpHub.ts b/src/services/mcp/McpHub.ts index 46f59858f9..6184f5c016 100644 --- a/src/services/mcp/McpHub.ts +++ b/src/services/mcp/McpHub.ts @@ -941,20 +941,26 @@ export class McpHub { // Update the connection object if (connection) { - try { - connection.server.disabled = disabled + const wasDisabled = connection.server.disabled + connection.server.disabled = disabled + + // When the server is activated, reload the configuration + if (wasDisabled && !disabled) { + await this.initializeMcpServers(serverSource) + } - // Only refresh capabilities if connected - if (connection.server.status === "connected") { + // Only refresh capabilities if connected + if (connection.server.status === "connected") { + try { connection.server.tools = await this.fetchToolsList(serverName, serverSource) connection.server.resources = await this.fetchResourcesList(serverName, serverSource) connection.server.resourceTemplates = await this.fetchResourceTemplatesList( serverName, serverSource, ) + } catch (error) { + console.error(`Failed to refresh capabilities for ${serverName}:`, error) } - } catch (error) { - console.error(`Failed to refresh capabilities for ${serverName}:`, error) } } @@ -1287,4 +1293,33 @@ export class McpHub { } this.disposables.forEach((d) => d.dispose()) } + + /** + * Enables or disables all global MCP servers at once. + * When activated, the configuration is reloaded. + * @param disabled true = disable all, false = enable all + */ + public async toggleAllServersDisabled(disabled: boolean): Promise { + // Collect all global server names + const globalConnections = this.connections.filter( + (conn) => conn.server.source === "global" || !conn.server.source, + ) + const serverNames = globalConnections.map((conn) => conn.server.name) + + // Set the Disabled flag for all serversv + for (const name of serverNames) { + await this.updateServerConfig(name, { disabled }, "global") + const conn = this.findConnection(name, "global") + if (conn) { + conn.server.disabled = disabled + } + } + + // If activated, reload configuration + if (!disabled) { + await this.initializeMcpServers("global") + } + + await this.notifyWebviewOfServerChanges() + } } diff --git a/src/services/mcp/__tests__/McpHub.test.ts b/src/services/mcp/__tests__/McpHub.test.ts index ffd98ff6bd..e85ae9d1e2 100644 --- a/src/services/mcp/__tests__/McpHub.test.ts +++ b/src/services/mcp/__tests__/McpHub.test.ts @@ -560,5 +560,77 @@ describe("McpHub", () => { ) }) }) + describe("toggleServerDisabled (Configuration reload)", () => { + it("should reload configuration when enabling a server (calls initializeMcpServers)", async () => { + const mockConfig: any = { + mcpServers: { + "test-server": { + type: "stdio", + command: "node", + args: ["test.js"], + disabled: true, + }, + }, + } + + ;(fs.readFile as jest.Mock).mockResolvedValueOnce(JSON.stringify(mockConfig)) + + mcpHub.connections = [ + { + server: { + name: "test-server", + config: JSON.stringify(mockConfig.mcpServers["test-server"]), + status: "connected", + disabled: true, + }, + client: { request: jest.fn() }, + transport: { close: jest.fn() }, + } as any, + ] + + const spy = jest.spyOn(mcpHub as any, "initializeMcpServers").mockResolvedValue(undefined) + + await mcpHub.toggleServerDisabled("test-server", false) + // Expectation: initializeMcpServers was called + expect(spy).toHaveBeenCalledWith("global") + spy.mockRestore() + }) + }) + describe("toggleAllServersDisabled (global enable/disable)", () => { + it("should reload configuration when globally enabling all servers (calls initializeMcpServers)", async () => { + // Two global servers, both disabled + mcpHub.connections = [ + { + server: { + name: "server1", + config: JSON.stringify({ type: "stdio", command: "node", args: ["a.js"], disabled: true }), + status: "disconnected", + disabled: true, + source: "global", + }, + client: { request: jest.fn() }, + transport: { close: jest.fn() }, + } as any, + { + server: { + name: "server2", + config: JSON.stringify({ type: "stdio", command: "node", args: ["b.js"], disabled: true }), + status: "disconnected", + disabled: true, + source: "global", + }, + client: { request: jest.fn() }, + transport: { close: jest.fn() }, + } as any, + ] + + const spy = jest.spyOn(mcpHub as any, "initializeMcpServers").mockResolvedValue(undefined) + + await mcpHub.toggleAllServersDisabled(false) // actiate global + + expect(spy).toHaveBeenCalledWith("global") + spy.mockRestore() + }) + }) }) }) From 74384e2dffa75dba002e3549b99469064e8e12d3 Mon Sep 17 00:00:00 2001 From: seedlord Date: Tue, 20 May 2025 03:48:34 +0200 Subject: [PATCH 02/11] Update src/services/mcp/McpHub.ts Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com> --- src/services/mcp/McpHub.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/mcp/McpHub.ts b/src/services/mcp/McpHub.ts index d40798b160..cdf65812da 100644 --- a/src/services/mcp/McpHub.ts +++ b/src/services/mcp/McpHub.ts @@ -1325,7 +1325,7 @@ export class McpHub { ) const serverNames = globalConnections.map((conn) => conn.server.name) - // Set the Disabled flag for all serversv + // Set the Disabled flag for all servers for (const name of serverNames) { await this.updateServerConfig(name, { disabled }, "global") const conn = this.findConnection(name, "global") From 783cbc6b02999e8b8d90d6d67326b4d08f0b33fa Mon Sep 17 00:00:00 2001 From: seedlord Date: Tue, 20 May 2025 03:51:52 +0200 Subject: [PATCH 03/11] Update McpHub.test.ts --- src/services/mcp/__tests__/McpHub.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/mcp/__tests__/McpHub.test.ts b/src/services/mcp/__tests__/McpHub.test.ts index e85ae9d1e2..84a3386bf1 100644 --- a/src/services/mcp/__tests__/McpHub.test.ts +++ b/src/services/mcp/__tests__/McpHub.test.ts @@ -626,7 +626,7 @@ describe("McpHub", () => { const spy = jest.spyOn(mcpHub as any, "initializeMcpServers").mockResolvedValue(undefined) - await mcpHub.toggleAllServersDisabled(false) // actiate global + await mcpHub.toggleAllServersDisabled(false) // activate global expect(spy).toHaveBeenCalledWith("global") spy.mockRestore() From ac26ff62b374fe9f8e62c406e6d6b3047e73ce6d Mon Sep 17 00:00:00 2001 From: seedlord Date: Wed, 21 May 2025 03:41:48 +0200 Subject: [PATCH 04/11] revert mcp changes --- src/services/mcp/McpHub.ts | 47 ++------------- src/services/mcp/__tests__/McpHub.test.ts | 72 ----------------------- 2 files changed, 6 insertions(+), 113 deletions(-) diff --git a/src/services/mcp/McpHub.ts b/src/services/mcp/McpHub.ts index cdf65812da..e83f7afb9e 100644 --- a/src/services/mcp/McpHub.ts +++ b/src/services/mcp/McpHub.ts @@ -960,26 +960,20 @@ export class McpHub { // Update the connection object if (connection) { - const wasDisabled = connection.server.disabled - connection.server.disabled = disabled - - // When the server is activated, reload the configuration - if (wasDisabled && !disabled) { - await this.initializeMcpServers(serverSource) - } + try { + connection.server.disabled = disabled - // Only refresh capabilities if connected - if (connection.server.status === "connected") { - try { + // Only refresh capabilities if connected + if (connection.server.status === "connected") { connection.server.tools = await this.fetchToolsList(serverName, serverSource) connection.server.resources = await this.fetchResourcesList(serverName, serverSource) connection.server.resourceTemplates = await this.fetchResourceTemplatesList( serverName, serverSource, ) - } catch (error) { - console.error(`Failed to refresh capabilities for ${serverName}:`, error) } + } catch (error) { + console.error(`Failed to refresh capabilities for ${serverName}:`, error) } } @@ -1312,33 +1306,4 @@ export class McpHub { } this.disposables.forEach((d) => d.dispose()) } - - /** - * Enables or disables all global MCP servers at once. - * When activated, the configuration is reloaded. - * @param disabled true = disable all, false = enable all - */ - public async toggleAllServersDisabled(disabled: boolean): Promise { - // Collect all global server names - const globalConnections = this.connections.filter( - (conn) => conn.server.source === "global" || !conn.server.source, - ) - const serverNames = globalConnections.map((conn) => conn.server.name) - - // Set the Disabled flag for all servers - for (const name of serverNames) { - await this.updateServerConfig(name, { disabled }, "global") - const conn = this.findConnection(name, "global") - if (conn) { - conn.server.disabled = disabled - } - } - - // If activated, reload configuration - if (!disabled) { - await this.initializeMcpServers("global") - } - - await this.notifyWebviewOfServerChanges() - } } diff --git a/src/services/mcp/__tests__/McpHub.test.ts b/src/services/mcp/__tests__/McpHub.test.ts index 84a3386bf1..ffd98ff6bd 100644 --- a/src/services/mcp/__tests__/McpHub.test.ts +++ b/src/services/mcp/__tests__/McpHub.test.ts @@ -560,77 +560,5 @@ describe("McpHub", () => { ) }) }) - describe("toggleServerDisabled (Configuration reload)", () => { - it("should reload configuration when enabling a server (calls initializeMcpServers)", async () => { - const mockConfig: any = { - mcpServers: { - "test-server": { - type: "stdio", - command: "node", - args: ["test.js"], - disabled: true, - }, - }, - } - - ;(fs.readFile as jest.Mock).mockResolvedValueOnce(JSON.stringify(mockConfig)) - - mcpHub.connections = [ - { - server: { - name: "test-server", - config: JSON.stringify(mockConfig.mcpServers["test-server"]), - status: "connected", - disabled: true, - }, - client: { request: jest.fn() }, - transport: { close: jest.fn() }, - } as any, - ] - - const spy = jest.spyOn(mcpHub as any, "initializeMcpServers").mockResolvedValue(undefined) - - await mcpHub.toggleServerDisabled("test-server", false) - // Expectation: initializeMcpServers was called - expect(spy).toHaveBeenCalledWith("global") - spy.mockRestore() - }) - }) - describe("toggleAllServersDisabled (global enable/disable)", () => { - it("should reload configuration when globally enabling all servers (calls initializeMcpServers)", async () => { - // Two global servers, both disabled - mcpHub.connections = [ - { - server: { - name: "server1", - config: JSON.stringify({ type: "stdio", command: "node", args: ["a.js"], disabled: true }), - status: "disconnected", - disabled: true, - source: "global", - }, - client: { request: jest.fn() }, - transport: { close: jest.fn() }, - } as any, - { - server: { - name: "server2", - config: JSON.stringify({ type: "stdio", command: "node", args: ["b.js"], disabled: true }), - status: "disconnected", - disabled: true, - source: "global", - }, - client: { request: jest.fn() }, - transport: { close: jest.fn() }, - } as any, - ] - - const spy = jest.spyOn(mcpHub as any, "initializeMcpServers").mockResolvedValue(undefined) - - await mcpHub.toggleAllServersDisabled(false) // activate global - - expect(spy).toHaveBeenCalledWith("global") - spy.mockRestore() - }) - }) }) }) From a7a96fbf0db785035b16805059758f68097e0edb Mon Sep 17 00:00:00 2001 From: seedlord Date: Wed, 21 May 2025 02:38:47 +0200 Subject: [PATCH 05/11] Add commands to reload and toggle MCP servers; update UI and localization --- package.json | 10 +++++ src/activate/registerCommands.ts | 29 ++++++++++++++ src/core/webview/webviewMessageHandler.ts | 11 ++++++ src/services/mcp/McpHub.ts | 48 +++++++++++++++++++++++ src/shared/WebviewMessage.ts | 2 + webview-ui/src/components/mcp/McpView.tsx | 23 +++++++++++ webview-ui/src/i18n/locales/de/mcp.json | 4 +- webview-ui/src/i18n/locales/en/mcp.json | 4 +- 8 files changed, 129 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 2ad83a6e86..bbbe6d0c96 100644 --- a/package.json +++ b/package.json @@ -169,6 +169,16 @@ "command": "roo.acceptInput", "title": "%command.acceptInput.title%", "category": "%configuration.title%" + }, + { + "command": "extension.reloadAllMcpServers", + "title": "Reload All MCP Servers", + "icon": "$(sync)" + }, + { + "command": "extension.toggleAllMcpServersDisabled", + "title": "Toggle All MCP Servers Enabled/Disabled", + "icon": "$(power)" } ], "menus": { diff --git a/src/activate/registerCommands.ts b/src/activate/registerCommands.ts index f90fc1e1d6..5e394befef 100644 --- a/src/activate/registerCommands.ts +++ b/src/activate/registerCommands.ts @@ -174,6 +174,35 @@ const getCommandsMap = ({ context, outputChannel, provider }: RegisterCommandOpt visibleProvider.postMessageToWebview({ type: "acceptInput" }) }, + "extension.reloadAllMcpServers": async () => { + const visibleProvider = getVisibleProviderOrLog(outputChannel) + if (!visibleProvider) { + return + } + try { + await visibleProvider.getMcpHub()?.restartAllMcpServers() + } catch (error) { + outputChannel.appendLine(`Failed to reload all MCP servers: ${error}`) + vscode.window.showErrorMessage(`Failed to reload all MCP servers: ${error}`) + } + }, + "extension.toggleAllMcpServersDisabled": async () => { + const visibleProvider = getVisibleProviderOrLog(outputChannel) + if (!visibleProvider) { + return + } + try { + const mcpHub = visibleProvider.getMcpHub() + if (mcpHub) { + const allServers = mcpHub.getAllServers() + const anyEnabled = allServers.some((server) => !server.disabled) + await mcpHub.toggleAllServersDisabled(anyEnabled) + } + } catch (error) { + outputChannel.appendLine(`Failed to toggle all MCP servers: ${error}`) + vscode.window.showErrorMessage(`Failed to toggle all MCP servers: ${error}`) + } + }, } } diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index c96615edd1..bbb82290c5 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -1255,5 +1255,16 @@ export const webviewMessageHandler = async (provider: ClineProvider, message: We await provider.postStateToWebview() break } + case "executeVSCodeCommand": { + if (message.command) { + try { + await vscode.commands.executeCommand(message.command) + } catch (error) { + provider.log(`Failed to execute VS Code command ${message.command}: ${error}`) + vscode.window.showErrorMessage(`Failed to execute command: ${message.command}`) + } + } + break + } } } diff --git a/src/services/mcp/McpHub.ts b/src/services/mcp/McpHub.ts index e83f7afb9e..2135319fce 100644 --- a/src/services/mcp/McpHub.ts +++ b/src/services/mcp/McpHub.ts @@ -1306,4 +1306,52 @@ export class McpHub { } this.disposables.forEach((d) => d.dispose()) } + + /** + * Enables or disables all global MCP servers at once. + * When activated, the configuration is reloaded. + * @param disabled true = disable all, false = enable all + */ + public async toggleAllServersDisabled(disabled: boolean): Promise { + // Collect all global server names + const allServers = this.getAllServers() + + // Set the Disabled flag for all servers + for (const server of allServers) { + await this.updateServerConfig(server.name, { disabled }, server.source) + const conn = this.findConnection(server.name, server.source) + if (conn) { + conn.server.disabled = disabled + } + } + + // If activated, reload configuration + if (!disabled) { + // Re-initialize all servers, both global and project + await this.initializeMcpServers("global") + await this.initializeMcpServers("project") + } + + await this.notifyWebviewOfServerChanges() + } + + /** + * Restarts all currently active MCP servers. + * This will trigger a popup for each server being restarted. + */ + public async restartAllMcpServers(): Promise { + const allServers = this.getAllServers() // Get all servers, regardless of disabled state + const restartPromises = allServers.map(async (server) => { + // Only restart if not disabled + if (!server.disabled) { + try { + await this.restartConnection(server.name, server.source) + } catch (error) { + this.showErrorMessage(`Failed to restart MCP server ${server.name}`, error) + } + } + }) + await Promise.all(restartPromises) + await this.notifyWebviewOfServerChanges() + } } diff --git a/src/shared/WebviewMessage.ts b/src/shared/WebviewMessage.ts index 6d6c832e75..3506d5d0b4 100644 --- a/src/shared/WebviewMessage.ts +++ b/src/shared/WebviewMessage.ts @@ -132,6 +132,7 @@ export interface WebviewMessage { | "toggleApiConfigPin" | "setHistoryPreviewCollapsed" | "condenseTaskContextRequest" + | "executeVSCodeCommand" text?: string disabled?: boolean askResponse?: ClineAskResponse @@ -161,6 +162,7 @@ export interface WebviewMessage { hasSystemPromptOverride?: boolean terminalOperation?: "continue" | "abort" historyPreviewCollapsed?: boolean + command?: string } export const checkoutDiffPayloadSchema = z.object({ diff --git a/webview-ui/src/components/mcp/McpView.tsx b/webview-ui/src/components/mcp/McpView.tsx index 7b6a89799b..9ce07f7757 100644 --- a/webview-ui/src/components/mcp/McpView.tsx +++ b/webview-ui/src/components/mcp/McpView.tsx @@ -105,6 +105,29 @@ const McpView = ({ onDone }: McpViewProps) => { +
+ + +
+ {/* Server List */} {servers.length > 0 && (
diff --git a/webview-ui/src/i18n/locales/de/mcp.json b/webview-ui/src/i18n/locales/de/mcp.json index 1140d443ac..ababea79cd 100644 --- a/webview-ui/src/i18n/locales/de/mcp.json +++ b/webview-ui/src/i18n/locales/de/mcp.json @@ -53,5 +53,7 @@ "serverStatus": { "retrying": "Wiederhole...", "retryConnection": "Verbindung wiederholen" - } + }, + "reloadAllServers": "Alle MCP-Server neu laden", + "toggleAllServers": "Alle MCP-Server aktivieren/deaktivieren" } diff --git a/webview-ui/src/i18n/locales/en/mcp.json b/webview-ui/src/i18n/locales/en/mcp.json index c7d6f851ff..77fa994427 100644 --- a/webview-ui/src/i18n/locales/en/mcp.json +++ b/webview-ui/src/i18n/locales/en/mcp.json @@ -53,5 +53,7 @@ "serverStatus": { "retrying": "Retrying...", "retryConnection": "Retry Connection" - } + }, + "reloadAllServers": "Reload All MCP Servers", + "toggleAllServers": "Toggle All MCP Servers Enabled/Disabled" } From e8f99e260bd9b7cf739d99e3c101af3302f77476 Mon Sep 17 00:00:00 2001 From: seedlord Date: Wed, 21 May 2025 03:36:47 +0200 Subject: [PATCH 06/11] Add tests for toggleAllServersDisabled and restartAllMcpServers methods in McpHub --- src/services/mcp/__tests__/McpHub.test.ts | 287 +++++++++++++++++++++- 1 file changed, 283 insertions(+), 4 deletions(-) diff --git a/src/services/mcp/__tests__/McpHub.test.ts b/src/services/mcp/__tests__/McpHub.test.ts index 182802e7df..cc94bb279e 100644 --- a/src/services/mcp/__tests__/McpHub.test.ts +++ b/src/services/mcp/__tests__/McpHub.test.ts @@ -4,7 +4,9 @@ import type { ExtensionContext, Uri } from "vscode" import { ServerConfigSchema } from "../McpHub" const fs = require("fs/promises") -const { McpHub } = require("../McpHub") +const { McpHub } = jest.requireActual("../McpHub") // Use requireActual to get the real module + +let originalConsoleError: typeof console.error = console.error // Store original console methods globally jest.mock("vscode", () => ({ workspace: { @@ -30,17 +32,39 @@ jest.mock("vscode", () => ({ jest.mock("fs/promises") jest.mock("../../../core/webview/ClineProvider") +// Mock the McpHub module itself +jest.mock("../McpHub", () => { + const originalModule = jest.requireActual("../McpHub") + return { + __esModule: true, + ...originalModule, + McpHub: jest.fn().mockImplementation((provider) => { + const instance = new originalModule.McpHub(provider) + // Spy on private methods + jest.spyOn(instance, "updateServerConfig" as any).mockResolvedValue(undefined) + jest.spyOn(instance, "findConnection" as any).mockReturnValue({ server: { disabled: false } } as any) + jest.spyOn(instance, "initializeMcpServers" as any).mockResolvedValue(undefined) + jest.spyOn(instance, "notifyWebviewOfServerChanges" as any).mockResolvedValue(undefined) + jest.spyOn(instance, "restartConnection" as any).mockResolvedValue(undefined) + jest.spyOn(instance, "showErrorMessage" as any).mockImplementation(jest.fn()) + jest.spyOn(instance, "getAllServers" as any).mockReturnValue([ + { name: "server1", source: "global", disabled: false, config: "{}", status: "connected" }, + { name: "server2", source: "project", disabled: false, config: "{}", status: "connected" }, + ]) + return instance + }), + } +}) + describe("McpHub", () => { let mcpHub: McpHubType let mockProvider: Partial - // Store original console methods - const originalConsoleError = console.error - beforeEach(() => { jest.clearAllMocks() // Mock console.error to suppress error messages during tests + originalConsoleError = console.error // Store original before mocking console.error = jest.fn() const mockUri: Uri = { @@ -317,6 +341,136 @@ describe("McpHub", () => { }) }) + describe("toggleAllServersDisabled", () => { + it("should disable all servers when passed true", async () => { + const mockConnections: McpConnection[] = [ + { + server: { + name: "server1", + config: "{}", + status: "connected", + disabled: false, + }, + client: {} as any, + transport: {} as any, + }, + { + server: { + name: "server2", + config: "{}", + status: "connected", + disabled: false, + }, + client: {} as any, + transport: {} as any, + }, + ] + mcpHub.connections = mockConnections + + // Mock fs.readFile to return a config with both servers enabled + ;(fs.readFile as jest.Mock).mockResolvedValueOnce( + JSON.stringify({ + mcpServers: { + server1: { disabled: false }, + server2: { disabled: false }, + }, + }), + ) + + await mcpHub.toggleAllServersDisabled(true) + + // Verify that both servers are now disabled in the connections + expect(mcpHub.connections[0].server.disabled).toBe(true) + expect(mcpHub.connections[1].server.disabled).toBe(true) + + // Mock fs.readFile and fs.writeFile to track config changes + let currentConfig = JSON.stringify({ + mcpServers: { + server1: { disabled: false }, + server2: { disabled: false }, + }, + }) + ;(fs.readFile as jest.Mock).mockImplementation(async () => currentConfig) + ;(fs.writeFile as jest.Mock).mockImplementation(async (path, data) => { + currentConfig = data + }) + + await mcpHub.toggleAllServersDisabled(true) + + // Verify that both servers are now disabled in the connections + expect(mcpHub.connections[0].server.disabled).toBe(true) + expect(mcpHub.connections[1].server.disabled).toBe(true) + + // Verify that fs.writeFile was called to persist the changes + const writtenConfig = JSON.parse(currentConfig) + expect(writtenConfig.mcpServers.server1.disabled).toBe(true) + expect(writtenConfig.mcpServers.server2.disabled).toBe(true) + + // Verify that postMessageToWebview was called + expect(mockProvider.postMessageToWebview).toHaveBeenCalledWith( + expect.objectContaining({ + type: "mcpServers", + }), + ) + }) + + it("should enable all servers when passed false", async () => { + const mockConnections: McpConnection[] = [ + { + server: { + name: "server1", + config: "{}", + status: "connected", + disabled: true, + }, + client: {} as any, + transport: {} as any, + }, + { + server: { + name: "server2", + config: "{}", + status: "connected", + disabled: true, + }, + client: {} as any, + transport: {} as any, + }, + ] + mcpHub.connections = mockConnections + + // Mock fs.readFile to return a config with both servers disabled + let currentConfig = JSON.stringify({ + mcpServers: { + server1: { disabled: true }, + server2: { disabled: true }, + }, + }) + ;(fs.readFile as jest.Mock).mockImplementation(async () => currentConfig) + ;(fs.writeFile as jest.Mock).mockImplementation(async (path, data) => { + currentConfig = data + }) + + await mcpHub.toggleAllServersDisabled(false) + + // Verify that both servers are now enabled in the connections + expect(mcpHub.connections[0].server.disabled).toBe(false) + expect(mcpHub.connections[1].server.disabled).toBe(false) + + // Verify that fs.writeFile was called to persist the changes + const writtenConfig = JSON.parse(currentConfig) + expect(writtenConfig.mcpServers.server1.disabled).toBe(false) + expect(writtenConfig.mcpServers.server2.disabled).toBe(false) + + // Verify that postMessageToWebview was called + expect(mockProvider.postMessageToWebview).toHaveBeenCalledWith( + expect.objectContaining({ + type: "mcpServers", + }), + ) + }) + }) + describe("callTool", () => { it("should execute tool successfully", async () => { // Mock the connection with a minimal client implementation @@ -560,4 +714,129 @@ describe("McpHub", () => { }) }) }) + + describe("restartAllMcpServers", () => { + let mcpHub: McpHubType + let mockProvider: Partial + + beforeEach(() => { + jest.clearAllMocks() + // Mock console.error to suppress error messages during tests + originalConsoleError = console.error // Store original before mocking + console.error = jest.fn() + + const mockUri: Uri = { + scheme: "file", + authority: "", + path: "/test/path", + query: "", + fragment: "", + fsPath: "/test/path", + with: jest.fn(), + toJSON: jest.fn(), + } + + mockProvider = { + ensureSettingsDirectoryExists: jest.fn().mockResolvedValue("/mock/settings/path"), + ensureMcpServersDirectoryExists: jest.fn().mockResolvedValue("/mock/settings/path"), + postMessageToWebview: jest.fn(), + context: { + subscriptions: [], + workspaceState: {} as any, + globalState: {} as any, + secrets: {} as any, + extensionUri: mockUri, + extensionPath: "/test/path", + storagePath: "/test/storage", + globalStoragePath: "/test/global-storage", + environmentVariableCollection: {} as any, + extension: { + id: "test-extension", + extensionUri: mockUri, + extensionPath: "/test/path", + extensionKind: 1, + isActive: true, + packageJSON: { + version: "1.0.0", + }, + activate: jest.fn(), + exports: undefined, + } as any, + asAbsolutePath: (path: string) => path, + storageUri: mockUri, + globalStorageUri: mockUri, + logUri: mockUri, + extensionMode: 1, + logPath: "/test/path", + languageModelAccessInformation: {} as any, + } as ExtensionContext, + } + + // Mock fs.readFile for initial settings + ;(fs.readFile as jest.Mock).mockResolvedValue( + JSON.stringify({ + mcpServers: { + "test-server": { + type: "stdio", + command: "node", + args: ["test.js"], + alwaysAllow: ["allowed-tool"], + }, + }, + }), + ) + + mcpHub = new McpHub(mockProvider as ClineProvider) + jest.spyOn(mcpHub as any, "showErrorMessage").mockImplementation(jest.fn()) + + // Mock internal methods + jest.spyOn(mcpHub, "getAllServers" as any).mockReturnValue([ + { name: "server1", source: "global", disabled: false }, + { name: "server2", source: "project", disabled: true }, // Disabled server + { name: "server3", source: "global", disabled: false }, + ]) + jest.spyOn(mcpHub, "restartConnection" as any).mockResolvedValue(undefined) + jest.spyOn(mcpHub as any, "notifyWebviewOfServerChanges").mockResolvedValue(undefined) + }) + + afterEach(() => { + // Restore original console methods + console.error = originalConsoleError + jest.restoreAllMocks() // Clean up spies + }) + + it("should restart only active servers", async () => { + await mcpHub.restartAllMcpServers() + + expect(mcpHub.getAllServers).toHaveBeenCalled() + expect(mcpHub.restartConnection).toHaveBeenCalledTimes(2) // Only server1 and server3 should be restarted + expect(mcpHub.restartConnection).toHaveBeenCalledWith("server1", "global") + expect(mcpHub.restartConnection).not.toHaveBeenCalledWith("server2", "project") + expect(mcpHub.restartConnection).toHaveBeenCalledWith("server3", "global") + expect((mcpHub as any).notifyWebviewOfServerChanges).toHaveBeenCalledTimes(1) + }) + + it("should call showErrorMessage if a restart fails", async () => { + jest.spyOn(mcpHub, "restartConnection" as any).mockRejectedValueOnce(new Error("Restart failed")) + + await mcpHub.restartAllMcpServers() + + expect(mcpHub.getAllServers).toHaveBeenCalled() + expect(mcpHub.restartConnection).toHaveBeenCalledTimes(2) // Only active servers are attempted to restart + expect((mcpHub as any).showErrorMessage).toHaveBeenCalledTimes(1) + expect((mcpHub as any).showErrorMessage).toHaveBeenCalledWith( + "Failed to restart MCP server server1", + expect.any(Error), + ) + expect((mcpHub as any).notifyWebviewOfServerChanges).toHaveBeenCalledTimes(1) + }) + + it("should call notifyWebviewOfServerChanges even if some restarts fail", async () => { + jest.spyOn(mcpHub, "restartConnection").mockRejectedValue(new Error("Restart failed")) + + await mcpHub.restartAllMcpServers() + + expect((mcpHub as any).notifyWebviewOfServerChanges).toHaveBeenCalledTimes(1) + }) + }) }) From cc278a6602c2f425579bf01b3067fc36f75ee7bf Mon Sep 17 00:00:00 2001 From: seedlord Date: Wed, 21 May 2025 04:14:46 +0200 Subject: [PATCH 07/11] Add new commands for reloading and toggling MCP server states --- src/schemas/index.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/schemas/index.ts b/src/schemas/index.ts index 9e47050ac1..daa6a90453 100644 --- a/src/schemas/index.ts +++ b/src/schemas/index.ts @@ -57,6 +57,8 @@ const commandIds = [ "setCustomStoragePath", "focusInput", "acceptInput", + "extension.reloadAllMcpServers", + "extension.toggleAllMcpServersDisabled", ] as const export type CommandId = (typeof commandIds)[number] From 8856dabd2c311f70d53fa1833883f6f4c6e5e423 Mon Sep 17 00:00:00 2001 From: seedlord Date: Thu, 22 May 2025 06:20:40 +0200 Subject: [PATCH 08/11] refactor: update command identifiers to include extension prefix --- package.json | 4 ++-- src/activate/registerCommands.ts | 4 ++-- src/schemas/index.ts | 4 ++-- webview-ui/src/components/mcp/McpView.tsx | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/package.json b/package.json index cff70a5558..95c96a2d84 100644 --- a/package.json +++ b/package.json @@ -161,12 +161,12 @@ "category": "%configuration.title%" }, { - "command": "extension.reloadAllMcpServers", + "command": "roo-cline.reloadAllMcpServers", "title": "Reload All MCP Servers", "icon": "$(sync)" }, { - "command": "extension.toggleAllMcpServersDisabled", + "command": "roo-cline.toggleAllMcpServersDisabled", "title": "Toggle All MCP Servers Enabled/Disabled", "icon": "$(power)" } diff --git a/src/activate/registerCommands.ts b/src/activate/registerCommands.ts index 88ab0cb382..03f008ce2b 100644 --- a/src/activate/registerCommands.ts +++ b/src/activate/registerCommands.ts @@ -176,7 +176,7 @@ const getCommandsMap = ({ context, outputChannel, provider }: RegisterCommandOpt visibleProvider.postMessageToWebview({ type: "acceptInput" }) }, - "extension.reloadAllMcpServers": async () => { + reloadAllMcpServers: async () => { const visibleProvider = getVisibleProviderOrLog(outputChannel) if (!visibleProvider) { return @@ -188,7 +188,7 @@ const getCommandsMap = ({ context, outputChannel, provider }: RegisterCommandOpt vscode.window.showErrorMessage(`Failed to reload all MCP servers: ${error}`) } }, - "extension.toggleAllMcpServersDisabled": async () => { + toggleAllMcpServersDisabled: async () => { const visibleProvider = getVisibleProviderOrLog(outputChannel) if (!visibleProvider) { return diff --git a/src/schemas/index.ts b/src/schemas/index.ts index 861c1e8ca0..f10a0b5f6e 100644 --- a/src/schemas/index.ts +++ b/src/schemas/index.ts @@ -74,8 +74,8 @@ export const commandIds = [ "focusInput", "acceptInput", - "extension.reloadAllMcpServers", - "extension.toggleAllMcpServersDisabled", + "reloadAllMcpServers", + "toggleAllMcpServersDisabled", ] as const export type CommandId = (typeof commandIds)[number] diff --git a/webview-ui/src/components/mcp/McpView.tsx b/webview-ui/src/components/mcp/McpView.tsx index 9ce07f7757..093904ce29 100644 --- a/webview-ui/src/components/mcp/McpView.tsx +++ b/webview-ui/src/components/mcp/McpView.tsx @@ -110,7 +110,7 @@ const McpView = ({ onDone }: McpViewProps) => { onClick={() => vscode.postMessage({ type: "executeVSCodeCommand", - command: "extension.reloadAllMcpServers", + command: "roo-cline.reloadAllMcpServers", }) } data-testid="reload-all-mcp-servers-button"> @@ -120,7 +120,7 @@ const McpView = ({ onDone }: McpViewProps) => { onClick={() => vscode.postMessage({ type: "executeVSCodeCommand", - command: "extension.toggleAllMcpServersDisabled", + command: "roo-cline.toggleAllMcpServersDisabled", }) } data-testid="toggle-all-mcp-servers-button"> From 8d937c3946cc82321d757b480aba0ce188704013 Mon Sep 17 00:00:00 2001 From: seedlord Date: Thu, 22 May 2025 06:24:50 +0200 Subject: [PATCH 09/11] refactor: optimize server configuration updates using Promise.all --- src/services/mcp/McpHub.ts | 16 +++++---- src/services/mcp/__tests__/McpHub.test.ts | 44 ++++++++++------------- 2 files changed, 28 insertions(+), 32 deletions(-) diff --git a/src/services/mcp/McpHub.ts b/src/services/mcp/McpHub.ts index 2135319fce..87033cedbd 100644 --- a/src/services/mcp/McpHub.ts +++ b/src/services/mcp/McpHub.ts @@ -1317,13 +1317,15 @@ export class McpHub { const allServers = this.getAllServers() // Set the Disabled flag for all servers - for (const server of allServers) { - await this.updateServerConfig(server.name, { disabled }, server.source) - const conn = this.findConnection(server.name, server.source) - if (conn) { - conn.server.disabled = disabled - } - } + await Promise.all( + allServers.map(async (server) => { + await this.updateServerConfig(server.name, { disabled }, server.source) + const conn = this.findConnection(server.name, server.source) + if (conn) { + conn.server.disabled = disabled + } + }), + ) // If activated, reload configuration if (!disabled) { diff --git a/src/services/mcp/__tests__/McpHub.test.ts b/src/services/mcp/__tests__/McpHub.test.ts index cc94bb279e..e616869461 100644 --- a/src/services/mcp/__tests__/McpHub.test.ts +++ b/src/services/mcp/__tests__/McpHub.test.ts @@ -384,15 +384,12 @@ describe("McpHub", () => { expect(mcpHub.connections[1].server.disabled).toBe(true) // Mock fs.readFile and fs.writeFile to track config changes - let currentConfig = JSON.stringify({ - mcpServers: { - server1: { disabled: false }, - server2: { disabled: false }, - }, - }) - ;(fs.readFile as jest.Mock).mockImplementation(async () => currentConfig) + const writeCalls: any[] = [] + ;(fs.readFile as jest.Mock).mockResolvedValue( + JSON.stringify({ mcpServers: { server1: { disabled: false }, server2: { disabled: false } } }), + ) ;(fs.writeFile as jest.Mock).mockImplementation(async (path, data) => { - currentConfig = data + writeCalls.push(JSON.parse(data)) }) await mcpHub.toggleAllServersDisabled(true) @@ -401,10 +398,10 @@ describe("McpHub", () => { expect(mcpHub.connections[0].server.disabled).toBe(true) expect(mcpHub.connections[1].server.disabled).toBe(true) - // Verify that fs.writeFile was called to persist the changes - const writtenConfig = JSON.parse(currentConfig) - expect(writtenConfig.mcpServers.server1.disabled).toBe(true) - expect(writtenConfig.mcpServers.server2.disabled).toBe(true) + // Verify that fs.writeFile was called for each server + expect(writeCalls.length).toBe(2) + expect(writeCalls[0].mcpServers.server1.disabled).toBe(true) + expect(writeCalls[1].mcpServers.server2.disabled).toBe(true) // Verify that postMessageToWebview was called expect(mockProvider.postMessageToWebview).toHaveBeenCalledWith( @@ -439,16 +436,13 @@ describe("McpHub", () => { ] mcpHub.connections = mockConnections - // Mock fs.readFile to return a config with both servers disabled - let currentConfig = JSON.stringify({ - mcpServers: { - server1: { disabled: true }, - server2: { disabled: true }, - }, - }) - ;(fs.readFile as jest.Mock).mockImplementation(async () => currentConfig) + // Mock fs.readFile and fs.writeFile to track config changes + const writeCalls: any[] = [] + ;(fs.readFile as jest.Mock).mockResolvedValue( + JSON.stringify({ mcpServers: { server1: { disabled: true }, server2: { disabled: true } } }), + ) ;(fs.writeFile as jest.Mock).mockImplementation(async (path, data) => { - currentConfig = data + writeCalls.push(JSON.parse(data)) }) await mcpHub.toggleAllServersDisabled(false) @@ -457,10 +451,10 @@ describe("McpHub", () => { expect(mcpHub.connections[0].server.disabled).toBe(false) expect(mcpHub.connections[1].server.disabled).toBe(false) - // Verify that fs.writeFile was called to persist the changes - const writtenConfig = JSON.parse(currentConfig) - expect(writtenConfig.mcpServers.server1.disabled).toBe(false) - expect(writtenConfig.mcpServers.server2.disabled).toBe(false) + // Verify that fs.writeFile was called for each server + expect(writeCalls.length).toBe(2) + expect(writeCalls[0].mcpServers.server1.disabled).toBe(false) + expect(writeCalls[1].mcpServers.server2.disabled).toBe(false) // Verify that postMessageToWebview was called expect(mockProvider.postMessageToWebview).toHaveBeenCalledWith( From fe0a70e1f7792bd18e53c721bee284c8df91fea2 Mon Sep 17 00:00:00 2001 From: seedlord Date: Thu, 22 May 2025 07:23:07 +0200 Subject: [PATCH 10/11] revert Promise.all for toggleAllServerDisabled --- src/services/mcp/McpHub.ts | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/services/mcp/McpHub.ts b/src/services/mcp/McpHub.ts index 87033cedbd..2135319fce 100644 --- a/src/services/mcp/McpHub.ts +++ b/src/services/mcp/McpHub.ts @@ -1317,15 +1317,13 @@ export class McpHub { const allServers = this.getAllServers() // Set the Disabled flag for all servers - await Promise.all( - allServers.map(async (server) => { - await this.updateServerConfig(server.name, { disabled }, server.source) - const conn = this.findConnection(server.name, server.source) - if (conn) { - conn.server.disabled = disabled - } - }), - ) + for (const server of allServers) { + await this.updateServerConfig(server.name, { disabled }, server.source) + const conn = this.findConnection(server.name, server.source) + if (conn) { + conn.server.disabled = disabled + } + } // If activated, reload configuration if (!disabled) { From f0596acb246b71427042a11179c6a37927122992 Mon Sep 17 00:00:00 2001 From: seedlord Date: Fri, 23 May 2025 01:30:42 +0200 Subject: [PATCH 11/11] revert eslint changes --- webview-ui/eslint.config.mjs | 1 - webview-ui/package.json | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/webview-ui/eslint.config.mjs b/webview-ui/eslint.config.mjs index a910b17fec..208700018b 100644 --- a/webview-ui/eslint.config.mjs +++ b/webview-ui/eslint.config.mjs @@ -6,7 +6,6 @@ export default [ { rules: { "@typescript-eslint/no-unused-vars": "off", - "@typescript-eslint/no-unused-expressions": "off", "@typescript-eslint/no-explicit-any": "off", "react/prop-types": "off", "react/display-name": "off", diff --git a/webview-ui/package.json b/webview-ui/package.json index 0352e04454..cb9e5c91e3 100644 --- a/webview-ui/package.json +++ b/webview-ui/package.json @@ -3,7 +3,7 @@ "private": true, "type": "module", "scripts": { - "lint": "eslint src --max-warnings=0", + "lint": "eslint src --ext=ts,tsx --max-warnings=0", "check-types": "tsc", "test": "jest -w=40%", "format": "prettier --write src",