Skip to content

runtime: use install context for bundled bwrap #23634

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bolinfest merged 1 commit into from
May 20, 2026
Merged

runtime: use install context for bundled bwrap #23634

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
View file
Open in desktop

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

76 changes: 71 additions & 5 deletions codex-rs/install-context/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ impl InstallContext {
&& let Some(path_dir) = &package_layout.path_dir
{
let bundled_rg = path_dir.join(default_rg_command());
if bundled_rg.exists() {
if bundled_rg.is_file() {
return bundled_rg.into_path_buf();
}
}
Expand All @@ -135,13 +135,37 @@ impl InstallContext {
} = &self.method
{
let bundled_rg = resources_dir.join(default_rg_command());
if bundled_rg.exists() {
if bundled_rg.is_file() {
return bundled_rg.into_path_buf();
}
}

default_rg_command()
}

pub fn bundled_resource(&self, file_name: impl AsRef<Path>) -> Option<AbsolutePathBuf> {
if let Some(package_layout) = &self.package_layout
&& let Some(resources_dir) = &package_layout.resources_dir
{
let resource = resources_dir.join(file_name.as_ref());
if resource.is_file() {
return Some(resource);
}
}

if let InstallMethod::Standalone {
resources_dir: Some(resources_dir),
..
} = &self.method
{
let resource = resources_dir.join(file_name);
if resource.is_file() {
return Some(resource);
}
}

None
}
}

impl CodexPackageLayout {
Expand Down Expand Up @@ -242,6 +266,8 @@ mod tests {
use pretty_assertions::assert_eq;
use std::fs;

const TEST_RESOURCE_NAME: &str = "codex-test-helper";

#[test]
fn detects_standalone_install_from_release_layout() -> std::io::Result<()> {
let codex_home = tempfile::tempdir()?;
Expand All @@ -253,6 +279,7 @@ mod tests {
let exe_path = release_dir.join(if cfg!(windows) { "codex.exe" } else { "codex" });
fs::write(&exe_path, "")?;
fs::write(resources_dir.join(default_rg_command()), "")?;
fs::write(resources_dir.join(TEST_RESOURCE_NAME), "")?;
let canonical_release_dir =
AbsolutePathBuf::from_absolute_path(release_dir.canonicalize()?)?;
let canonical_resources_dir =
Expand All @@ -270,12 +297,16 @@ mod tests {
InstallContext {
method: InstallMethod::Standalone {
release_dir: canonical_release_dir,
resources_dir: Some(canonical_resources_dir),
resources_dir: Some(canonical_resources_dir.clone()),
platform: standalone_platform(),
},
package_layout: None,
}
);
assert_eq!(
context.bundled_resource(TEST_RESOURCE_NAME),
Some(canonical_resources_dir.join(TEST_RESOURCE_NAME))
);
Ok(())
}

Expand Down Expand Up @@ -312,6 +343,7 @@ mod tests {
fs::write(package_dir.path().join(PACKAGE_METADATA_FILENAME), "{}")?;
let exe_path = bin_dir.join(if cfg!(windows) { "codex.exe" } else { "codex" });
fs::write(&exe_path, "")?;
fs::write(resources_dir.join(TEST_RESOURCE_NAME), "")?;
fs::write(path_dir.join(default_rg_command()), "")?;
let canonical_package_dir =
AbsolutePathBuf::from_absolute_path(package_dir.path().canonicalize()?)?;
Expand All @@ -322,7 +354,7 @@ mod tests {
let package_layout = CodexPackageLayout {
package_dir: canonical_package_dir,
bin_dir: canonical_bin_dir,
resources_dir: Some(canonical_resources_dir),
resources_dir: Some(canonical_resources_dir.clone()),
path_dir: Some(canonical_path_dir.clone()),
};

Expand All @@ -346,6 +378,10 @@ mod tests {
.join(default_rg_command())
.into_path_buf()
);
assert_eq!(
context.bundled_resource(TEST_RESOURCE_NAME),
Some(canonical_resources_dir.join(TEST_RESOURCE_NAME))
);
Ok(())
}

Expand All @@ -364,6 +400,7 @@ mod tests {
fs::write(package_dir.join(PACKAGE_METADATA_FILENAME), "{}")?;
let exe_path = bin_dir.join(if cfg!(windows) { "codex.exe" } else { "codex" });
fs::write(&exe_path, "")?;
fs::write(resources_dir.join(TEST_RESOURCE_NAME), "")?;
fs::write(path_dir.join(default_rg_command()), "")?;
let canonical_package_dir =
AbsolutePathBuf::from_absolute_path(package_dir.canonicalize()?)?;
Expand All @@ -390,7 +427,7 @@ mod tests {
package_layout: Some(CodexPackageLayout {
package_dir: canonical_package_dir,
bin_dir: canonical_bin_dir,
resources_dir: Some(canonical_resources_dir),
resources_dir: Some(canonical_resources_dir.clone()),
path_dir: Some(canonical_path_dir.clone()),
}),
}
Expand All @@ -401,6 +438,10 @@ mod tests {
.join(default_rg_command())
.into_path_buf()
);
assert_eq!(
context.bundled_resource(TEST_RESOURCE_NAME),
Some(canonical_resources_dir.join(TEST_RESOURCE_NAME))
);
Ok(())
}

Expand Down Expand Up @@ -455,6 +496,31 @@ mod tests {
Ok(())
}

#[test]
fn bundled_file_lookups_ignore_directories() -> std::io::Result<()> {
let package_dir = tempfile::tempdir()?;
let bin_dir = package_dir.path().join(BIN_DIRNAME);
let resources_dir = package_dir.path().join(RESOURCES_DIRNAME);
let path_dir = package_dir.path().join(PATH_DIRNAME);
fs::create_dir_all(&bin_dir)?;
fs::create_dir_all(resources_dir.join(TEST_RESOURCE_NAME))?;
fs::create_dir_all(path_dir.join(default_rg_command()))?;
fs::write(package_dir.path().join(PACKAGE_METADATA_FILENAME), "{}")?;
let exe_path = bin_dir.join(if cfg!(windows) { "codex.exe" } else { "codex" });
fs::write(&exe_path, "")?;

let context = InstallContext::from_exe_with_codex_home(
/*is_macos*/ false,
/*current_exe*/ Some(&exe_path),
/*managed_by_npm*/ false,
/*managed_by_bun*/ false,
/*codex_home*/ None,
);
assert_eq!(context.rg_command(), default_rg_command());
assert_eq!(context.bundled_resource(TEST_RESOURCE_NAME), None);
Ok(())
}

#[test]
fn npm_and_bun_take_precedence() {
let npm_context = InstallContext::from_exe_with_codex_home(
Expand Down
1 change: 1 addition & 0 deletions codex-rs/linux-sandbox/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ workspace = true

[target.'cfg(target_os = "linux")'.dependencies]
clap = { workspace = true, features = ["derive"] }
codex-install-context = { workspace = true }
codex-process-hardening = { workspace = true }
codex-protocol = { workspace = true }
codex-sandboxing = { workspace = true }
Expand Down
56 changes: 48 additions & 8 deletions codex-rs/linux-sandbox/src/bundled_bwrap.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use std::sync::OnceLock;
use crate::bazel_bwrap;
use crate::exec_util::argv_to_cstrings;
use crate::exec_util::make_files_inheritable;
use codex_install_context::InstallContext;
use codex_utils_absolute_path::AbsolutePathBuf;
use sha2::Digest as _;
use sha2::Sha256;
Expand All @@ -26,7 +27,9 @@ pub(crate) struct BundledBwrapLauncher {

pub(crate) fn launcher() -> Option<BundledBwrapLauncher> {
let current_exe = std::env::current_exe().ok()?;
find_for_exe(&current_exe).map(|program| BundledBwrapLauncher { program })
find_for_install_context(InstallContext::current())
.or_else(|| find_legacy_for_exe(&current_exe))
.map(|program| BundledBwrapLauncher { program })
}

impl BundledBwrapLauncher {
Expand Down Expand Up @@ -66,8 +69,14 @@ impl BundledBwrapLauncher {
}
}

fn find_for_exe(exe: &Path) -> Option<AbsolutePathBuf> {
candidates_for_exe(exe)
fn find_for_install_context(context: &InstallContext) -> Option<AbsolutePathBuf> {
context
.bundled_resource("bwrap")
.filter(|path| is_executable_file(path))
}

fn find_legacy_for_exe(exe: &Path) -> Option<AbsolutePathBuf> {
legacy_candidates_for_exe(exe)
.into_iter()
.find(|candidate| is_executable_file(candidate))
.map(|path| {
Expand All @@ -80,7 +89,7 @@ fn find_for_exe(exe: &Path) -> Option<AbsolutePathBuf> {
})
}

fn candidates_for_exe(exe: &Path) -> Vec<PathBuf> {
fn legacy_candidates_for_exe(exe: &Path) -> Vec<PathBuf> {
let Some(exe_dir) = exe.parent() else {
return Vec::new();
};
Expand Down Expand Up @@ -180,21 +189,52 @@ fn bytes_to_hex(bytes: &[u8; 32]) -> String {
#[cfg(test)]
mod tests {
use super::*;
use codex_install_context::CodexPackageLayout;
use codex_install_context::InstallContext;
use codex_install_context::InstallMethod;
use pretty_assertions::assert_eq;
use std::fs;
use tempfile::NamedTempFile;
use tempfile::tempdir;

#[test]
fn finds_standalone_bundled_bwrap_next_to_exe_resources() {
fn finds_package_layout_bwrap_from_install_context() {
let temp_dir = tempdir().expect("temp dir");
let package_dir = temp_dir.path();
let bin_dir = package_dir.join("bin");
let resources_dir = package_dir.join("codex-resources");
let expected_bwrap = resources_dir.join("bwrap");
fs::create_dir_all(&bin_dir).expect("create bin dir");
write_executable(&expected_bwrap);

let context = InstallContext {
method: InstallMethod::Other,
package_layout: Some(CodexPackageLayout {
package_dir: AbsolutePathBuf::from_absolute_path(package_dir).expect("absolute"),
bin_dir: AbsolutePathBuf::from_absolute_path(&bin_dir).expect("absolute"),
resources_dir: Some(
AbsolutePathBuf::from_absolute_path(&resources_dir).expect("absolute"),
),
path_dir: None,
}),
};

assert_eq!(
find_for_install_context(&context),
Some(AbsolutePathBuf::from_absolute_path(&expected_bwrap).expect("absolute"))
);
}

#[test]
fn finds_legacy_standalone_bundled_bwrap_next_to_exe_resources() {
let temp_dir = tempdir().expect("temp dir");
let exe = temp_dir.path().join("codex");
let expected_bwrap = temp_dir.path().join("codex-resources").join("bwrap");
write_executable(&exe);
write_executable(&expected_bwrap);

assert_eq!(
find_for_exe(&exe),
find_legacy_for_exe(&exe),
Some(AbsolutePathBuf::from_absolute_path(&expected_bwrap).expect("absolute"))
);
}
Expand All @@ -209,7 +249,7 @@ mod tests {
write_executable(&expected_bwrap);

assert_eq!(
find_for_exe(&exe),
find_legacy_for_exe(&exe),
Some(AbsolutePathBuf::from_absolute_path(&expected_bwrap).expect("absolute"))
);
}
Expand All @@ -223,7 +263,7 @@ mod tests {
write_executable(&expected_bwrap);

assert_eq!(
find_for_exe(&exe),
find_legacy_for_exe(&exe),
Some(AbsolutePathBuf::from_absolute_path(&expected_bwrap).expect("absolute"))
);
}
Expand Down
Loading