mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
package: include zsh fork in Codex package (#23756)
## Why The package layout gives Codex a stable place for runtime helpers that should travel with the entrypoint. `shell_zsh_fork` still required users to configure `zsh_path` manually, even though we already publish prebuilt zsh fork artifacts. This PR builds on #24129 and uses the shared DotSlash artifact fetcher to include the zsh fork in Codex packages when a matching target artifact exists. Packaged Codex builds can then discover the bundled fork automatically; the user/profile `zsh_path` override is removed so the feature uses the package-managed artifact instead of a legacy path knob. ## What Changed - Added `scripts/codex_package/codex-zsh`, a checked-in DotSlash manifest for the current macOS arm64 and Linux zsh fork artifacts. - Taught `scripts/build_codex_package.py` to fetch the matching zsh fork artifact and install it at `codex-resources/zsh/bin/zsh` when available for the selected target. - Added package layout validation for the optional bundled zsh resource. - Added `InstallContext::bundled_zsh_path()` and `InstallContext::bundled_zsh_bin_dir()` for package-layout resource discovery. - Threaded the packaged zsh path through config loading as the runtime `zsh_path` for packaged installs, and removed the config/profile/CLI override path. - Kept the packaged default zsh override typed as `AbsolutePathBuf` until the existing runtime `Config::zsh_path` boundary. - Updated app-server zsh-fork integration tests to spawn `codex-app-server` from a temporary package layout with `codex-resources/zsh/bin/zsh`, matching the new packaged discovery path instead of setting `zsh_path` in config. - Switched package executable copying from metadata-preserving `copy2()` to `copyfile()` plus explicit executable bits, which avoids macOS file-flag failures when local smoke tests use system binaries as inputs. ## Testing To verify that the `zsh` executable from the Codex package is picked up correctly, first I ran: ```shell ./scripts/build_codex_package.py ``` which created: ``` /private/var/folders/vw/x2knqmks50sfhfpy27nftl900000gp/T/codex-package-pms94kdp/ ``` so then I ran: ``` /private/var/folders/vw/x2knqmks50sfhfpy27nftl900000gp/T/codex-package-pms94kdp/bin/codex exec --enable shell_zsh_fork 'run `echo $0`' ``` which reported the following, as expected: ``` /private/var/folders/vw/x2knqmks50sfhfpy27nftl900000gp/T/codex-package-pms94kdp/codex-resources/zsh/bin/zsh ``` --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/23756). * #23768 * __->__ #23756
This commit is contained in:
@@ -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()?;
|
||||
|
||||
@@ -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;
|
||||
@@ -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(),
|
||||
@@ -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>,
|
||||
@@ -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,
|
||||
@@ -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);
|
||||
|
||||
|
||||
@@ -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()
|
||||
},
|
||||
|
||||
@@ -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()
|
||||
)
|
||||
})?
|
||||
|
||||
@@ -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
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user