diff --git a/crates/tui/src/sandbox/mod.rs b/crates/tui/src/sandbox/mod.rs index 54e43085..36ac39c0 100644 --- a/crates/tui/src/sandbox/mod.rs +++ b/crates/tui/src/sandbox/mod.rs @@ -157,7 +157,10 @@ impl CommandSpec { /// Get the original command as a single string (for display). pub fn display_command(&self) -> String { - if self.program == "sh" && self.args.len() == 2 && self.args[0] == "-c" { + if (self.program == "sh" || self.program == "bash") + && self.args.len() == 2 + && self.args[0] == "-c" + { // For shell commands, show the actual command self.args[1].clone() } else if self.program.eq_ignore_ascii_case("cmd") @@ -170,9 +173,13 @@ impl CommandSpec { raw.strip_prefix("chcp 65001 >NUL & ") .unwrap_or(raw) .to_string() - } else if (self.program.eq_ignore_ascii_case("pwsh") - || self.program.eq_ignore_ascii_case("powershell")) - && self.args.len() >= 3 + } else if { + let program = self.program.to_ascii_lowercase(); + program == "pwsh" + || program == "pwsh.exe" + || program == "powershell" + || program == "powershell.exe" + } && self.args.len() >= 3 && self.args[0].eq_ignore_ascii_case("-NoProfile") && self.args[1].eq_ignore_ascii_case("-Command") { @@ -601,26 +608,11 @@ impl SandboxManager { mod tests { use super::*; - fn expected_shell_command(command: &str) -> Vec { - #[cfg(windows)] - { - vec![ - "cmd".to_string(), - "/C".to_string(), - format!("chcp 65001 >NUL & {command}"), - ] - } - #[cfg(not(windows))] - { - vec!["sh".to_string(), "-c".to_string(), command.to_string()] - } - } - #[test] fn test_command_spec_shell() { let spec = CommandSpec::shell("echo hello", PathBuf::from("/tmp"), Duration::from_secs(30)); - // Program and args depend on the detected shell (pwsh, cmd, sh, …). + // Program and args depend on the detected shell. assert!(!spec.program.is_empty(), "program must not be empty"); assert!(!spec.args.is_empty(), "args must not be empty"); assert_eq!(spec.display_command(), "echo hello"); @@ -636,19 +628,30 @@ mod tests { let cmd = r#"git commit -m "feat: complete sub-pages""#; let spec = CommandSpec::shell(cmd, PathBuf::from("/tmp"), Duration::from_secs(30)); - #[cfg(windows)] - { - assert_eq!(spec.program, "cmd"); + let dispatcher = crate::shell_dispatcher::global_dispatcher(); + assert_eq!(spec.program, dispatcher.kind().binary()); + if dispatcher.kind().is_powershell() { assert_eq!( spec.args, - vec!["/C".to_string(), format!("chcp 65001 >NUL & {cmd}")] + vec![ + dispatcher.kind().command_flag().to_string(), + "-Command".to_string(), + format!("[Console]::OutputEncoding = [System.Text.Encoding]::UTF8; {cmd}") + ] ); - } - #[cfg(not(windows))] - { - assert_eq!(spec.program, "sh"); - assert_eq!(spec.args, vec!["-c".to_string(), cmd.to_string()]); - // The quoted message is intact in a single argv slot — `sh -c` + } else { + assert_eq!( + spec.args, + if cfg!(windows) { + vec!["/C".to_string(), format!("chcp 65001 >NUL & {cmd}")] + } else { + vec![ + dispatcher.kind().command_flag().to_string(), + cmd.to_string(), + ] + } + ); + // The quoted message is intact in a single argv slot — shell `-c` // performs POSIX tokenization, yielding the correct argv: // ["git","commit","-m","feat: complete sub-pages"]. assert_eq!(spec.args.len(), 2); @@ -710,9 +713,39 @@ mod tests { .with_policy(SandboxPolicy::DangerFullAccess); let env = manager.prepare(&spec); + let dispatcher = crate::shell_dispatcher::global_dispatcher(); assert_eq!(env.sandbox_type, SandboxType::None); - assert_eq!(env.command, expected_shell_command("echo test")); + if dispatcher.kind().is_powershell() { + assert_eq!( + env.command, + vec![ + dispatcher.kind().binary().to_string(), + dispatcher.kind().command_flag().to_string(), + "-Command".to_string(), + "[Console]::OutputEncoding = [System.Text.Encoding]::UTF8; echo test" + .to_string(), + ] + ); + } else if cfg!(windows) { + assert_eq!( + env.command, + vec![ + dispatcher.kind().binary().to_string(), + "/C".to_string(), + "chcp 65001 >NUL & echo test".to_string(), + ] + ); + } else { + assert_eq!( + env.command, + vec![ + dispatcher.kind().binary().to_string(), + dispatcher.kind().command_flag().to_string(), + "echo test".to_string(), + ] + ); + } assert!(!env.is_sandboxed()); } diff --git a/crates/tui/src/tools/shell.rs b/crates/tui/src/tools/shell.rs index 31f09f1b..2cfae192 100644 --- a/crates/tui/src/tools/shell.rs +++ b/crates/tui/src/tools/shell.rs @@ -846,16 +846,25 @@ impl ShellManager { child_env::apply_to_command(&mut cmd, child_env::string_map_env(&exec_env.env)); - // Disable raw mode before spawn; restore on drop regardless of - // success/failure/timeout (issue #1690). - let _ = crossterm::terminal::disable_raw_mode(); - struct SyncRawModeGuard; + // Disable raw mode before spawn; restore only if raw mode was active + // on entry (issue #1690). + let raw_mode_was_enabled = crossterm::terminal::is_raw_mode_enabled().unwrap_or(false); + if raw_mode_was_enabled { + let _ = crossterm::terminal::disable_raw_mode(); + } + struct SyncRawModeGuard { + restore: bool, + } impl Drop for SyncRawModeGuard { fn drop(&mut self) { - let _ = crossterm::terminal::enable_raw_mode(); + if self.restore { + let _ = crossterm::terminal::enable_raw_mode(); + } } } - let _guard = SyncRawModeGuard; + let _guard = SyncRawModeGuard { + restore: raw_mode_was_enabled, + }; let mut child = cmd .spawn() @@ -991,15 +1000,25 @@ impl ShellManager { } install_parent_death_signal(&mut cmd); - // Disable raw mode before spawn; restore on drop (issue #1690). - let _ = crossterm::terminal::disable_raw_mode(); - struct InteractiveRawModeGuard; + // Disable raw mode before spawn; restore only if raw mode was active + // on entry (issue #1690). + let raw_mode_was_enabled = crossterm::terminal::is_raw_mode_enabled().unwrap_or(false); + if raw_mode_was_enabled { + let _ = crossterm::terminal::disable_raw_mode(); + } + struct InteractiveRawModeGuard { + restore: bool, + } impl Drop for InteractiveRawModeGuard { fn drop(&mut self) { - let _ = crossterm::terminal::enable_raw_mode(); + if self.restore { + let _ = crossterm::terminal::enable_raw_mode(); + } } } - let _guard = InteractiveRawModeGuard; + let _guard = InteractiveRawModeGuard { + restore: raw_mode_was_enabled, + }; child_env::apply_to_command(&mut cmd, child_env::string_map_env(&exec_env.env)); diff --git a/crates/tui/src/tools/shell/tests.rs b/crates/tui/src/tools/shell/tests.rs index 477c785f..e43cf1db 100644 --- a/crates/tui/src/tools/shell/tests.rs +++ b/crates/tui/src/tools/shell/tests.rs @@ -910,41 +910,48 @@ fn issue_1691_quoted_commit_message_round_trips() { Duration::from_secs(5), ); - #[cfg(not(windows))] - { - // `sh -c `: the whole command (with quotes) is a single argv - // entry. `sh` then POSIX-tokenizes it → correct git argv. We never - // split the command string ourselves. - assert_eq!(spec.program, "sh"); - assert_eq!(spec.args, ["-c".to_string(), cmd.to_string()]); - assert_eq!(spec.args.len(), 2); - - // push_shell_args is a faithful pass-through on Unix. - let mut built = Command::new(&spec.program); - push_shell_args(&mut built, &spec.program, &spec.args); - let got: Vec = built - .get_args() - .map(|a| a.to_string_lossy().into_owned()) - .collect(); - assert_eq!(got, ["-c".to_string(), cmd.to_string()]); - } - - #[cfg(windows)] - { - // `cmd /C `: payload carries the quotes verbatim. The fix - // routes /C + payload through `raw_arg` so `cmd.exe` (not MSVCRT) - // parses it, matching what a terminal does. - assert_eq!(spec.program, "cmd"); + let dispatcher = crate::shell_dispatcher::global_dispatcher(); + // The whole command (with quotes) is a single argv entry. The actual + // shell binary can vary by platform, but the payload itself must stay + // intact in one shell arg. We never split the command string ourselves. + assert_eq!(spec.program, dispatcher.kind().binary()); + if dispatcher.kind().is_powershell() { + assert_eq!( + spec.args, + [ + dispatcher.kind().command_flag().to_string(), + "-Command".to_string(), + format!("[Console]::OutputEncoding = [System.Text.Encoding]::UTF8; {cmd}") + ] + ); + } else if cfg!(windows) { assert_eq!( spec.args, ["/C".to_string(), format!("chcp 65001 >NUL & {cmd}")] ); - let mut built = Command::new(&spec.program); - push_shell_args(&mut built, &spec.program, &spec.args); - let got: Vec = built - .get_args() - .map(|a| a.to_string_lossy().into_owned()) - .collect(); - assert_eq!(got, spec.args); + } else { + assert_eq!( + spec.args, + [ + dispatcher.kind().command_flag().to_string(), + cmd.to_string() + ] + ); } + assert_eq!( + spec.args.len(), + if dispatcher.kind().is_powershell() { + 3 + } else { + 2 + } + ); + + let mut built = Command::new(&spec.program); + push_shell_args(&mut built, &spec.program, &spec.args); + let got: Vec = built + .get_args() + .map(|a| a.to_string_lossy().into_owned()) + .collect(); + assert_eq!(got, spec.args); }