diff --git a/codex-rs/core/src/tools/handlers/unified_exec/exec_command.rs b/codex-rs/core/src/tools/handlers/unified_exec/exec_command.rs index eb89e56ef2..31f60e93d8 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec/exec_command.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec/exec_command.rs @@ -1,3 +1,4 @@ +use std::path::Path; use std::sync::Arc; use crate::function_tool::FunctionCallError; @@ -139,7 +140,7 @@ impl ExecCommandHandler { ); let environment = Arc::clone(&turn_environment.environment); let fs = environment.get_filesystem(); - let args: ExecCommandArgs = parse_arguments_with_base_path(&arguments, &cwd)?; + let mut args: ExecCommandArgs = parse_arguments_with_base_path(&arguments, &cwd)?; let hook_command = args.cmd.clone(); maybe_emit_implicit_skill_invocation( session.as_ref(), @@ -151,13 +152,28 @@ impl ExecCommandHandler { let process_id = manager.allocate_process_id().await; let shell_mode = shell_mode_for_environment(&turn.unified_exec_shell_mode, environment.as_ref()); - // Remote environments may use a different OS and must build commands with their native - // shell; fall back to the session shell when the environment did not report one. - let shell = turn_environment - .shell - .clone() - .map(Arc::new) - .unwrap_or_else(|| session.user_shell()); + let shell = if environment.is_remote() { + let environment_shell = turn_environment.shell.clone().ok_or_else(|| { + FunctionCallError::RespondToModel(format!( + "shell is unavailable in environment `{}`", + turn_environment.environment_id + )) + })?; + if let Some(provided_shell) = args.shell.as_deref() { + let matches_environment = provided_shell == environment_shell.name() + || Path::new(provided_shell) == environment_shell.shell_path; + if !matches_environment { + return Err(FunctionCallError::RespondToModel(format!( + "`shell` must match the selected remote environment shell `{}`", + environment_shell.shell_path.display() + ))); + } + args.shell = None; + } + Arc::new(environment_shell) + } else { + session.user_shell() + }; let resolved_command = get_command( &args, shell, diff --git a/codex-rs/core/tests/suite/remote_env.rs b/codex-rs/core/tests/suite/remote_env.rs index 311485c112..48f07199f0 100644 --- a/codex-rs/core/tests/suite/remote_env.rs +++ b/codex-rs/core/tests/suite/remote_env.rs @@ -366,18 +366,43 @@ async fn exec_command_routes_to_selected_remote_environment() -> Result<()> { environment_id: REMOTE_ENVIRONMENT_ID.to_string(), cwd: PathUri::from_abs_path(&remote_cwd), }; + let selections = vec![local_selection, remote_selection]; + let mismatched_shell_output = exec_command_routing_output( + &test, + &server, + "call-remote-mismatched-shell", + json!({ + "shell": "/definitely/not/the/remote/shell", + "cmd": "echo should-not-run", + "environment_id": REMOTE_ENVIRONMENT_ID, + }), + Some(selections.clone()), + ) + .await?; + assert!( + mismatched_shell_output + .contains("`shell` must match the selected remote environment shell"), + "unexpected mismatched shell output: {mismatched_shell_output}", + ); + let remote_shell_path = test + .executor_environment() + .environment() + .info() + .await? + .shell + .path; let multi_env_output = exec_command_routing_output( &test, &server, "call-multi-env", json!({ - "shell": "/bin/sh", + "shell": remote_shell_path, "cmd": format!("cat {remote_marker_name}"), "login": false, "yield_time_ms": 1_000, "environment_id": REMOTE_ENVIRONMENT_ID, }), - Some(vec![local_selection, remote_selection]), + Some(selections), ) .await?; assert!( @@ -496,7 +521,6 @@ async fn remote_request_permissions_grant_unblocks_later_remote_exec() -> Result "exec-call", "exec_command", &json!({ - "shell": "/bin/sh", "cmd": command, "login": false, "yield_time_ms": 1_000, @@ -925,7 +949,6 @@ async fn apply_patch_intercepted_exec_command_routes_to_selected_remote_environm call_id, "exec_command", &serde_json::to_string(&json!({ - "shell": "/bin/sh", "cmd": command, "login": false, "yield_time_ms": 5_000, diff --git a/codex-rs/core/tests/suite/unified_exec.rs b/codex-rs/core/tests/suite/unified_exec.rs index 9f8d931da2..b24eb81997 100644 --- a/codex-rs/core/tests/suite/unified_exec.rs +++ b/codex-rs/core/tests/suite/unified_exec.rs @@ -405,10 +405,16 @@ async fn unified_exec_emits_exec_command_begin_event() -> Result<()> { }); let test = builder.build_with_remote_env(&server).await?; let cwd = test.config.cwd.to_path_buf(); + let shell_path = test + .executor_environment() + .environment() + .info() + .await? + .shell + .path; 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, }); @@ -435,7 +441,12 @@ async fn unified_exec_emits_exec_command_begin_event() -> Result<()> { }) .await; - assert_command(&begin_event.command, "-lc", "/bin/echo hello unified exec"); + assert_command( + &begin_event.command, + &shell_path, + "-lc", + "/bin/echo hello unified exec", + ); assert_eq!(begin_event.cwd.as_path(), cwd.as_path()); @@ -1300,10 +1311,16 @@ async fn unified_exec_emits_one_begin_and_one_end_event() -> Result<()> { .expect("test config should allow feature update"); }); let test = builder.build_with_remote_env(&server).await?; + let shell_path = test + .executor_environment() + .environment() + .info() + .await? + .shell + .path; let open_call_id = "uexec-open-session"; let open_args = json!({ - "shell": "bash".to_string(), "cmd": "sleep 0.1".to_string(), "yield_time_ms": 10, }); @@ -1393,7 +1410,7 @@ async fn unified_exec_emits_one_begin_and_one_end_event() -> Result<()> { let open_event = &begin_events[0]; - assert_command(&open_event.command, "-lc", "sleep 0.1"); + assert_command(&open_event.command, &shell_path, "-lc", "sleep 0.1"); assert!( open_event.interaction_input.is_none(), @@ -2204,7 +2221,6 @@ async fn write_stdin_ctrl_c_reports_unsupported_interrupt_to_model_on_windows() let interrupt_call_id = "uexec-windows-interrupt"; let start_args = serde_json::json!({ - "shell": "cmd", "cmd": "echo READY && ping -n 30 127.0.0.1 >NUL", "yield_time_ms": 250, "tty": false, @@ -3552,16 +3568,14 @@ async fn unified_exec_prunes_exited_sessions_first() -> Result<()> { Ok(()) } -fn assert_command(command: &[String], expected_args: &str, expected_cmd: &str) { +fn assert_command( + command: &[String], + expected_shell: &str, + 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[0], expected_shell); assert_eq!(command[1], expected_args); assert_eq!(command[2], expected_cmd); }