Use shell to launch MCP and ACP servers (#42382)

`npx`, and any `npm install`-ed programs, exist as batch scripts/PowerShell scripts on the PATH. We have to use a shell to launch these programs. Fixes https://github.com/zed-industries/zed/issues/41435 Closes https://github.com/zed-industries/zed/pull/42651 Release Notes: - windows: Custom MCP and ACP servers installed through `npm` now launch correctly. --------- Co-authored-by: Lukas Wirth <me@lukaswirth.dev>
2025-12-10 12:08:37 -05:00
parent 8ee85eab3c
commit d83201256d
12 changed files with 389 additions and 43 deletions
@@ -56,7 +56,10 @@ pub enum ShellKind {
    Tcsh,
    Rc,
    Fish,
+    /// Pre-installed "legacy" powershell for windows
    PowerShell,
+    /// PowerShell 7.x
+    Pwsh,
    Nushell,
    Cmd,
    Xonsh,
@@ -238,6 +241,7 @@ impl fmt::Display for ShellKind {
            ShellKind::Tcsh => write!(f, "tcsh"),
            ShellKind::Fish => write!(f, "fish"),
            ShellKind::PowerShell => write!(f, "powershell"),
+            ShellKind::Pwsh => write!(f, "pwsh"),
            ShellKind::Nushell => write!(f, "nu"),
            ShellKind::Cmd => write!(f, "cmd"),
            ShellKind::Rc => write!(f, "rc"),
@@ -260,7 +264,8 @@ impl ShellKind {
            .to_string_lossy();

        match &*program {
-            "powershell" | "pwsh" => ShellKind::PowerShell,
+            "powershell" => ShellKind::PowerShell,
+            "pwsh" => ShellKind::Pwsh,
            "cmd" => ShellKind::Cmd,
            "nu" => ShellKind::Nushell,
            "fish" => ShellKind::Fish,
@@ -279,7 +284,7 @@ impl ShellKind {

    pub fn to_shell_variable(self, input: &str) -> String {
        match self {
-            Self::PowerShell => Self::to_powershell_variable(input),
+            Self::PowerShell | Self::Pwsh => Self::to_powershell_variable(input),
            Self::Cmd => Self::to_cmd_variable(input),
            Self::Posix => input.to_owned(),
            Self::Fish => input.to_owned(),
@@ -407,8 +412,12 @@ impl ShellKind {

    pub fn args_for_shell(&self, interactive: bool, combined_command: String) -> Vec<String> {
        match self {
-            ShellKind::PowerShell => vec!["-C".to_owned(), combined_command],
-            ShellKind::Cmd => vec!["/C".to_owned(), combined_command],
+            ShellKind::PowerShell | ShellKind::Pwsh => vec!["-C".to_owned(), combined_command],
+            ShellKind::Cmd => vec![
+                "/S".to_owned(),
+                "/C".to_owned(),
+                format!("\"{combined_command}\""),
+            ],
            ShellKind::Posix
            | ShellKind::Nushell
            | ShellKind::Fish
@@ -426,7 +435,7 @@ impl ShellKind {

    pub const fn command_prefix(&self) -> Option<char> {
        match self {
-            ShellKind::PowerShell => Some('&'),
+            ShellKind::PowerShell | ShellKind::Pwsh => Some('&'),
            ShellKind::Nushell => Some('^'),
            ShellKind::Posix
            | ShellKind::Csh
@@ -457,6 +466,7 @@ impl ShellKind {
            | ShellKind::Rc
            | ShellKind::Fish
            | ShellKind::PowerShell
+            | ShellKind::Pwsh
            | ShellKind::Nushell
            | ShellKind::Xonsh
            | ShellKind::Elvish => ';',
@@ -471,6 +481,7 @@ impl ShellKind {
            | ShellKind::Tcsh
            | ShellKind::Rc
            | ShellKind::Fish
+            | ShellKind::Pwsh
            | ShellKind::PowerShell
            | ShellKind::Xonsh => "&&",
            ShellKind::Nushell | ShellKind::Elvish => ";",
@@ -478,11 +489,10 @@ impl ShellKind {
    }

    pub fn try_quote<'a>(&self, arg: &'a str) -> Option<Cow<'a, str>> {
-        shlex::try_quote(arg).ok().map(|arg| match self {
-            // If we are running in PowerShell, we want to take extra care when escaping strings.
-            // In particular, we want to escape strings with a backtick (`) rather than a backslash (\).
-            ShellKind::PowerShell => Cow::Owned(arg.replace("\\\"", "`\"").replace("\\\\", "\\")),
-            ShellKind::Cmd => Cow::Owned(arg.replace("\\\\", "\\")),
+        match self {
+            ShellKind::PowerShell => Some(Self::quote_powershell(arg)),
+            ShellKind::Pwsh => Some(Self::quote_pwsh(arg)),
+            ShellKind::Cmd => Some(Self::quote_cmd(arg)),
            ShellKind::Posix
            | ShellKind::Csh
            | ShellKind::Tcsh
@@ -490,8 +500,173 @@ impl ShellKind {
            | ShellKind::Fish
            | ShellKind::Nushell
            | ShellKind::Xonsh
-            | ShellKind::Elvish => arg,
-        })
+            | ShellKind::Elvish => shlex::try_quote(arg).ok(),
+        }
+    }
+
+    fn quote_windows(arg: &str, enclose: bool) -> Cow<'_, str> {
+        if arg.is_empty() {
+            return Cow::Borrowed("\"\"");
+        }
+
+        let needs_quoting = arg.chars().any(|c| c == ' ' || c == '\t' || c == '"');
+        if !needs_quoting {
+            return Cow::Borrowed(arg);
+        }
+
+        let mut result = String::with_capacity(arg.len() + 2);
+
+        if enclose {
+            result.push('"');
+        }
+
+        let chars: Vec<char> = arg.chars().collect();
+        let mut i = 0;
+
+        while i < chars.len() {
+            if chars[i] == '\\' {
+                let mut num_backslashes = 0;
+                while i < chars.len() && chars[i] == '\\' {
+                    num_backslashes += 1;
+                    i += 1;
+                }
+
+                if i < chars.len() && chars[i] == '"' {
+                    // Backslashes followed by quote: double the backslashes and escape the quote
+                    for _ in 0..(num_backslashes * 2 + 1) {
+                        result.push('\\');
+                    }
+                    result.push('"');
+                    i += 1;
+                } else if i >= chars.len() {
+                    // Trailing backslashes: double them (they precede the closing quote)
+                    for _ in 0..(num_backslashes * 2) {
+                        result.push('\\');
+                    }
+                } else {
+                    // Backslashes not followed by quote: output as-is
+                    for _ in 0..num_backslashes {
+                        result.push('\\');
+                    }
+                }
+            } else if chars[i] == '"' {
+                // Quote not preceded by backslash: escape it
+                result.push('\\');
+                result.push('"');
+                i += 1;
+            } else {
+                result.push(chars[i]);
+                i += 1;
+            }
+        }
+
+        if enclose {
+            result.push('"');
+        }
+        Cow::Owned(result)
+    }
+
+    fn needs_quoting_powershell(s: &str) -> bool {
+        s.is_empty()
+            || s.chars().any(|c| {
+                c.is_whitespace()
+                    || matches!(
+                        c,
+                        '"' | '`'
+                            | '$'
+                            | '&'
+                            | '|'
+                            | '<'
+                            | '>'
+                            | ';'
+                            | '('
+                            | ')'
+                            | '['
+                            | ']'
+                            | '{'
+                            | '}'
+                            | ','
+                            | '\''
+                            | '@'
+                    )
+            })
+    }
+
+    fn need_quotes_powershell(arg: &str) -> bool {
+        let mut quote_count = 0;
+        for c in arg.chars() {
+            if c == '"' {
+                quote_count += 1;
+            } else if c.is_whitespace() && (quote_count % 2 == 0) {
+                return true;
+            }
+        }
+        false
+    }
+
+    fn escape_powershell_quotes(s: &str) -> String {
+        let mut result = String::with_capacity(s.len() + 4);
+        result.push('\'');
+        for c in s.chars() {
+            if c == '\'' {
+                result.push('\'');
+            }
+            result.push(c);
+        }
+        result.push('\'');
+        result
+    }
+
+    pub fn quote_powershell(arg: &str) -> Cow<'_, str> {
+        let ps_will_quote = Self::need_quotes_powershell(arg);
+        let crt_quoted = Self::quote_windows(arg, !ps_will_quote);
+
+        if !Self::needs_quoting_powershell(arg) {
+            return crt_quoted;
+        }
+
+        Cow::Owned(Self::escape_powershell_quotes(&crt_quoted))
+    }
+
+    pub fn quote_pwsh(arg: &str) -> Cow<'_, str> {
+        if arg.is_empty() {
+            return Cow::Borrowed("''");
+        }
+
+        if !Self::needs_quoting_powershell(arg) {
+            return Cow::Borrowed(arg);
+        }
+
+        Cow::Owned(Self::escape_powershell_quotes(arg))
+    }
+
+    pub fn quote_cmd(arg: &str) -> Cow<'_, str> {
+        let crt_quoted = Self::quote_windows(arg, true);
+
+        let needs_cmd_escaping = crt_quoted.contains('"')
+            || crt_quoted.contains('%')
+            || crt_quoted
+                .chars()
+                .any(|c| matches!(c, '^' | '<' | '>' | '&' | '|' | '(' | ')'));
+
+        if !needs_cmd_escaping {
+            return crt_quoted;
+        }
+
+        let mut result = String::with_capacity(crt_quoted.len() * 2);
+        for c in crt_quoted.chars() {
+            match c {
+                '^' | '"' | '<' | '>' | '&' | '|' | '(' | ')' => {
+                    result.push('^');
+                    result.push(c);
+                }
+                '%' => {
+                    result.push_str("%%cd:~,%");
+                }
+                _ => result.push(c),
+            }
+        }
+        Cow::Owned(result)
    }

    /// Quotes the given argument if necessary, taking into account the command prefix.
@@ -538,7 +713,7 @@ impl ShellKind {
        match self {
            ShellKind::Cmd => "",
            ShellKind::Nushell => "overlay use",
-            ShellKind::PowerShell => ".",
+            ShellKind::PowerShell | ShellKind::Pwsh => ".",
            ShellKind::Fish
            | ShellKind::Csh
            | ShellKind::Tcsh
@@ -558,6 +733,7 @@ impl ShellKind {
            | ShellKind::Rc
            | ShellKind::Fish
            | ShellKind::PowerShell
+            | ShellKind::Pwsh
            | ShellKind::Nushell
            | ShellKind::Xonsh
            | ShellKind::Elvish => "clear",
@@ -576,6 +752,7 @@ impl ShellKind {
            | ShellKind::Rc
            | ShellKind::Fish
            | ShellKind::PowerShell
+            | ShellKind::Pwsh
            | ShellKind::Nushell
            | ShellKind::Xonsh
            | ShellKind::Elvish => true,
@@ -605,7 +782,7 @@ mod tests {
                .try_quote("C:\\Users\\johndoe\\dev\\python\\39007\\tests\\.venv\\Scripts\\python.exe -m pytest \"test_foo.py::test_foo\"")
                .unwrap()
                .into_owned(),
-            "\"C:\\Users\\johndoe\\dev\\python\\39007\\tests\\.venv\\Scripts\\python.exe -m pytest `\"test_foo.py::test_foo`\"\"".to_string()
+            "'C:\\Users\\johndoe\\dev\\python\\39007\\tests\\.venv\\Scripts\\python.exe -m pytest \\\"test_foo.py::test_foo\\\"'".to_string()
        );
    }

@@ -617,7 +794,113 @@ mod tests {
                .try_quote("C:\\Users\\johndoe\\dev\\python\\39007\\tests\\.venv\\Scripts\\python.exe -m pytest \"test_foo.py::test_foo\"")
                .unwrap()
                .into_owned(),
-            "\"C:\\Users\\johndoe\\dev\\python\\39007\\tests\\.venv\\Scripts\\python.exe -m pytest \\\"test_foo.py::test_foo\\\"\"".to_string()
+            "^\"C:\\Users\\johndoe\\dev\\python\\39007\\tests\\.venv\\Scripts\\python.exe -m pytest \\^\"test_foo.py::test_foo\\^\"^\"".to_string()
+        );
+    }
+
+    #[test]
+    fn test_try_quote_powershell_edge_cases() {
+        let shell_kind = ShellKind::PowerShell;
+
+        // Empty string
+        assert_eq!(
+            shell_kind.try_quote("").unwrap().into_owned(),
+            "'\"\"'".to_string()
+        );
+
+        // String without special characters (no quoting needed)
+        assert_eq!(shell_kind.try_quote("simple").unwrap(), "simple");
+
+        // String with spaces
+        assert_eq!(
+            shell_kind.try_quote("hello world").unwrap().into_owned(),
+            "'hello world'".to_string()
+        );
+
+        // String with dollar signs
+        assert_eq!(
+            shell_kind.try_quote("$variable").unwrap().into_owned(),
+            "'$variable'".to_string()
+        );
+
+        // String with backticks
+        assert_eq!(
+            shell_kind.try_quote("test`command").unwrap().into_owned(),
+            "'test`command'".to_string()
+        );
+
+        // String with multiple special characters
+        assert_eq!(
+            shell_kind
+                .try_quote("test `\"$var`\" end")
+                .unwrap()
+                .into_owned(),
+            "'test `\\\"$var`\\\" end'".to_string()
+        );
+
+        // String with backslashes and colon (path without spaces doesn't need quoting)
+        assert_eq!(
+            shell_kind.try_quote("C:\\path\\to\\file").unwrap(),
+            "C:\\path\\to\\file"
+        );
+    }
+
+    #[test]
+    fn test_try_quote_cmd_edge_cases() {
+        let shell_kind = ShellKind::Cmd;
+
+        // Empty string
+        assert_eq!(
+            shell_kind.try_quote("").unwrap().into_owned(),
+            "^\"^\"".to_string()
+        );
+
+        // String without special characters (no quoting needed)
+        assert_eq!(shell_kind.try_quote("simple").unwrap(), "simple");
+
+        // String with spaces
+        assert_eq!(
+            shell_kind.try_quote("hello world").unwrap().into_owned(),
+            "^\"hello world^\"".to_string()
+        );
+
+        // String with space and backslash (backslash not at end, so not doubled)
+        assert_eq!(
+            shell_kind.try_quote("path\\ test").unwrap().into_owned(),
+            "^\"path\\ test^\"".to_string()
+        );
+
+        // String ending with backslash (must be doubled before closing quote)
+        assert_eq!(
+            shell_kind.try_quote("test path\\").unwrap().into_owned(),
+            "^\"test path\\\\^\"".to_string()
+        );
+
+        // String ending with multiple backslashes (all doubled before closing quote)
+        assert_eq!(
+            shell_kind.try_quote("test path\\\\").unwrap().into_owned(),
+            "^\"test path\\\\\\\\^\"".to_string()
+        );
+
+        // String with embedded quote (quote is escaped, backslash before it is doubled)
+        assert_eq!(
+            shell_kind.try_quote("test\\\"quote").unwrap().into_owned(),
+            "^\"test\\\\\\^\"quote^\"".to_string()
+        );
+
+        // String with multiple backslashes before embedded quote (all doubled)
+        assert_eq!(
+            shell_kind
+                .try_quote("test\\\\\"quote")
+                .unwrap()
+                .into_owned(),
+            "^\"test\\\\\\\\\\^\"quote^\"".to_string()
+        );
+
+        // String with backslashes not before quotes (path without spaces doesn't need quoting)
+        assert_eq!(
+            shell_kind.try_quote("C:\\path\\to\\file").unwrap(),
+            "C:\\path\\to\\file"
        );
    }

@@ -1,3 +1,5 @@
+use std::borrow::Cow;
+
 use crate::shell::get_system_shell;
 use crate::shell::{Shell, ShellKind};

@@ -42,7 +44,7 @@ impl ShellBuilder {
            self.program.clone()
        } else {
            match self.kind {
-                ShellKind::PowerShell => {
+                ShellKind::PowerShell | ShellKind::Pwsh => {
                    format!("{} -C '{}'", self.program, command_to_use_in_label)
                }
                ShellKind::Cmd => {
@@ -78,11 +80,27 @@ impl ShellBuilder {
        task_args: &[String],
    ) -> (String, Vec<String>) {
        if let Some(task_command) = task_command {
-            let mut combined_command = task_args.iter().fold(task_command, |mut command, arg| {
-                command.push(' ');
-                command.push_str(&self.kind.to_shell_variable(arg));
-                command
-            });
+            let task_command = self.kind.prepend_command_prefix(&task_command);
+            let task_command = if !task_args.is_empty() {
+                match self.kind.try_quote_prefix_aware(&task_command) {
+                    Some(task_command) => task_command,
+                    None => task_command,
+                }
+            } else {
+                task_command
+            };
+            let mut combined_command =
+                task_args
+                    .iter()
+                    .fold(task_command.into_owned(), |mut command, arg| {
+                        command.push(' ');
+                        let shell_variable = self.kind.to_shell_variable(arg);
+                        command.push_str(&match self.kind.try_quote(&shell_variable) {
+                            Some(shell_variable) => shell_variable,
+                            None => Cow::Owned(shell_variable),
+                        });
+                        command
+                    });
            if self.redirect_stdin {
                match self.kind {
                    ShellKind::Fish => {
@@ -99,7 +117,7 @@ impl ShellBuilder {
                        combined_command.insert(0, '(');
                        combined_command.push_str(") </dev/null");
                    }
-                    ShellKind::PowerShell => {
+                    ShellKind::PowerShell | ShellKind::Pwsh => {
                        combined_command.insert_str(0, "$null | & {");
                        combined_command.push_str("}");
                    }
@@ -115,6 +133,10 @@ impl ShellBuilder {

        (self.program, self.args)
    }
+
+    pub fn kind(&self) -> ShellKind {
+        self.kind
+    }
 }

 #[cfg(test)]
@@ -144,7 +166,7 @@ mod test {
            vec![
                "-i",
                "-c",
-                "echo $env.hello $env.world nothing --($env.something) $ ${test"
+                "^echo '$env.hello' '$env.world' nothing '--($env.something)' '$' '${test'"
            ]
        );
    }
@@ -159,7 +181,7 @@ mod test {
            .build(Some("echo".into()), &["nothing".to_string()]);

        assert_eq!(program, "nu");
-        assert_eq!(args, vec!["-i", "-c", "(echo nothing) </dev/null"]);
+        assert_eq!(args, vec!["-i", "-c", "(^echo nothing) </dev/null"]);
    }

    #[test]
@@ -159,7 +159,7 @@ async fn capture_windows(
                zed_path.display()
            ),
        ]),
-        ShellKind::PowerShell => cmd.args([
+        ShellKind::PowerShell | ShellKind::Pwsh => cmd.args([
            "-NonInteractive",
            "-NoProfile",
            "-Command",