fix(acp): honor configured terminal shell

src-tauri/src/acp/connection.rs

@@ -36,7 +36,9 @@ use crate::acp::file_system_runtime::{FileSystemRuntime, FileSystemRuntimeError,
 use crate::acp::registry::{self, AgentDistribution};
 use crate::acp::session_state::SessionState;
 use crate::acp::stderr_tail::{summarize_parser_error, StderrTail, TailScope};
-use crate::acp::terminal_runtime::{TerminalRuntime, TerminalRuntimeError};
+use crate::acp::terminal_runtime::{
+    TerminalRuntime, TerminalRuntimeError, TerminalShellRuntimeConfig,
+};
 use crate::acp::types::{
     AcpEvent, AvailableCommandInfo, ConnectionInfo, ConnectionStatus, GrokEffortSpec,
     PermissionOptionInfo, PlanEntryInfo, PromptCapabilitiesInfo, PromptInputBlock,
@@ -1154,6 +1156,7 @@ pub async fn spawn_agent_connection(
     preferred_mode_id: Option<String>,
     preferred_config_values: BTreeMap<String, String>,
     delegation_injection: Option<DelegationInjection>,
+    terminal_shell_config: TerminalShellRuntimeConfig,
 ) -> Result<tokio::sync::oneshot::Receiver<()>, AcpError> {
     // Create the authoritative session state up front. Subsequent emit_with_state
     // calls write through this state and increment its seq counter so the first
@@ -1317,6 +1320,7 @@ pub async fn spawn_agent_connection(
             emitter_clone.clone(),
             Arc::clone(&state_clone),
             terminal_base_env,
+            terminal_shell_config,
             preferred_mode_id,
             preferred_config_values,
             delegation_injection,
@@ -3170,6 +3174,7 @@ async fn run_connection(
     emitter: EventEmitter,
     state: Arc<RwLock<SessionState>>,
     terminal_base_env: BTreeMap<String, String>,
+    terminal_shell_config: TerminalShellRuntimeConfig,
     preferred_mode_id: Option<String>,
     preferred_config_values: BTreeMap<String, String>,
     delegation_injection: Option<DelegationInjection>,
@@ -3188,7 +3193,9 @@ async fn run_connection(
     // `terminal/create` without a `cwd` (e.g. CodeBuddy) runs in the folder the
     // conversation runs in rather than codeg's own process cwd.
     let terminal_runtime = Arc::new(
-        TerminalRuntime::with_base_env(terminal_base_env).with_default_cwd(Some(cwd.clone())),
+        TerminalRuntime::with_base_env(terminal_base_env)
+            .with_default_cwd(Some(cwd.clone()))
+            .with_default_shell_config(terminal_shell_config),
     );
     let cwd_string = cwd.to_string_lossy().to_string();
     tracing::info!("[ACP] fs policy {}", fs_policy.describe());

src-tauri/src/acp/manager.rs

@@ -25,6 +25,7 @@ use crate::acp::question::{
     build_outcome, QuestionAnswer, QuestionOutcome, QuestionSpec, RegisteredQuestion,
     SessionQuestionAccess,
 };
+use crate::acp::terminal_runtime::TerminalShellRuntimeConfig;
 use crate::acp::types::{
     AcpEvent, AgentOptionsSnapshot, ConfigStaleKind, ConnectionInfo, ConnectionStatus,
     ForkResultInfo, PromptCapabilitiesInfo, PromptInputBlock,
@@ -201,6 +202,10 @@ pub struct ConnectionManager {
     /// tests; in production initialized from env via
     /// `spawn_handshake_timeout_from_env`.
     spawn_handshake_timeout: Duration,
+    /// Shared General Settings shell used by ACP terminal fallbacks. Cloned
+    /// into each connection runtime so a setting update applies to existing
+    /// model sessions as well as newly spawned ones.
+    terminal_shell_config: TerminalShellRuntimeConfig,
     /// Delegation broker + token registry + UDS path installed during app
     /// bootstrap (`install_delegation`). When present, `spawn_agent` propagates
     /// the injection to `spawn_agent_connection`, which makes
@@ -262,6 +267,7 @@ impl ConnectionManager {
             connections: Arc::new(Mutex::new(HashMap::new())),
             spawn_locks: Arc::new(Mutex::new(HashMap::new())),
             spawn_handshake_timeout: spawn_handshake_timeout_from_env(),
+            terminal_shell_config: TerminalShellRuntimeConfig::new(),
             delegation_injection: Arc::new(std::sync::OnceLock::new()),
             probe_locks: Arc::new(Mutex::new(HashMap::new())),
             pending_questions: Arc::new(Mutex::new(HashMap::new())),
@@ -275,6 +281,7 @@ impl ConnectionManager {
             connections: self.connections.clone(),
             spawn_locks: self.spawn_locks.clone(),
             spawn_handshake_timeout: self.spawn_handshake_timeout,
+            terminal_shell_config: self.terminal_shell_config.clone(),
             delegation_injection: self.delegation_injection.clone(),
             probe_locks: self.probe_locks.clone(),
             pending_questions: self.pending_questions.clone(),
@@ -293,6 +300,13 @@ impl ConnectionManager {
         self.delegation_injection.get().cloned()
     }
 
+    /// Returns the shared terminal-shell setting consumed by ACP terminal
+    /// runtimes. Keeping the handle shared makes saves apply immediately to
+    /// connections that are already running.
+    pub fn terminal_shell_config(&self) -> TerminalShellRuntimeConfig {
+        self.terminal_shell_config.clone()
+    }
+
     /// Test-only constructor that overrides the spawn-handshake timeout.
     /// Production code should use `new()`.
     #[cfg(test)]
@@ -301,6 +315,7 @@ impl ConnectionManager {
             connections: Arc::new(Mutex::new(HashMap::new())),
             spawn_locks: Arc::new(Mutex::new(HashMap::new())),
             spawn_handshake_timeout: timeout,
+            terminal_shell_config: TerminalShellRuntimeConfig::new(),
             delegation_injection: Arc::new(std::sync::OnceLock::new()),
             probe_locks: Arc::new(Mutex::new(HashMap::new())),
             pending_questions: Arc::new(Mutex::new(HashMap::new())),
@@ -478,6 +493,7 @@ impl ConnectionManager {
             preferred_mode_id,
             preferred_config_values,
             self.delegation_snapshot(),
+            self.terminal_shell_config.clone(),
         )
         .await?;
 

src-tauri/src/acp/terminal_runtime.rs

@@ -1,5 +1,5 @@
 use std::collections::{BTreeMap, HashMap, HashSet};
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
 use std::process::Stdio;
 use std::sync::Arc;
 use std::time::Duration;
@@ -10,9 +10,11 @@ use sacp::schema::{
     TerminalOutputResponse, WaitForTerminalExitRequest, WaitForTerminalExitResponse,
 };
 use tokio::io::{AsyncRead, AsyncReadExt};
-use tokio::sync::{watch, Mutex, Notify};
+use tokio::sync::{watch, Mutex, Notify, RwLock};
 use tokio::task::JoinHandle;
 
+use crate::terminal::manager::resolve_shell;
+
 type TerminalMap = HashMap<String, Arc<TerminalInstance>>;
 const DEFAULT_OUTPUT_BYTE_LIMIT: u64 = 1_000_000;
 /// After the child process exits, wait up to this long for the stdout/stderr
@@ -390,6 +392,31 @@ async fn own_terminal_process(terminal: Arc<TerminalInstance>, mut child: tokio:
         .send_replace(TerminalCompletion::Exited(exit_status));
 }
 
+/// Shared, hot-swappable default shell for ACP terminal requests.
+///
+/// The setting is owned by [`crate::acp::manager::ConnectionManager`] and
+/// cloned into every connection runtime. Reading it when a terminal is created
+/// means a change in General Settings also affects already-running model
+/// sessions.
+#[derive(Clone, Default)]
+pub struct TerminalShellRuntimeConfig {
+    inner: Arc<RwLock<Option<String>>>,
+}
+
+impl TerminalShellRuntimeConfig {
+    pub fn new() -> Self {
+        Self::default()
+    }
+
+    pub async fn snapshot(&self) -> Option<String> {
+        self.inner.read().await.clone()
+    }
+
+    pub async fn set(&self, default_shell: Option<String>) {
+        *self.inner.write().await = default_shell;
+    }
+}
+
 pub struct TerminalRuntime {
     terminals: Mutex<TerminalMap>,
     /// Base environment merged into every spawned terminal command before
@@ -407,6 +434,10 @@ pub struct TerminalRuntime {
     /// process cwd (often "/" on desktop, the dev crate dir in development).
     /// `None` leaves the process cwd inherited (legacy behavior).
     default_cwd: Option<PathBuf>,
+    /// The current General Settings default shell. Structured ACP requests
+    /// still direct-exec real programs, while shell command lines and shell
+    /// builtins use this selected shell as their fallback.
+    default_shell: TerminalShellRuntimeConfig,
 }
 
 #[derive(Debug, Clone)]
@@ -428,6 +459,7 @@ impl TerminalRuntime {
             terminals: Mutex::new(HashMap::new()),
             base_env,
             default_cwd: None,
+            default_shell: TerminalShellRuntimeConfig::new(),
         }
     }
 
@@ -438,6 +470,17 @@ impl TerminalRuntime {
         self
     }
 
+    /// Use a shared General Settings shell value for ACP terminal fallbacks.
+    /// The config is read at command creation time so existing connections pick
+    /// up setting changes without being restarted.
+    pub fn with_default_shell_config(
+        mut self,
+        default_shell: TerminalShellRuntimeConfig,
+    ) -> Self {
+        self.default_shell = default_shell;
+        self
+    }
+
     /// Apply stdio, working directory, and environment to a freshly built
     /// terminal command. Shared by the direct-exec and shell-fallback spawn
     /// paths in `create_terminal` so both honor the same cwd precedence and
@@ -514,14 +557,15 @@ impl TerminalRuntime {
         // as unrunnable (`NotFound`, or `InvalidFilename` when the whole line is
         // longer than the OS path limit — grok crams `<shell> -lc "<script>"`
         // into `command`, so a multi-KB heredoc exceeds PATH_MAX and the direct
-        // exec fails with ENAMETOOLONG, not ENOENT) AND the request looks like a
-        // whole shell line crammed into `command` (empty args + embedded
-        // whitespace, the shape CodeBuddy and grok send) do we retry through the
-        // platform shell so its `&&`, pipes, `$VAR`, and globs evaluate. A path
-        // that long can never be a real executable, so rerouting it is always at
-        // least as correct. Deciding off a real failed spawn — rather than a
-        // pre-spawn `which` guess that runs in codeg's own cwd/env — means we
-        // never reroute a command that would otherwise have run.
+        // exec fails with ENAMETOOLONG, not ENOENT) do we retry through a shell.
+        // Whole shell lines (empty args + embedded whitespace, the shape
+        // CodeBuddy and grok send) always qualify. A configured default shell
+        // additionally qualifies a no-argv shell builtin such as PowerShell's
+        // `Get-ChildItem`. A path that long can never be a real executable, so
+        // rerouting it is always at least as correct. Deciding off a real failed
+        // spawn — rather than a pre-spawn `which` guess that runs in codeg's own
+        // cwd/env — means we never reroute a command that would otherwise have
+        // run.
         //
         // A spawn the kernel refuses with `ETXTBSY` is retried in place instead
         // (see `spawn_retrying_exec_busy`): the program *is* runnable, it is
@@ -532,17 +576,24 @@ impl TerminalRuntime {
         direct.args(&request.args);
         self.configure_command(&mut direct, &request);
 
+        let configured_shell = self.default_shell.snapshot().await;
+        // A structured ACP request normally names an executable plus argv, so
+        // preserve direct execution for it. If an explicit shell is selected,
+        // also let no-argv shell builtins (for example PowerShell's
+        // `Get-ChildItem`) fall back through that shell after direct exec fails.
+        let can_retry_through_shell = request.args.is_empty()
+            && (request.command.contains(char::is_whitespace) || configured_shell.is_some());
         let spawned = crate::process::spawn_retrying_exec_busy(|| direct.spawn()).await;
         let mut child = match spawned {
             Ok(child) => child,
             Err(err)
                 if matches!(
                     err.kind(),
                     std::io::ErrorKind::NotFound | std::io::ErrorKind::InvalidFilename
-                ) && request.args.is_empty()
-                    && request.command.contains(char::is_whitespace) =>
+                ) && can_retry_through_shell =>
             {
-                let mut shell = shell_wrapped_command(&request.command);
+                let shell = configured_shell.unwrap_or_else(resolve_shell);
+                let mut shell = shell_wrapped_command(&shell, &request.command);
                 self.configure_command(&mut shell, &request);
                 shell.spawn().map_err(|err| {
                     TerminalRuntimeError::Internal(format!(
@@ -782,23 +833,24 @@ where
     }
 }
 
-/// Wrap a full shell command line so it executes through the platform shell.
-/// Used when an agent passes an entire command line in `command` with empty
-/// `args` (see `create_terminal`); the shell preserves the `&&`, pipes,
-/// `$VAR`, and globs the agent's line relies on. Reuses `tokio_command` so the
-/// shell still inherits codeg's UTF-8 env and Windows program normalization.
-#[cfg(not(windows))]
-fn shell_wrapped_command(line: &str) -> tokio::process::Command {
-    let mut command = crate::process::tokio_command("/bin/sh");
-    command.arg("-c").arg(line);
-    command
-}
-
-#[cfg(windows)]
-fn shell_wrapped_command(line: &str) -> tokio::process::Command {
-    let comspec = std::env::var("COMSPEC").unwrap_or_else(|_| "cmd.exe".to_string());
-    let mut command = crate::process::tokio_command(comspec);
-    command.arg("/C").arg(line);
+/// Wrap a command line through the configured shell. This matches the shell
+/// semantics used by the built-in terminal instead of hard-coding `/bin/sh` or
+/// `cmd.exe` for model tool calls.
+fn shell_wrapped_command(shell: &str, line: &str) -> tokio::process::Command {
+    let shell_name = Path::new(shell)
+        .file_name()
+        .and_then(|value| value.to_str())
+        .unwrap_or(shell)
+        .to_ascii_lowercase();
+    let mut command = crate::process::tokio_command(shell);
+
+    if shell_name.contains("pwsh") || shell_name.contains("powershell") {
+        command.args(["-NoLogo", "-NoProfile", "-Command", line]);
+    } else if shell_name == "cmd" || shell_name == "cmd.exe" {
+        command.args(["/D", "/S", "/C", line]);
+    } else {
+        command.arg("-c").arg(line);
+    }
     command
 }
 
@@ -868,6 +920,41 @@ fn decode_available_utf8(pending: &mut Vec<u8>) -> String {
     output
 }
 
+#[cfg(test)]
+mod shell_config_tests {
+    use super::*;
+
+    #[tokio::test]
+    async fn default_shell_config_hot_swaps() {
+        let config = TerminalShellRuntimeConfig::new();
+        assert_eq!(config.snapshot().await, None);
+
+        config.set(Some("pwsh.exe".to_string())).await;
+        assert_eq!(config.snapshot().await.as_deref(), Some("pwsh.exe"));
+    }
+
+    #[test]
+    fn powershell_fallback_uses_the_selected_executable() {
+        let command = shell_wrapped_command("pwsh.exe", "Get-ChildItem");
+        let std_command = command.as_std();
+        let args: Vec<_> = std_command
+            .get_args()
+            .map(|value| value.to_string_lossy().to_string())
+            .collect();
+
+        assert_eq!(std_command.get_program().to_string_lossy(), "pwsh.exe");
+        assert_eq!(
+            args,
+            vec![
+                "-NoLogo".to_string(),
+                "-NoProfile".to_string(),
+                "-Command".to_string(),
+                "Get-ChildItem".to_string(),
+            ]
+        );
+    }
+}
+
 #[cfg(all(test, unix))]
 mod tests {
     use super::*;
@@ -969,6 +1056,49 @@ mod tests {
         out.output
     }
 
+    /// The shell handle is shared with a live connection, so changing General
+    /// Settings after the agent connects affects its next shell command.
+    #[tokio::test]
+    async fn shell_fallback_uses_the_live_selected_shell() {
+        use std::os::unix::fs::PermissionsExt;
+
+        let dir = tempfile::tempdir().expect("temp dir");
+        let shell = dir.path().join("selected-shell");
+        std::fs::write(
+            &shell,
+            "#!/bin/sh\nprintf '%s\\n' selected-shell\nexec /bin/sh \"$@\"\n",
+        )
+        .expect("write shell");
+        let mut permissions = std::fs::metadata(&shell)
+            .expect("shell metadata")
+            .permissions();
+        permissions.set_mode(0o755);
+        std::fs::set_permissions(&shell, permissions).expect("make shell executable");
+
+        let config = TerminalShellRuntimeConfig::new();
+        let runtime = TerminalRuntime::with_base_env(BTreeMap::new())
+            .with_default_shell_config(config.clone());
+        config
+            .set(Some(shell.to_string_lossy().to_string()))
+            .await;
+
+        let session_id = SessionId::new("selected-shell".to_string());
+        let request = CreateTerminalRequest::new(
+            session_id.clone(),
+            "printf 'command-output\\n'".to_string(),
+        );
+        let output = run_and_capture(&runtime, &session_id, request).await;
+
+        assert!(
+            output.contains("selected-shell"),
+            "configured shell did not run; got:\n{output}"
+        );
+        assert!(
+            output.contains("command-output"),
+            "configured shell did not execute the command; got:\n{output}"
+        );
+    }
+
     /// A `terminal/create` that omits `cwd` defaults to the runtime's
     /// configured working directory rather than codeg's own process cwd.
     #[tokio::test]