diff --git a/codex-rs/core/src/environment_selection.rs b/codex-rs/core/src/environment_selection.rs index 8b0a2d6b5..8fb59ba92 100644 --- a/codex-rs/core/src/environment_selection.rs +++ b/codex-rs/core/src/environment_selection.rs @@ -194,6 +194,12 @@ impl TurnEnvironmentSnapshot { self.turn_environments.first() } + pub(crate) fn local(&self) -> Option<&TurnEnvironment> { + self.turn_environments + .iter() + .find(|environment| !environment.environment.is_remote()) + } + #[cfg(test)] pub(crate) fn primary_environment(&self) -> Option> { self.primary() diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 927bf0469..207f7eab9 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -36,6 +36,7 @@ use crate::exec_policy::ExecPolicyManager; use crate::image_preparation::prepare_response_items; use crate::parse_turn_item; use crate::realtime_conversation::RealtimeConversationManager; +use crate::session::turn_context::TurnEnvironment; use crate::session_prefix::format_subagent_notification_message; use crate::skills::SkillRenderSideEffects; use crate::skills_load_input_from_config; @@ -1504,10 +1505,11 @@ impl Session { self.emit_config_changed_contributors(previous_config.as_ref(), new_config.as_ref()); self.services.skills_manager.clear_cache(); self.services.plugins_manager.clear_cache(); + let environments = self.services.turn_environments.snapshot().await; let hooks = build_hooks_for_config( config.as_ref(), self.services.plugins_manager.as_ref(), - self.services.user_shell.as_ref(), + environments.single_local_environment(), ) .await; @@ -3528,11 +3530,17 @@ pub(crate) fn emit_subagent_session_started( async fn build_hooks_for_config( config: &Config, plugins_manager: &PluginsManager, - user_shell: &crate::shell::Shell, + environment: Option<&TurnEnvironment>, ) -> Hooks { - let mut hook_shell_argv = user_shell.derive_exec_args("", /*use_login_shell*/ false); - let hook_shell_program = hook_shell_argv.remove(0); - let _ = hook_shell_argv.pop(); + let (hook_shell_program, hook_shell_argv) = environment + .and_then(|environment| environment.shell.as_ref()) + .map(|shell| { + let mut argv = shell.derive_exec_args("", /*use_login_shell*/ false); + let program = argv.remove(0); + let _ = argv.pop(); + (Some(program), argv) + }) + .unwrap_or_default(); let plugins_input = config.plugins_config_input(); let plugin_outcome = plugins_manager.plugins_for_config(&plugins_input).await; let plugin_hook_sources = plugin_outcome.effective_plugin_hook_sources(); @@ -3544,7 +3552,7 @@ async fn build_hooks_for_config( config_layer_stack: Some(config.config_layer_stack.clone()), plugin_hook_sources, plugin_hook_load_warnings, - shell_program: Some(hook_shell_program), + shell_program: hook_shell_program, shell_args: hook_shell_argv, }) } diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index 864090033..46f20eaf5 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -439,7 +439,7 @@ async fn warm_plugins_and_skills_for_session_init( config: Arc, plugins_manager: Arc, skills_manager: Arc, - turn_environments: TurnEnvironmentSnapshot, + turn_environments: &TurnEnvironmentSnapshot, ) -> Vec { let fs = turn_environments.primary_filesystem(); let plugins_input = config.plugins_config_input(); @@ -829,7 +829,7 @@ impl Session { Arc::clone(&config), Arc::clone(&plugins_manager), Arc::clone(&skills_manager), - resolved_environments, + &resolved_environments, ) .instrument(info_span!( "session_init.plugin_skill_warmup", @@ -913,8 +913,12 @@ impl Session { (None, None) }; - let hooks = - build_hooks_for_config(&config, plugins_manager.as_ref(), &default_shell).await; + let hooks = build_hooks_for_config( + &config, + plugins_manager.as_ref(), + resolved_environments.single_local_environment(), + ) + .await; for warning in hooks.startup_warnings() { post_session_configured_events.push(Event { id: INITIAL_SUBMIT_ID.to_owned(), diff --git a/codex-rs/core/src/tasks/user_shell.rs b/codex-rs/core/src/tasks/user_shell.rs index 4a2a76494..6a01a87a0 100644 --- a/codex-rs/core/src/tasks/user_shell.rs +++ b/codex-rs/core/src/tasks/user_shell.rs @@ -29,6 +29,7 @@ use crate::turn_timing::now_unix_timestamp_ms; use crate::user_shell_command::user_shell_command_record_item; use codex_protocol::exec_output::ExecToolCallOutput; use codex_protocol::exec_output::StreamOutput; +use codex_protocol::protocol::ErrorEvent; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::ExecCommandBeginEvent; use codex_protocol::protocol::ExecCommandEndEvent; @@ -124,18 +125,26 @@ pub(crate) async fn execute_user_shell_command( session.send_event(turn_context.as_ref(), event).await; } - // Execute the user's script under their default shell when known; this + let Some((turn_environment, environment_shell)) = turn_context + .environments + .local() + .and_then(|environment| environment.shell.as_ref().map(|shell| (environment, shell))) + else { + send_user_shell_error( + &session, + turn_context.as_ref(), + "shell is unavailable in this session", + ) + .await; + return; + }; + + // Execute the user's script under the environment's shell; this // allows commands that use shell features (pipes, &&, redirects, etc.). // We do not source rc files or otherwise reformat the script. let use_login_shell = true; - let session_shell = session.user_shell(); - let environment = turn_context.environments.single_local_environment(); - let shell = environment - .and_then(|environment| environment.shell.as_ref()) - .unwrap_or(session_shell.as_ref()); - let shell_snapshot_location = - environment.and_then(|environment| environment.shell_snapshot(environment.cwd())); - let display_command = shell.derive_exec_args(&command, use_login_shell); + let display_command = environment_shell.derive_exec_args(&command, use_login_shell); + let shell_snapshot_location = turn_environment.shell_snapshot(turn_environment.cwd()); let mut exec_env_map = create_env( &turn_context.shell_environment_policy, Some(session.thread_id), @@ -145,7 +154,7 @@ pub(crate) async fn execute_user_shell_command( } let exec_command = prepare_user_shell_exec_command( &display_command, - shell, + environment_shell, shell_snapshot_location.as_ref(), &turn_context.shell_environment_policy.r#set, &mut exec_env_map, @@ -153,10 +162,7 @@ pub(crate) async fn execute_user_shell_command( let call_id = Uuid::new_v4().to_string(); let raw_command = command; - #[allow(deprecated)] - let cwd = environment - .map(|environment| environment.cwd().clone()) - .unwrap_or_else(|| turn_context.cwd.clone()); + let cwd = turn_environment.cwd().clone(); let parsed_cmd = parse_command(&display_command); session @@ -341,9 +347,21 @@ pub(crate) async fn execute_user_shell_command( } } +async fn send_user_shell_error(session: &Session, turn_context: &TurnContext, message: &str) { + session + .send_event( + turn_context, + EventMsg::Error(ErrorEvent { + message: message.to_string(), + codex_error_info: None, + }), + ) + .await; +} + fn prepare_user_shell_exec_command( display_command: &[String], - session_shell: &Shell, + shell: &Shell, shell_snapshot: Option<&AbsolutePathBuf>, shell_environment_set: &HashMap, exec_env_map: &mut HashMap, @@ -352,7 +370,7 @@ fn prepare_user_shell_exec_command( { prepare_user_shell_exec_command_with_path_prepend( display_command, - session_shell, + shell, shell_snapshot, shell_environment_set, exec_env_map, @@ -364,7 +382,7 @@ fn prepare_user_shell_exec_command( { maybe_wrap_shell_lc_with_snapshot( display_command, - session_shell, + shell, shell_snapshot, shell_environment_set, exec_env_map, @@ -384,7 +402,7 @@ fn prepare_user_shell_exec_command( #[cfg(unix)] fn prepare_user_shell_exec_command_with_path_prepend( display_command: &[String], - session_shell: &Shell, + shell: &Shell, shell_snapshot: Option<&AbsolutePathBuf>, shell_environment_set: &HashMap, exec_env_map: &mut HashMap, @@ -395,7 +413,7 @@ fn prepare_user_shell_exec_command_with_path_prepend( prepend_runtime_path(exec_env_map, &mut runtime_path_prepends); maybe_wrap_shell_lc_with_snapshot( display_command, - session_shell, + shell, shell_snapshot, &explicit_env_overrides, exec_env_map, diff --git a/codex-rs/core/src/tools/runtimes/unified_exec.rs b/codex-rs/core/src/tools/runtimes/unified_exec.rs index 23c65bdeb..25a1b932c 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -322,7 +322,7 @@ impl<'a> ToolRuntime for UnifiedExecRunt attempt.sandbox, attempt.windows_sandbox_level, ); - let command = if matches!(session_shell.shell_type, ShellType::PowerShell) { + let command = if matches!(req.shell_type, ShellType::PowerShell) { prefix_powershell_script_with_utf8(&command) } else { command diff --git a/codex-rs/core/tests/suite/user_shell_cmd.rs b/codex-rs/core/tests/suite/user_shell_cmd.rs index f7aed6b47..4a9605497 100644 --- a/codex-rs/core/tests/suite/user_shell_cmd.rs +++ b/codex-rs/core/tests/suite/user_shell_cmd.rs @@ -21,6 +21,7 @@ use core_test_support::responses::mount_sse_once; use core_test_support::responses::sse; use core_test_support::responses::start_mock_server; use core_test_support::skip_if_no_network; +use core_test_support::submit_thread_settings; use core_test_support::test_codex::local_selections; use core_test_support::test_codex::test_codex; use core_test_support::test_codex::turn_permission_fields; @@ -99,6 +100,40 @@ async fn user_shell_cmd_ls_and_cat_in_temp_dir() { assert_eq!(stdout, contents); } +#[tokio::test] +async fn user_shell_command_without_local_environment_emits_error() -> anyhow::Result<()> { + let server = start_mock_server().await; + let mut builder = test_codex(); + let test = builder.build(&server).await?; + submit_thread_settings( + &test.codex, + codex_protocol::protocol::ThreadSettingsOverrides { + environments: Some(codex_protocol::protocol::TurnEnvironmentSelections::new( + test.config.cwd.clone(), + vec![], + )), + ..Default::default() + }, + ) + .await?; + + test.codex + .submit(Op::RunUserShellCommand { + command: "echo shell".to_string(), + }) + .await?; + + let EventMsg::Error(error) = + wait_for_event(&test.codex, |event| matches!(event, EventMsg::Error(_))).await + else { + unreachable!() + }; + assert_eq!(error.message, "shell is unavailable in this session"); + assert_eq!(error.codex_error_info, None); + + Ok(()) +} + #[tokio::test] async fn user_shell_cmd_can_be_interrupted() { // Set up isolated config and conversation.