From 3099353c9304f1db8fb7e6bf535cc56dcffec7c6 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Sun, 26 Apr 2026 08:02:55 -0700 Subject: [PATCH] test: harden app-server integration tests --- codex-rs/app-server/src/message_processor.rs | 14 +++++---- codex-rs/app-server/tests/common/lib.rs | 1 + .../app-server/tests/common/mcp_process.rs | 5 +++- .../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 | 19 ++++++++++-- .../tests/suite/v2/realtime_conversation.rs | 30 +++++++++++-------- codex-rs/core/tests/common/responses.rs | 9 +++++- codex-rs/rmcp-client/src/program_resolver.rs | 16 ++++------ 10 files changed, 79 insertions(+), 37 deletions(-) diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index d3eee87ccd..8b8fb317f4 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -95,6 +95,8 @@ use tokio::time::timeout; use tracing::Instrument; const EXTERNAL_AUTH_REFRESH_TIMEOUT: Duration = Duration::from_secs(10); +const TEST_DISABLE_PLUGIN_STARTUP_TASKS_ENV_VAR: &str = + "CODEX_APP_SERVER_TEST_DISABLE_PLUGIN_STARTUP_TASKS"; #[derive(Clone)] struct ExternalAuthRefreshBridge { @@ -315,11 +317,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 std::env::var_os(TEST_DISABLE_PLUGIN_STARTUP_TASKS_ENV_VAR).is_none() { + // 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/tests/common/lib.rs b/codex-rs/app-server/tests/common/lib.rs index 6ac26d8a56..2010235d20 100644 --- a/codex-rs/app-server/tests/common/lib.rs +++ b/codex-rs/app-server/tests/common/lib.rs @@ -26,6 +26,7 @@ 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::McpProcess; +pub use mcp_process::TEST_DISABLE_PLUGIN_STARTUP_TASKS_ENV_VAR; pub use mock_model_server::create_mock_responses_server_repeating_assistant; pub use mock_model_server::create_mock_responses_server_sequence; pub use mock_model_server::create_mock_responses_server_sequence_unchecked; diff --git a/codex-rs/app-server/tests/common/mcp_process.rs b/codex-rs/app-server/tests/common/mcp_process.rs index befa248e80..9b7f5b5339 100644 --- a/codex-rs/app-server/tests/common/mcp_process.rs +++ b/codex-rs/app-server/tests/common/mcp_process.rs @@ -106,6 +106,8 @@ pub struct McpProcess { } pub const DEFAULT_CLIENT_NAME: &str = "codex-app-server-tests"; +pub const TEST_DISABLE_PLUGIN_STARTUP_TASKS_ENV_VAR: &str = + "CODEX_APP_SERVER_TEST_DISABLE_PLUGIN_STARTUP_TASKS"; const DISABLE_MANAGED_CONFIG_ENV_VAR: &str = "CODEX_APP_SERVER_DISABLE_MANAGED_CONFIG"; impl McpProcess { @@ -147,7 +149,8 @@ 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"); + cmd.env(TEST_DISABLE_PLUGIN_STARTUP_TASKS_ENV_VAR, "1"); // 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 456ae1577a..d5f82a66bb 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::TEST_DISABLE_PLUGIN_STARTUP_TASKS_ENV_VAR; use app_test_support::create_mock_responses_server_sequence_unchecked; use app_test_support::to_response; use base64::Engine; @@ -394,7 +395,8 @@ pub(super) async fn spawn_websocket_server_with_args( .stdout(Stdio::null()) .stderr(Stdio::piped()) .env("CODEX_HOME", codex_home) - .env("RUST_LOG", "debug"); + .env("RUST_LOG", "warn") + .env(TEST_DISABLE_PLUGIN_STARTUP_TASKS_ENV_VAR, "1"); let mut process = cmd .kill_on_drop(true) .spawn() @@ -529,7 +531,8 @@ async fn run_websocket_server_to_completion_with_args( .stdout(Stdio::null()) .stderr(Stdio::piped()) .env("CODEX_HOME", codex_home) - .env("RUST_LOG", "debug"); + .env("RUST_LOG", "warn") + .env(TEST_DISABLE_PLUGIN_STARTUP_TASKS_ENV_VAR, "1"); 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 049256b602..6b1715dc7a 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 642844eb92..a780a51e0b 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 f885f2cb7a..a10ce3e5af 100644 --- a/codex-rs/app-server/tests/suite/v2/plugin_list.rs +++ b/codex-rs/app-server/tests/suite/v2/plugin_list.rs @@ -4,6 +4,7 @@ use anyhow::Result; use anyhow::bail; use app_test_support::ChatGptAuthFixture; use app_test_support::McpProcess; +use app_test_support::TEST_DISABLE_PLUGIN_STARTUP_TASKS_ENV_VAR; use app_test_support::to_response; use app_test_support::write_chatgpt_auth; use codex_app_server_protocol::JSONRPCResponse; @@ -1066,7 +1067,11 @@ 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_env( + codex_home.path(), + &[(TEST_DISABLE_PLUGIN_STARTUP_TASKS_ENV_VAR, None)], + ) + .await?; timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; wait_for_path_exists(&marker_path).await?; @@ -1102,7 +1107,11 @@ 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_env( + codex_home.path(), + &[(TEST_DISABLE_PLUGIN_STARTUP_TASKS_ENV_VAR, None)], + ) + .await?; timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; } @@ -1490,7 +1499,11 @@ 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_env( + codex_home.path(), + &[(TEST_DISABLE_PLUGIN_STARTUP_TASKS_ENV_VAR, None)], + ) + .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 dfc3fea318..62ba19cc4f 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 171ec36756..2dcfd5203d 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 cb32ae3114..3666b1871b 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;