refactor(mcp): use getDefaultEnvironment for stdio client transport (#4259)

- Replace manual PATH and HOME env var handling with getDefaultEnvironment()
- Improves consistency and reliability of MCP client environment setup
- Leverages SDK's built-in environment configuration

Author: samhvw8

src/services/mcp/McpHub.ts

@@ -1,5 +1,5 @@
 import { Client } from "@modelcontextprotocol/sdk/client/index.js"
-import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js"
+import { StdioClientTransport, getDefaultEnvironment } from "@modelcontextprotocol/sdk/client/stdio.js"
 import { SSEClientTransport } from "@modelcontextprotocol/sdk/client/sse.js"
 import { StreamableHTTPClientTransport } from "@modelcontextprotocol/sdk/client/streamableHttp.js"
 import ReconnectingEventSource from "reconnecting-eventsource"
@@ -483,9 +483,8 @@ export class McpHub {
 					args: configInjected.args,
 					cwd: configInjected.cwd,
 					env: {
+						...getDefaultEnvironment(),
 						...(configInjected.env || {}),
-						...(process.env.PATH ? { PATH: process.env.PATH } : {}),
-						...(process.env.HOME ? { HOME: process.env.HOME } : {}),
 					},
 					stderr: "pipe",
 				})

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

@@ -128,6 +128,20 @@ describe("McpHub", () => {
 			// Mock reading initial config
 			;(fs.readFile as jest.Mock).mockResolvedValueOnce(JSON.stringify(mockConfig))
 
+			// Set up mock connection without alwaysAllow
+			const mockConnection: McpConnection = {
+				server: {
+					name: "test-server",
+					type: "stdio",
+					command: "node",
+					args: ["test.js"],
+					source: "global",
+				} as any,
+				client: {} as any,
+				transport: {} as any,
+			}
+			mcpHub.connections = [mockConnection]
+
 			await mcpHub.toggleToolAlwaysAllow("test-server", "global", "new-tool", true)
 
 			// Verify the config was updated correctly
@@ -162,6 +176,21 @@ describe("McpHub", () => {
 			// Mock reading initial config
 			;(fs.readFile as jest.Mock).mockResolvedValueOnce(JSON.stringify(mockConfig))
 
+			// Set up mock connection
+			const mockConnection: McpConnection = {
+				server: {
+					name: "test-server",
+					type: "stdio",
+					command: "node",
+					args: ["test.js"],
+					alwaysAllow: ["existing-tool"],
+					source: "global",
+				} as any,
+				client: {} as any,
+				transport: {} as any,
+			}
+			mcpHub.connections = [mockConnection]
+
 			await mcpHub.toggleToolAlwaysAllow("test-server", "global", "existing-tool", false)
 
 			// Verify the config was updated correctly
@@ -195,6 +224,21 @@ describe("McpHub", () => {
 			// Mock reading initial config
 			;(fs.readFile as jest.Mock).mockResolvedValueOnce(JSON.stringify(mockConfig))
 
+			// Set up mock connection
+			const mockConnection: McpConnection = {
+				server: {
+					name: "test-server",
+					type: "stdio",
+					command: "node",
+					args: ["test.js"],
+					alwaysAllow: [],
+					source: "global",
+				} as any,
+				client: {} as any,
+				transport: {} as any,
+			}
+			mcpHub.connections = [mockConnection]
+
 			await mcpHub.toggleToolAlwaysAllow("test-server", "global", "new-tool", true)
 
 			// Verify the config was updated with initialized alwaysAllow
@@ -228,6 +272,21 @@ describe("McpHub", () => {
 			// Mock reading initial config
 			;(fs.readFile as jest.Mock).mockResolvedValueOnce(JSON.stringify(mockConfig))
 
+			// Set up mock connection
+			const mockConnection: McpConnection = {
+				server: {
+					name: "test-server",
+					type: "stdio",
+					command: "node",
+					args: ["test.js"],
+					disabled: false,
+					source: "global",
+				} as any,
+				client: {} as any,
+				transport: {} as any,
+			}
+			mcpHub.connections = [mockConnection]
+
 			await mcpHub.toggleServerDisabled("test-server", true)
 
 			// Verify the config was updated correctly
@@ -445,6 +504,21 @@ describe("McpHub", () => {
 				// Mock reading initial config
 				;(fs.readFile as jest.Mock).mockResolvedValueOnce(JSON.stringify(mockConfig))
 
+				// Set up mock connection
+				const mockConnection: McpConnection = {
+					server: {
+						name: "test-server",
+						type: "stdio",
+						command: "node",
+						args: ["test.js"],
+						timeout: 60,
+						source: "global",
+					} as any,
+					client: {} as any,
+					transport: {} as any,
+				}
+				mcpHub.connections = [mockConnection]
+
 				await mcpHub.updateServerTimeout("test-server", 120)
 
 				// Verify the config was updated correctly
@@ -475,14 +549,31 @@ describe("McpHub", () => {
 				// Mock initial read
 				;(fs.readFile as jest.Mock).mockResolvedValueOnce(JSON.stringify(mockConfig))
 
+				// Set up mock connection before updating
+				const mockConnectionInitial: McpConnection = {
+					server: {
+						name: "test-server",
+						type: "stdio",
+						command: "node",
+						args: ["test.js"],
+						timeout: 60,
+						source: "global",
+					} as any,
+					client: {
+						request: jest.fn().mockResolvedValue({ content: [] }),
+					} as any,
+					transport: {} as any,
+				}
+				mcpHub.connections = [mockConnectionInitial]
+
 				// Update with invalid timeout
 				await mcpHub.updateServerTimeout("test-server", 3601)
 
 				// Config is written
 				expect(fs.writeFile).toHaveBeenCalled()
 
 				// Setup connection with invalid timeout
-				const mockConnection: McpConnection = {
+				const mockConnectionInvalid: McpConnection = {
 					server: {
 						name: "test-server",
 						config: JSON.stringify({
@@ -499,13 +590,13 @@ describe("McpHub", () => {
 					transport: {} as any,
 				}
 
-				mcpHub.connections = [mockConnection]
+				mcpHub.connections = [mockConnectionInvalid]
 
 				// Call tool - should use default timeout
 				await mcpHub.callTool("test-server", "test-tool")
 
 				// Verify default timeout was used
-				expect(mockConnection.client.request).toHaveBeenCalledWith(
+				expect(mockConnectionInvalid.client.request).toHaveBeenCalledWith(
 					expect.anything(),
 					expect.anything(),
 					expect.objectContaining({ timeout: 60000 }), // Default 60 seconds
@@ -526,6 +617,21 @@ describe("McpHub", () => {
 
 				;(fs.readFile as jest.Mock).mockResolvedValueOnce(JSON.stringify(mockConfig))
 
+				// Set up mock connection
+				const mockConnection: McpConnection = {
+					server: {
+						name: "test-server",
+						type: "stdio",
+						command: "node",
+						args: ["test.js"],
+						timeout: 60,
+						source: "global",
+					} as any,
+					client: {} as any,
+					transport: {} as any,
+				}
+				mcpHub.connections = [mockConnection]
+
 				// Test valid timeout values
 				const validTimeouts = [1, 60, 3600]
 				for (const timeout of validTimeouts) {
@@ -550,6 +656,21 @@ describe("McpHub", () => {
 
 				;(fs.readFile as jest.Mock).mockResolvedValueOnce(JSON.stringify(mockConfig))
 
+				// Set up mock connection
+				const mockConnection: McpConnection = {
+					server: {
+						name: "test-server",
+						type: "stdio",
+						command: "node",
+						args: ["test.js"],
+						timeout: 60,
+						source: "global",
+					} as any,
+					client: {} as any,
+					transport: {} as any,
+				}
+				mcpHub.connections = [mockConnection]
+
 				await mcpHub.updateServerTimeout("test-server", 120)
 
 				expect(mockProvider.postMessageToWebview).toHaveBeenCalledWith(