diff --git a/codex-rs/core/src/tools/handlers/unified_exec.rs b/codex-rs/core/src/tools/handlers/unified_exec.rs index 344b4afaf..4c943c628 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec.rs @@ -6,6 +6,7 @@ use crate::protocol::EventMsg; use crate::protocol::ExecCommandOutputDeltaEvent; use crate::protocol::ExecCommandSource; use crate::protocol::ExecOutputStream; +use crate::shell::default_user_shell; use crate::shell::get_shell_by_model_provided_path; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolOutput; @@ -31,8 +32,8 @@ struct ExecCommandArgs { cmd: String, #[serde(default)] workdir: Option, - #[serde(default = "default_shell")] - shell: String, + #[serde(default)] + shell: Option, #[serde(default = "default_login")] login: bool, #[serde(default = "default_exec_yield_time_ms")] @@ -65,10 +66,6 @@ fn default_write_stdin_yield_time_ms() -> u64 { 250 } -fn default_shell() -> String { - "/bin/bash".to_string() -} - fn default_login() -> bool { true } @@ -257,7 +254,12 @@ impl ToolHandler for UnifiedExecHandler { } fn get_command(args: &ExecCommandArgs) -> Vec { - let shell = get_shell_by_model_provided_path(&PathBuf::from(args.shell.clone())); + let shell = if let Some(shell_str) = &args.shell { + get_shell_by_model_provided_path(&PathBuf::from(shell_str)) + } else { + default_user_shell() + }; + shell.derive_exec_args(&args.cmd, args.login) } @@ -289,3 +291,65 @@ fn format_response(response: &UnifiedExecResponse) -> String { sections.join("\n") } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_get_command_uses_default_shell_when_unspecified() { + let json = r#"{"cmd": "echo hello"}"#; + + let args: ExecCommandArgs = + serde_json::from_str(json).expect("deserialize ExecCommandArgs"); + + assert!(args.shell.is_none()); + + let command = get_command(&args); + + assert_eq!(command.len(), 3); + assert_eq!(command[2], "echo hello"); + } + + #[test] + fn test_get_command_respects_explicit_bash_shell() { + let json = r#"{"cmd": "echo hello", "shell": "/bin/bash"}"#; + + let args: ExecCommandArgs = + serde_json::from_str(json).expect("deserialize ExecCommandArgs"); + + assert_eq!(args.shell.as_deref(), Some("/bin/bash")); + + let command = get_command(&args); + + assert_eq!(command[2], "echo hello"); + } + + #[test] + fn test_get_command_respects_explicit_powershell_shell() { + let json = r#"{"cmd": "echo hello", "shell": "powershell"}"#; + + let args: ExecCommandArgs = + serde_json::from_str(json).expect("deserialize ExecCommandArgs"); + + assert_eq!(args.shell.as_deref(), Some("powershell")); + + let command = get_command(&args); + + assert_eq!(command[2], "echo hello"); + } + + #[test] + fn test_get_command_respects_explicit_cmd_shell() { + let json = r#"{"cmd": "echo hello", "shell": "cmd"}"#; + + let args: ExecCommandArgs = + serde_json::from_str(json).expect("deserialize ExecCommandArgs"); + + assert_eq!(args.shell.as_deref(), Some("cmd")); + + let command = get_command(&args); + + assert_eq!(command[2], "echo hello"); + } +} diff --git a/codex-rs/core/tests/suite/unified_exec.rs b/codex-rs/core/tests/suite/unified_exec.rs index 584882ce7..dc7bdb6b1 100644 --- a/codex-rs/core/tests/suite/unified_exec.rs +++ b/codex-rs/core/tests/suite/unified_exec.rs @@ -295,6 +295,7 @@ async fn unified_exec_emits_exec_command_begin_event() -> Result<()> { let call_id = "uexec-begin-event"; let args = json!({ + "shell": "bash".to_string(), "cmd": "/bin/echo hello unified exec".to_string(), "yield_time_ms": 250, }); @@ -336,14 +337,8 @@ async fn unified_exec_emits_exec_command_begin_event() -> Result<()> { }) .await; - assert_eq!( - begin_event.command, - vec![ - "/bin/bash".to_string(), - "-lc".to_string(), - "/bin/echo hello unified exec".to_string() - ] - ); + assert_command(&begin_event.command, "-lc", "/bin/echo hello unified exec"); + assert_eq!(begin_event.cwd, cwd.path()); wait_for_event(&codex, |event| matches!(event, EventMsg::TaskComplete(_))).await; @@ -782,6 +777,7 @@ async fn unified_exec_emits_begin_for_write_stdin() -> Result<()> { let open_call_id = "uexec-open-for-begin"; let open_args = json!({ + "shell": "bash".to_string(), "cmd": "bash -i".to_string(), "yield_time_ms": 200, }); @@ -843,14 +839,7 @@ async fn unified_exec_emits_begin_for_write_stdin() -> Result<()> { }) .await; - assert_eq!( - begin_event.command, - vec![ - "/bin/bash".to_string(), - "-lc".to_string(), - "bash -i".to_string() - ] - ); + assert_command(&begin_event.command, "-lc", "bash -i"); assert_eq!( begin_event.interaction_input, Some("echo hello".to_string()) @@ -884,6 +873,7 @@ async fn unified_exec_emits_begin_event_for_write_stdin_requests() -> Result<()> let open_call_id = "uexec-open-session"; let open_args = json!({ + "shell": "bash".to_string(), "cmd": "bash -i".to_string(), "yield_time_ms": 250, }); @@ -959,14 +949,9 @@ async fn unified_exec_emits_begin_event_for_write_stdin_requests() -> Result<()> .iter() .find(|ev| ev.call_id == open_call_id) .expect("missing exec_command begin"); - assert_eq!( - open_event.command, - vec![ - "/bin/bash".to_string(), - "-lc".to_string(), - "bash -i".to_string() - ] - ); + + assert_command(&open_event.command, "-lc", "bash -i"); + assert!( open_event.interaction_input.is_none(), "startup begin events should not include interaction input" @@ -977,14 +962,9 @@ async fn unified_exec_emits_begin_event_for_write_stdin_requests() -> Result<()> .iter() .find(|ev| ev.call_id == poll_call_id) .expect("missing write_stdin begin"); - assert_eq!( - poll_event.command, - vec![ - "/bin/bash".to_string(), - "-lc".to_string(), - "bash -i".to_string() - ] - ); + + assert_command(&poll_event.command, "-lc", "bash -i"); + assert!( poll_event.interaction_input.is_none(), "poll begin events should omit interaction input" @@ -2121,3 +2101,17 @@ async fn unified_exec_prunes_exited_sessions_first() -> Result<()> { Ok(()) } + +fn assert_command(command: &[String], expected_args: &str, expected_cmd: &str) { + assert_eq!(command.len(), 3); + let shell_path = &command[0]; + assert!( + shell_path == "/bin/bash" + || shell_path == "/usr/bin/bash" + || shell_path == "/usr/local/bin/bash" + || shell_path.ends_with("/bash"), + "unexpected bash path: {shell_path}" + ); + assert_eq!(command[1], expected_args); + assert_eq!(command[2], expected_cmd); +}