diff --git a/codex-rs/cli/tests/app_server_daemon.rs b/codex-rs/cli/tests/app_server_daemon.rs index 9c3c820d49..002012a95a 100644 --- a/codex-rs/cli/tests/app_server_daemon.rs +++ b/codex-rs/cli/tests/app_server_daemon.rs @@ -1,8 +1,5 @@ #![cfg(unix)] -#[path = "support/executable.rs"] -mod executable; - use std::path::PathBuf; use std::process::Child; use std::process::Command; @@ -13,7 +10,7 @@ use std::time::Instant; use anyhow::Context; use anyhow::Result; use anyhow::ensure; -use executable::copy_executable; +use codex_utils_cargo_bin::copy_executable; use pretty_assertions::assert_eq; use serde_json::Value; use tempfile::TempDir; diff --git a/codex-rs/cli/tests/doctor_path_safety.rs b/codex-rs/cli/tests/doctor_path_safety.rs index d67b7fd2dd..c6bd556870 100644 --- a/codex-rs/cli/tests/doctor_path_safety.rs +++ b/codex-rs/cli/tests/doctor_path_safety.rs @@ -1,8 +1,5 @@ //! Black-box coverage for safe diagnostic execution and config error reporting. -#[path = "support/executable.rs"] -mod executable; - use std::ffi::OsString; use std::path::PathBuf; use std::time::Duration; @@ -15,7 +12,7 @@ use codex_app_server_protocol::RequestId; use codex_config::loader::project_trust_key; use codex_state::SqliteConfig; use codex_utils_absolute_path::AbsolutePathBuf; -use executable::copy_executable; +use codex_utils_cargo_bin::copy_executable; use pretty_assertions::assert_eq; use serde_json::Value; use serde_json::json; diff --git a/codex-rs/cli/tests/exec_server.rs b/codex-rs/cli/tests/exec_server.rs index 9d018b8423..75bb8344cf 100644 --- a/codex-rs/cli/tests/exec_server.rs +++ b/codex-rs/cli/tests/exec_server.rs @@ -1,6 +1,3 @@ -#[path = "support/executable.rs"] -mod executable; - #[cfg(target_os = "linux")] #[path = "exec_server/pid_namespace_tests.rs"] mod pid_namespace_tests; @@ -36,7 +33,7 @@ use codex_exec_server::NoiseRendezvousConnectBundle; use codex_exec_server::ProcessId; use codex_http_client::HttpClientFactory; use codex_http_client::OutboundProxyPolicy; -use executable::copy_executable; +use codex_utils_cargo_bin::copy_executable; use futures::SinkExt; use futures::StreamExt; use predicates::prelude::PredicateBooleanExt; diff --git a/codex-rs/cli/tests/support/executable.rs b/codex-rs/cli/tests/support/executable.rs deleted file mode 100644 index 2ee51da727..0000000000 --- a/codex-rs/cli/tests/support/executable.rs +++ /dev/null @@ -1,39 +0,0 @@ -//! Copies executable fixtures without exposing writable descriptors to sibling test spawns. -//! Linux copies finish in a separate process before the fixture can be launched. - -use std::path::Path; -#[cfg(target_os = "linux")] -use std::process::Command; - -#[cfg(target_os = "linux")] -use anyhow::Context; -use anyhow::Result; -#[cfg(target_os = "linux")] -use anyhow::ensure; - -pub(super) fn copy_executable(source: &Path, destination: &Path) -> Result<()> { - #[cfg(target_os = "linux")] - { - // Sibling tests can spawn children while a copy holds the executable open - // for writing. Keep that descriptor in a separate process and wait for it - // to exit, so inherited descriptors cannot cause ETXTBSY during launch. - let output = Command::new("/bin/cp") - .arg("--") - .arg(source) - .arg(destination) - .output() - .context("failed to start fixture executable copy")?; - ensure!( - output.status.success(), - "failed to copy fixture executable {} to {}: {}", - source.display(), - destination.display(), - String::from_utf8_lossy(&output.stderr) - ); - // Match fs::copy even when the test process has a restrictive umask. - std::fs::set_permissions(destination, std::fs::metadata(source)?.permissions())?; - } - #[cfg(not(target_os = "linux"))] - std::fs::copy(source, destination)?; - Ok(()) -} diff --git a/codex-rs/exec-server/tests/common/fake_bwrap.rs b/codex-rs/exec-server/tests/common/fake_bwrap.rs index ddd2936480..4a77f5e436 100644 --- a/codex-rs/exec-server/tests/common/fake_bwrap.rs +++ b/codex-rs/exec-server/tests/common/fake_bwrap.rs @@ -1,11 +1,10 @@ -use std::os::unix::fs::PermissionsExt; use std::path::Path; use std::path::PathBuf; pub(crate) fn write_fake_bwrap(bin_dir: &Path) -> anyhow::Result { std::fs::create_dir_all(bin_dir)?; let fake_bwrap = bin_dir.join("bwrap"); - std::fs::write( + codex_utils_cargo_bin::write_executable( &fake_bwrap, r#"#!/bin/bash set -euo pipefail @@ -55,8 +54,5 @@ fi exec "${cmd[@]}" "#, )?; - let mut permissions = std::fs::metadata(&fake_bwrap)?.permissions(); - permissions.set_mode(0o755); - std::fs::set_permissions(&fake_bwrap, permissions)?; Ok(fake_bwrap) } diff --git a/codex-rs/rmcp-client/src/program_resolver.rs b/codex-rs/rmcp-client/src/program_resolver.rs index 53db522166..7f15fe84ef 100644 --- a/codex-rs/rmcp-client/src/program_resolver.rs +++ b/codex-rs/rmcp-client/src/program_resolver.rs @@ -69,6 +69,7 @@ mod tests { use super::*; use crate::utils::create_env_for_mcp_server; use anyhow::Result; + #[cfg(windows)] use std::fs; use std::path::Path; use tempfile::TempDir; @@ -79,24 +80,10 @@ mod tests { #[tokio::test] async fn test_unix_executes_script_without_extension() -> Result<()> { let env = TestExecutableEnv::new()?; - // Linux can transiently report ETXTBSY while the freshly written test - // script is becoming executable on the backing filesystem. - let mut retries = 0; - let output = loop { - let mut cmd = Command::new(&env.program_name); - cmd.envs(&env.mcp_env); - - let output = cmd.output().await; - if !output - .as_ref() - .is_err_and(|err| err.kind() == std::io::ErrorKind::ExecutableFileBusy) - || retries == 2 - { - break output; - } - retries += 1; - tokio::time::sleep(std::time::Duration::from_millis(10)).await; - }; + let output = Command::new(&env.program_name) + .envs(&env.mcp_env) + .output() + .await; assert!( output.is_ok(), @@ -214,22 +201,12 @@ mod tests { #[cfg(unix)] { let file = dir.join(Self::TEST_PROGRAM); - fs::write(&file, "#!/bin/sh\nexit 0")?; - Self::set_executable(&file)?; + codex_utils_cargo_bin::write_executable(&file, "#!/bin/sh\nexit 0")?; } Ok(()) } - #[cfg(unix)] - fn set_executable(path: &Path) -> Result<()> { - use std::os::unix::fs::PermissionsExt; - let mut perms = fs::metadata(path)?.permissions(); - perms.set_mode(0o755); - fs::set_permissions(path, perms)?; - Ok(()) - } - /// Prepends the given directory to the system's PATH variable. fn build_path_env_var(dir: &Path) -> OsString { let mut path = OsString::from(dir.as_os_str()); diff --git a/codex-rs/rmcp-client/tests/mcp_2026_stdio.rs b/codex-rs/rmcp-client/tests/mcp_2026_stdio.rs index 3c7e5ee117..9658b81ecd 100644 --- a/codex-rs/rmcp-client/tests/mcp_2026_stdio.rs +++ b/codex-rs/rmcp-client/tests/mcp_2026_stdio.rs @@ -53,29 +53,9 @@ async fn exercise_stdio_server( let (server, cwd) = if use_executor { (server, cwd) } else { - use std::os::unix::fs::PermissionsExt; let wrapper = root.path().join("server"); let script = "#!/bin/sh\nprintf '%s' \"$0\" > argv0\nexec \"$MCP_SERVER\" \"$@\"\n"; - #[cfg(target_os = "linux")] - { - // Keep the writable descriptor out of this process so concurrent test - // spawns cannot inherit it and cause ETXTBSY when launching the wrapper. - let output = std::process::Command::new("/bin/sh") - .arg("-c") - .arg("printf '%s' \"$1\" > \"$2\"") - .arg("write-mcp-fixture") - .arg(script) - .arg(&wrapper) - .output()?; - anyhow::ensure!( - output.status.success(), - "failed to write MCP wrapper: {}", - String::from_utf8_lossy(&output.stderr) - ); - } - #[cfg(not(target_os = "linux"))] - std::fs::write(&wrapper, script)?; - std::fs::set_permissions(wrapper, std::fs::Permissions::from_mode(0o755))?; + codex_utils_cargo_bin::write_executable(&wrapper, script)?; env.insert(OsString::from("MCP_SERVER"), server.into_os_string()); ( std::path::PathBuf::from("./server"), diff --git a/codex-rs/rmcp-client/tests/process_group_cleanup.rs b/codex-rs/rmcp-client/tests/process_group_cleanup.rs index 1dcd5586ef..704310583a 100644 --- a/codex-rs/rmcp-client/tests/process_group_cleanup.rs +++ b/codex-rs/rmcp-client/tests/process_group_cleanup.rs @@ -3,7 +3,6 @@ use std::collections::HashMap; use std::ffi::OsString; use std::fs; -use std::os::unix::fs::PermissionsExt; use std::path::Path; use std::sync::Arc; use std::time::Duration; @@ -87,11 +86,10 @@ async fn drop_kills_wrapper_process_group() -> Result<()> { let child_pid_file = temp_dir.path().join("child.pid"); let child_pid_file_str = child_pid_file.to_string_lossy().into_owned(); let wrapper = temp_dir.path().join("wrapper"); - fs::write( + codex_utils_cargo_bin::write_executable( &wrapper, "#!/bin/sh\nsleep 300 & child_pid=$!; echo \"$child_pid\" > \"$CHILD_PID_FILE\"; cat >/dev/null\n", )?; - fs::set_permissions(wrapper, fs::Permissions::from_mode(0o755))?; let client = RmcpClient::new_stdio_client( OsString::from("./wrapper"), diff --git a/codex-rs/utils/cargo-bin/src/executable.rs b/codex-rs/utils/cargo-bin/src/executable.rs new file mode 100644 index 0000000000..cb4cb9f08c --- /dev/null +++ b/codex-rs/utils/cargo-bin/src/executable.rs @@ -0,0 +1,62 @@ +//! Creates executable test fixtures without exposing writable descriptors to sibling spawns. +//! On Linux, writes and copies finish in a separate process before the fixture can be launched. + +use std::fs; +use std::io; +use std::path::Path; +#[cfg(target_os = "linux")] +use std::process::Command; + +/// Writes a small Unix script fixture and makes it executable with mode 0o755. +#[cfg(unix)] +pub fn write_executable(path: &Path, script: &str) -> io::Result<()> { + use std::os::unix::fs::PermissionsExt; + + #[cfg(target_os = "linux")] + { + // A sibling test can fork while a parent-owned file is open for writing. + // CLOEXEC does not release that inherited descriptor until the sibling execs. + let output = Command::new("/bin/sh") + .arg("-c") + .arg("printf '%s' \"$1\" > \"$2\"") + .arg("write-executable-fixture") + .arg(script) + .arg(path) + .output()?; + if !output.status.success() { + return Err(io::Error::other(format!( + "failed to write executable fixture {}: {}", + path.display(), + String::from_utf8_lossy(&output.stderr), + ))); + } + } + #[cfg(not(target_os = "linux"))] + fs::write(path, script)?; + fs::set_permissions(path, fs::Permissions::from_mode(/*mode*/ 0o755)) +} + +/// Copies an executable fixture, preserving the source permissions like [`fs::copy`]. +pub fn copy_executable(source: &Path, destination: &Path) -> io::Result<()> { + #[cfg(target_os = "linux")] + { + let output = Command::new("/bin/cp") + .arg("--") + .arg(source) + .arg(destination) + .output()?; + if !output.status.success() { + return Err(io::Error::other(format!( + "failed to copy executable fixture {} to {}: {}", + source.display(), + destination.display(), + String::from_utf8_lossy(&output.stderr), + ))); + } + // Match fs::copy even when the test process has a restrictive umask. + fs::set_permissions(destination, fs::metadata(source)?.permissions())?; + } + #[cfg(not(target_os = "linux"))] + fs::copy(source, destination)?; + Ok(()) +} diff --git a/codex-rs/utils/cargo-bin/src/lib.rs b/codex-rs/utils/cargo-bin/src/lib.rs index 2cb25a6b17..6ef7080cbc 100644 --- a/codex-rs/utils/cargo-bin/src/lib.rs +++ b/codex-rs/utils/cargo-bin/src/lib.rs @@ -5,6 +5,11 @@ use std::path::PathBuf; pub use runfiles; +mod executable; +pub use executable::copy_executable; +#[cfg(unix)] +pub use executable::write_executable; + /// Bazel sets this when runfiles directories are disabled, which we do on all platforms for consistency. const RUNFILES_MANIFEST_ONLY_ENV: &str = "RUNFILES_MANIFEST_ONLY";