fix(stdio): handle spaces in Command path on Windows (cmd /d /s /c)

src/ModelContextProtocol.Core/Client/StdioClientTransport.cs

@@ -63,13 +63,24 @@ public async Task<ITransport> ConnectAsync(CancellationToken cancellationToken =
 
         string command = _options.Command;
         IList<string>? arguments = _options.Arguments;
+
+        // On Windows, for stdio, we need to wrap non-cmd.exe commands with cmd.exe (usually npx or uvicorn):
+        // the stdio transport will not work correctly if the command is not run in a shell. The wrapped
+        // "cmd.exe /d /s /c ..." command line is built by hand (see GetWindowsCmdArguments below).
+        string? cmdArguments = null;
         if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows) &&
             !string.Equals(Path.GetFileName(command), "cmd.exe", StringComparison.OrdinalIgnoreCase))
         {
-            // On Windows, for stdio, we need to wrap non-shell commands with cmd.exe /c {command} (usually npx or uvicorn).
-            // The stdio transport will not work correctly if the command is not run in a shell.
-            arguments = arguments is null or [] ? ["/c", command] : ["/c", command, ..arguments];
+            // Build the full "cmd.exe /d /s /c ..." command line by hand rather than relying on
+            // ProcessStartInfo.ArgumentList. ArgumentList applies the Win32 C-runtime quoting rules to
+            // each argument independently, which is incompatible with how cmd.exe /c re-parses its command
+            // line: when more than two quote characters follow /c, cmd strips the first and last quote and
+            // re-parses the middle, mangling command paths that contain spaces (e.g. under "Program Files").
+            // Using /s together with a single outer pair of quotes makes cmd strip exactly that outer pair
+            // and treat the remainder verbatim, so a space-containing command path launches correctly.
+            cmdArguments = GetWindowsCmdArguments(command, arguments);
             command = "cmd.exe";
+            arguments = null;
         }
 
         ILogger logger = (ILogger?)_loggerFactory?.CreateLogger<StdioClientTransport>() ?? NullLogger.Instance;
@@ -93,7 +104,13 @@ public async Task<ITransport> ConnectAsync(CancellationToken cancellationToken =
 #endif
             };
 
-            if (arguments is not null)
+            if (cmdArguments is not null)
+            {
+                // The cmd.exe wrapper builds its own already-quoted command line (see GetWindowsCmdArguments);
+                // assign it directly instead of going through ArgumentList's per-argument quoting.
+                startInfo.Arguments = cmdArguments;
+            }
+            else if (arguments is not null)
             {
 #if NET
                 foreach (string arg in arguments)
@@ -293,6 +310,115 @@ private static string EscapeArgumentString(string argument) =>
         WindowsCliSpecialArgumentsRegex.Replace(argument, static match => "^" + match.Value) :
         argument;
 
+    /// <summary>
+    /// Builds the raw command line passed to <c>cmd.exe</c> when wrapping a non-<c>cmd.exe</c> command on Windows.
+    /// </summary>
+    /// <remarks>
+    /// Produces <c>/d /s /c "&lt;command&gt; &lt;arg1&gt; &lt;arg2&gt; ..."</c> where:
+    /// <list type="bullet">
+    /// <item><c>/d</c> skips any AutoRun commands, and <c>/s</c> together with the single outer pair of quotes
+    /// makes <c>cmd.exe</c> strip exactly that outer pair and treat the remainder verbatim.</item>
+    /// <item>The command and each argument are wrapped in double quotes when they contain whitespace, a quote,
+    /// or a <c>cmd.exe</c> metacharacter, using the Win32 <c>CommandLineToArgvW</c> escaping rules, so both
+    /// <c>cmd.exe</c> and the launched child process parse the tokens back to their original values. Quoting
+    /// also neutralizes <c>cmd.exe</c> metacharacters (<c>&amp; | &lt; &gt; ^ ( )</c>), which are literal inside
+    /// double quotes. A bare command name is left unquoted so that <c>cmd.exe</c> still resolves it against the
+    /// current directory and PATH.</item>
+    /// </list>
+    /// </remarks>
+    internal static string GetWindowsCmdArguments(string command, IList<string>? arguments)
+    {
+        StringBuilder builder = new("/d /s /c \"");
+
+        // The command is quoted only when it needs it (e.g. a path under "Program Files" that contains a
+        // space): that is what fixes #1601. A bare command name (such as "npx") is deliberately left
+        // unquoted, because cmd.exe resolves an unquoted bare name against the current directory and PATH
+        // but does not resolve a quoted one (the outer /s means cmd does not strip the inner quotes).
+        AppendCmdArgument(builder, command);
+
+        if (arguments is not null)
+        {
+            foreach (string argument in arguments)
+            {
+                builder.Append(' ');
+                AppendCmdArgument(builder, argument);
+            }
+        }
+
+        builder.Append('"');
+        return builder.ToString();
+    }
+
+    private static void AppendCmdArgument(StringBuilder builder, string argument)
+    {
+        if (argument.Length != 0 && !RequiresCmdQuoting(argument))
+        {
+            builder.Append(argument);
+            return;
+        }
+
+        // Quote using the Win32 backslash/quote escaping rules (same rules as PasteArguments), so the child
+        // process's CommandLineToArgvW recovers the exact original argument.
+        builder.Append('"');
+        int idx = 0;
+        while (idx < argument.Length)
+        {
+            char c = argument[idx++];
+            if (c == '\\')
+            {
+                int backslashes = 1;
+                while (idx < argument.Length && argument[idx] == '\\')
+                {
+                    idx++;
+                    backslashes++;
+                }
+
+                if (idx == argument.Length)
+                {
+                    // Backslashes precede the closing quote, so they must be doubled.
+                    builder.Append('\\', backslashes * 2);
+                }
+                else if (argument[idx] == '"')
+                {
+                    // Backslashes precede an embedded quote, so they must be doubled and the quote escaped.
+                    builder.Append('\\', backslashes * 2 + 1);
+                    builder.Append('"');
+                    idx++;
+                }
+                else
+                {
+                    builder.Append('\\', backslashes);
+                }
+
+                continue;
+            }
+
+            if (c == '"')
+            {
+                builder.Append('\\');
+                builder.Append('"');
+                continue;
+            }
+
+            builder.Append(c);
+        }
+
+        builder.Append('"');
+    }
+
+    private static bool RequiresCmdQuoting(string argument)
+    {
+        foreach (char c in argument)
+        {
+            if (char.IsWhiteSpace(c) || c is '"' or '&' or '|' or '<' or '>' or '^' or '(' or ')')
+            {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
     private const string WindowsCliSpecialArgumentsRegexString = "[&^><|]";
 
 #if NET

tests/ModelContextProtocol.Tests/Transport/StdioClientTransportTests.cs

@@ -13,6 +13,8 @@ public class StdioClientTransportTests(ITestOutputHelper testOutputHelper) : Log
 {
     public static bool IsStdErrCallbackSupported => !PlatformDetection.IsMonoRuntime;
 
+    public static bool IsWindows => RuntimeInformation.IsOSPlatform(OSPlatform.Windows);
+
     [Fact]
     public async Task ConnectAsync_DoesNotLogEnvironmentVariablesAtTrace()
     {
@@ -187,6 +189,157 @@ public async Task EscapesCliArgumentsCorrectly(string? cliArgumentValue)
         Assert.Equal(cliArgumentValue ?? "", content.Text);
     }
 
+    // Regression tests for https://github.com/modelcontextprotocol/csharp-sdk/issues/1601:
+    // on Windows, non-cmd.exe commands are wrapped with cmd.exe. The wrapper must launch the
+    // command correctly even when the command path contains a space (e.g. under "Program Files").
+
+    [Fact(Skip = "Windows-only: StdioClientTransport only wraps commands with cmd.exe on Windows.", SkipUnless = nameof(IsWindows))]
+    public async Task WindowsCmdWrapper_UsesDsCWithSingleOuterQuotePair()
+    {
+        // The command and each argument are quoted only when they contain whitespace, a quote, or a cmd.exe
+        // metacharacter (& | < > ^ ( )); a bare command name is left unquoted so cmd.exe still resolves it.
+        static string Wrap(string inner) => "/d /s /c \"" + inner + "\"";
+        static string Quote(string value) => "\"" + value + "\"";
+
+        // Use guaranteed-nonexistent command names (under a unique, nonexistent directory, and a bare name
+        // that will not resolve from PATH) so the wrapper's cmd.exe never actually launches a real program;
+        // the test only inspects the logged command line, not the process outcome.
+        string id = Guid.NewGuid().ToString("N");
+        string spacedCommand = $@"C:\{id} dir\My Server\server_{id}.exe"; // contains spaces
+        string plainCommand = $@"C:\{id}dir\server_{id}.exe";            // no space, has a directory
+        string bareCommand = $"nonexistent_{id}.exe";                    // bare name, not on PATH
+
+        var cases = new (string command, string[]? arguments, string expectedArguments)[]
+        {
+            // Space in the command path with a trailing argument — the exact shape that failed in #1601.
+            (spacedCommand, ["--flag"], Wrap(Quote(spacedCommand) + " --flag")),
+            // Space in the command path with an argument that itself contains spaces.
+            (spacedCommand, ["arg with spaces"], Wrap(Quote(spacedCommand) + " " + Quote("arg with spaces"))),
+            // Space in the command path with no arguments.
+            (spacedCommand, null, Wrap(Quote(spacedCommand))),
+            // A space-free command path is left unquoted; cmd.exe metacharacters in arguments are quoted so
+            // cmd treats them literally rather than as operators.
+            (plainCommand, ["a&b", "c|d", "e>f", "g<h", "i^j"],
+                Wrap(plainCommand + " " + Quote("a&b") + " " + Quote("c|d") + " " + Quote("e>f") + " " + Quote("g<h") + " " + Quote("i^j"))),
+            // A bare command name and ordinary arguments without special characters are passed through unquoted.
+            (bareCommand, ["--key=value", "plain"], Wrap(bareCommand + " --key=value plain")),
+            // An empty argument is preserved as an empty quoted token.
+            (bareCommand, [""], Wrap(bareCommand + " " + Quote(""))),
+        };
+
+        foreach (var (command, arguments, expectedArguments) in cases)
+        {
+            string logged = await GetLoggedProcessInvocationAsync(command, arguments);
+            Assert.Contains($"Arguments: {expectedArguments}, Working directory:", logged);
+        }
+    }
+
+    // Connects a transport far enough to capture the trace log that records the exact process command line
+    // (emitted before the process is started), then returns the rendered log message.
+    private static async Task<string> GetLoggedProcessInvocationAsync(string command, IList<string>? arguments)
+    {
+        var provider = new MockLoggerProvider();
+        using var loggerFactory = Microsoft.Extensions.Logging.LoggerFactory.Create(builder =>
+        {
+            builder.AddProvider(provider);
+            builder.SetMinimumLevel(LogLevel.Trace);
+        });
+
+        var transport = new StdioClientTransport(new() { Name = "TestServer", Command = command, Arguments = arguments }, loggerFactory);
+        try
+        {
+            // The command path need not exist: the invocation is logged before the process is started, and
+            // whether cmd.exe then fails to find the target is irrelevant to the command line we are asserting.
+            await using var _ = await transport.ConnectAsync(TestContext.Current.CancellationToken);
+        }
+        catch
+        {
+            // Ignore: we only need the pre-start trace log.
+        }
+
+        var log = Assert.Single(provider.LogMessages, m =>
+            m.LogLevel == LogLevel.Trace && m.Message.Contains("starting server process", StringComparison.Ordinal));
+        return log.Message;
+    }
+
+    [Theory(Skip = "Windows-only: reproduces #1601 (a space in the command path on Windows).", SkipUnless = nameof(IsWindows))]
+    [InlineData("simple")]
+    [InlineData("value with spaces")]
+    [InlineData("a & b | c")]
+    [InlineData("&^<>|")]
+    [InlineData("value with \"quotes\" and spaces")]
+    [InlineData(@"C:\Program Files\Test App\app.dll")]
+    [InlineData(@"C:\EndsWithBackslash\")]
+    [InlineData("http://localhost:1234/callback?foo=1&bar=2")]
+    public async Task CommandPathWithSpaces_RoundTripsArgumentsThroughCmd(string cliArgumentValue)
+    {
+        // Copy the (self-contained) TestServer build output to a directory whose path contains a space,
+        // then launch it by absolute path. Before the fix this failed to start; the argument echoed back
+        // confirms both that the process launched and that the argument survived the cmd.exe wrapping.
+        string serverDirectory = CopyTestServerToDirectoryWithSpaces();
+        try
+        {
+            var options = new StdioClientTransportOptions
+            {
+                Name = "TestServer",
+                Command = Path.Combine(serverDirectory, "TestServer.exe"),
+                Arguments = [$"--cli-arg={cliArgumentValue}"],
+            };
+
+            var transport = new StdioClientTransport(options, LoggerFactory);
+            await using var client = await McpClient.CreateAsync(transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken);
+
+            var tools = await client.ListToolsAsync(cancellationToken: TestContext.Current.CancellationToken);
+            Assert.NotEmpty(tools);
+
+            var result = await client.CallToolAsync("echoCliArg", cancellationToken: TestContext.Current.CancellationToken);
+            var content = Assert.IsType<TextContentBlock>(Assert.Single(result.Content));
+            Assert.Equal(cliArgumentValue, content.Text);
+        }
+        finally
+        {
+            try
+            {
+                Directory.Delete(serverDirectory, recursive: true);
+            }
+            catch
+            {
+                // Best effort cleanup.
+            }
+        }
+    }
+
+    private static string CopyTestServerToDirectoryWithSpaces()
+    {
+        // The TestServer build output is a sibling of the test build output under the artifacts layout,
+        // differing only by the project-name segment of the path.
+        string sourceDirectory = AppContext.BaseDirectory.Replace(
+            $"{Path.DirectorySeparatorChar}ModelContextProtocol.Tests{Path.DirectorySeparatorChar}",
+            $"{Path.DirectorySeparatorChar}ModelContextProtocol.TestServer{Path.DirectorySeparatorChar}");
+
+        Assert.True(
+            File.Exists(Path.Combine(sourceDirectory, "TestServer.exe")),
+            $"Could not locate the TestServer build output at '{sourceDirectory}'.");
+
+        string destinationDirectory = Path.Combine(Path.GetTempPath(), "mcp sdk 1601 " + Guid.NewGuid().ToString("N"));
+        CopyDirectory(sourceDirectory, destinationDirectory);
+        return destinationDirectory;
+
+        static void CopyDirectory(string source, string destination)
+        {
+            Directory.CreateDirectory(destination);
+            foreach (string file in Directory.GetFiles(source))
+            {
+                File.Copy(file, Path.Combine(destination, Path.GetFileName(file)), overwrite: true);
+            }
+
+            foreach (string directory in Directory.GetDirectories(source))
+            {
+                CopyDirectory(directory, Path.Combine(destination, Path.GetFileName(directory)));
+            }
+        }
+    }
+
     [Fact(Skip = "Platform not supported by this test.", SkipUnless = nameof(IsStdErrCallbackSupported))]
     public async Task InheritEnvironmentVariables_DefaultTrue_ChildSeesParentEnvVars()
     {