diff --git a/codex-rs/app-server/tests/common/Cargo.toml b/codex-rs/app-server/tests/common/Cargo.toml index d91ed4e675..de58509f0d 100644 --- a/codex-rs/app-server/tests/common/Cargo.toml +++ b/codex-rs/app-server/tests/common/Cargo.toml @@ -12,7 +12,7 @@ anyhow = { workspace = true } base64 = { workspace = true } chrono = { workspace = true } codex-app-server-protocol = { workspace = true } -codex-core = { workspace = true, features = ["test-support"] } +codex-core = { workspace = true } codex-protocol = { workspace = true } codex-utils-cargo-bin = { workspace = true } serde = { workspace = true } diff --git a/codex-rs/core/BUILD.bazel b/codex-rs/core/BUILD.bazel index 6e5ce680ae..2ef299c5c0 100644 --- a/codex-rs/core/BUILD.bazel +++ b/codex-rs/core/BUILD.bazel @@ -3,10 +3,6 @@ load("//:defs.bzl", "codex_rust_crate") codex_rust_crate( name = "core", crate_name = "codex_core", - # TODO(mbolin): Eliminate the use of features in the version of the - # rust_library() that is used by rust_binary() rules for release artifacts - # such as the Codex CLI. - crate_features = ["test-support"], compile_data = glob( include = ["**"], exclude = [ diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index 26e7f17cc8..e759072fac 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -113,10 +113,6 @@ which = { workspace = true } wildmatch = { workspace = true } zip = { workspace = true } -[features] -test-support = [] - - [target.'cfg(target_os = "linux")'.dependencies] keyring = { workspace = true, features = ["linux-native-async-persistent"] } landlock = { workspace = true } diff --git a/codex-rs/core/src/auth.rs b/codex-rs/core/src/auth.rs index a6282305ea..198af0bbce 100644 --- a/codex-rs/core/src/auth.rs +++ b/codex-rs/core/src/auth.rs @@ -981,8 +981,8 @@ impl AuthManager { } } - #[cfg(any(test, feature = "test-support"))] /// Create an AuthManager with a specific CodexAuth, for testing only. + #[doc(hidden)] pub fn from_auth_for_testing(auth: CodexAuth) -> Arc { let cached = CachedAuth { auth: Some(auth), @@ -998,8 +998,8 @@ impl AuthManager { }) } - #[cfg(any(test, feature = "test-support"))] /// Create an AuthManager with a specific CodexAuth and codex home, for testing only. + #[doc(hidden)] pub fn from_auth_for_testing_with_home(auth: CodexAuth, codex_home: PathBuf) -> Arc { let cached = CachedAuth { auth: Some(auth), diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 85095bc5f5..39ea4d34ee 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -143,9 +143,11 @@ pub use exec_policy::check_execpolicy_for_warnings; pub use exec_policy::load_exec_policy; pub use file_watcher::FileWatcherEvent; pub use safety::get_platform_sandbox; +#[doc(hidden)] +pub use thread_manager::set_thread_manager_test_mode_for_tests; pub use tools::spec::parse_tool_input_schema; pub use turn_metadata::build_turn_metadata_header; -#[cfg(any(test, feature = "test-support"))] +#[doc(hidden)] pub use unified_exec::set_deterministic_process_ids_for_tests; // Re-export the protocol types from the standalone `codex-protocol` crate so existing // `codex_core::protocol::...` references continue to work across the workspace. diff --git a/codex-rs/core/src/models_manager/collaboration_mode_presets.rs b/codex-rs/core/src/models_manager/collaboration_mode_presets.rs index cdc5134038..393762d8f5 100644 --- a/codex-rs/core/src/models_manager/collaboration_mode_presets.rs +++ b/codex-rs/core/src/models_manager/collaboration_mode_presets.rs @@ -13,7 +13,7 @@ pub(super) fn builtin_collaboration_mode_presets() -> Vec vec![plan_preset(), default_preset()] } -#[cfg(any(test, feature = "test-support"))] +#[doc(hidden)] pub fn test_builtin_collaboration_mode_presets() -> Vec { builtin_collaboration_mode_presets() } diff --git a/codex-rs/core/src/models_manager/manager.rs b/codex-rs/core/src/models_manager/manager.rs index d3bf497e63..b0e9ff2552 100644 --- a/codex-rs/core/src/models_manager/manager.rs +++ b/codex-rs/core/src/models_manager/manager.rs @@ -336,8 +336,8 @@ impl ModelsManager { } } - #[cfg(any(test, feature = "test-support"))] /// Construct a manager with a specific provider for testing. + #[doc(hidden)] pub fn with_provider( codex_home: PathBuf, auth_manager: Arc, @@ -355,8 +355,8 @@ impl ModelsManager { } } - #[cfg(any(test, feature = "test-support"))] /// Get model identifier without consulting remote state or cache. + #[doc(hidden)] pub fn get_model_offline(model: Option<&str>) -> String { if let Some(model) = model { return model.to_string(); @@ -370,8 +370,8 @@ impl ModelsManager { .unwrap_or_default() } - #[cfg(any(test, feature = "test-support"))] /// Build `ModelInfo` without consulting remote state or cache. + #[doc(hidden)] pub fn construct_model_info_offline(model: &str, config: &Config) -> ModelInfo { model_info::with_config_overrides(model_info::model_info_from_slug(model), config) } diff --git a/codex-rs/core/src/models_manager/mod.rs b/codex-rs/core/src/models_manager/mod.rs index 9527466252..3929ea367f 100644 --- a/codex-rs/core/src/models_manager/mod.rs +++ b/codex-rs/core/src/models_manager/mod.rs @@ -4,7 +4,6 @@ pub mod manager; pub mod model_info; pub mod model_presets; -#[cfg(any(test, feature = "test-support"))] pub use collaboration_mode_presets::test_builtin_collaboration_mode_presets; /// Convert the client version string to a whole version string (e.g. "1.2.3-alpha.4" -> "1.2.3"). diff --git a/codex-rs/core/src/models_manager/model_presets.rs b/codex-rs/core/src/models_manager/model_presets.rs index a597f7f922..b4c191603b 100644 --- a/codex-rs/core/src/models_manager/model_presets.rs +++ b/codex-rs/core/src/models_manager/model_presets.rs @@ -359,7 +359,7 @@ pub(super) fn builtin_model_presets(_auth_mode: Option) -> Vec &'static Vec { &PRESETS } diff --git a/codex-rs/core/src/thread_manager.rs b/codex-rs/core/src/thread_manager.rs index 13f9efafb0..de89275e82 100644 --- a/codex-rs/core/src/thread_manager.rs +++ b/codex-rs/core/src/thread_manager.rs @@ -1,7 +1,5 @@ use crate::AuthManager; -#[cfg(any(test, feature = "test-support"))] use crate::CodexAuth; -#[cfg(any(test, feature = "test-support"))] use crate::ModelProviderInfo; use crate::agent::AgentControl; use crate::codex::Codex; @@ -31,25 +29,48 @@ use codex_protocol::protocol::SessionSource; use std::collections::HashMap; use std::path::PathBuf; use std::sync::Arc; -#[cfg(any(test, feature = "test-support"))] -use tempfile::TempDir; +use std::sync::atomic::AtomicBool; +use std::sync::atomic::Ordering; use tokio::runtime::Handle; -#[cfg(any(test, feature = "test-support"))] use tokio::runtime::RuntimeFlavor; use tokio::sync::RwLock; use tokio::sync::broadcast; use tracing::warn; const THREAD_CREATED_CHANNEL_CAPACITY: usize = 1024; +/// Test-only override for enabling thread-manager behaviors used by integration +/// tests. +/// +/// In production builds this value should remain at its default (`false`) and +/// must not be toggled. +static FORCE_TEST_THREAD_MANAGER_BEHAVIOR: AtomicBool = AtomicBool::new(false); + +#[doc(hidden)] +pub fn set_thread_manager_test_mode_for_tests(enabled: bool) { + FORCE_TEST_THREAD_MANAGER_BEHAVIOR.store(enabled, Ordering::Relaxed); +} + +fn should_use_test_thread_manager_behavior() -> bool { + FORCE_TEST_THREAD_MANAGER_BEHAVIOR.load(Ordering::Relaxed) +} + +struct TempCodexHomeGuard { + path: PathBuf, +} + +impl Drop for TempCodexHomeGuard { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.path); + } +} fn build_file_watcher(codex_home: PathBuf, skills_manager: Arc) -> Arc { - #[cfg(any(test, feature = "test-support"))] - if let Ok(handle) = Handle::try_current() + if should_use_test_thread_manager_behavior() + && let Ok(handle) = Handle::try_current() && handle.runtime_flavor() == RuntimeFlavor::CurrentThread { // The real watcher spins background tasks that can starve the // current-thread test runtime and cause event waits to time out. - // Integration tests compile with the `test-support` feature. warn!("using noop file watcher under current-thread test runtime"); return Arc::new(FileWatcher::noop()); } @@ -95,8 +116,7 @@ pub struct NewThread { /// them in memory. pub struct ThreadManager { state: Arc, - #[cfg(any(test, feature = "test-support"))] - _test_codex_home_guard: Option, + _test_codex_home_guard: Option, } /// Shared, `Arc`-owned state for [`ThreadManager`]. This `Arc` is required to have a single @@ -110,10 +130,8 @@ pub(crate) struct ThreadManagerState { skills_manager: Arc, file_watcher: Arc, session_source: SessionSource, - #[cfg(any(test, feature = "test-support"))] - #[allow(dead_code)] - // Captures submitted ops for testing purpose. - ops_log: Arc>>, + // Captures submitted ops for testing purpose when test mode is enabled. + ops_log: Option>>>, } impl ThreadManager { @@ -134,33 +152,38 @@ impl ThreadManager { file_watcher, auth_manager, session_source, - #[cfg(any(test, feature = "test-support"))] - ops_log: Arc::new(std::sync::Mutex::new(Vec::new())), + ops_log: should_use_test_thread_manager_behavior() + .then(|| Arc::new(std::sync::Mutex::new(Vec::new()))), }), - #[cfg(any(test, feature = "test-support"))] _test_codex_home_guard: None, } } - #[cfg(any(test, feature = "test-support"))] /// Construct with a dummy AuthManager containing the provided CodexAuth. /// Used for integration tests: should not be used by ordinary business logic. + #[doc(hidden)] pub fn with_models_provider(auth: CodexAuth, provider: ModelProviderInfo) -> Self { - let temp_dir = tempfile::tempdir().unwrap_or_else(|err| panic!("temp codex home: {err}")); - let codex_home = temp_dir.path().to_path_buf(); - let mut manager = Self::with_models_provider_and_home(auth, provider, codex_home); - manager._test_codex_home_guard = Some(temp_dir); + set_thread_manager_test_mode_for_tests(true); + let codex_home = std::env::temp_dir().join(format!( + "codex-thread-manager-test-{}", + uuid::Uuid::new_v4() + )); + std::fs::create_dir_all(&codex_home) + .unwrap_or_else(|err| panic!("temp codex home dir create failed: {err}")); + let mut manager = Self::with_models_provider_and_home(auth, provider, codex_home.clone()); + manager._test_codex_home_guard = Some(TempCodexHomeGuard { path: codex_home }); manager } - #[cfg(any(test, feature = "test-support"))] /// Construct with a dummy AuthManager containing the provided CodexAuth and codex home. /// Used for integration tests: should not be used by ordinary business logic. + #[doc(hidden)] pub fn with_models_provider_and_home( auth: CodexAuth, provider: ModelProviderInfo, codex_home: PathBuf, ) -> Self { + set_thread_manager_test_mode_for_tests(true); let auth_manager = AuthManager::from_auth_for_testing(auth); let (thread_created_tx, _) = broadcast::channel(THREAD_CREATED_CHANNEL_CAPACITY); let skills_manager = Arc::new(SkillsManager::new(codex_home.clone())); @@ -178,8 +201,8 @@ impl ThreadManager { file_watcher, auth_manager, session_source: SessionSource::Exec, - #[cfg(any(test, feature = "test-support"))] - ops_log: Arc::new(std::sync::Mutex::new(Vec::new())), + ops_log: should_use_test_thread_manager_behavior() + .then(|| Arc::new(std::sync::Mutex::new(Vec::new()))), }), _test_codex_home_guard: None, } @@ -340,13 +363,13 @@ impl ThreadManager { AgentControl::new(Arc::downgrade(&self.state)) } - #[cfg(any(test, feature = "test-support"))] + #[doc(hidden)] #[allow(dead_code)] pub(crate) fn captured_ops(&self) -> Vec<(ThreadId, Op)> { self.state .ops_log - .lock() - .map(|log| log.clone()) + .as_ref() + .and_then(|ops_log| ops_log.lock().ok().map(|log| log.clone())) .unwrap_or_default() } } @@ -364,11 +387,10 @@ impl ThreadManagerState { /// Send an operation to a thread by ID. pub(crate) async fn send_op(&self, thread_id: ThreadId, op: Op) -> CodexResult { let thread = self.get_thread(thread_id).await?; - #[cfg(any(test, feature = "test-support"))] + if let Some(ops_log) = &self.ops_log + && let Ok(mut log) = ops_log.lock() { - if let Ok(mut log) = self.ops_log.lock() { - log.push((thread_id, op.clone())); - } + log.push((thread_id, op.clone())); } thread.submit(op).await } diff --git a/codex-rs/core/src/unified_exec/mod.rs b/codex-rs/core/src/unified_exec/mod.rs index bbc7790c63..3cd6813dd4 100644 --- a/codex-rs/core/src/unified_exec/mod.rs +++ b/codex-rs/core/src/unified_exec/mod.rs @@ -42,7 +42,6 @@ mod head_tail_buffer; mod process; mod process_manager; -#[cfg(any(test, feature = "test-support"))] pub fn set_deterministic_process_ids_for_tests(enabled: bool) { process_manager::set_deterministic_process_ids_for_tests(enabled); } diff --git a/codex-rs/core/src/unified_exec/process_manager.rs b/codex-rs/core/src/unified_exec/process_manager.rs index 6740850092..251a4910ba 100644 --- a/codex-rs/core/src/unified_exec/process_manager.rs +++ b/codex-rs/core/src/unified_exec/process_manager.rs @@ -62,28 +62,20 @@ const UNIFIED_EXEC_ENV: [(&str, &str); 10] = [ ("CODEX_CI", "1"), ]; -#[cfg(any(test, feature = "test-support"))] /// Test-only override for deterministic unified exec process IDs. /// /// In production builds this value should remain at its default (`false`) and /// must not be toggled. static FORCE_DETERMINISTIC_PROCESS_IDS: AtomicBool = AtomicBool::new(false); -#[cfg(any(test, feature = "test-support"))] pub(super) fn set_deterministic_process_ids_for_tests(enabled: bool) { FORCE_DETERMINISTIC_PROCESS_IDS.store(enabled, Ordering::Relaxed); } -#[cfg(any(test, feature = "test-support"))] fn deterministic_process_ids_forced_for_tests() -> bool { FORCE_DETERMINISTIC_PROCESS_IDS.load(Ordering::Relaxed) } -#[cfg(not(any(test, feature = "test-support")))] -fn deterministic_process_ids_forced_for_tests() -> bool { - false -} - fn should_use_deterministic_process_ids() -> bool { cfg!(test) || deterministic_process_ids_forced_for_tests() } diff --git a/codex-rs/core/tests/common/Cargo.toml b/codex-rs/core/tests/common/Cargo.toml index 1fb43bb768..a7d35c0de7 100644 --- a/codex-rs/core/tests/common/Cargo.toml +++ b/codex-rs/core/tests/common/Cargo.toml @@ -11,7 +11,7 @@ path = "lib.rs" anyhow = { workspace = true } assert_cmd = { workspace = true } base64 = { workspace = true } -codex-core = { workspace = true, features = ["test-support"] } +codex-core = { workspace = true } codex-protocol = { workspace = true } codex-utils-absolute-path = { workspace = true } codex-utils-cargo-bin = { workspace = true } diff --git a/codex-rs/core/tests/common/lib.rs b/codex-rs/core/tests/common/lib.rs index 0144eeec5a..e81067d6c3 100644 --- a/codex-rs/core/tests/common/lib.rs +++ b/codex-rs/core/tests/common/lib.rs @@ -20,6 +20,7 @@ pub mod test_codex_exec; #[ctor] fn enable_deterministic_unified_exec_process_ids_for_tests() { + codex_core::set_thread_manager_test_mode_for_tests(true); codex_core::set_deterministic_process_ids_for_tests(true); } diff --git a/codex-rs/tui/Cargo.toml b/codex-rs/tui/Cargo.toml index 1dced39b76..5e313f19b1 100644 --- a/codex-rs/tui/Cargo.toml +++ b/codex-rs/tui/Cargo.toml @@ -120,7 +120,7 @@ arboard = { workspace = true } [dev-dependencies] codex-cli = { workspace = true } -codex-core = { workspace = true, features = ["test-support"] } +codex-core = { workspace = true } codex-utils-cargo-bin = { workspace = true } codex-utils-pty = { workspace = true } assert_matches = { workspace = true }