From ac2bffa443612c70fc755caccc18d5689339fb67 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Sun, 26 Apr 2026 12:43:16 -0700 Subject: [PATCH] test: harden app-server integration tests (#19683) ## Why Windows Bazel runs in the permissions stack exposed that app-server integration tests were launching normal plugin startup warmups in every subprocess. Those warmups can call `https://chatgpt.com/backend-api/plugins/featured` when a test is not specifically exercising plugin startup, which adds slow background work, noisy stderr, and dependence on external network state. The relevant startup/featured-plugin behavior was introduced across #15042 and #15264. A few app-server tests also had long optional waits or unbounded cleanup paths, making failures expensive to diagnose and contributing to slow Windows shards. One external-agent config test from #18246 used a GitHub-style marketplace source, which was enough to exercise the pending remote-import path but also meant the background completion task could attempt a real clone. ## What Changed - Adds explicit `AppServerRuntimeOptions` / `PluginStartupTasks` plumbing and a hidden debug-only `--disable-plugin-startup-tasks-for-tests` app-server flag, so integration tests can suppress startup plugin warmups without adding a production env-var gate. - Has the app-server test harness pass that hidden flag by default, while opting plugin-startup coverage back in for tests that intentionally exercise startup sync and featured-plugin warmup behavior. - Lowers normal app-server subprocess logging from `info`/`debug` to `warn` to avoid multi-megabyte stderr output in Bazel logs. - Prevents the external-agent config test from attempting a real marketplace clone by using an invalid non-local source while still exercising the pending-import completion path. - Bounds optional filesystem/realtime waits and fake WebSocket test-server shutdown so failures produce targeted timeouts instead of hanging a shard. - Fixes the Unix script-resolution test in `rmcp-client` to exercise PATH resolution directly and include the actual spawn error in failures. ## Verification - `cargo check -p codex-app-server` - `cargo clippy -p codex-app-server --tests -- -D warnings` - `cargo test -p codex-rmcp-client program_resolver::tests::test_unix_executes_script_without_extension` - `cargo test -p codex-app-server --test all external_agent_config_import_sends_completion_notification_after_pending_plugins_finish -- --nocapture` - `cargo test -p codex-app-server --test all plugin_list_uses_warmed_featured_plugin_ids_cache_on_first_request -- --nocapture` - Windows Local Bazel passed with this test-hardening bundle before it was extracted from #19606. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/19683). * #19395 * #19394 * #19393 * #19392 * #19606 * __->__ #19683 --- codex-rs/app-server/src/in_process.rs | 1 + codex-rs/app-server/src/lib.rs | 44 +++++++++++++++++++ codex-rs/app-server/src/main.rs | 18 +++++++- codex-rs/app-server/src/message_processor.rs | 15 ++++--- .../src/message_processor/tracing_tests.rs | 1 + codex-rs/app-server/tests/common/lib.rs | 1 + .../app-server/tests/common/mcp_process.rs | 20 +++++++-- .../suite/v2/connection_handling_websocket.rs | 7 ++- .../tests/suite/v2/external_agent_config.rs | 4 +- codex-rs/app-server/tests/suite/v2/fs.rs | 11 ++++- .../app-server/tests/suite/v2/plugin_list.rs | 6 +-- .../tests/suite/v2/realtime_conversation.rs | 30 ++++++++----- codex-rs/core/tests/common/responses.rs | 9 +++- codex-rs/rmcp-client/src/program_resolver.rs | 16 +++---- 14 files changed, 140 insertions(+), 43 deletions(-) diff --git a/codex-rs/app-server/src/in_process.rs b/codex-rs/app-server/src/in_process.rs index 729f6d04a..dac25b693 100644 --- a/codex-rs/app-server/src/in_process.rs +++ b/codex-rs/app-server/src/in_process.rs @@ -415,6 +415,7 @@ fn start_uninitialized(args: InProcessStartArgs) -> InProcessClientHandle { auth_manager, rpc_transport: AppServerRpcTransport::InProcess, remote_control_handle: None, + plugin_startup_tasks: crate::PluginStartupTasks::Start, })); let mut thread_created_rx = processor.thread_created_receiver(); let session = Arc::new(ConnectionSessionState::new(ConnectionOrigin::InProcess)); diff --git a/codex-rs/app-server/src/lib.rs b/codex-rs/app-server/src/lib.rs index d9b403165..64f487482 100644 --- a/codex-rs/app-server/src/lib.rs +++ b/codex-rs/app-server/src/lib.rs @@ -362,6 +362,25 @@ pub async fn run_main( .await } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum PluginStartupTasks { + Start, + Skip, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct AppServerRuntimeOptions { + pub plugin_startup_tasks: PluginStartupTasks, +} + +impl Default for AppServerRuntimeOptions { + fn default() -> Self { + Self { + plugin_startup_tasks: PluginStartupTasks::Start, + } + } +} + pub async fn run_main_with_transport( arg0_paths: Arg0DispatchPaths, cli_config_overrides: CliConfigOverrides, @@ -370,6 +389,30 @@ pub async fn run_main_with_transport( transport: AppServerTransport, session_source: SessionSource, auth: AppServerWebsocketAuthSettings, +) -> IoResult<()> { + run_main_with_transport_options( + arg0_paths, + cli_config_overrides, + loader_overrides, + default_analytics_enabled, + transport, + session_source, + auth, + AppServerRuntimeOptions::default(), + ) + .await +} + +#[allow(clippy::too_many_arguments)] +pub async fn run_main_with_transport_options( + arg0_paths: Arg0DispatchPaths, + cli_config_overrides: CliConfigOverrides, + loader_overrides: LoaderOverrides, + default_analytics_enabled: bool, + transport: AppServerTransport, + session_source: SessionSource, + auth: AppServerWebsocketAuthSettings, + runtime_options: AppServerRuntimeOptions, ) -> IoResult<()> { let environment_manager = Arc::new(EnvironmentManager::new(EnvironmentManagerArgs::from_env( ExecServerRuntimePaths::from_optional_paths( @@ -683,6 +726,7 @@ pub async fn run_main_with_transport( auth_manager, rpc_transport: analytics_rpc_transport(&transport), remote_control_handle: Some(remote_control_handle), + plugin_startup_tasks: runtime_options.plugin_startup_tasks, })); let mut thread_created_rx = processor.thread_created_receiver(); let mut running_turn_count_rx = processor.subscribe_running_assistant_turn_count(); diff --git a/codex-rs/app-server/src/main.rs b/codex-rs/app-server/src/main.rs index e37916093..67098c2b3 100644 --- a/codex-rs/app-server/src/main.rs +++ b/codex-rs/app-server/src/main.rs @@ -1,7 +1,9 @@ use clap::Parser; +use codex_app_server::AppServerRuntimeOptions; use codex_app_server::AppServerTransport; use codex_app_server::AppServerWebsocketAuthArgs; -use codex_app_server::run_main_with_transport; +use codex_app_server::PluginStartupTasks; +use codex_app_server::run_main_with_transport_options; use codex_arg0::Arg0DispatchPaths; use codex_arg0::arg0_dispatch_or_else; use codex_core::config_loader::LoaderOverrides; @@ -36,6 +38,12 @@ struct AppServerArgs { #[command(flatten)] auth: AppServerWebsocketAuthArgs, + + /// Hidden debug-only test hook used by integration tests that spawn the + /// production app-server binary. + #[cfg(debug_assertions)] + #[arg(long = "disable-plugin-startup-tasks-for-tests", hide = true)] + disable_plugin_startup_tasks_for_tests: bool, } fn main() -> anyhow::Result<()> { @@ -51,8 +59,13 @@ fn main() -> anyhow::Result<()> { let transport = args.listen; let session_source = args.session_source; let auth = args.auth.try_into_settings()?; + let mut runtime_options = AppServerRuntimeOptions::default(); + #[cfg(debug_assertions)] + if args.disable_plugin_startup_tasks_for_tests { + runtime_options.plugin_startup_tasks = PluginStartupTasks::Skip; + } - run_main_with_transport( + run_main_with_transport_options( arg0_paths, CliConfigOverrides::default(), loader_overrides, @@ -60,6 +73,7 @@ fn main() -> anyhow::Result<()> { transport, session_source, auth, + runtime_options, ) .await?; Ok(()) diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index d3eee87cc..2def169c4 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -95,7 +95,6 @@ use tokio::time::timeout; use tracing::Instrument; const EXTERNAL_AUTH_REFRESH_TIMEOUT: Duration = Duration::from_secs(10); - #[derive(Clone)] struct ExternalAuthRefreshBridge { outgoing: Arc, @@ -260,6 +259,7 @@ pub(crate) struct MessageProcessorArgs { pub(crate) auth_manager: Arc, pub(crate) rpc_transport: AppServerRpcTransport, pub(crate) remote_control_handle: Option, + pub(crate) plugin_startup_tasks: crate::PluginStartupTasks, } impl MessageProcessor { @@ -279,6 +279,7 @@ impl MessageProcessor { auth_manager, rpc_transport, remote_control_handle, + plugin_startup_tasks, } = args; auth_manager.set_external_auth(Arc::new(ExternalAuthRefreshBridge { outgoing: outgoing.clone(), @@ -315,11 +316,13 @@ impl MessageProcessor { feedback, log_db, }); - // Keep plugin startup warmups aligned at app-server startup. - // TODO(xl): Move into PluginManager once this no longer depends on config feature gating. - thread_manager - .plugins_manager() - .maybe_start_plugin_startup_tasks_for_config(&config, auth_manager.clone()); + if matches!(plugin_startup_tasks, crate::PluginStartupTasks::Start) { + // Keep plugin startup warmups aligned at app-server startup. + // TODO(xl): Move into PluginManager once this no longer depends on config feature gating. + thread_manager + .plugins_manager() + .maybe_start_plugin_startup_tasks_for_config(&config, auth_manager.clone()); + } let config_api = ConfigApi::new( config_manager, thread_manager.clone(), diff --git a/codex-rs/app-server/src/message_processor/tracing_tests.rs b/codex-rs/app-server/src/message_processor/tracing_tests.rs index 5b6690c0b..7160b57d5 100644 --- a/codex-rs/app-server/src/message_processor/tracing_tests.rs +++ b/codex-rs/app-server/src/message_processor/tracing_tests.rs @@ -288,6 +288,7 @@ fn build_test_processor( auth_manager, rpc_transport: AppServerRpcTransport::Stdio, remote_control_handle: None, + plugin_startup_tasks: crate::PluginStartupTasks::Start, })); (processor, outgoing_rx) } diff --git a/codex-rs/app-server/tests/common/lib.rs b/codex-rs/app-server/tests/common/lib.rs index 6ac26d8a5..6bb600bd8 100644 --- a/codex-rs/app-server/tests/common/lib.rs +++ b/codex-rs/app-server/tests/common/lib.rs @@ -25,6 +25,7 @@ pub use core_test_support::test_path_buf_with_windows; pub use core_test_support::test_tmp_path; pub use core_test_support::test_tmp_path_buf; pub use mcp_process::DEFAULT_CLIENT_NAME; +pub use mcp_process::DISABLE_PLUGIN_STARTUP_TASKS_ARG; pub use mcp_process::McpProcess; pub use mock_model_server::create_mock_responses_server_repeating_assistant; pub use mock_model_server::create_mock_responses_server_sequence; diff --git a/codex-rs/app-server/tests/common/mcp_process.rs b/codex-rs/app-server/tests/common/mcp_process.rs index befa248e8..bcd364c74 100644 --- a/codex-rs/app-server/tests/common/mcp_process.rs +++ b/codex-rs/app-server/tests/common/mcp_process.rs @@ -106,19 +106,26 @@ pub struct McpProcess { } pub const DEFAULT_CLIENT_NAME: &str = "codex-app-server-tests"; +pub const DISABLE_PLUGIN_STARTUP_TASKS_ARG: &str = "--disable-plugin-startup-tasks-for-tests"; const DISABLE_MANAGED_CONFIG_ENV_VAR: &str = "CODEX_APP_SERVER_DISABLE_MANAGED_CONFIG"; impl McpProcess { pub async fn new(codex_home: &Path) -> anyhow::Result { - Self::new_with_env_and_args(codex_home, &[], &[]).await + Self::new_with_env_and_args(codex_home, &[], &[DISABLE_PLUGIN_STARTUP_TASKS_ARG]).await } pub async fn new_without_managed_config(codex_home: &Path) -> anyhow::Result { Self::new_with_env(codex_home, &[(DISABLE_MANAGED_CONFIG_ENV_VAR, Some("1"))]).await } + pub async fn new_with_plugin_startup_tasks(codex_home: &Path) -> anyhow::Result { + Self::new_with_env_and_args(codex_home, &[], &[]).await + } + pub async fn new_with_args(codex_home: &Path, args: &[&str]) -> anyhow::Result { - Self::new_with_env_and_args(codex_home, &[], args).await + let mut all_args = vec![DISABLE_PLUGIN_STARTUP_TASKS_ARG]; + all_args.extend_from_slice(args); + Self::new_with_env_and_args(codex_home, &[], &all_args).await } /// Creates a new MCP process, allowing tests to override or remove @@ -130,7 +137,12 @@ impl McpProcess { codex_home: &Path, env_overrides: &[(&str, Option<&str>)], ) -> anyhow::Result { - Self::new_with_env_and_args(codex_home, env_overrides, &[]).await + Self::new_with_env_and_args( + codex_home, + env_overrides, + &[DISABLE_PLUGIN_STARTUP_TASKS_ARG], + ) + .await } async fn new_with_env_and_args( @@ -147,7 +159,7 @@ impl McpProcess { cmd.stderr(Stdio::piped()); cmd.current_dir(codex_home); cmd.env("CODEX_HOME", codex_home); - cmd.env("RUST_LOG", "info"); + cmd.env("RUST_LOG", "warn"); // Keep integration tests isolated from host managed configuration. cmd.env( "CODEX_APP_SERVER_MANAGED_CONFIG_PATH", diff --git a/codex-rs/app-server/tests/suite/v2/connection_handling_websocket.rs b/codex-rs/app-server/tests/suite/v2/connection_handling_websocket.rs index 456ae1577..6581c1467 100644 --- a/codex-rs/app-server/tests/suite/v2/connection_handling_websocket.rs +++ b/codex-rs/app-server/tests/suite/v2/connection_handling_websocket.rs @@ -1,6 +1,7 @@ use anyhow::Context; use anyhow::Result; use anyhow::bail; +use app_test_support::DISABLE_PLUGIN_STARTUP_TASKS_ARG; use app_test_support::create_mock_responses_server_sequence_unchecked; use app_test_support::to_response; use base64::Engine; @@ -389,12 +390,13 @@ pub(super) async fn spawn_websocket_server_with_args( let mut cmd = Command::new(program); cmd.arg("--listen") .arg(listen_url) + .arg(DISABLE_PLUGIN_STARTUP_TASKS_ARG) .args(extra_args) .stdin(Stdio::null()) .stdout(Stdio::null()) .stderr(Stdio::piped()) .env("CODEX_HOME", codex_home) - .env("RUST_LOG", "debug"); + .env("RUST_LOG", "warn"); let mut process = cmd .kill_on_drop(true) .spawn() @@ -524,12 +526,13 @@ async fn run_websocket_server_to_completion_with_args( let mut cmd = Command::new(program); cmd.arg("--listen") .arg(listen_url) + .arg(DISABLE_PLUGIN_STARTUP_TASKS_ARG) .args(extra_args) .stdin(Stdio::null()) .stdout(Stdio::null()) .stderr(Stdio::piped()) .env("CODEX_HOME", codex_home) - .env("RUST_LOG", "debug"); + .env("RUST_LOG", "warn"); timeout(DEFAULT_READ_TIMEOUT, cmd.output()) .await .context("timed out waiting for websocket app-server to exit")? diff --git a/codex-rs/app-server/tests/suite/v2/external_agent_config.rs b/codex-rs/app-server/tests/suite/v2/external_agent_config.rs index 049256b60..6b1715dc7 100644 --- a/codex-rs/app-server/tests/suite/v2/external_agent_config.rs +++ b/codex-rs/app-server/tests/suite/v2/external_agent_config.rs @@ -127,6 +127,8 @@ async fn external_agent_config_import_sends_completion_notification_after_pendin -> Result<()> { let codex_home = TempDir::new()?; std::fs::create_dir_all(codex_home.path().join(".claude"))?; + // This test only needs a pending non-local plugin import. Use an invalid + // source so the background completion path cannot make a real network clone. std::fs::write( codex_home.path().join(".claude").join("settings.json"), r#"{ @@ -135,7 +137,7 @@ async fn external_agent_config_import_sends_completion_notification_after_pendin }, "extraKnownMarketplaces": { "acme-tools": { - "source": "owner/debug-marketplace" + "source": "not a valid marketplace source" } } }"#, diff --git a/codex-rs/app-server/tests/suite/v2/fs.rs b/codex-rs/app-server/tests/suite/v2/fs.rs index 642844eb9..a780a51e0 100644 --- a/codex-rs/app-server/tests/suite/v2/fs.rs +++ b/codex-rs/app-server/tests/suite/v2/fs.rs @@ -33,6 +33,7 @@ use std::process::Command; const DEFAULT_READ_TIMEOUT: Duration = Duration::from_secs(60); #[cfg(not(any(target_os = "macos", windows)))] const DEFAULT_READ_TIMEOUT: Duration = Duration::from_secs(10); +const OPTIONAL_FS_CHANGE_TIMEOUT: Duration = Duration::from_secs(2); async fn initialized_mcp(codex_home: &TempDir) -> Result { let mut mcp = McpProcess::new(codex_home.path()).await?; @@ -832,7 +833,7 @@ async fn maybe_fs_changed_notification( mcp: &mut McpProcess, ) -> Result> { match timeout( - DEFAULT_READ_TIMEOUT, + OPTIONAL_FS_CHANGE_TIMEOUT, mcp.read_stream_until_notification_message("fs/changed"), ) .await @@ -845,6 +846,14 @@ async fn maybe_fs_changed_notification( fn replace_file_atomically(path: &PathBuf, contents: &str) -> Result<()> { let temp_path = path.with_extension("lock"); std::fs::write(&temp_path, contents)?; + + #[cfg(windows)] + match std::fs::remove_file(path) { + Ok(()) => {} + Err(err) if err.kind() == std::io::ErrorKind::NotFound => {} + Err(err) => return Err(err.into()), + } + std::fs::rename(temp_path, path)?; Ok(()) } diff --git a/codex-rs/app-server/tests/suite/v2/plugin_list.rs b/codex-rs/app-server/tests/suite/v2/plugin_list.rs index f885f2cb7..8735c20ff 100644 --- a/codex-rs/app-server/tests/suite/v2/plugin_list.rs +++ b/codex-rs/app-server/tests/suite/v2/plugin_list.rs @@ -1066,7 +1066,7 @@ async fn app_server_startup_remote_plugin_sync_runs_once() -> Result<()> { .join(STARTUP_REMOTE_PLUGIN_SYNC_MARKER_FILE); { - let mut mcp = McpProcess::new(codex_home.path()).await?; + let mut mcp = McpProcess::new_with_plugin_startup_tasks(codex_home.path()).await?; timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; wait_for_path_exists(&marker_path).await?; @@ -1102,7 +1102,7 @@ async fn app_server_startup_remote_plugin_sync_runs_once() -> Result<()> { assert!(config.contains(r#"[plugins."linear@openai-curated"]"#)); { - let mut mcp = McpProcess::new(codex_home.path()).await?; + let mut mcp = McpProcess::new_with_plugin_startup_tasks(codex_home.path()).await?; timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; } @@ -1490,7 +1490,7 @@ async fn plugin_list_uses_warmed_featured_plugin_ids_cache_on_first_request() -> .mount(&server) .await; - let mut mcp = McpProcess::new(codex_home.path()).await?; + let mut mcp = McpProcess::new_with_plugin_startup_tasks(codex_home.path()).await?; timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; wait_for_featured_plugin_request_count(&server, /*expected_count*/ 1).await?; diff --git a/codex-rs/app-server/tests/suite/v2/realtime_conversation.rs b/codex-rs/app-server/tests/suite/v2/realtime_conversation.rs index dfc3fea31..62ba19cc4 100644 --- a/codex-rs/app-server/tests/suite/v2/realtime_conversation.rs +++ b/codex-rs/app-server/tests/suite/v2/realtime_conversation.rs @@ -281,7 +281,7 @@ impl RealtimeE2eHarness { )?; let mut mcp = McpProcess::new(codex_home.path()).await?; - mcp.initialize().await?; + timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; login_with_api_key(&mut mcp, "sk-test-key").await?; let thread_start_request_id = mcp @@ -345,10 +345,16 @@ impl RealtimeE2eHarness { /// Returns the nth JSON message app-server wrote to the fake Realtime API /// sideband websocket. async fn sideband_outbound_request(&self, request_index: usize) -> Value { - self.realtime_server - .wait_for_request(/*connection_index*/ 0, request_index) - .await - .body_json() + timeout( + DEFAULT_TIMEOUT, + self.realtime_server + .wait_for_request(/*connection_index*/ 0, request_index), + ) + .await + .unwrap_or_else(|_| { + panic!("timed out waiting for realtime sideband request {request_index}") + }) + .body_json() } async fn append_audio(&mut self, thread_id: String) -> Result<()> { @@ -534,7 +540,7 @@ async fn realtime_conversation_streams_v2_notifications() -> Result<()> { )?; let mut mcp = McpProcess::new(codex_home.path()).await?; - mcp.initialize().await?; + timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; login_with_api_key(&mut mcp, "sk-test-key").await?; let thread_start_request_id = mcp @@ -783,7 +789,7 @@ async fn realtime_text_output_modality_requests_text_output_and_final_transcript )?; let mut mcp = McpProcess::new(codex_home.path()).await?; - mcp.initialize().await?; + timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; login_with_api_key(&mut mcp, "sk-test-key").await?; let thread_start_request_id = mcp @@ -885,7 +891,7 @@ async fn realtime_list_voices_returns_supported_names() -> Result<()> { )?; let mut mcp = McpProcess::new(codex_home.path()).await?; - mcp.initialize().await?; + timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; let request_id = mcp .send_thread_realtime_list_voices_request(ThreadRealtimeListVoicesParams {}) @@ -957,7 +963,7 @@ async fn realtime_conversation_stop_emits_closed_notification() -> Result<()> { )?; let mut mcp = McpProcess::new(codex_home.path()).await?; - mcp.initialize().await?; + timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; login_with_api_key(&mut mcp, "sk-test-key").await?; let thread_start_request_id = mcp @@ -1053,7 +1059,7 @@ async fn realtime_webrtc_start_emits_sdp_notification() -> Result<()> { )?; let mut mcp = McpProcess::new(codex_home.path()).await?; - mcp.initialize().await?; + timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; login_with_api_key(&mut mcp, "sk-test-key").await?; let thread_start_request_id = mcp @@ -1968,7 +1974,7 @@ async fn realtime_webrtc_start_surfaces_backend_error() -> Result<()> { )?; let mut mcp = McpProcess::new(codex_home.path()).await?; - mcp.initialize().await?; + timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; login_with_api_key(&mut mcp, "sk-test-key").await?; // Phase 2: start a normal app-server thread and request realtime over WebRTC. @@ -2029,7 +2035,7 @@ async fn realtime_conversation_requires_feature_flag() -> Result<()> { )?; let mut mcp = McpProcess::new(codex_home.path()).await?; - mcp.initialize().await?; + timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; let thread_start_request_id = mcp .send_thread_start_request(ThreadStartParams::default()) diff --git a/codex-rs/core/tests/common/responses.rs b/codex-rs/core/tests/common/responses.rs index 171ec3675..2dcfd5203 100644 --- a/codex-rs/core/tests/common/responses.rs +++ b/codex-rs/core/tests/common/responses.rs @@ -541,7 +541,14 @@ impl WebSocketTestServer { pub async fn shutdown(self) { let _ = self.shutdown.send(()); - let _ = self.task.await; + let mut task = self.task; + if tokio::time::timeout(Duration::from_secs(10), &mut task) + .await + .is_err() + { + task.abort(); + let _ = task.await; + } } } diff --git a/codex-rs/rmcp-client/src/program_resolver.rs b/codex-rs/rmcp-client/src/program_resolver.rs index cb32ae311..3666b1871 100644 --- a/codex-rs/rmcp-client/src/program_resolver.rs +++ b/codex-rs/rmcp-client/src/program_resolver.rs @@ -75,11 +75,14 @@ mod tests { #[tokio::test] async fn test_unix_executes_script_without_extension() -> Result<()> { let env = TestExecutableEnv::new()?; - let mut cmd = Command::new(&env.executable_path); + let mut cmd = Command::new(&env.program_name); cmd.envs(&env.mcp_env); let output = cmd.output().await; - assert!(output.is_ok(), "Unix should execute scripts directly"); + assert!( + output.is_ok(), + "Unix should execute PATH-resolved scripts directly: {output:?}" + ); Ok(()) } @@ -143,8 +146,6 @@ mod tests { // Held to prevent the temporary directory from being deleted. _temp_dir: TempDir, program_name: String, - #[cfg(unix)] - executable_path: std::path::PathBuf, mcp_env: HashMap, } @@ -167,8 +168,6 @@ mod tests { let mcp_env = create_env_for_mcp_server(Some(extra_env), &[])?; Ok(Self { - #[cfg(unix)] - executable_path: Self::executable_path(dir_path), _temp_dir: temp_dir, program_name: Self::TEST_PROGRAM.to_string(), mcp_env, @@ -193,11 +192,6 @@ mod tests { Ok(()) } - #[cfg(unix)] - fn executable_path(dir: &Path) -> std::path::PathBuf { - dir.join(Self::TEST_PROGRAM) - } - #[cfg(unix)] fn set_executable(path: &Path) -> Result<()> { use std::os::unix::fs::PermissionsExt;