diff --git a/src/services/mcp/McpHub.ts b/src/services/mcp/McpHub.ts index f5ddf3e57b..6e6015e21f 100644 --- a/src/services/mcp/McpHub.ts +++ b/src/services/mcp/McpHub.ts @@ -677,11 +677,30 @@ export class McpHub { const isAlreadyWrapped = configInjected.command.toLowerCase() === "cmd.exe" || configInjected.command.toLowerCase() === "cmd" - const command = isWindows && !isAlreadyWrapped ? "cmd.exe" : configInjected.command - const args = - isWindows && !isAlreadyWrapped - ? ["/c", configInjected.command, ...(configInjected.args || [])] - : configInjected.args + // Special handling for npx with package@version syntax + const isNpxWithPackage = + configInjected.command.toLowerCase() === "npx" && + configInjected.args && + configInjected.args.length > 0 && + configInjected.args[0].includes("@") + + let command = configInjected.command + let args = configInjected.args + + if (isWindows && !isAlreadyWrapped) { + if (isNpxWithPackage) { + // For npx with package@version, combine npx and package name into a single command string + // This ensures proper package resolution on Windows + const packageName = configInjected.args![0] + const remainingArgs = configInjected.args!.slice(1) + command = "cmd.exe" + args = ["/c", `npx ${packageName}`, ...remainingArgs] + } else { + // Standard wrapping for other commands + command = "cmd.exe" + args = ["/c", configInjected.command, ...(configInjected.args || [])] + } + } transport = new StdioClientTransport({ command, @@ -1378,8 +1397,8 @@ export class McpHub { await this.deleteConnection(serverName, serverSource) // Re-add as a disabled connection // Re-read config from file to get updated disabled state - const updatedConfig = await this.readServerConfigFromFile(serverName, serverSource) - await this.connectToServer(serverName, updatedConfig, serverSource) + const updatedConfig = await this.readServerConfigFromFile(serverName, serverSource) + await this.connectToServer(serverName, updatedConfig, serverSource) } else if (!disabled && connection.server.status === "disconnected") { // If enabling a disabled server, connect it // Re-read config from file to get updated disabled state diff --git a/src/services/mcp/__tests__/McpHub.spec.ts b/src/services/mcp/__tests__/McpHub.spec.ts index 1db924ed6c..8f52167da7 100644 --- a/src/services/mcp/__tests__/McpHub.spec.ts +++ b/src/services/mcp/__tests__/McpHub.spec.ts @@ -2146,5 +2146,131 @@ describe("McpHub", () => { }), ) }) + + it("should handle npx with package@version syntax on Windows", async () => { + // Mock Windows platform + Object.defineProperty(process, "platform", { + value: "win32", + writable: true, + enumerable: true, + configurable: true, + }) + + // Mock StdioClientTransport + const mockTransport = { + start: vi.fn().mockResolvedValue(undefined), + close: vi.fn().mockResolvedValue(undefined), + stderr: { + on: vi.fn(), + }, + onerror: null, + onclose: null, + } + + StdioClientTransport.mockImplementation((config: any) => { + // Verify that npx with package@version is properly wrapped + expect(config.command).toBe("cmd.exe") + // The package@version should be combined with npx in a single string + expect(config.args).toEqual(["/c", "npx chrome-devtools-mcp@latest"]) + return mockTransport + }) + + // Mock Client + Client.mockImplementation(() => ({ + connect: vi.fn().mockResolvedValue(undefined), + close: vi.fn().mockResolvedValue(undefined), + getInstructions: vi.fn().mockReturnValue("test instructions"), + request: vi.fn().mockResolvedValue({ tools: [], resources: [], resourceTemplates: [] }), + })) + + // Create a new McpHub instance + const mcpHub = new McpHub(mockProvider as ClineProvider) + + // Mock the config file read with chrome-devtools-mcp@latest + vi.mocked(fs.readFile).mockResolvedValue( + JSON.stringify({ + mcpServers: { + "chrome-devtools": { + command: "npx", + args: ["chrome-devtools-mcp@latest"], + }, + }, + }), + ) + + // Initialize servers (this will trigger connectToServer) + await mcpHub["initializeGlobalMcpServers"]() + + // Verify StdioClientTransport was called with properly wrapped command + expect(StdioClientTransport).toHaveBeenCalledWith( + expect.objectContaining({ + command: "cmd.exe", + args: ["/c", "npx chrome-devtools-mcp@latest"], + }), + ) + }) + + it("should handle npx with package@version and additional args on Windows", async () => { + // Mock Windows platform + Object.defineProperty(process, "platform", { + value: "win32", + writable: true, + enumerable: true, + configurable: true, + }) + + // Mock StdioClientTransport + const mockTransport = { + start: vi.fn().mockResolvedValue(undefined), + close: vi.fn().mockResolvedValue(undefined), + stderr: { + on: vi.fn(), + }, + onerror: null, + onclose: null, + } + + StdioClientTransport.mockImplementation((config: any) => { + // Verify that npx with package@version and additional args is properly wrapped + expect(config.command).toBe("cmd.exe") + // The package@version should be combined with npx, and additional args follow + expect(config.args).toEqual(["/c", "npx some-package@1.2.3", "--config", "test.json"]) + return mockTransport + }) + + // Mock Client + Client.mockImplementation(() => ({ + connect: vi.fn().mockResolvedValue(undefined), + close: vi.fn().mockResolvedValue(undefined), + getInstructions: vi.fn().mockReturnValue("test instructions"), + request: vi.fn().mockResolvedValue({ tools: [], resources: [], resourceTemplates: [] }), + })) + + // Create a new McpHub instance + const mcpHub = new McpHub(mockProvider as ClineProvider) + + // Mock the config file read with package@version and additional args + vi.mocked(fs.readFile).mockResolvedValue( + JSON.stringify({ + mcpServers: { + "test-server": { + command: "npx", + args: ["some-package@1.2.3", "--config", "test.json"], + }, + }, + }), + ) + + // Initialize servers (this will trigger connectToServer) + await mcpHub["initializeGlobalMcpServers"]() + + // Verify StdioClientTransport was called with properly wrapped command + expect(StdioClientTransport).toHaveBeenCalledWith( + expect.objectContaining({ + command: "cmd.exe", + args: ["/c", "npx some-package@1.2.3", "--config", "test.json"], + }), + ) + }) }) })