Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions codex-rs/core/src/environment_selection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Arc<codex_exec_server::Environment>> {
self.primary()
Expand Down
20 changes: 14 additions & 6 deletions codex-rs/core/src/session/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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();
Expand All @@ -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,
})
}
Expand Down
12 changes: 8 additions & 4 deletions codex-rs/core/src/session/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,7 @@ async fn warm_plugins_and_skills_for_session_init(
config: Arc<Config>,
plugins_manager: Arc<PluginsManager>,
skills_manager: Arc<SkillsManager>,
turn_environments: TurnEnvironmentSnapshot,
turn_environments: &TurnEnvironmentSnapshot,
) -> Vec<SkillError> {
let fs = turn_environments.primary_filesystem();
let plugins_input = config.plugins_config_input();
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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(),
Expand Down
56 changes: 37 additions & 19 deletions codex-rs/core/src/tasks/user_shell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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),
Expand All @@ -145,18 +154,15 @@ pub(crate) async fn execute_user_shell_command(
}
let exec_command = prepare_user_shell_exec_command(
&display_command,
shell,
environment_shell,
Comment thread
pakrym-oai marked this conversation as resolved.
shell_snapshot_location.as_ref(),
&turn_context.shell_environment_policy.r#set,
&mut exec_env_map,
);

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
Expand Down Expand Up @@ -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<String, String>,
exec_env_map: &mut HashMap<String, String>,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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<String, String>,
exec_env_map: &mut HashMap<String, String>,
Expand All @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion codex-rs/core/src/tools/runtimes/unified_exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> 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
Expand Down
35 changes: 35 additions & 0 deletions codex-rs/core/tests/suite/user_shell_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down
Loading