From e61bae12e33cb6b773c3cc42d5775e79d566de1a Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Tue, 23 Dec 2025 19:29:32 -0800 Subject: [PATCH] feat: introduce codex-utils-cargo-bin as an alternative to assert_cmd::Command (#8496) This PR introduces a `codex-utils-cargo-bin` utility crate that wraps/replaces our use of `assert_cmd::Command` and `escargot::CargoBuild`. As you can infer from the introduction of `buck_project_root()` in this PR, I am attempting to make it possible to build Codex under [Buck2](https://buck2.build) as well as `cargo`. With Buck2, I hope to achieve faster incremental local builds (largely due to Buck2's [dice](https://buck2.build/docs/insights_and_knowledge/modern_dice/) build strategy, as well as benefits from its local build daemon) as well as faster CI builds if we invest in remote execution and caching. See https://buck2.build/docs/getting_started/what_is_buck2/#why-use-buck2-key-advantages for more details about the performance advantages of Buck2. Buck2 enforces stronger requirements in terms of build and test isolation. It discourages assumptions about absolute paths (which is key to enabling remote execution). Because the `CARGO_BIN_EXE_*` environment variables that Cargo provides are absolute paths (which `assert_cmd::Command` reads), this is a problem for Buck2, which is why we need this `codex-utils-cargo-bin` utility. My WIP-Buck2 setup sets the `CARGO_BIN_EXE_*` environment variables passed to a `rust_test()` build rule as relative paths. `codex-utils-cargo-bin` will resolve these values to absolute paths, when necessary. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/8496). * #8498 * __->__ #8496 --- AGENTS.md | 6 + codex-rs/Cargo.lock | 26 +++- codex-rs/Cargo.toml | 2 + codex-rs/app-server/Cargo.toml | 1 - codex-rs/app-server/tests/common/Cargo.toml | 2 +- .../app-server/tests/common/mcp_process.rs | 10 +- codex-rs/apply-patch/Cargo.toml | 1 + codex-rs/apply-patch/tests/suite/cli.rs | 29 ++-- codex-rs/apply-patch/tests/suite/scenarios.rs | 3 +- codex-rs/apply-patch/tests/suite/tool.rs | 4 +- codex-rs/cli/Cargo.toml | 1 + codex-rs/cli/tests/execpolicy.rs | 2 +- codex-rs/cli/tests/mcp_add_remove.rs | 2 +- codex-rs/cli/tests/mcp_list.rs | 2 +- codex-rs/core/Cargo.toml | 1 + codex-rs/core/tests/common/Cargo.toml | 1 + codex-rs/core/tests/common/lib.rs | 8 +- codex-rs/core/tests/common/test_codex.rs | 4 +- codex-rs/core/tests/common/test_codex_exec.rs | 6 +- codex-rs/core/tests/suite/cli_stream.rs | 11 +- codex-rs/core/tests/suite/live_cli.rs | 2 +- codex-rs/exec-server/Cargo.toml | 2 +- codex-rs/exec-server/tests/common/Cargo.toml | 2 +- codex-rs/exec-server/tests/common/lib.rs | 43 +++--- .../tests/suite/accept_elicitation.rs | 6 +- codex-rs/exec/Cargo.toml | 1 + codex-rs/exec/tests/suite/apply_patch.rs | 3 +- codex-rs/exec/tests/suite/sandbox.rs | 3 +- codex-rs/mcp-server/Cargo.toml | 1 - codex-rs/mcp-server/tests/common/Cargo.toml | 2 +- .../mcp-server/tests/common/mcp_process.rs | 8 +- codex-rs/rmcp-client/Cargo.toml | 2 +- codex-rs/rmcp-client/tests/resources.rs | 10 +- codex-rs/stdio-to-uds/Cargo.toml | 1 + codex-rs/stdio-to-uds/tests/stdio_to_uds.rs | 2 +- codex-rs/utils/cargo-bin/Cargo.toml | 12 ++ codex-rs/utils/cargo-bin/src/lib.rs | 127 ++++++++++++++++++ 37 files changed, 248 insertions(+), 101 deletions(-) create mode 100644 codex-rs/utils/cargo-bin/Cargo.toml create mode 100644 codex-rs/utils/cargo-bin/src/lib.rs diff --git a/AGENTS.md b/AGENTS.md index 50c10b1da1..e924ceeae1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -77,6 +77,12 @@ If you don’t have the tool: - Prefer deep equals comparisons whenever possible. Perform `assert_eq!()` on entire objects, rather than individual fields. - Avoid mutating process environment in tests; prefer passing environment-derived flags or dependencies from above. +### Spawning workspace binaries in tests (Cargo vs Buck2) + +- Prefer `codex_utils_cargo_bin::cargo_bin("...")` over `assert_cmd::Command::cargo_bin(...)` or `escargot` when tests need to spawn first-party binaries. + - Under Buck2, `CARGO_BIN_EXE_*` may be project-relative (e.g. `buck-out/...`), which breaks if a test changes its working directory. `codex_utils_cargo_bin::cargo_bin` resolves to an absolute path first. +- When locating fixture files under Buck2, avoid `env!("CARGO_MANIFEST_DIR")` (Buck codegen sets it to `"."`). Prefer deriving paths from `codex_utils_cargo_bin::buck_project_root()` when needed. + ### Integration tests (core) - Prefer the utilities in `core_test_support::responses` when writing end-to-end Codex tests. diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 42c8c557dc..70902b4bd5 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -329,12 +329,12 @@ name = "app_test_support" version = "0.0.0" dependencies = [ "anyhow", - "assert_cmd", "base64", "chrono", "codex-app-server-protocol", "codex-core", "codex-protocol", + "codex-utils-cargo-bin", "core_test_support", "serde", "serde_json", @@ -993,7 +993,6 @@ version = "0.0.0" dependencies = [ "anyhow", "app_test_support", - "assert_cmd", "base64", "chrono", "codex-app-server-protocol", @@ -1064,6 +1063,7 @@ dependencies = [ "anyhow", "assert_cmd", "assert_matches", + "codex-utils-cargo-bin", "pretty_assertions", "similar", "tempfile", @@ -1161,6 +1161,7 @@ dependencies = [ "codex-tui", "codex-tui2", "codex-utils-absolute-path", + "codex-utils-cargo-bin", "codex-windows-sandbox", "ctor 0.5.0", "libc", @@ -1285,6 +1286,7 @@ dependencies = [ "codex-protocol", "codex-rmcp-client", "codex-utils-absolute-path", + "codex-utils-cargo-bin", "codex-utils-pty", "codex-utils-readiness", "codex-utils-string", @@ -1360,6 +1362,7 @@ dependencies = [ "codex-core", "codex-protocol", "codex-utils-absolute-path", + "codex-utils-cargo-bin", "core_test_support", "libc", "mcp-types", @@ -1385,11 +1388,11 @@ name = "codex-exec-server" version = "0.0.0" dependencies = [ "anyhow", - "assert_cmd", "async-trait", "clap", "codex-core", "codex-execpolicy", + "codex-utils-cargo-bin", "exec_server_test_support", "libc", "maplit", @@ -1548,7 +1551,6 @@ name = "codex-mcp-server" version = "0.0.0" dependencies = [ "anyhow", - "assert_cmd", "codex-arg0", "codex-common", "codex-core", @@ -1672,8 +1674,8 @@ dependencies = [ "axum", "codex-keyring-store", "codex-protocol", + "codex-utils-cargo-bin", "dirs", - "escargot", "futures", "keyring", "mcp-types", @@ -1700,6 +1702,7 @@ version = "0.0.0" dependencies = [ "anyhow", "assert_cmd", + "codex-utils-cargo-bin", "pretty_assertions", "tempfile", "uds_windows", @@ -1866,6 +1869,14 @@ dependencies = [ "tokio", ] +[[package]] +name = "codex-utils-cargo-bin" +version = "0.0.0" +dependencies = [ + "assert_cmd", + "thiserror 2.0.17", +] + [[package]] name = "codex-utils-image" version = "0.0.0" @@ -2069,6 +2080,7 @@ dependencies = [ "codex-core", "codex-protocol", "codex-utils-absolute-path", + "codex-utils-cargo-bin", "notify", "pretty_assertions", "regex-lite", @@ -2788,8 +2800,8 @@ name = "exec_server_test_support" version = "0.0.0" dependencies = [ "anyhow", - "assert_cmd", "codex-core", + "codex-utils-cargo-bin", "rmcp", "serde_json", "tokio", @@ -4157,9 +4169,9 @@ name = "mcp_test_support" version = "0.0.0" dependencies = [ "anyhow", - "assert_cmd", "codex-core", "codex-mcp-server", + "codex-utils-cargo-bin", "core_test_support", "mcp-types", "os_info", diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index 3dc6618ce2..d79e87acce 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -36,6 +36,7 @@ members = [ "tui", "tui2", "utils/absolute-path", + "utils/cargo-bin", "utils/git", "utils/cache", "utils/image", @@ -93,6 +94,7 @@ codex-tui = { path = "tui" } codex-tui2 = { path = "tui2" } codex-utils-absolute-path = { path = "utils/absolute-path" } codex-utils-cache = { path = "utils/cache" } +codex-utils-cargo-bin = { path = "utils/cargo-bin" } codex-utils-image = { path = "utils/image" } codex-utils-json-to-toml = { path = "utils/json-to-toml" } codex-utils-pty = { path = "utils/pty" } diff --git a/codex-rs/app-server/Cargo.toml b/codex-rs/app-server/Cargo.toml index ca7fac30ce..fbe9150a1a 100644 --- a/codex-rs/app-server/Cargo.toml +++ b/codex-rs/app-server/Cargo.toml @@ -48,7 +48,6 @@ uuid = { workspace = true, features = ["serde", "v7"] } [dev-dependencies] app_test_support = { workspace = true } -assert_cmd = { workspace = true } base64 = { workspace = true } core_test_support = { workspace = true } mcp-types = { workspace = true } diff --git a/codex-rs/app-server/tests/common/Cargo.toml b/codex-rs/app-server/tests/common/Cargo.toml index 67ceeae4f4..d91ed4e675 100644 --- a/codex-rs/app-server/tests/common/Cargo.toml +++ b/codex-rs/app-server/tests/common/Cargo.toml @@ -9,12 +9,12 @@ path = "lib.rs" [dependencies] anyhow = { workspace = true } -assert_cmd = { workspace = true } base64 = { workspace = true } chrono = { workspace = true } codex-app-server-protocol = { workspace = true } codex-core = { workspace = true, features = ["test-support"] } codex-protocol = { workspace = true } +codex-utils-cargo-bin = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } tokio = { workspace = true, features = [ diff --git a/codex-rs/app-server/tests/common/mcp_process.rs b/codex-rs/app-server/tests/common/mcp_process.rs index e2da40bf2d..98b2cabaaa 100644 --- a/codex-rs/app-server/tests/common/mcp_process.rs +++ b/codex-rs/app-server/tests/common/mcp_process.rs @@ -11,7 +11,6 @@ use tokio::process::ChildStdin; use tokio::process::ChildStdout; use anyhow::Context; -use assert_cmd::prelude::*; use codex_app_server_protocol::AddConversationListenerParams; use codex_app_server_protocol::ArchiveConversationParams; use codex_app_server_protocol::CancelLoginAccountParams; @@ -49,7 +48,6 @@ use codex_app_server_protocol::ThreadResumeParams; use codex_app_server_protocol::ThreadStartParams; use codex_app_server_protocol::TurnInterruptParams; use codex_app_server_protocol::TurnStartParams; -use std::process::Command as StdCommand; use tokio::process::Command; pub struct McpProcess { @@ -78,12 +76,8 @@ impl McpProcess { codex_home: &Path, env_overrides: &[(&str, Option<&str>)], ) -> anyhow::Result { - // Use assert_cmd to locate the binary path and then switch to tokio::process::Command - let std_cmd = StdCommand::cargo_bin("codex-app-server") - .context("should find binary for codex-mcp-server")?; - - let program = std_cmd.get_program().to_owned(); - + let program = codex_utils_cargo_bin::cargo_bin("codex-app-server") + .context("should find binary for codex-app-server")?; let mut cmd = Command::new(program); cmd.stdin(Stdio::piped()); diff --git a/codex-rs/apply-patch/Cargo.toml b/codex-rs/apply-patch/Cargo.toml index 1a918ce93b..2fad46c2af 100644 --- a/codex-rs/apply-patch/Cargo.toml +++ b/codex-rs/apply-patch/Cargo.toml @@ -25,5 +25,6 @@ tree-sitter-bash = { workspace = true } [dev-dependencies] assert_cmd = { workspace = true } assert_matches = { workspace = true } +codex-utils-cargo-bin = { workspace = true } pretty_assertions = { workspace = true } tempfile = { workspace = true } diff --git a/codex-rs/apply-patch/tests/suite/cli.rs b/codex-rs/apply-patch/tests/suite/cli.rs index ed95aba17c..c982c7aa86 100644 --- a/codex-rs/apply-patch/tests/suite/cli.rs +++ b/codex-rs/apply-patch/tests/suite/cli.rs @@ -1,8 +1,13 @@ -use assert_cmd::prelude::*; +use assert_cmd::Command; use std::fs; -use std::process::Command; use tempfile::tempdir; +fn apply_patch_command() -> anyhow::Result { + Ok(Command::new(codex_utils_cargo_bin::cargo_bin( + "apply_patch", + )?)) +} + #[test] fn test_apply_patch_cli_add_and_update() -> anyhow::Result<()> { let tmp = tempdir()?; @@ -16,8 +21,7 @@ fn test_apply_patch_cli_add_and_update() -> anyhow::Result<()> { +hello *** End Patch"# ); - Command::cargo_bin("apply_patch") - .expect("should find apply_patch binary") + apply_patch_command()? .arg(add_patch) .current_dir(tmp.path()) .assert() @@ -34,8 +38,7 @@ fn test_apply_patch_cli_add_and_update() -> anyhow::Result<()> { +world *** End Patch"# ); - Command::cargo_bin("apply_patch") - .expect("should find apply_patch binary") + apply_patch_command()? .arg(update_patch) .current_dir(tmp.path()) .assert() @@ -59,10 +62,9 @@ fn test_apply_patch_cli_stdin_add_and_update() -> anyhow::Result<()> { +hello *** End Patch"# ); - let mut cmd = - assert_cmd::Command::cargo_bin("apply_patch").expect("should find apply_patch binary"); - cmd.current_dir(tmp.path()); - cmd.write_stdin(add_patch) + apply_patch_command()? + .current_dir(tmp.path()) + .write_stdin(add_patch) .assert() .success() .stdout(format!("Success. Updated the following files:\nA {file}\n")); @@ -77,10 +79,9 @@ fn test_apply_patch_cli_stdin_add_and_update() -> anyhow::Result<()> { +world *** End Patch"# ); - let mut cmd = - assert_cmd::Command::cargo_bin("apply_patch").expect("should find apply_patch binary"); - cmd.current_dir(tmp.path()); - cmd.write_stdin(update_patch) + apply_patch_command()? + .current_dir(tmp.path()) + .write_stdin(update_patch) .assert() .success() .stdout(format!("Success. Updated the following files:\nM {file}\n")); diff --git a/codex-rs/apply-patch/tests/suite/scenarios.rs b/codex-rs/apply-patch/tests/suite/scenarios.rs index 4b3eb3c84a..b35ed8590d 100644 --- a/codex-rs/apply-patch/tests/suite/scenarios.rs +++ b/codex-rs/apply-patch/tests/suite/scenarios.rs @@ -1,4 +1,3 @@ -use assert_cmd::prelude::*; use pretty_assertions::assert_eq; use std::collections::BTreeMap; use std::fs; @@ -36,7 +35,7 @@ fn run_apply_patch_scenario(dir: &Path) -> anyhow::Result<()> { // Run apply_patch in the temporary directory. We intentionally do not assert // on the exit status here; the scenarios are specified purely in terms of // final filesystem state, which we compare below. - Command::cargo_bin("apply_patch")? + Command::new(codex_utils_cargo_bin::cargo_bin("apply_patch")?) .arg(patch) .current_dir(tmp.path()) .output()?; diff --git a/codex-rs/apply-patch/tests/suite/tool.rs b/codex-rs/apply-patch/tests/suite/tool.rs index c937719da0..56cd6e57a7 100644 --- a/codex-rs/apply-patch/tests/suite/tool.rs +++ b/codex-rs/apply-patch/tests/suite/tool.rs @@ -5,13 +5,13 @@ use std::path::Path; use tempfile::tempdir; fn run_apply_patch_in_dir(dir: &Path, patch: &str) -> anyhow::Result { - let mut cmd = Command::cargo_bin("apply_patch")?; + let mut cmd = Command::new(codex_utils_cargo_bin::cargo_bin("apply_patch")?); cmd.current_dir(dir); Ok(cmd.arg(patch).assert()) } fn apply_patch_command(dir: &Path) -> anyhow::Result { - let mut cmd = Command::cargo_bin("apply_patch")?; + let mut cmd = Command::new(codex_utils_cargo_bin::cargo_bin("apply_patch")?); cmd.current_dir(dir); Ok(cmd) } diff --git a/codex-rs/cli/Cargo.toml b/codex-rs/cli/Cargo.toml index 3cbdd97f5a..0768d7a42f 100644 --- a/codex-rs/cli/Cargo.toml +++ b/codex-rs/cli/Cargo.toml @@ -60,6 +60,7 @@ codex_windows_sandbox = { package = "codex-windows-sandbox", path = "../windows- [dev-dependencies] assert_cmd = { workspace = true } assert_matches = { workspace = true } +codex-utils-cargo-bin = { workspace = true } predicates = { workspace = true } pretty_assertions = { workspace = true } tempfile = { workspace = true } diff --git a/codex-rs/cli/tests/execpolicy.rs b/codex-rs/cli/tests/execpolicy.rs index 241a873d59..7d8a2b1c45 100644 --- a/codex-rs/cli/tests/execpolicy.rs +++ b/codex-rs/cli/tests/execpolicy.rs @@ -24,7 +24,7 @@ prefix_rule( "#, )?; - let output = Command::cargo_bin("codex")? + let output = Command::new(codex_utils_cargo_bin::cargo_bin("codex")?) .env("CODEX_HOME", codex_home.path()) .args([ "execpolicy", diff --git a/codex-rs/cli/tests/mcp_add_remove.rs b/codex-rs/cli/tests/mcp_add_remove.rs index 2911637331..bc3fedc2a7 100644 --- a/codex-rs/cli/tests/mcp_add_remove.rs +++ b/codex-rs/cli/tests/mcp_add_remove.rs @@ -8,7 +8,7 @@ use pretty_assertions::assert_eq; use tempfile::TempDir; fn codex_command(codex_home: &Path) -> Result { - let mut cmd = assert_cmd::Command::cargo_bin("codex")?; + let mut cmd = assert_cmd::Command::new(codex_utils_cargo_bin::cargo_bin("codex")?); cmd.env("CODEX_HOME", codex_home); Ok(cmd) } diff --git a/codex-rs/cli/tests/mcp_list.rs b/codex-rs/cli/tests/mcp_list.rs index 1492365afb..400f53365e 100644 --- a/codex-rs/cli/tests/mcp_list.rs +++ b/codex-rs/cli/tests/mcp_list.rs @@ -12,7 +12,7 @@ use serde_json::json; use tempfile::TempDir; fn codex_command(codex_home: &Path) -> Result { - let mut cmd = assert_cmd::Command::cargo_bin("codex")?; + let mut cmd = assert_cmd::Command::new(codex_utils_cargo_bin::cargo_bin("codex")?); cmd.env("CODEX_HOME", codex_home); Ok(cmd) } diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index ec33343161..51fab19dec 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -123,6 +123,7 @@ assert_cmd = { workspace = true } assert_matches = { workspace = true } codex-arg0 = { workspace = true } codex-core = { path = ".", features = ["deterministic_process_ids"] } +codex-utils-cargo-bin = { workspace = true } core_test_support = { workspace = true } ctor = { workspace = true } escargot = { workspace = true } diff --git a/codex-rs/core/tests/common/Cargo.toml b/codex-rs/core/tests/common/Cargo.toml index 0f0abbb3cb..c61a095686 100644 --- a/codex-rs/core/tests/common/Cargo.toml +++ b/codex-rs/core/tests/common/Cargo.toml @@ -14,6 +14,7 @@ base64 = { workspace = true } codex-core = { workspace = true, features = ["test-support"] } codex-protocol = { workspace = true } codex-utils-absolute-path = { workspace = true } +codex-utils-cargo-bin = { workspace = true } notify = { workspace = true } regex-lite = { workspace = true } serde_json = { workspace = true } diff --git a/codex-rs/core/tests/common/lib.rs b/codex-rs/core/tests/common/lib.rs index 63791127bc..9568ec2786 100644 --- a/codex-rs/core/tests/common/lib.rs +++ b/codex-rs/core/tests/common/lib.rs @@ -10,9 +10,6 @@ use codex_utils_absolute_path::AbsolutePathBuf; use regex_lite::Regex; use std::path::PathBuf; -#[cfg(target_os = "linux")] -use assert_cmd::cargo::cargo_bin; - pub mod process; pub mod responses; pub mod streaming_sse; @@ -87,7 +84,10 @@ pub async fn load_default_config_for_test(codex_home: &TempDir) -> Config { #[cfg(target_os = "linux")] fn default_test_overrides() -> ConfigOverrides { ConfigOverrides { - codex_linux_sandbox_exe: Some(cargo_bin("codex-linux-sandbox")), + codex_linux_sandbox_exe: Some( + codex_utils_cargo_bin::cargo_bin("codex-linux-sandbox") + .expect("should find binary for codex-linux-sandbox"), + ), ..ConfigOverrides::default() } } diff --git a/codex-rs/core/tests/common/test_codex.rs b/codex-rs/core/tests/common/test_codex.rs index 1e574cdef1..a081a6bf12 100644 --- a/codex-rs/core/tests/common/test_codex.rs +++ b/codex-rs/core/tests/common/test_codex.rs @@ -184,8 +184,8 @@ impl TestCodexBuilder { for hook in self.pre_build_hooks.drain(..) { hook(home.path()); } - if let Ok(cmd) = assert_cmd::Command::cargo_bin("codex") { - config.codex_linux_sandbox_exe = Some(PathBuf::from(cmd.get_program().to_os_string())); + if let Ok(path) = codex_utils_cargo_bin::cargo_bin("codex") { + config.codex_linux_sandbox_exe = Some(path); } let mut mutators = vec![]; diff --git a/codex-rs/core/tests/common/test_codex_exec.rs b/codex-rs/core/tests/common/test_codex_exec.rs index 478d0c57f4..5f3ea0d5ac 100644 --- a/codex-rs/core/tests/common/test_codex_exec.rs +++ b/codex-rs/core/tests/common/test_codex_exec.rs @@ -11,8 +11,10 @@ pub struct TestCodexExecBuilder { impl TestCodexExecBuilder { pub fn cmd(&self) -> assert_cmd::Command { - let mut cmd = assert_cmd::Command::cargo_bin("codex-exec") - .expect("should find binary for codex-exec"); + let mut cmd = assert_cmd::Command::new( + codex_utils_cargo_bin::cargo_bin("codex-exec") + .expect("should find binary for codex-exec"), + ); cmd.current_dir(self.cwd.path()) .env("CODEX_HOME", self.home.path()) .env(CODEX_API_KEY_ENV_VAR, "dummy"); diff --git a/codex-rs/core/tests/suite/cli_stream.rs b/codex-rs/core/tests/suite/cli_stream.rs index d7f0fb9830..8901dd210e 100644 --- a/codex-rs/core/tests/suite/cli_stream.rs +++ b/codex-rs/core/tests/suite/cli_stream.rs @@ -1,5 +1,4 @@ use assert_cmd::Command as AssertCommand; -use assert_cmd::cargo::cargo_bin; use codex_core::RolloutRecorder; use codex_core::protocol::GitInfo; use core_test_support::fs_wait; @@ -45,7 +44,7 @@ async fn chat_mode_stream_cli() { "model_providers.mock={{ name = \"mock\", base_url = \"{}/v1\", env_key = \"PATH\", wire_api = \"chat\" }}", server.uri() ); - let bin = cargo_bin("codex"); + let bin = codex_utils_cargo_bin::cargo_bin("codex").unwrap(); let mut cmd = AssertCommand::new(bin); cmd.arg("exec") .arg("--skip-git-repo-check") @@ -128,7 +127,7 @@ async fn exec_cli_applies_experimental_instructions_file() { ); let home = TempDir::new().unwrap(); - let bin = cargo_bin("codex"); + let bin = codex_utils_cargo_bin::cargo_bin("codex").unwrap(); let mut cmd = AssertCommand::new(bin); cmd.arg("exec") .arg("--skip-git-repo-check") @@ -182,7 +181,7 @@ async fn responses_api_stream_cli() { std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/cli_responses_fixture.sse"); let home = TempDir::new().unwrap(); - let bin = cargo_bin("codex"); + let bin = codex_utils_cargo_bin::cargo_bin("codex").unwrap(); let mut cmd = AssertCommand::new(bin); cmd.arg("exec") .arg("--skip-git-repo-check") @@ -218,7 +217,7 @@ async fn integration_creates_and_checks_session_file() -> anyhow::Result<()> { std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/cli_responses_fixture.sse"); // 4. Run the codex CLI and invoke `exec`, which is what records a session. - let bin = cargo_bin("codex"); + let bin = codex_utils_cargo_bin::cargo_bin("codex").unwrap(); let mut cmd = AssertCommand::new(bin); cmd.arg("exec") .arg("--skip-git-repo-check") @@ -339,7 +338,7 @@ async fn integration_creates_and_checks_session_file() -> anyhow::Result<()> { // Second run: resume should update the existing file. let marker2 = format!("integration-resume-{}", Uuid::new_v4()); let prompt2 = format!("echo {marker2}"); - let bin2 = cargo_bin("codex"); + let bin2 = codex_utils_cargo_bin::cargo_bin("codex").unwrap(); let mut cmd2 = AssertCommand::new(bin2); cmd2.arg("exec") .arg("--skip-git-repo-check") diff --git a/codex-rs/core/tests/suite/live_cli.rs b/codex-rs/core/tests/suite/live_cli.rs index e77fe6998e..c5c26a1c8e 100644 --- a/codex-rs/core/tests/suite/live_cli.rs +++ b/codex-rs/core/tests/suite/live_cli.rs @@ -30,7 +30,7 @@ fn run_live(prompt: &str) -> (assert_cmd::assert::Assert, TempDir) { // implementation). Instead we configure the std `Command` ourselves, then later hand the // resulting `Output` to `assert_cmd` for the familiar assertions. - let mut cmd = Command::cargo_bin("codex-rs").unwrap(); + let mut cmd = Command::new(codex_utils_cargo_bin::cargo_bin("codex-rs").unwrap()); cmd.current_dir(dir.path()); cmd.env("OPENAI_API_KEY", require_api_key()); diff --git a/codex-rs/exec-server/Cargo.toml b/codex-rs/exec-server/Cargo.toml index 4f8df05dbe..9346db63c1 100644 --- a/codex-rs/exec-server/Cargo.toml +++ b/codex-rs/exec-server/Cargo.toml @@ -56,7 +56,7 @@ tracing = { workspace = true } tracing-subscriber = { workspace = true, features = ["env-filter", "fmt"] } [dev-dependencies] -assert_cmd = { workspace = true } +codex-utils-cargo-bin = { workspace = true } exec_server_test_support = { workspace = true } maplit = { workspace = true } pretty_assertions = { workspace = true } diff --git a/codex-rs/exec-server/tests/common/Cargo.toml b/codex-rs/exec-server/tests/common/Cargo.toml index ba7d2af0a1..6444b61f97 100644 --- a/codex-rs/exec-server/tests/common/Cargo.toml +++ b/codex-rs/exec-server/tests/common/Cargo.toml @@ -8,9 +8,9 @@ license.workspace = true path = "lib.rs" [dependencies] -assert_cmd = { workspace = true } anyhow = { workspace = true } codex-core = { workspace = true } +codex-utils-cargo-bin = { workspace = true } rmcp = { workspace = true } serde_json = { workspace = true } tokio = { workspace = true } diff --git a/codex-rs/exec-server/tests/common/lib.rs b/codex-rs/exec-server/tests/common/lib.rs index c2202a168a..a5a6a3cac5 100644 --- a/codex-rs/exec-server/tests/common/lib.rs +++ b/codex-rs/exec-server/tests/common/lib.rs @@ -32,14 +32,18 @@ pub async fn create_transport

( where P: AsRef, { - let mcp_executable = assert_cmd::Command::cargo_bin("codex-exec-mcp-server")?; - let execve_wrapper = assert_cmd::Command::cargo_bin("codex-execve-wrapper")?; - let bash = Path::new(env!("CARGO_MANIFEST_DIR")) - .join("..") - .join("..") - .join("tests") - .join("suite") - .join("bash"); + let mcp_executable = codex_utils_cargo_bin::cargo_bin("codex-exec-mcp-server")?; + let execve_wrapper = codex_utils_cargo_bin::cargo_bin("codex-execve-wrapper")?; + // `bash` requires a special lookup when running under Buck because it is a + // _resource_ rather than a binary target. + let bash = if let Some(root) = codex_utils_cargo_bin::buck_project_root()? { + root.join("codex-rs/exec-server/tests/suite/bash") + } else { + Path::new(env!("CARGO_MANIFEST_DIR")) + .join("..") + .join("suite") + .join("bash") + }; // Need to ensure the artifact associated with the bash DotSlash file is // available before it is run in a read-only sandbox. @@ -52,20 +56,19 @@ where .await?; assert!(status.success(), "dotslash fetch failed: {status:?}"); - let transport = - TokioChildProcess::new(Command::new(mcp_executable.get_program()).configure(|cmd| { - cmd.arg("--bash").arg(bash); - cmd.arg("--execve").arg(execve_wrapper.get_program()); - cmd.env("CODEX_HOME", codex_home.as_ref()); - cmd.env("DOTSLASH_CACHE", dotslash_cache.as_ref()); + let transport = TokioChildProcess::new(Command::new(&mcp_executable).configure(|cmd| { + cmd.arg("--bash").arg(bash); + cmd.arg("--execve").arg(&execve_wrapper); + cmd.env("CODEX_HOME", codex_home.as_ref()); + cmd.env("DOTSLASH_CACHE", dotslash_cache.as_ref()); - // Important: pipe stdio so rmcp can speak JSON-RPC over stdin/stdout - cmd.stdin(Stdio::piped()); - cmd.stdout(Stdio::piped()); + // Important: pipe stdio so rmcp can speak JSON-RPC over stdin/stdout + cmd.stdin(Stdio::piped()); + cmd.stdout(Stdio::piped()); - // Optional but very helpful while debugging: - cmd.stderr(Stdio::inherit()); - }))?; + // Optional but very helpful while debugging: + cmd.stderr(Stdio::inherit()); + }))?; Ok(transport) } diff --git a/codex-rs/exec-server/tests/suite/accept_elicitation.rs b/codex-rs/exec-server/tests/suite/accept_elicitation.rs index 81283a91d5..1e0ea32e32 100644 --- a/codex-rs/exec-server/tests/suite/accept_elicitation.rs +++ b/codex-rs/exec-server/tests/suite/accept_elicitation.rs @@ -142,11 +142,7 @@ prefix_rule( } fn ensure_codex_cli() -> Result { - let codex_cli = PathBuf::from( - assert_cmd::Command::cargo_bin("codex")? - .get_program() - .to_os_string(), - ); + let codex_cli = codex_utils_cargo_bin::cargo_bin("codex")?; let metadata = codex_cli.metadata().with_context(|| { format!( diff --git a/codex-rs/exec/Cargo.toml b/codex-rs/exec/Cargo.toml index c205215421..9156c22ea3 100644 --- a/codex-rs/exec/Cargo.toml +++ b/codex-rs/exec/Cargo.toml @@ -51,6 +51,7 @@ ts-rs = { workspace = true, features = [ [dev-dependencies] assert_cmd = { workspace = true } +codex-utils-cargo-bin = { workspace = true } core_test_support = { workspace = true } libc = { workspace = true } mcp-types = { workspace = true } diff --git a/codex-rs/exec/tests/suite/apply_patch.rs b/codex-rs/exec/tests/suite/apply_patch.rs index a32f83ba5e..fdc5415dc6 100644 --- a/codex-rs/exec/tests/suite/apply_patch.rs +++ b/codex-rs/exec/tests/suite/apply_patch.rs @@ -23,8 +23,7 @@ fn test_standalone_exec_cli_can_use_apply_patch() -> anyhow::Result<()> { let absolute_path = tmp.path().join(relative_path); fs::write(&absolute_path, "original content\n")?; - Command::cargo_bin("codex-exec") - .context("should find binary for codex-exec")? + Command::new(codex_utils_cargo_bin::cargo_bin("codex-exec")?) .arg(CODEX_APPLY_PATCH_ARG1) .arg( r#"*** Begin Patch diff --git a/codex-rs/exec/tests/suite/sandbox.rs b/codex-rs/exec/tests/suite/sandbox.rs index d995d91030..303023b158 100644 --- a/codex-rs/exec/tests/suite/sandbox.rs +++ b/codex-rs/exec/tests/suite/sandbox.rs @@ -42,7 +42,8 @@ async fn spawn_command_under_sandbox( env: HashMap, ) -> std::io::Result { use codex_core::landlock::spawn_command_under_linux_sandbox; - let codex_linux_sandbox_exe = assert_cmd::cargo::cargo_bin("codex-exec"); + let codex_linux_sandbox_exe = codex_utils_cargo_bin::cargo_bin("codex-exec") + .map_err(|err| io::Error::new(io::ErrorKind::NotFound, err))?; spawn_command_under_linux_sandbox( codex_linux_sandbox_exe, command, diff --git a/codex-rs/mcp-server/Cargo.toml b/codex-rs/mcp-server/Cargo.toml index 00a2204578..6236384c95 100644 --- a/codex-rs/mcp-server/Cargo.toml +++ b/codex-rs/mcp-server/Cargo.toml @@ -38,7 +38,6 @@ tracing = { workspace = true, features = ["log"] } tracing-subscriber = { workspace = true, features = ["env-filter", "fmt"] } [dev-dependencies] -assert_cmd = { workspace = true } core_test_support = { workspace = true } mcp_test_support = { workspace = true } os_info = { workspace = true } diff --git a/codex-rs/mcp-server/tests/common/Cargo.toml b/codex-rs/mcp-server/tests/common/Cargo.toml index c549a03fc9..aba984edab 100644 --- a/codex-rs/mcp-server/tests/common/Cargo.toml +++ b/codex-rs/mcp-server/tests/common/Cargo.toml @@ -9,9 +9,9 @@ path = "lib.rs" [dependencies] anyhow = { workspace = true } -assert_cmd = { workspace = true } codex-core = { workspace = true } codex-mcp-server = { workspace = true } +codex-utils-cargo-bin = { workspace = true } mcp-types = { workspace = true } os_info = { workspace = true } pretty_assertions = { workspace = true } diff --git a/codex-rs/mcp-server/tests/common/mcp_process.rs b/codex-rs/mcp-server/tests/common/mcp_process.rs index a6bc966d7e..56e134b223 100644 --- a/codex-rs/mcp-server/tests/common/mcp_process.rs +++ b/codex-rs/mcp-server/tests/common/mcp_process.rs @@ -10,7 +10,6 @@ use tokio::process::ChildStdin; use tokio::process::ChildStdout; use anyhow::Context; -use assert_cmd::prelude::*; use codex_mcp_server::CodexToolCallParam; use mcp_types::CallToolRequestParams; @@ -27,7 +26,6 @@ use mcp_types::ModelContextProtocolRequest; use mcp_types::RequestId; use pretty_assertions::assert_eq; use serde_json::json; -use std::process::Command as StdCommand; use tokio::process::Command; pub struct McpProcess { @@ -55,12 +53,8 @@ impl McpProcess { codex_home: &Path, env_overrides: &[(&str, Option<&str>)], ) -> anyhow::Result { - // Use assert_cmd to locate the binary path and then switch to tokio::process::Command - let std_cmd = StdCommand::cargo_bin("codex-mcp-server") + let program = codex_utils_cargo_bin::cargo_bin("codex-mcp-server") .context("should find binary for codex-mcp-server")?; - - let program = std_cmd.get_program().to_owned(); - let mut cmd = Command::new(program); cmd.stdin(Stdio::piped()); diff --git a/codex-rs/rmcp-client/Cargo.toml b/codex-rs/rmcp-client/Cargo.toml index f2525c18d3..efcea2d805 100644 --- a/codex-rs/rmcp-client/Cargo.toml +++ b/codex-rs/rmcp-client/Cargo.toml @@ -55,7 +55,7 @@ webbrowser = { workspace = true } which = { workspace = true } [dev-dependencies] -escargot = { workspace = true } +codex-utils-cargo-bin = { workspace = true } pretty_assertions = { workspace = true } serial_test = { workspace = true } tempfile = { workspace = true } diff --git a/codex-rs/rmcp-client/tests/resources.rs b/codex-rs/rmcp-client/tests/resources.rs index fda21d14e2..3d627ebbf4 100644 --- a/codex-rs/rmcp-client/tests/resources.rs +++ b/codex-rs/rmcp-client/tests/resources.rs @@ -5,7 +5,7 @@ use std::time::Duration; use codex_rmcp_client::ElicitationAction; use codex_rmcp_client::ElicitationResponse; use codex_rmcp_client::RmcpClient; -use escargot::CargoBuild; +use codex_utils_cargo_bin::CargoBinError; use futures::FutureExt as _; use mcp_types::ClientCapabilities; use mcp_types::Implementation; @@ -20,12 +20,8 @@ use serde_json::json; const RESOURCE_URI: &str = "memo://codex/example-note"; -fn stdio_server_bin() -> anyhow::Result { - let build = CargoBuild::new() - .package("codex-rmcp-client") - .bin("test_stdio_server") - .run()?; - Ok(build.path().to_path_buf()) +fn stdio_server_bin() -> Result { + codex_utils_cargo_bin::cargo_bin("test_stdio_server") } fn init_params() -> InitializeRequestParams { diff --git a/codex-rs/stdio-to-uds/Cargo.toml b/codex-rs/stdio-to-uds/Cargo.toml index 71ed3722c3..20e3ade720 100644 --- a/codex-rs/stdio-to-uds/Cargo.toml +++ b/codex-rs/stdio-to-uds/Cargo.toml @@ -23,5 +23,6 @@ uds_windows = { workspace = true } [dev-dependencies] assert_cmd = { workspace = true } +codex-utils-cargo-bin = { workspace = true } pretty_assertions = { workspace = true } tempfile = { workspace = true } diff --git a/codex-rs/stdio-to-uds/tests/stdio_to_uds.rs b/codex-rs/stdio-to-uds/tests/stdio_to_uds.rs index 11888a8134..c6062d50dd 100644 --- a/codex-rs/stdio-to-uds/tests/stdio_to_uds.rs +++ b/codex-rs/stdio-to-uds/tests/stdio_to_uds.rs @@ -47,7 +47,7 @@ fn pipes_stdin_and_stdout_through_socket() -> anyhow::Result<()> { Ok(()) }); - Command::cargo_bin("codex-stdio-to-uds")? + Command::new(codex_utils_cargo_bin::cargo_bin("codex-stdio-to-uds")?) .arg(&socket_path) .write_stdin("request") .assert() diff --git a/codex-rs/utils/cargo-bin/Cargo.toml b/codex-rs/utils/cargo-bin/Cargo.toml new file mode 100644 index 0000000000..d8e6877e6e --- /dev/null +++ b/codex-rs/utils/cargo-bin/Cargo.toml @@ -0,0 +1,12 @@ +[package] +name = "codex-utils-cargo-bin" +version.workspace = true +edition.workspace = true +license.workspace = true + +[lints] +workspace = true + +[dependencies] +assert_cmd = { workspace = true } +thiserror = { workspace = true } diff --git a/codex-rs/utils/cargo-bin/src/lib.rs b/codex-rs/utils/cargo-bin/src/lib.rs new file mode 100644 index 0000000000..16f5493915 --- /dev/null +++ b/codex-rs/utils/cargo-bin/src/lib.rs @@ -0,0 +1,127 @@ +use std::ffi::OsString; +use std::path::PathBuf; + +#[derive(Debug, thiserror::Error)] +pub enum CargoBinError { + #[error("failed to read current exe")] + CurrentExe { + #[source] + source: std::io::Error, + }, + #[error("failed to read current directory")] + CurrentDir { + #[source] + source: std::io::Error, + }, + #[error("CARGO_BIN_EXE env var {key} resolved to {path:?}, but it does not exist")] + ResolvedPathDoesNotExist { key: String, path: PathBuf }, + #[error("could not locate binary {name:?}; tried env vars {env_keys:?}; {fallback}")] + NotFound { + name: String, + env_keys: Vec, + fallback: String, + }, +} + +/// Returns an absolute path to a binary target built for the current test run. +/// +/// In `cargo test`, `CARGO_BIN_EXE_*` env vars are absolute, but Buck2 may set +/// them to project-relative paths (e.g. `buck-out/...`). Those paths break if a +/// test later changes its working directory. This helper makes the path +/// absolute up-front so callers can safely `chdir` afterwards. +pub fn cargo_bin(name: &str) -> Result { + let env_keys = cargo_bin_env_keys(name); + for key in &env_keys { + if let Some(value) = std::env::var_os(key) { + return resolve_bin_from_env(key, value); + } + } + + match assert_cmd::Command::cargo_bin(name) { + Ok(cmd) => { + let abs = absolutize_from_buck_or_cwd(PathBuf::from(cmd.get_program()))?; + if abs.exists() { + Ok(abs) + } else { + Err(CargoBinError::ResolvedPathDoesNotExist { + key: "assert_cmd::Command::cargo_bin".to_owned(), + path: abs, + }) + } + } + Err(err) => Err(CargoBinError::NotFound { + name: name.to_owned(), + env_keys, + fallback: format!("assert_cmd fallback failed: {err}"), + }), + } +} + +fn cargo_bin_env_keys(name: &str) -> Vec { + let mut keys = Vec::with_capacity(2); + keys.push(format!("CARGO_BIN_EXE_{name}")); + + // Cargo replaces dashes in target names when exporting env vars. + let underscore_name = name.replace('-', "_"); + if underscore_name != name { + keys.push(format!("CARGO_BIN_EXE_{underscore_name}")); + } + + keys +} + +fn resolve_bin_from_env(key: &str, value: OsString) -> Result { + let abs = absolutize_from_buck_or_cwd(PathBuf::from(value))?; + + if abs.exists() { + Ok(abs) + } else { + Err(CargoBinError::ResolvedPathDoesNotExist { + key: key.to_owned(), + path: abs, + }) + } +} + +fn absolutize_from_buck_or_cwd(path: PathBuf) -> Result { + if path.is_absolute() { + return Ok(path); + } + + if let Some(root) = + buck_project_root().map_err(|source| CargoBinError::CurrentExe { source })? + { + return Ok(root.join(path)); + } + + Ok(std::env::current_dir() + .map_err(|source| CargoBinError::CurrentDir { source })? + .join(path)) +} + +/// Best-effort attempt to find the Buck project root for the currently running +/// process. +/// +/// Prefer this over `env!("CARGO_MANIFEST_DIR")` when running under Buck2: our +/// Buck generator sets `CARGO_MANIFEST_DIR="."` for compilation, which makes +/// `env!("CARGO_MANIFEST_DIR")` unusable for locating workspace files. +pub fn buck_project_root() -> Result, std::io::Error> { + if let Some(root) = std::env::var_os("BUCK_PROJECT_ROOT") { + let root = PathBuf::from(root); + if root.is_absolute() { + return Ok(Some(root)); + } + } + + // Fall back to deriving the project root from the location of the test + // runner executable: + // /buck-out/v2/gen/.../__tests__/test-binary + let exe = std::env::current_exe()?; + for ancestor in exe.ancestors() { + if ancestor.file_name().is_some_and(|name| name == "buck-out") { + return Ok(ancestor.parent().map(PathBuf::from)); + } + } + + Ok(None) +}