From 87f5b69b242d378ce04c38f639112758472a4eb5 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Wed, 10 Dec 2025 15:17:13 -0800 Subject: [PATCH] fix: ensure accept_elicitation_for_prompt_rule() test passes locally (#7832) When I originally introduced `accept_elicitation_for_prompt_rule()` in https://github.com/openai/codex/pull/7617, it worked for me locally because I had run `codex-rs/exec-server/tests/suite/bash` once myself, which had the side-effect of installing the corresponding DotSlash artifact. In CI, I added explicit logic to do this as part of `.github/workflows/rust-ci.yml`, which meant the test also passed in CI, but this logic should have been done as part of the test so that it would work locally for devs who had not installed the DotSlash artifact for `codex-rs/exec-server/tests/suite/bash` before. This PR updates the test to do this (and deletes the setup logic from `rust-ci.yml`), creating a new `DOTSLASH_CACHE` in a temp directory so that this is handled independently for each test. While here, also added a check to ensure that the `codex` binary has been built prior to running the test, as we have to ensure it is symlinked as `codex-linux-sandbox` on Linux in order for the integration test to work on that platform. --- .github/workflows/rust-ci.yml | 8 ---- codex-rs/exec-server/tests/common/lib.rs | 17 ++++++- .../tests/suite/accept_elicitation.rs | 44 ++++++++++++++++--- .../exec-server/tests/suite/list_tools.rs | 4 +- 4 files changed, 57 insertions(+), 16 deletions(-) diff --git a/.github/workflows/rust-ci.yml b/.github/workflows/rust-ci.yml index 0be45540c..13fcaacbb 100644 --- a/.github/workflows/rust-ci.yml +++ b/.github/workflows/rust-ci.yml @@ -412,14 +412,6 @@ jobs: - name: Install DotSlash uses: facebook/install-dotslash@v2 - - name: Pre-fetch DotSlash artifacts - # The Bash wrapper is not available on Windows. - if: ${{ !startsWith(matrix.runner, 'windows') }} - shell: bash - run: | - set -euo pipefail - dotslash -- fetch exec-server/tests/suite/bash - - uses: dtolnay/rust-toolchain@1.90 with: targets: ${{ matrix.target }} diff --git a/codex-rs/exec-server/tests/common/lib.rs b/codex-rs/exec-server/tests/common/lib.rs index c6df5c32c..0c2952822 100644 --- a/codex-rs/exec-server/tests/common/lib.rs +++ b/codex-rs/exec-server/tests/common/lib.rs @@ -23,7 +23,10 @@ use std::sync::Arc; use std::sync::Mutex; use tokio::process::Command; -pub fn create_transport

(codex_home: P) -> anyhow::Result +pub async fn create_transport

( + codex_home: P, + dotslash_cache: P, +) -> anyhow::Result where P: AsRef, { @@ -36,11 +39,23 @@ where .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. + let status = Command::new("dotslash") + .arg("--") + .arg("fetch") + .arg(bash.clone()) + .env("DOTSLASH_CACHE", dotslash_cache.as_ref()) + .status() + .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()); // Important: pipe stdio so rmcp can speak JSON-RPC over stdin/stdout cmd.stdin(Stdio::piped()); diff --git a/codex-rs/exec-server/tests/suite/accept_elicitation.rs b/codex-rs/exec-server/tests/suite/accept_elicitation.rs index 2093f9a57..1858deeec 100644 --- a/codex-rs/exec-server/tests/suite/accept_elicitation.rs +++ b/codex-rs/exec-server/tests/suite/accept_elicitation.rs @@ -1,9 +1,12 @@ #![allow(clippy::unwrap_used, clippy::expect_used)] use std::borrow::Cow; +use std::path::PathBuf; use std::sync::Arc; use std::sync::Mutex; +use anyhow::Context; use anyhow::Result; +use anyhow::ensure; use codex_exec_server::ExecResult; use exec_server_test_support::InteractiveClient; use exec_server_test_support::create_transport; @@ -17,6 +20,7 @@ use rmcp::model::CallToolResult; use rmcp::model::CreateElicitationRequestParam; use rmcp::model::object; use serde_json::json; +use std::os::unix::fs::PermissionsExt; use std::os::unix::fs::symlink; use tempfile::TempDir; @@ -42,7 +46,9 @@ prefix_rule( codex_home.as_ref(), ) .await?; - let transport = create_transport(codex_home.as_ref())?; + let dotslash_cache_temp_dir = TempDir::new()?; + let dotslash_cache = dotslash_cache_temp_dir.path(); + let transport = create_transport(codex_home.as_ref(), dotslash_cache).await?; // Create an MCP client that approves expected elicitation messages. let project_root = TempDir::new()?; @@ -68,11 +74,8 @@ prefix_rule( let linux_sandbox_exe_folder = TempDir::new()?; let codex_linux_sandbox_exe = if cfg!(target_os = "linux") { let codex_linux_sandbox_exe = linux_sandbox_exe_folder.path().join("codex-linux-sandbox"); - let codex_cli = assert_cmd::Command::cargo_bin("codex")? - .get_program() - .to_os_string(); - let codex_cli_path = std::path::PathBuf::from(codex_cli); - symlink(&codex_cli_path, &codex_linux_sandbox_exe)?; + let codex_cli = ensure_codex_cli()?; + symlink(&codex_cli, &codex_linux_sandbox_exe)?; Some(codex_linux_sandbox_exe) } else { None @@ -129,3 +132,32 @@ prefix_rule( Ok(()) } + +fn ensure_codex_cli() -> Result { + let codex_cli = PathBuf::from( + assert_cmd::Command::cargo_bin("codex")? + .get_program() + .to_os_string(), + ); + + let metadata = codex_cli.metadata().with_context(|| { + format!( + "failed to read metadata for codex binary at {}", + codex_cli.display() + ) + })?; + ensure!( + metadata.is_file(), + "expected codex binary at {} to be a file; run `cargo build -p codex-cli --bin codex` before this test", + codex_cli.display() + ); + + let mode = metadata.permissions().mode(); + ensure!( + mode & 0o111 != 0, + "codex binary at {} is not executable (mode {mode:o}); run `cargo build -p codex-cli --bin codex` before this test", + codex_cli.display() + ); + + Ok(codex_cli) +} diff --git a/codex-rs/exec-server/tests/suite/list_tools.rs b/codex-rs/exec-server/tests/suite/list_tools.rs index 17505c761..2f3d412df 100644 --- a/codex-rs/exec-server/tests/suite/list_tools.rs +++ b/codex-rs/exec-server/tests/suite/list_tools.rs @@ -22,7 +22,9 @@ async fn list_tools() -> Result<()> { policy_dir.join("default.codexpolicy"), r#"prefix_rule(pattern=["ls"], decision="prompt")"#, )?; - let transport = create_transport(codex_home.path())?; + let dotslash_cache_temp_dir = TempDir::new()?; + let dotslash_cache = dotslash_cache_temp_dir.path(); + let transport = create_transport(codex_home.path(), dotslash_cache).await?; let service = ().serve(transport).await?; let tools = service.list_tools(Default::default()).await?.tools;