diff --git a/codex-rs/core/src/session/turn_context.rs b/codex-rs/core/src/session/turn_context.rs index 7ff1a5b2cf..e6ad409e86 100644 --- a/codex-rs/core/src/session/turn_context.rs +++ b/codex-rs/core/src/session/turn_context.rs @@ -299,13 +299,6 @@ impl TurnContext { } } - #[deprecated(note = "resolve paths from the selected turn environment cwd instead")] - pub(crate) fn resolve_path(&self, path: Option) -> AbsolutePathBuf { - #[allow(deprecated)] - path.as_ref() - .map_or_else(|| self.cwd.clone(), |path| self.cwd.join(path)) - } - pub(crate) fn file_system_sandbox_context( &self, additional_permissions: Option, diff --git a/codex-rs/core/src/tools/handlers/shell/shell_command.rs b/codex-rs/core/src/tools/handlers/shell/shell_command.rs index cf5ef10998..1faa89b31f 100644 --- a/codex-rs/core/src/tools/handlers/shell/shell_command.rs +++ b/codex-rs/core/src/tools/handlers/shell/shell_command.rs @@ -1,7 +1,7 @@ -use codex_protocol::ThreadId; use codex_protocol::models::ShellCommandToolCallParams; use codex_tools::ShellCommandBackendConfig; use codex_tools::ToolName; +use codex_utils_absolute_path::AbsolutePathBuf; use crate::exec::ExecCapturePolicy; use crate::exec::ExecParams; @@ -9,6 +9,7 @@ use crate::exec_env::create_env; use crate::function_tool::FunctionCallError; use crate::maybe_emit_implicit_skill_invocation; use crate::session::turn_context::TurnContext; +use crate::session::turn_context::TurnEnvironment; use crate::shell::Shell; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; @@ -86,14 +87,17 @@ impl ShellCommandHandler { params: &ShellCommandToolCallParams, session: &crate::session::session::Session, turn_context: &TurnContext, - thread_id: ThreadId, + turn_environment: &TurnEnvironment, + cwd: AbsolutePathBuf, allow_login_shell: bool, ) -> Result { - let shell = session.user_shell(); + let session_shell = session.user_shell(); + let shell = turn_environment + .shell + .as_ref() + .unwrap_or(session_shell.as_ref()); let use_login_shell = Self::resolve_use_login_shell(params.login, allow_login_shell)?; - let command = Self::base_command(shell.as_ref(), ¶ms.command, use_login_shell); - #[allow(deprecated)] - let cwd = turn_context.resolve_path(params.workdir.clone()); + let command = Self::base_command(shell, ¶ms.command, use_login_shell); Ok(ExecParams { command, @@ -102,13 +106,10 @@ impl ShellCommandHandler { capture_policy: ExecCapturePolicy::ShellTool, env: create_env( &turn_context.config.permissions.shell_environment_policy, - Some(thread_id), + Some(session.thread_id), ), network: turn_context.network.clone(), - network_environment_id: turn_context - .environments - .primary() - .map(|environment| environment.environment_id.clone()), + network_environment_id: Some(turn_environment.environment_id.clone()), sandbox_permissions: params.sandbox_permissions.unwrap_or_default(), windows_sandbox_level: turn_context.windows_sandbox_level, windows_sandbox_private_desktop: turn_context @@ -181,16 +182,19 @@ impl ShellCommandHandler { )); }; - #[allow(deprecated)] - let cwd = resolve_workdir_base_path(&arguments, &turn.cwd)?; + let environment_cwd = turn_environment.cwd().to_abs_path().map_err(|err| { + FunctionCallError::RespondToModel(format!( + "shell_command cwd `{}` is not native to the Codex host: {err}", + turn_environment.cwd() + )) + })?; + let cwd = resolve_workdir_base_path(&arguments, &environment_cwd)?; let params: ShellCommandToolCallParams = parse_arguments_with_base_path(&arguments, &cwd)?; - #[allow(deprecated)] - let workdir = turn.resolve_path(params.workdir.clone()); maybe_emit_implicit_skill_invocation( session.as_ref(), turn.as_ref(), ¶ms.command, - &workdir, + &cwd, ) .await; let prefix_rule = params.prefix_rule.clone(); @@ -198,10 +202,16 @@ impl ShellCommandHandler { ¶ms, session.as_ref(), turn.as_ref(), - session.thread_id, + &turn_environment, + cwd, turn.config.permissions.allow_login_shell, )?; - let shell_type = Some(session.user_shell().shell_type); + let shell_type = Some( + turn_environment + .shell + .as_ref() + .map_or_else(|| session.user_shell().shell_type, |shell| shell.shell_type), + ); run_exec_like(RunExecLikeArgs { tool_name, exec_params, diff --git a/codex-rs/core/src/tools/handlers/shell_tests.rs b/codex-rs/core/src/tools/handlers/shell_tests.rs index 885e0d2dce..bcc60885b0 100644 --- a/codex-rs/core/src/tools/handlers/shell_tests.rs +++ b/codex-rs/core/src/tools/handlers/shell_tests.rs @@ -8,6 +8,7 @@ use crate::exec_env::create_env; use crate::sandboxing::SandboxPermissions; use crate::session::step_context::StepContext; use crate::session::tests::make_session_and_context; +use crate::session::turn_context::TurnEnvironment; use crate::shell::Shell; use crate::shell::ShellType; use crate::tools::context::FunctionToolOutput; @@ -21,6 +22,7 @@ use crate::turn_diff_tracker::TurnDiffTracker; use codex_shell_command::is_safe_command::is_known_safe_command; use codex_shell_command::powershell::try_find_powershell_executable_blocking; use codex_shell_command::powershell::try_find_pwsh_executable_blocking; +use codex_utils_path_uri::PathUri; use serde_json::json; use tokio::sync::Mutex; @@ -68,7 +70,7 @@ fn assert_safe(shell: &Shell, command: &str) { } #[tokio::test] -async fn shell_command_handler_to_exec_params_uses_session_shell_and_turn_context() { +async fn shell_command_handler_to_exec_params_uses_selected_environment() { let (session, turn_context) = make_session_and_context().await; let command = "echo hello".to_string(); @@ -78,11 +80,25 @@ async fn shell_command_handler_to_exec_params_uses_session_shell_and_turn_contex let sandbox_permissions = SandboxPermissions::RequireEscalated; let justification = Some("because tests".to_string()); - let expected_command = session - .user_shell() - .derive_exec_args(&command, /*use_login_shell*/ true); - #[allow(deprecated)] - let expected_cwd = turn_context.resolve_path(workdir.clone()); + let selected_shell = Shell { + shell_type: ShellType::Bash, + shell_path: PathBuf::from("/selected/bin/bash"), + }; + let expected_command = selected_shell.derive_exec_args(&command, /*use_login_shell*/ true); + let selected_cwd = turn_context.config.cwd.join("selected-environment"); + let expected_cwd = selected_cwd.join("subdir"); + let selected_environment = TurnEnvironment::new( + "selected-environment".to_string(), + Arc::clone( + &turn_context + .environments + .primary() + .expect("primary environment") + .environment, + ), + PathUri::from_abs_path(&selected_cwd), + Some(selected_shell), + ); let expected_env = create_env( &turn_context.config.permissions.shell_environment_policy, Some(session.thread_id), @@ -103,7 +119,8 @@ async fn shell_command_handler_to_exec_params_uses_session_shell_and_turn_contex ¶ms, &session, &turn_context, - session.thread_id, + &selected_environment, + expected_cwd.clone(), /*allow_login_shell*/ true, ) .expect("login shells should be allowed"); @@ -113,6 +130,10 @@ async fn shell_command_handler_to_exec_params_uses_session_shell_and_turn_contex assert_eq!(exec_params.cwd, expected_cwd); assert_eq!(exec_params.env, expected_env); assert_eq!(exec_params.network, turn_context.network); + assert_eq!( + exec_params.network_environment_id.as_deref(), + Some("selected-environment") + ); assert_eq!(exec_params.expiration.timeout_ms(), timeout_ms); assert_eq!(exec_params.sandbox_permissions, sandbox_permissions); assert_eq!(exec_params.justification, justification); @@ -150,6 +171,14 @@ fn shell_command_handler_respects_explicit_login_flag() { #[tokio::test] async fn shell_command_handler_defaults_to_non_login_when_disallowed() { let (session, turn_context) = make_session_and_context().await; + let turn_environment = turn_context + .environments + .primary() + .expect("primary environment"); + let cwd = turn_environment + .cwd() + .to_abs_path() + .expect("native environment cwd"); let params = ShellCommandToolCallParams { command: "echo hello".to_string(), workdir: None, @@ -165,7 +194,8 @@ async fn shell_command_handler_defaults_to_non_login_when_disallowed() { ¶ms, &session, &turn_context, - session.thread_id, + turn_environment, + cwd, /*allow_login_shell*/ false, ) .expect("non-login shells should still be allowed");