fix(tui): align shell tests with detected shell

This commit is contained in:
Paulo Aboim Pinto
2026-05-27 17:24:28 +02:00
committed by Hunter Bown
parent f1afcf316f
commit ec643c5054
3 changed files with 133 additions and 74 deletions
+64 -31
View File
@@ -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<String> {
#[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());
}
+30 -11
View File
@@ -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));
+39 -32
View File
@@ -910,41 +910,48 @@ fn issue_1691_quoted_commit_message_round_trips() {
Duration::from_secs(5),
);
#[cfg(not(windows))]
{
// `sh -c <cmd>`: 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<String> = 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>`: 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<String> = 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<String> = built
.get_args()
.map(|a| a.to_string_lossy().into_owned())
.collect();
assert_eq!(got, spec.args);
}