diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 8175adcbc59..9a9951d49ea 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -84,6 +84,7 @@ use crate::config::Config; use crate::config::Constrained; use crate::config::ConstraintResult; use crate::config::GhostSnapshotConfig; +use crate::config::types::ShellConfig; use crate::config::types::ShellEnvironmentPolicy; use crate::context_manager::ContextManager; use crate::environment_context::EnvironmentContext; @@ -382,6 +383,7 @@ pub(crate) struct TurnContext { pub(crate) approval_policy: AskForApproval, pub(crate) sandbox_policy: SandboxPolicy, pub(crate) shell_environment_policy: ShellEnvironmentPolicy, + pub(crate) shell: ShellConfig, pub(crate) tools_config: ToolsConfig, pub(crate) ghost_snapshot: GhostSnapshotConfig, pub(crate) final_output_json_schema: Option, @@ -540,6 +542,7 @@ impl Session { approval_policy: session_configuration.approval_policy.value(), sandbox_policy: session_configuration.sandbox_policy.get().clone(), shell_environment_policy: per_turn_config.shell_environment_policy.clone(), + shell: per_turn_config.shell.clone(), tools_config, ghost_snapshot: per_turn_config.ghost_snapshot.clone(), final_output_json_schema: None, @@ -2266,6 +2269,7 @@ async fn spawn_review_thread( approval_policy: parent_turn_context.approval_policy, sandbox_policy: parent_turn_context.sandbox_policy.clone(), shell_environment_policy: parent_turn_context.shell_environment_policy.clone(), + shell: parent_turn_context.shell.clone(), cwd: parent_turn_context.cwd.clone(), final_output_json_schema: None, codex_linux_sandbox_exe: parent_turn_context.codex_linux_sandbox_exe.clone(), diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 668d205cccc..6c985d5ec03 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -9,6 +9,7 @@ use crate::config::types::OtelConfigToml; use crate::config::types::OtelExporterKind; use crate::config::types::SandboxWorkspaceWrite; use crate::config::types::ScrollInputMode; +use crate::config::types::ShellConfig; use crate::config::types::ShellEnvironmentPolicy; use crate::config::types::ShellEnvironmentPolicyToml; use crate::config::types::Tui; @@ -130,6 +131,8 @@ pub struct Config { pub forced_auto_mode_downgraded_on_windows: bool, pub shell_environment_policy: ShellEnvironmentPolicy, + /// Shell execution defaults applied when tools do not specify overrides. + pub shell: ShellConfig, /// When `true`, `AgentReasoning` events emitted by the backend will be /// suppressed from the frontend output. This can reduce visual noise when @@ -694,6 +697,9 @@ pub struct ConfigToml { #[serde(default)] pub shell_environment_policy: ShellEnvironmentPolicyToml, + /// Default shell configuration applied when tools do not override it. + #[serde(default)] + pub shell: ShellConfig, /// Sandbox mode to use. pub sandbox_mode: Option, @@ -1227,6 +1233,7 @@ impl Config { .clone(); let shell_environment_policy = cfg.shell_environment_policy.into(); + let shell = cfg.shell; let history = cfg.history.unwrap_or_default(); @@ -1340,6 +1347,7 @@ impl Config { did_user_set_custom_approval_policy_or_sandbox_mode, forced_auto_mode_downgraded_on_windows, shell_environment_policy, + shell, notify: cfg.notify, user_instructions, base_instructions, @@ -1918,6 +1926,43 @@ trust_level = "trusted" Ok(()) } + #[test] + fn shell_defaults_to_login_shell() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + let cfg = ConfigToml::default(); + + let config = Config::load_from_base_config_with_overrides( + cfg, + ConfigOverrides::default(), + codex_home.path().to_path_buf(), + )?; + + assert!(config.shell.default_login); + + Ok(()) + } + + #[test] + fn shell_respects_configured_default_login() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + let cfg = ConfigToml { + shell: ShellConfig { + default_login: false, + }, + ..Default::default() + }; + + let config = Config::load_from_base_config_with_overrides( + cfg, + ConfigOverrides::default(), + codex_home.path().to_path_buf(), + )?; + + assert!(!config.shell.default_login); + + Ok(()) + } + #[test] fn profile_legacy_toggles_override_base() -> std::io::Result<()> { let codex_home = TempDir::new()?; @@ -3225,6 +3270,7 @@ model_verbosity = "high" did_user_set_custom_approval_policy_or_sandbox_mode: true, forced_auto_mode_downgraded_on_windows: false, shell_environment_policy: ShellEnvironmentPolicy::default(), + shell: ShellConfig::default(), user_instructions: None, notify: None, cwd: fixture.cwd(), @@ -3310,6 +3356,7 @@ model_verbosity = "high" did_user_set_custom_approval_policy_or_sandbox_mode: true, forced_auto_mode_downgraded_on_windows: false, shell_environment_policy: ShellEnvironmentPolicy::default(), + shell: ShellConfig::default(), user_instructions: None, notify: None, cwd: fixture.cwd(), @@ -3410,6 +3457,7 @@ model_verbosity = "high" did_user_set_custom_approval_policy_or_sandbox_mode: true, forced_auto_mode_downgraded_on_windows: false, shell_environment_policy: ShellEnvironmentPolicy::default(), + shell: ShellConfig::default(), user_instructions: None, notify: None, cwd: fixture.cwd(), @@ -3496,6 +3544,7 @@ model_verbosity = "high" did_user_set_custom_approval_policy_or_sandbox_mode: true, forced_auto_mode_downgraded_on_windows: false, shell_environment_policy: ShellEnvironmentPolicy::default(), + shell: ShellConfig::default(), user_instructions: None, notify: None, cwd: fixture.cwd(), diff --git a/codex-rs/core/src/config/types.rs b/codex-rs/core/src/config/types.rs index 657bb052aff..8b3959e1ed9 100644 --- a/codex-rs/core/src/config/types.rs +++ b/codex-rs/core/src/config/types.rs @@ -555,6 +555,24 @@ impl Notice { pub(crate) const TABLE_KEY: &'static str = "notice"; } +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq)] +pub struct ShellConfig { + #[serde(default = "default_login_shell")] + pub default_login: bool, +} + +const fn default_login_shell() -> bool { + true +} + +impl Default for ShellConfig { + fn default() -> Self { + Self { + default_login: true, + } + } +} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Default)] pub struct SandboxWorkspaceWrite { #[serde(default)] diff --git a/codex-rs/core/src/tools/handlers/shell.rs b/codex-rs/core/src/tools/handlers/shell.rs index 0e14da68f26..b73610e2911 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -43,8 +43,13 @@ impl ShellHandler { } impl ShellCommandHandler { - fn base_command(shell: &Shell, command: &str, login: Option) -> Vec { - let use_login_shell = login.unwrap_or(true); + fn base_command( + shell: &Shell, + command: &str, + login: Option, + default_login: bool, + ) -> Vec { + let use_login_shell = login.unwrap_or(default_login); shell.derive_exec_args(command, use_login_shell) } @@ -54,7 +59,12 @@ impl ShellCommandHandler { turn_context: &TurnContext, ) -> ExecParams { let shell = session.user_shell(); - let command = Self::base_command(shell.as_ref(), ¶ms.command, params.login); + let command = Self::base_command( + shell.as_ref(), + ¶ms.command, + params.login, + turn_context.shell.default_login, + ); ExecParams { command, @@ -156,7 +166,12 @@ impl ToolHandler for ShellCommandHandler { serde_json::from_str::(arguments) .map(|params| { let shell = invocation.session.user_shell(); - let command = Self::base_command(shell.as_ref(), ¶ms.command, params.login); + let command = Self::base_command( + shell.as_ref(), + ¶ms.command, + params.login, + invocation.turn.shell.default_login, + ); !is_known_safe_command(&command) }) .unwrap_or(true) @@ -400,14 +415,14 @@ mod tests { }; let login_command = - ShellCommandHandler::base_command(&shell, "echo login shell", Some(true)); + ShellCommandHandler::base_command(&shell, "echo login shell", Some(true), true); assert_eq!( login_command, shell.derive_exec_args("echo login shell", true) ); let non_login_command = - ShellCommandHandler::base_command(&shell, "echo non login shell", Some(false)); + ShellCommandHandler::base_command(&shell, "echo non login shell", Some(false), true); assert_eq!( non_login_command, shell.derive_exec_args("echo non login shell", false) diff --git a/codex-rs/core/src/tools/handlers/unified_exec.rs b/codex-rs/core/src/tools/handlers/unified_exec.rs index 7769f262a9e..35401877d0d 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec.rs @@ -31,8 +31,8 @@ struct ExecCommandArgs { workdir: Option, #[serde(default)] shell: Option, - #[serde(default = "default_login")] - login: bool, + #[serde(default)] + login: Option, #[serde(default = "default_exec_yield_time_ms")] yield_time_ms: u64, #[serde(default)] @@ -63,10 +63,6 @@ fn default_write_stdin_yield_time_ms() -> u64 { 250 } -fn default_login() -> bool { - true -} - #[async_trait] impl ToolHandler for UnifiedExecHandler { fn kind(&self) -> ToolKind { @@ -89,7 +85,11 @@ impl ToolHandler for UnifiedExecHandler { let Ok(params) = serde_json::from_str::(arguments) else { return true; }; - let command = get_command(¶ms, invocation.session.user_shell()); + let command = get_command( + ¶ms, + invocation.session.user_shell(), + invocation.turn.shell.default_login, + ); !is_known_safe_command(&command) } @@ -120,8 +120,7 @@ impl ToolHandler for UnifiedExecHandler { "exec_command" => { let args: ExecCommandArgs = parse_arguments(&arguments)?; let process_id = manager.allocate_process_id().await; - let command = get_command(&args, session.user_shell()); - + let command = get_command(&args, session.user_shell(), turn.shell.default_login); let ExecCommandArgs { workdir, yield_time_ms, @@ -225,16 +224,32 @@ impl ToolHandler for UnifiedExecHandler { } } -fn get_command(args: &ExecCommandArgs, session_shell: Arc) -> Vec { - let model_shell = args.shell.as_ref().map(|shell_str| { +fn resolve_login_choice( + requested_login: Option, + default_login: bool, + has_snapshot: bool, +) -> bool { + requested_login.unwrap_or(if has_snapshot { true } else { default_login }) +} + +fn get_command( + args: &ExecCommandArgs, + session_shell: Arc, + default_login: bool, +) -> Vec { + if let Some(shell_str) = &args.shell { let mut shell = get_shell_by_model_provided_path(&PathBuf::from(shell_str)); shell.shell_snapshot = None; - shell - }); - - let shell = model_shell.as_ref().unwrap_or(session_shell.as_ref()); + let use_login_shell = resolve_login_choice(args.login, default_login, false); + return shell.derive_exec_args(&args.cmd, use_login_shell); + } - shell.derive_exec_args(&args.cmd, args.login) + let use_login_shell = resolve_login_choice( + args.login, + default_login, + session_shell.shell_snapshot.is_some(), + ); + session_shell.derive_exec_args(&args.cmd, use_login_shell) } fn format_response(response: &UnifiedExecResponse) -> String { @@ -270,7 +285,11 @@ fn format_response(response: &UnifiedExecResponse) -> String { mod tests { use super::*; use crate::shell::default_user_shell; + #[cfg(not(windows))] + use crate::shell_snapshot::ShellSnapshot; use std::sync::Arc; + #[cfg(not(windows))] + use tempfile::TempDir; #[test] fn test_get_command_uses_default_shell_when_unspecified() -> anyhow::Result<()> { @@ -280,7 +299,7 @@ mod tests { assert!(args.shell.is_none()); - let command = get_command(&args, Arc::new(default_user_shell())); + let command = get_command(&args, Arc::new(default_user_shell()), true); assert_eq!(command.len(), 3); assert_eq!(command[2], "echo hello"); @@ -295,7 +314,7 @@ mod tests { assert_eq!(args.shell.as_deref(), Some("/bin/bash")); - let command = get_command(&args, Arc::new(default_user_shell())); + let command = get_command(&args, Arc::new(default_user_shell()), true); assert_eq!(command.last(), Some(&"echo hello".to_string())); if command @@ -315,7 +334,7 @@ mod tests { assert_eq!(args.shell.as_deref(), Some("powershell")); - let command = get_command(&args, Arc::new(default_user_shell())); + let command = get_command(&args, Arc::new(default_user_shell()), true); assert_eq!(command[2], "echo hello"); Ok(()) @@ -329,9 +348,29 @@ mod tests { assert_eq!(args.shell.as_deref(), Some("cmd")); - let command = get_command(&args, Arc::new(default_user_shell())); + let command = get_command(&args, Arc::new(default_user_shell()), true); assert_eq!(command[2], "echo hello"); Ok(()) } + + #[test] + #[cfg(not(windows))] + fn test_get_command_prefers_login_for_snapshot_defaults() { + let json = r#"{"cmd": "echo hello"}"#; + + let args: ExecCommandArgs = + serde_json::from_str(json).expect("deserialize ExecCommandArgs"); + let temp_dir = TempDir::new().expect("create temp dir"); + let snapshot_path = temp_dir.path().join("snapshot.sh"); + std::fs::write(&snapshot_path, "").expect("write snapshot"); + + let mut shell = default_user_shell(); + shell.shell_snapshot = Some(Arc::new(ShellSnapshot { + path: snapshot_path, + })); + + let command = get_command(&args, Arc::new(shell), false); + assert_eq!(command[1], "-lc"); + } }