mirror of
https://github.com/openai/codex.git
synced 2026-09-13 11:47:17 +00:00
Resolve model-provided shells by type (#39607)
## Why A model-provided shell path should select the requested shell type without allowing that path to determine which executable Codex runs. ## What changed - Resolve model-provided shells through Codex's normal shell discovery and fallback logic after detecting their type. - Keep the configured packaged zsh executable when the zsh-fork feature is enabled and the file exists. - Update shell, unified exec, and network approval expectations to use the resolved local executable and arguments. GitOrigin-RevId: ebe6f7eec2cfd1c0548d5bf1a26b7a30dba02cc2
This commit is contained in:
@@ -1130,13 +1130,19 @@ impl Session {
|
||||
"zsh fork feature enabled, but no packaged zsh fork is available for this install"
|
||||
)
|
||||
})?;
|
||||
let zsh_path = zsh_path.to_path_buf();
|
||||
shell::get_shell(shell::ShellType::Zsh, Some(&zsh_path)).ok_or_else(|| {
|
||||
anyhow::anyhow!(
|
||||
"zsh fork feature enabled, but packaged zsh fork `{}` is not usable",
|
||||
zsh_path.display()
|
||||
)
|
||||
})?
|
||||
if zsh_path.is_file() {
|
||||
shell::Shell {
|
||||
shell_type: shell::ShellType::Zsh,
|
||||
shell_path: zsh_path.clone(),
|
||||
}
|
||||
} else {
|
||||
shell::get_shell(shell::ShellType::Zsh).ok_or_else(|| {
|
||||
anyhow::anyhow!(
|
||||
"zsh fork feature enabled, but packaged zsh fork `{}` is not usable",
|
||||
zsh_path.display()
|
||||
)
|
||||
})?
|
||||
}
|
||||
} else {
|
||||
shell::default_user_shell()
|
||||
};
|
||||
|
||||
@@ -85,8 +85,8 @@ pub fn get_shell_by_model_provided_path(shell_path: &PathBuf) -> Shell {
|
||||
codex_shell_command::shell_detect::get_shell_by_model_provided_path(shell_path).into()
|
||||
}
|
||||
|
||||
pub fn get_shell(shell_type: ShellType, path: Option<&PathBuf>) -> Option<Shell> {
|
||||
codex_shell_command::shell_detect::get_shell(shell_type, path).map(Into::into)
|
||||
pub fn get_shell(shell_type: ShellType) -> Option<Shell> {
|
||||
codex_shell_command::shell_detect::get_shell(shell_type).map(Into::into)
|
||||
}
|
||||
|
||||
pub fn default_user_shell() -> Shell {
|
||||
|
||||
@@ -206,8 +206,8 @@ async fn write_shell_snapshot(
|
||||
if shell_type == ShellType::PowerShell || shell_type == ShellType::Cmd {
|
||||
bail!("Shell snapshot not supported yet for {shell_type:?}");
|
||||
}
|
||||
let shell = get_shell(shell_type, /*path*/ None)
|
||||
.with_context(|| format!("No available shell for {shell_type:?}"))?;
|
||||
let shell =
|
||||
get_shell(shell_type).with_context(|| format!("No available shell for {shell_type:?}"))?;
|
||||
|
||||
let raw_snapshot = capture_snapshot(&shell, cwd).await?;
|
||||
let snapshot = strip_snapshot_preamble(&raw_snapshot)?;
|
||||
|
||||
@@ -5,7 +5,7 @@ use std::process::Command;
|
||||
#[test]
|
||||
#[cfg(target_os = "macos")]
|
||||
fn detects_zsh() {
|
||||
let zsh_shell = get_shell(ShellType::Zsh, /*path*/ None).unwrap();
|
||||
let zsh_shell = get_shell(ShellType::Zsh).unwrap();
|
||||
|
||||
let shell_path = zsh_shell.shell_path;
|
||||
|
||||
@@ -24,7 +24,7 @@ fn fish_fallback_to_zsh() {
|
||||
|
||||
#[test]
|
||||
fn detects_bash() {
|
||||
let bash_shell = get_shell(ShellType::Bash, /*path*/ None).unwrap();
|
||||
let bash_shell = get_shell(ShellType::Bash).unwrap();
|
||||
let shell_path = bash_shell.shell_path;
|
||||
|
||||
assert!(
|
||||
@@ -35,7 +35,7 @@ fn detects_bash() {
|
||||
|
||||
#[test]
|
||||
fn detects_sh() {
|
||||
let sh_shell = get_shell(ShellType::Sh, /*path*/ None).unwrap();
|
||||
let sh_shell = get_shell(ShellType::Sh).unwrap();
|
||||
let shell_path = sh_shell.shell_path;
|
||||
assert!(
|
||||
shell_path.file_name().and_then(|name| name.to_str()) == Some("sh"),
|
||||
@@ -48,12 +48,12 @@ fn can_run_on_shell_test() {
|
||||
let cmd = "echo \"Works\"";
|
||||
if cfg!(windows) {
|
||||
assert!(shell_works(
|
||||
get_shell(ShellType::PowerShell, /*path*/ None),
|
||||
get_shell(ShellType::PowerShell),
|
||||
"Out-String 'Works'",
|
||||
/*required*/ true,
|
||||
));
|
||||
assert!(shell_works(
|
||||
get_shell(ShellType::Cmd, /*path*/ None),
|
||||
get_shell(ShellType::Cmd),
|
||||
cmd,
|
||||
/*required*/ true,
|
||||
));
|
||||
@@ -69,17 +69,17 @@ fn can_run_on_shell_test() {
|
||||
/*required*/ true
|
||||
));
|
||||
assert!(shell_works(
|
||||
get_shell(ShellType::Zsh, /*path*/ None),
|
||||
get_shell(ShellType::Zsh),
|
||||
cmd,
|
||||
/*required*/ false
|
||||
));
|
||||
assert!(shell_works(
|
||||
get_shell(ShellType::Bash, /*path*/ None),
|
||||
get_shell(ShellType::Bash),
|
||||
cmd,
|
||||
/*required*/ true
|
||||
));
|
||||
assert!(shell_works(
|
||||
get_shell(ShellType::Sh, /*path*/ None),
|
||||
get_shell(ShellType::Sh),
|
||||
cmd,
|
||||
/*required*/ true
|
||||
));
|
||||
@@ -181,7 +181,7 @@ fn finds_powershell() {
|
||||
return;
|
||||
}
|
||||
|
||||
let powershell_shell = get_shell(ShellType::PowerShell, /*path*/ None).unwrap();
|
||||
let powershell_shell = get_shell(ShellType::PowerShell).unwrap();
|
||||
let shell_path = powershell_shell.shell_path;
|
||||
|
||||
assert!(shell_path.ends_with("pwsh.exe") || shell_path.ends_with("powershell.exe"));
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
use super::*;
|
||||
use crate::shell::ShellType;
|
||||
use crate::shell::default_user_shell;
|
||||
use crate::shell::get_shell;
|
||||
use codex_exec_server::Environment;
|
||||
use codex_tools::UnifiedExecShellMode;
|
||||
use codex_tools::ZshForkConfig;
|
||||
@@ -95,7 +96,7 @@ fn test_get_command_respects_explicit_bash_shell() -> anyhow::Result<()> {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_get_command_respects_explicit_powershell_shell() -> anyhow::Result<()> {
|
||||
fn test_get_command_resolves_powershell_by_type() -> anyhow::Result<()> {
|
||||
let temp_dir = tempfile::tempdir()?;
|
||||
let powershell_path = temp_dir.path().join(if cfg!(windows) {
|
||||
"powershell.exe"
|
||||
@@ -123,10 +124,13 @@ fn test_get_command_respects_explicit_powershell_shell() -> anyhow::Result<()> {
|
||||
/*allow_login_shell*/ true,
|
||||
)
|
||||
.map_err(anyhow::Error::msg)?;
|
||||
let command = resolved.command;
|
||||
|
||||
assert_eq!(command[2], "echo hello");
|
||||
assert_eq!(resolved.shell_type, ShellType::PowerShell);
|
||||
let expected_shell = get_shell(ShellType::PowerShell)
|
||||
.unwrap_or_else(|| codex_shell_command::shell_detect::ultimate_fallback_shell().into());
|
||||
assert_eq!(
|
||||
resolved.command,
|
||||
expected_shell.derive_exec_args("echo hello", /*use_login_shell*/ true)
|
||||
);
|
||||
assert_eq!(resolved.shell_type, expected_shell.shell_type);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user