Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 26 additions & 7 deletions src/services/mcp/McpHub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
126 changes: 126 additions & 0 deletions src/services/mcp/__tests__/McpHub.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 [email protected]", "--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: ["[email protected]", "--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 [email protected]", "--config", "test.json"],
}),
)
})
})
})