fix(core): normalize Windows PATH for MCP stdio servers (#3451)

* fix(core): normalize Windows PATH for MCP stdio servers

* test(core): stabilize MCP stdio env assertion on Windows

* refactor(core): extract shared windowsPath utility and fix PATH override semantics

Address PR #3451 review comments:

1. [Critical] Fix PATH override semantics: normalize process.env before
   merging with server config so that a server-provided PATH fully
   replaces the parent value instead of being merged with a stale
   case-variant (Path).

2. [Suggestion] Extract mergeWindowsPathValues and
   normalizePathEnvForWindows into a shared utility module
   (src/utils/windowsPath.ts), eliminating near-identical copies in
   shellExecutionService.ts and mcp-client.ts. The shared version
   includes the fingerprint caching from shellExecutionService.

3. [Suggestion] Revert the "should connect via command" test to its
   original non-Windows form, restoring generic coverage. Add a
   dedicated test for server config PATH override behavior.

* fix(core): mock process.platform in shellExecutionService PATH tests

The shared windowsPath utility uses process.platform (not os.platform),
so the two Windows PATH normalization tests need to mock both to ensure
the utility detects the win32 platform correctly in test environments.

* fix(core): use objectContaining for env assertion in command transport test

On Windows, normalizePathEnvForWindows deduplicates PATH entries, so the
env passed to StdioClientTransport won't exactly match a raw process.env
spread. Use expect.objectContaining to verify server config env vars are
passed through without requiring an exact match on all platform-dependent
env keys.

packages/core/src/services/shellExecutionService.test.ts

@@ -649,6 +649,7 @@ describe('ShellExecutionService', () => {
 
     it('should normalize PATH-like env keys on Windows for pty execution', async () => {
       mockPlatform.mockReturnValue('win32');
+      vi.spyOn(process, 'platform', 'get').mockReturnValue('win32');
       setupConflictingPathEnv();
 
       await simulateExecution('dir', (pty) =>
@@ -1125,6 +1126,7 @@ describe('ShellExecutionService child_process fallback', () => {
 
     it('should normalize PATH-like env keys on Windows for child_process fallback', async () => {
       mockPlatform.mockReturnValue('win32');
+      vi.spyOn(process, 'platform', 'get').mockReturnValue('win32');
       setupConflictingPathEnv();
 
       await simulateExecution('dir', (cp) => cp.emit('exit', 0, null));

packages/core/src/services/shellExecutionService.ts

@@ -19,94 +19,10 @@ import {
   serializeTerminalToObject,
   type AnsiOutput,
 } from '../utils/terminalSerializer.js';
+import { normalizePathEnvForWindows } from '../utils/windowsPath.js';
 const { Terminal } = pkg;
 
 const SIGKILL_TIMEOUT_MS = 200;
-const WINDOWS_PATH_DELIMITER = ';';
-let cachedWindowsPathFingerprint: string | undefined;
-let cachedMergedWindowsPath: string | undefined;
-
-function mergeWindowsPathValues(
-  env: NodeJS.ProcessEnv,
-  pathKeys: string[],
-): string | undefined {
-  const mergedEntries: string[] = [];
-  const seenEntries = new Set<string>();
-
-  for (const key of pathKeys) {
-    const value = env[key];
-    if (value === undefined) {
-      continue;
-    }
-
-    for (const entry of value.split(WINDOWS_PATH_DELIMITER)) {
-      if (seenEntries.has(entry)) {
-        continue;
-      }
-      seenEntries.add(entry);
-      mergedEntries.push(entry);
-    }
-  }
-
-  return mergedEntries.length > 0
-    ? mergedEntries.join(WINDOWS_PATH_DELIMITER)
-    : undefined;
-}
-
-function getWindowsPathFingerprint(
-  env: NodeJS.ProcessEnv,
-  pathKeys: string[],
-): string {
-  return pathKeys.map((key) => `${key}=${env[key] ?? ''}`).join('\0');
-}
-
-function normalizePathEnvForWindows(env: NodeJS.ProcessEnv): NodeJS.ProcessEnv {
-  if (os.platform() !== 'win32') {
-    return env;
-  }
-
-  const normalized: NodeJS.ProcessEnv = { ...env };
-  const pathKeys = Object.keys(normalized).filter(
-    (key) => key.toLowerCase() === 'path',
-  );
-
-  if (pathKeys.length === 0) {
-    return normalized;
-  }
-
-  const orderedPathKeys = [...pathKeys].sort((left, right) => {
-    if (left === 'PATH') {
-      return -1;
-    }
-    if (right === 'PATH') {
-      return 1;
-    }
-    return left.localeCompare(right);
-  });
-
-  const fingerprint = getWindowsPathFingerprint(normalized, orderedPathKeys);
-  const canonicalValue =
-    fingerprint === cachedWindowsPathFingerprint
-      ? cachedMergedWindowsPath
-      : mergeWindowsPathValues(normalized, orderedPathKeys);
-
-  if (fingerprint !== cachedWindowsPathFingerprint) {
-    cachedWindowsPathFingerprint = fingerprint;
-    cachedMergedWindowsPath = canonicalValue;
-  }
-
-  for (const key of pathKeys) {
-    if (key !== 'PATH') {
-      delete normalized[key];
-    }
-  }
-
-  if (canonicalValue !== undefined) {
-    normalized['PATH'] = canonicalValue;
-  }
-
-  return normalized;
-}
 
 /**
  * On Windows with PowerShell, prefix the command with a statement that forces

packages/core/src/tools/mcp-client.test.ts

@@ -24,6 +24,7 @@ import {
 import type { ToolRegistry } from './tool-registry.js';
 
 const mockExistsSync = vi.hoisted(() => vi.fn(() => true));
+const ORIGINAL_ENV = process.env;
 
 vi.mock('node:fs', () => ({
   existsSync: mockExistsSync,
@@ -37,6 +38,7 @@ vi.mock('../mcp/oauth-token-storage.js');
 describe('mcp-client', () => {
   afterEach(() => {
     vi.restoreAllMocks();
+    process.env = ORIGINAL_ENV;
   });
 
   describe('McpClient', () => {
@@ -289,11 +291,73 @@ describe('mcp-client', () => {
         command: 'test-command',
         args: ['--foo', 'bar'],
         cwd: 'test/cwd',
-        env: { ...process.env, FOO: 'bar' },
+        // Use objectContaining because normalizePathEnvForWindows deduplicates
+        // PATH entries on Windows, so the env won't be an exact spread match.
+        env: expect.objectContaining({ FOO: 'bar' }),
         stderr: 'pipe',
       });
     });
 
+    it('should normalize PATH-like env keys on Windows for stdio transport', async () => {
+      vi.spyOn(process, 'platform', 'get').mockReturnValue('win32');
+      process.env = {
+        ...ORIGINAL_ENV,
+        PATH: 'C:\\Windows\\System32;C:\\Shared\\Tools',
+        Path: 'C:\\Users\\tester\\bin;C:\\Shared\\Tools',
+      };
+      const mockedTransport = vi
+        .spyOn(SdkClientStdioLib, 'StdioClientTransport')
+        .mockReturnValue({} as SdkClientStdioLib.StdioClientTransport);
+
+      await createTransport(
+        'test-server',
+        {
+          command: 'test-command',
+          env: { FOO: 'bar' },
+        },
+        false,
+      );
+
+      expect(mockedTransport).toHaveBeenCalledWith({
+        command: 'test-command',
+        args: [],
+        cwd: undefined,
+        env: expect.objectContaining({
+          PATH: 'C:\\Windows\\System32;C:\\Shared\\Tools;C:\\Users\\tester\\bin',
+          FOO: 'bar',
+        }),
+        stderr: 'pipe',
+      });
+      const transportOptions = mockedTransport.mock.calls[0]?.[0];
+      expect(transportOptions?.env?.['Path']).toBeUndefined();
+    });
+
+    it('should let server config PATH override parent PATH on Windows', async () => {
+      vi.spyOn(process, 'platform', 'get').mockReturnValue('win32');
+      process.env = {
+        ...ORIGINAL_ENV,
+        PATH: 'C:\\Windows\\System32;C:\\Shared\\Tools',
+        Path: 'C:\\Users\\tester\\bin;C:\\Shared\\Tools',
+      };
+      const mockedTransport = vi
+        .spyOn(SdkClientStdioLib, 'StdioClientTransport')
+        .mockReturnValue({} as SdkClientStdioLib.StdioClientTransport);
+
+      await createTransport(
+        'test-server',
+        {
+          command: 'test-command',
+          env: { PATH: 'C:\\ServerToolchain\\bin' },
+        },
+        false,
+      );
+
+      const transportOptions = mockedTransport.mock.calls[0]?.[0];
+      // Server-provided PATH should fully replace the parent PATH, not merge
+      expect(transportOptions?.env?.['PATH']).toBe('C:\\ServerToolchain\\bin');
+      expect(transportOptions?.env?.['Path']).toBeUndefined();
+    });
+
     it('should connect via command without cwd', async () => {
       const mockedTransport = vi
         .spyOn(SdkClientStdioLib, 'StdioClientTransport')

packages/core/src/tools/mcp-client.ts

@@ -43,6 +43,7 @@ import { OAuthUtils } from '../mcp/oauth-utils.js';
 import type { PromptRegistry } from '../prompts/prompt-registry.js';
 import { getErrorMessage } from '../utils/errors.js';
 import { createDebugLogger } from '../utils/debugLogger.js';
+import { normalizePathEnvForWindows } from '../utils/windowsPath.js';
 import type {
   Unsubscribe,
   WorkspaceContext,
@@ -1409,13 +1410,19 @@ export async function createTransport(
       );
     }
 
+    // Normalize process.env PATH first (merge PATH+Path → single PATH on
+    // Windows), then apply server-specific overrides on top so that a server
+    // config providing its own PATH fully replaces the parent value instead of
+    // being merged with a stale case-variant.
+    const env = {
+      ...normalizePathEnvForWindows({ ...process.env }),
+      ...(mcpServerConfig.env || {}),
+    };
+
     const transport = new StdioClientTransport({
       command: mcpServerConfig.command,
       args: mcpServerConfig.args || [],
-      env: {
-        ...process.env,
-        ...(mcpServerConfig.env || {}),
-      } as Record<string, string>,
+      env: env as Record<string, string>,
       cwd: mcpServerConfig.cwd,
       stderr: 'pipe',
     });

packages/core/src/utils/windowsPath.ts

@@ -0,0 +1,117 @@
+const WINDOWS_PATH_DELIMITER = ';';
+let cachedWindowsPathFingerprint: string | undefined;
+let cachedMergedWindowsPath: string | undefined;
+
+/**
+ * Merges multiple PATH-like environment variable values into a single
+ * deduplicated string, preserving the original order and removing duplicates.
+ *
+ * @param env - The environment object containing PATH-like keys
+ * @param pathKeys - Ordered list of keys whose values should be merged
+ * @returns The merged PATH string, or undefined if no entries were found
+ */
+export function mergeWindowsPathValues(
+  env: NodeJS.ProcessEnv,
+  pathKeys: string[],
+): string | undefined {
+  const mergedEntries: string[] = [];
+  const seenEntries = new Set<string>();
+
+  for (const key of pathKeys) {
+    const value = env[key];
+    if (value === undefined) {
+      continue;
+    }
+
+    for (const entry of value.split(WINDOWS_PATH_DELIMITER)) {
+      if (seenEntries.has(entry)) {
+        continue;
+      }
+      seenEntries.add(entry);
+      mergedEntries.push(entry);
+    }
+  }
+
+  return mergedEntries.length > 0
+    ? mergedEntries.join(WINDOWS_PATH_DELIMITER)
+    : undefined;
+}
+
+/**
+ * Returns a fingerprint string for the given PATH-like keys, used for caching.
+ */
+function getWindowsPathFingerprint(
+  env: NodeJS.ProcessEnv,
+  pathKeys: string[],
+): string {
+  return pathKeys.map((key) => `${key}=${env[key] ?? ''}`).join('\0');
+}
+
+/**
+ * Sorts PATH-like keys so that uppercase `PATH` comes first, followed by
+ * other casing variants in lexicographic order.
+ */
+function sortPathKeys(pathKeys: string[]): string[] {
+  return [...pathKeys].sort((left, right) => {
+    if (left === 'PATH') {
+      return -1;
+    }
+    if (right === 'PATH') {
+      return 1;
+    }
+    return left.localeCompare(right);
+  });
+}
+
+/**
+ * Normalizes PATH-like environment variables on Windows by merging all
+ * case-variant keys (PATH, Path, path, etc.) into a single canonical `PATH`
+ * key with deduplicated entries. On non-Windows platforms this is a no-op.
+ *
+ * Results are cached by fingerprint to avoid redundant merges when the
+ * environment has not changed between calls.
+ *
+ * @param env - The environment object to normalize
+ * @returns A new environment object with a single canonical `PATH` key
+ */
+export function normalizePathEnvForWindows(
+  env: NodeJS.ProcessEnv,
+): NodeJS.ProcessEnv {
+  if (process.platform !== 'win32') {
+    return env;
+  }
+
+  const normalized: NodeJS.ProcessEnv = { ...env };
+  const pathKeys = Object.keys(normalized).filter(
+    (key) => key.toLowerCase() === 'path',
+  );
+
+  if (pathKeys.length === 0) {
+    return normalized;
+  }
+
+  const orderedPathKeys = sortPathKeys(pathKeys);
+
+  const fingerprint = getWindowsPathFingerprint(normalized, orderedPathKeys);
+  const canonicalValue =
+    fingerprint === cachedWindowsPathFingerprint
+      ? cachedMergedWindowsPath
+      : mergeWindowsPathValues(normalized, orderedPathKeys);
+
+  if (fingerprint !== cachedWindowsPathFingerprint) {
+    cachedWindowsPathFingerprint = fingerprint;
+    cachedMergedWindowsPath = canonicalValue;
+  }
+
+  for (const key of pathKeys) {
+    if (key !== 'PATH') {
+      delete normalized[key];
+    }
+  }
+
+  if (canonicalValue !== undefined) {
+    normalized['PATH'] = canonicalValue;
+  }
+
+  return normalized;
+}