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
1 change: 1 addition & 0 deletions codex-rs/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

23 changes: 23 additions & 0 deletions codex-rs/app-server/tests/common/mcp_process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,13 +173,36 @@ impl McpProcess {
.await
}

pub async fn new_with_program_and_env(
codex_home: &Path,
program: &Path,
env_overrides: &[(&str, Option<&str>)],
) -> anyhow::Result<Self> {
Self::new_with_program_env_and_args(
codex_home,
program,
env_overrides,
&[DISABLE_PLUGIN_STARTUP_TASKS_ARG],
)
.await
}

async fn new_with_env_and_args(
codex_home: &Path,
env_overrides: &[(&str, Option<&str>)],
args: &[&str],
) -> anyhow::Result<Self> {
let program = codex_utils_cargo_bin::cargo_bin("codex-app-server")
.context("should find binary for codex-app-server")?;
Self::new_with_program_env_and_args(codex_home, &program, env_overrides, args).await
}

async fn new_with_program_env_and_args(
codex_home: &Path,
program: &Path,
env_overrides: &[(&str, Option<&str>)],
args: &[&str],
) -> anyhow::Result<Self> {
let mut cmd = Command::new(program);

cmd.stdin(Stdio::piped());
Expand Down
77 changes: 60 additions & 17 deletions codex-rs/app-server/tests/suite/v2/turn_start_zsh_fork.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ use core_test_support::skip_if_no_network;
use pretty_assertions::assert_eq;
use std::collections::BTreeMap;
use std::path::Path;
use std::path::PathBuf;
use tempfile::TempDir;
use tokio::time::timeout;

Expand Down Expand Up @@ -94,10 +95,9 @@ async fn turn_start_shell_zsh_fork_executes_command_v2() -> Result<()> {
(Feature::UnifiedExec, false),
(Feature::ShellSnapshot, false),
]),
&zsh_path,
)?;

let mut mcp = create_zsh_test_mcp_process(&codex_home, &workspace).await?;
let mut mcp = create_zsh_test_mcp_process(&codex_home, &workspace, &zsh_path).await?;
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;

let start_id = mcp
Expand Down Expand Up @@ -162,7 +162,7 @@ async fn turn_start_shell_zsh_fork_executes_command_v2() -> Result<()> {
};
assert_eq!(id, "call-zsh-fork");
assert_eq!(status, CommandExecutionStatus::InProgress);
assert!(command.starts_with(&zsh_path.display().to_string()));
assert!(command.starts_with(&command_packaged_zsh_path(&codex_home).display().to_string()));
assert!(command.contains("/bin/sh -c"));
assert!(command.contains("sleep 0.01"));
assert!(command.contains(&release_marker.display().to_string()));
Expand Down Expand Up @@ -213,10 +213,9 @@ async fn turn_start_shell_zsh_fork_exec_approval_decline_v2() -> Result<()> {
(Feature::UnifiedExec, false),
(Feature::ShellSnapshot, false),
]),
&zsh_path,
)?;

let mut mcp = create_zsh_test_mcp_process(&codex_home, &workspace).await?;
let mut mcp = create_zsh_test_mcp_process(&codex_home, &workspace, &zsh_path).await?;
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;

let start_id = mcp
Expand Down Expand Up @@ -346,10 +345,9 @@ async fn turn_start_shell_zsh_fork_exec_approval_cancel_v2() -> Result<()> {
(Feature::UnifiedExec, false),
(Feature::ShellSnapshot, false),
]),
&zsh_path,
)?;

let mut mcp = create_zsh_test_mcp_process(&codex_home, &workspace).await?;
let mut mcp = create_zsh_test_mcp_process(&codex_home, &workspace, &zsh_path).await?;
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;

let start_id = mcp
Expand Down Expand Up @@ -505,10 +503,9 @@ async fn turn_start_shell_zsh_fork_subcommand_decline_marks_parent_declined_v2()
(Feature::UnifiedExec, false),
(Feature::ShellSnapshot, false),
]),
&zsh_path,
)?;

let mut mcp = create_zsh_test_mcp_process(&codex_home, &workspace).await?;
let mut mcp = create_zsh_test_mcp_process(&codex_home, &workspace, &zsh_path).await?;
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;

let start_id = mcp
Expand Down Expand Up @@ -603,7 +600,8 @@ async fn turn_start_shell_zsh_fork_subcommand_decline_marks_parent_declined_v2()
);
approved_subcommand_strings.push(approval_command.to_string());
}
let is_parent_approval = approval_command.contains(&zsh_path.display().to_string())
let is_parent_approval = approval_command
.contains(&command_packaged_zsh_path(&codex_home).display().to_string())
&& (approval_command.contains(&shell_command)
|| (has_first_file && has_second_file)
|| approval_command.contains(&parent_shell_hint));
Expand Down Expand Up @@ -738,17 +736,65 @@ async fn turn_start_shell_zsh_fork_subcommand_decline_marks_parent_declined_v2()
Ok(())
}

async fn create_zsh_test_mcp_process(codex_home: &Path, zdotdir: &Path) -> Result<McpProcess> {
async fn create_zsh_test_mcp_process(
codex_home: &Path,
zdotdir: &Path,
zsh_path: &Path,
) -> Result<McpProcess> {
let app_server = create_test_package_app_server(codex_home, zsh_path)?;
let zdotdir = zdotdir.to_string_lossy().into_owned();
McpProcess::new_with_env(codex_home, &[("ZDOTDIR", Some(zdotdir.as_str()))]).await
McpProcess::new_with_program_and_env(
codex_home,
&app_server,
&[("ZDOTDIR", Some(zdotdir.as_str()))],
)
.await
}

fn create_test_package_app_server(codex_home: &Path, zsh_path: &Path) -> Result<PathBuf> {
let package_dir = codex_home.join("test-package");
let bin_dir = package_dir.join("bin");
let package_zsh_path = packaged_zsh_path(codex_home);
let Some(zsh_bin_dir) = package_zsh_path.parent() else {
anyhow::bail!("packaged zsh path should have parent");
};
std::fs::create_dir_all(&bin_dir)?;
std::fs::create_dir_all(zsh_bin_dir)?;
std::fs::write(package_dir.join("codex-package.json"), "{}")?;

let app_server = bin_dir.join("codex-app-server");
copy_with_permissions(
&codex_utils_cargo_bin::cargo_bin("codex-app-server")?,
&app_server,
)?;
copy_with_permissions(zsh_path, &package_zsh_path)?;
Ok(app_server)
}

fn packaged_zsh_path(codex_home: &Path) -> PathBuf {
codex_home
.join("test-package")
.join("codex-resources")
.join("zsh")
.join("bin")
.join("zsh")
}

fn command_packaged_zsh_path(codex_home: &Path) -> PathBuf {
let path = packaged_zsh_path(codex_home);
std::fs::canonicalize(&path).unwrap_or(path)
}

fn copy_with_permissions(source: &Path, destination: &Path) -> std::io::Result<()> {
std::fs::copy(source, destination)?;
std::fs::set_permissions(destination, std::fs::metadata(source)?.permissions())
}

fn create_config_toml(
codex_home: &Path,
server_uri: &str,
approval_policy: &str,
feature_flags: &BTreeMap<Feature, bool>,
zsh_path: &Path,
) -> std::io::Result<()> {
let mut features = BTreeMap::from([(Feature::RemoteModels, false)]);
for (feature, enabled) in feature_flags {
Expand All @@ -774,7 +820,6 @@ fn create_config_toml(
model = "mock-model"
approval_policy = "{approval_policy}"
sandbox_mode = "read-only"
zsh_path = "{zsh_path}"

model_provider = "mock_provider"

Expand All @@ -787,9 +832,7 @@ base_url = "{server_uri}/v1"
wire_api = "responses"
request_max_retries = 0
stream_max_retries = 0
"#,
approval_policy = approval_policy,
zsh_path = zsh_path.display()
"#
),
)
}
Expand Down
3 changes: 0 additions & 3 deletions codex-rs/config/src/config_toml.rs
Original file line number Diff line number Diff line change
Expand Up @@ -291,9 +291,6 @@ pub struct ConfigToml {
#[schemars(skip)]
pub js_repl_node_module_dirs: Option<Vec<AbsolutePathBuf>>,

/// Optional absolute path to patched zsh used by zsh-exec-bridge-backed shell execution.
pub zsh_path: Option<AbsolutePathBuf>,

/// Profile to use from the `profiles` map.
pub profile: Option<String>,

Expand Down
2 changes: 0 additions & 2 deletions codex-rs/config/src/profile_toml.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,6 @@ pub struct ConfigProfile {
/// Deprecated: ignored.
#[schemars(skip)]
pub js_repl_node_module_dirs: Option<Vec<AbsolutePathBuf>>,
/// Optional absolute path to patched zsh used by zsh-exec-bridge-backed shell execution.
pub zsh_path: Option<AbsolutePathBuf>,
pub experimental_compact_prompt_file: Option<AbsolutePathBuf>,
pub include_permissions_instructions: Option<bool>,
pub include_apps_instructions: Option<bool>,
Expand Down
1 change: 1 addition & 0 deletions codex-rs/core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ codex-shell-command = { workspace = true }
codex-execpolicy = { workspace = true }
codex-git-utils = { workspace = true }
codex-hooks = { workspace = true }
codex-install-context = { workspace = true }
codex-network-proxy = { workspace = true }
codex-otel = { workspace = true }
codex-plugin = { workspace = true }
Expand Down
16 changes: 0 additions & 16 deletions codex-rs/core/config.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -711,14 +711,6 @@
}
],
"default": null
},
"zsh_path": {
"allOf": [
{
"$ref": "#/definitions/AbsolutePathBuf"
}
],
"description": "Optional absolute path to patched zsh used by zsh-exec-bridge-backed shell execution."
}
},
"type": "object"
Expand Down Expand Up @@ -4986,14 +4978,6 @@
],
"default": null,
"description": "Windows-specific configuration."
},
"zsh_path": {
"allOf": [
{
"$ref": "#/definitions/AbsolutePathBuf"
}
],
"description": "Optional absolute path to patched zsh used by zsh-exec-bridge-backed shell execution."
}
},
"title": "ConfigToml",
Expand Down
19 changes: 19 additions & 0 deletions codex-rs/core/src/config/config_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4470,6 +4470,25 @@ async fn add_dir_override_extends_workspace_writable_roots() -> std::io::Result<
Ok(())
}

#[tokio::test]
async fn default_zsh_path_sets_runtime_zsh_path() -> std::io::Result<()> {
let codex_home = TempDir::new()?;
let default_zsh_path = codex_home.path().join("packaged-zsh");

let config = Config::load_from_base_config_with_overrides(
ConfigToml::default(),
ConfigOverrides {
default_zsh_path: Some(default_zsh_path.abs()),
..Default::default()
},
codex_home.abs(),
)
.await?;
assert_eq!(config.zsh_path, Some(default_zsh_path));

Ok(())
}

#[tokio::test]
async fn sqlite_home_defaults_to_codex_home_for_workspace_write() -> std::io::Result<()> {
let codex_home = TempDir::new()?;
Expand Down
16 changes: 13 additions & 3 deletions codex-rs/core/src/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ use codex_features::FeaturesToml;
use codex_features::MultiAgentV2ConfigToml;
use codex_features::NetworkProxyConfigToml;
use codex_git_utils::resolve_root_git_project_for_trust;
use codex_install_context::InstallContext;
use codex_login::AuthManagerConfig;
use codex_mcp::McpConfig;
use codex_memories_read::memory_root;
Expand Down Expand Up @@ -1392,11 +1393,18 @@ impl Config {
.effective_config()
.try_into()
.map_err(|err| std::io::Error::new(std::io::ErrorKind::InvalidData, err))?;
let default_zsh_path = refreshed_config
.zsh_path
.clone()
.map(AbsolutePathBuf::try_from)
.transpose()?;

Self::load_config_with_layer_stack(
LOCAL_FS.as_ref(),
cfg,
ConfigOverrides {
cwd: Some(self.cwd.to_path_buf()),
default_zsh_path,
..Default::default()
},
refreshed_config.codex_home.clone(),
Expand Down Expand Up @@ -2128,7 +2136,7 @@ pub struct ConfigOverrides {
pub codex_self_exe: Option<PathBuf>,
pub codex_linux_sandbox_exe: Option<PathBuf>,
pub main_execve_wrapper_exe: Option<PathBuf>,
pub zsh_path: Option<PathBuf>,
pub default_zsh_path: Option<AbsolutePathBuf>,
pub base_instructions: Option<String>,
pub developer_instructions: Option<String>,
pub personality: Option<Personality>,
Expand Down Expand Up @@ -2480,7 +2488,7 @@ impl Config {
codex_self_exe,
codex_linux_sandbox_exe,
main_execve_wrapper_exe,
zsh_path: zsh_path_override,
default_zsh_path,
base_instructions,
developer_instructions,
personality,
Expand Down Expand Up @@ -3199,7 +3207,9 @@ impl Config {
)
.await?;
let compact_prompt = compact_prompt.or(file_compact_prompt);
let zsh_path = zsh_path_override.or(cfg.zsh_path.map(Into::into));
let zsh_path = default_zsh_path
.or_else(|| InstallContext::current().bundled_zsh_path())
.map(AbsolutePathBuf::into_path_buf);

let review_model = override_review_model.or(cfg.review_model);

Expand Down
2 changes: 1 addition & 1 deletion codex-rs/core/src/config/permissions_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ async fn restricted_read_implicitly_allows_helper_executables() -> std::io::Resu
},
ConfigOverrides {
cwd: Some(cwd.clone()),
zsh_path: Some(zsh_path.clone()),
default_zsh_path: Some(AbsolutePathBuf::try_from(zsh_path.clone())?),
main_execve_wrapper_exe: Some(execve_wrapper),
..Default::default()
},
Expand Down
4 changes: 2 additions & 2 deletions codex-rs/core/src/session/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -826,13 +826,13 @@ impl Session {
} else if use_zsh_fork_shell {
let zsh_path = config.zsh_path.as_ref().ok_or_else(|| {
anyhow::anyhow!(
"zsh fork feature enabled, but `zsh_path` is not configured; set `zsh_path` in config.toml"
"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 zsh_path `{}` is not usable; set `zsh_path` to a valid zsh executable",
"zsh fork feature enabled, but packaged zsh fork `{}` is not usable",
zsh_path.display()
)
})?
Expand Down
4 changes: 2 additions & 2 deletions codex-rs/core/src/session/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4319,7 +4319,7 @@ async fn absolute_cwd_update_with_turn_environment_is_allowed() {
}

#[tokio::test]
async fn session_new_fails_when_zsh_fork_enabled_without_zsh_path() {
async fn session_new_fails_when_zsh_fork_enabled_without_packaged_zsh() {
let codex_home = tempfile::tempdir().expect("create temp dir");
let mut config = build_test_config(codex_home.path()).await;
config
Expand Down Expand Up @@ -4420,7 +4420,7 @@ async fn session_new_fails_when_zsh_fork_enabled_without_zsh_path() {
Err(err) => err,
};
let msg = format!("{err:#}");
assert!(msg.contains("zsh fork feature enabled, but `zsh_path` is not configured"));
assert!(msg.contains("zsh fork feature enabled, but no packaged zsh fork is available"));
}

// todo: use online model info
Expand Down
2 changes: 1 addition & 1 deletion codex-rs/exec/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,7 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result
codex_self_exe: arg0_paths.codex_self_exe.clone(),
codex_linux_sandbox_exe: arg0_paths.codex_linux_sandbox_exe.clone(),
main_execve_wrapper_exe: arg0_paths.main_execve_wrapper_exe.clone(),
zsh_path: None,
default_zsh_path: None,
base_instructions: None,
developer_instructions: None,
personality: None,
Expand Down
Loading
Loading