fix: resolve MCP server execution on Windows with node version managers (#1246)

- Wrap commands with cmd.exe /c on Windows to handle non-exe executables
- Add platform detection for Windows-specific command handling
- Add comprehensive tests for Windows command wrapping behavior
- Maintain backward compatibility for non-Windows platforms

This fixes the 'spawn npx ENOENT' error that occurs when using node version
managers like fnm on Windows, where npx is a PowerShell script rather than
an executable file.

Author: hannesrudolph

src/services/mcp/McpHub.ts

@@ -586,9 +586,16 @@ export class McpHub {
 			})) as typeof config
 
 			if (configInjected.type === "stdio") {
+				// On Windows, wrap commands with cmd.exe to handle non-exe executables like npx.ps1
+				const isWindows = process.platform === "win32"
+				const command = isWindows ? "cmd.exe" : configInjected.command
+				const args = isWindows
+					? ["/c", configInjected.command, ...(configInjected.args || [])]
+					: configInjected.args
+
 				transport = new StdioClientTransport({
-					command: configInjected.command,
-					args: configInjected.args,
+					command,
+					args,
 					cwd: configInjected.cwd,
 					env: {
 						...getDefaultEnvironment(),

src/services/mcp/__tests__/McpHub.test.ts

@@ -33,12 +33,23 @@ jest.mock("vscode", () => ({
 jest.mock("fs/promises")
 jest.mock("../../../core/webview/ClineProvider")
 
+// Mock the MCP SDK modules
+jest.mock("@modelcontextprotocol/sdk/client/stdio", () => ({
+	StdioClientTransport: jest.fn(),
+	getDefaultEnvironment: jest.fn().mockReturnValue({ PATH: "/usr/bin" }),
+}))
+
+jest.mock("@modelcontextprotocol/sdk/client/index", () => ({
+	Client: jest.fn(),
+}))
+
 describe("McpHub", () => {
 	let mcpHub: McpHubType
 	let mockProvider: Partial<ClineProvider>
 
 	// Store original console methods
 	const originalConsoleError = console.error
+	const originalPlatform = Object.getOwnPropertyDescriptor(process, "platform")
 
 	beforeEach(() => {
 		jest.clearAllMocks()
@@ -113,6 +124,10 @@ describe("McpHub", () => {
 	afterEach(() => {
 		// Restore original console methods
 		console.error = originalConsoleError
+		// Restore original platform
+		if (originalPlatform) {
+			Object.defineProperty(process, "platform", originalPlatform)
+		}
 	})
 
 	describe("toggleToolAlwaysAllow", () => {
@@ -684,4 +699,148 @@ describe("McpHub", () => {
 			})
 		})
 	})
+
+	describe("Windows command wrapping", () => {
+		let StdioClientTransport: jest.Mock
+		let Client: jest.Mock
+
+		beforeEach(() => {
+			// Reset mocks
+			jest.clearAllMocks()
+
+			// Get references to the mocked constructors
+			StdioClientTransport = require("@modelcontextprotocol/sdk/client/stdio").StdioClientTransport as jest.Mock
+			Client = require("@modelcontextprotocol/sdk/client/index").Client as jest.Mock
+
+			// Mock Windows platform
+			Object.defineProperty(process, "platform", {
+				value: "win32",
+				writable: true,
+				enumerable: true,
+				configurable: true,
+			})
+		})
+
+		it("should wrap commands with cmd.exe on Windows", async () => {
+			// Mock StdioClientTransport
+			const mockTransport = {
+				start: jest.fn().mockResolvedValue(undefined),
+				close: jest.fn().mockResolvedValue(undefined),
+				stderr: {
+					on: jest.fn(),
+				},
+				onerror: null,
+				onclose: null,
+			}
+
+			StdioClientTransport.mockImplementation((config: any) => {
+				// Verify that cmd.exe wrapping is applied
+				expect(config.command).toBe("cmd.exe")
+				expect(config.args).toEqual([
+					"/c",
+					"npx",
+					"-y",
+					"@modelcontextprotocol/server-filesystem",
+					"/test/path",
+				])
+				return mockTransport
+			})
+
+			// Mock Client
+			Client.mockImplementation(() => ({
+				connect: jest.fn().mockResolvedValue(undefined),
+				close: jest.fn().mockResolvedValue(undefined),
+				getInstructions: jest.fn().mockReturnValue("test instructions"),
+				request: jest.fn().mockResolvedValue({ tools: [], resources: [], resourceTemplates: [] }),
+			}))
+
+			// Create a new McpHub instance
+			const mcpHub = new McpHub(mockProvider as ClineProvider)
+
+			// Mock the config file read
+			;(fs.readFile as jest.Mock).mockResolvedValue(
+				JSON.stringify({
+					mcpServers: {
+						"test-npx-server": {
+							command: "npx",
+							args: ["-y", "@modelcontextprotocol/server-filesystem", "/test/path"],
+						},
+					},
+				}),
+			)
+
+			// Initialize servers (this will trigger connectToServer)
+			await mcpHub["initializeGlobalMcpServers"]()
+
+			// Verify StdioClientTransport was called with wrapped command
+			expect(StdioClientTransport).toHaveBeenCalledWith(
+				expect.objectContaining({
+					command: "cmd.exe",
+					args: ["/c", "npx", "-y", "@modelcontextprotocol/server-filesystem", "/test/path"],
+				}),
+			)
+		})
+
+		it("should not wrap commands on non-Windows platforms", async () => {
+			// Mock non-Windows platform
+			Object.defineProperty(process, "platform", {
+				value: "darwin",
+				writable: true,
+				enumerable: true,
+				configurable: true,
+			})
+
+			// Mock StdioClientTransport
+			const mockTransport = {
+				start: jest.fn().mockResolvedValue(undefined),
+				close: jest.fn().mockResolvedValue(undefined),
+				stderr: {
+					on: jest.fn(),
+				},
+				onerror: null,
+				onclose: null,
+			}
+
+			StdioClientTransport.mockImplementation((config: any) => {
+				// Verify that no cmd.exe wrapping is applied
+				expect(config.command).toBe("npx")
+				expect(config.args).toEqual(["-y", "@modelcontextprotocol/server-filesystem", "/test/path"])
+				return mockTransport
+			})
+
+			// Mock Client
+			Client.mockImplementation(() => ({
+				connect: jest.fn().mockResolvedValue(undefined),
+				close: jest.fn().mockResolvedValue(undefined),
+				getInstructions: jest.fn().mockReturnValue("test instructions"),
+				request: jest.fn().mockResolvedValue({ tools: [], resources: [], resourceTemplates: [] }),
+			}))
+
+			// Create a new McpHub instance
+			const mcpHub = new McpHub(mockProvider as ClineProvider)
+
+			// Mock the config file read
+			;(fs.readFile as jest.Mock).mockResolvedValue(
+				JSON.stringify({
+					mcpServers: {
+						"test-npx-server": {
+							command: "npx",
+							args: ["-y", "@modelcontextprotocol/server-filesystem", "/test/path"],
+						},
+					},
+				}),
+			)
+
+			// Initialize servers (this will trigger connectToServer)
+			await mcpHub["initializeGlobalMcpServers"]()
+
+			// Verify StdioClientTransport was called without wrapping
+			expect(StdioClientTransport).toHaveBeenCalledWith(
+				expect.objectContaining({
+					command: "npx",
+					args: ["-y", "@modelcontextprotocol/server-filesystem", "/test/path"],
+				}),
+			)
+		})
+	})
 })