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) +}