diff --git a/codex-rs/app-server-client/src/lib.rs b/codex-rs/app-server-client/src/lib.rs index 1e7a84efe7ff..8165fb367344 100644 --- a/codex-rs/app-server-client/src/lib.rs +++ b/codex-rs/app-server-client/src/lib.rs @@ -2029,14 +2029,17 @@ mod tests { #[tokio::test] async fn runtime_start_args_forward_environment_manager() { let config = Arc::new(build_test_config().await); - let environment_manager = Arc::new(EnvironmentManager::new(EnvironmentManagerArgs { - exec_server_url: Some("ws://127.0.0.1:8765".to_string()), - local_runtime_paths: ExecServerRuntimePaths::new( - std::env::current_exe().expect("current exe"), - /*codex_linux_sandbox_exe*/ None, + let environment_manager = Arc::new( + EnvironmentManager::create_for_tests( + Some("ws://127.0.0.1:8765".to_string()), + ExecServerRuntimePaths::new( + std::env::current_exe().expect("current exe"), + /*codex_linux_sandbox_exe*/ None, + ) + .expect("runtime paths"), ) - .expect("runtime paths"), - })); + .await, + ); let runtime_args = InProcessClientStartArgs { arg0_paths: Arg0DispatchPaths::default(), diff --git a/codex-rs/app-server/src/lib.rs b/codex-rs/app-server/src/lib.rs index de8960810122..44fe776dd61a 100644 --- a/codex-rs/app-server/src/lib.rs +++ b/codex-rs/app-server/src/lib.rs @@ -414,12 +414,15 @@ pub async fn run_main_with_transport_options( auth: AppServerWebsocketAuthSettings, runtime_options: AppServerRuntimeOptions, ) -> IoResult<()> { - let environment_manager = Arc::new(EnvironmentManager::new(EnvironmentManagerArgs::from_env( - ExecServerRuntimePaths::from_optional_paths( - arg0_paths.codex_self_exe.clone(), - arg0_paths.codex_linux_sandbox_exe.clone(), - )?, - ))); + let environment_manager = Arc::new( + EnvironmentManager::new(EnvironmentManagerArgs::new( + ExecServerRuntimePaths::from_optional_paths( + arg0_paths.codex_self_exe.clone(), + arg0_paths.codex_linux_sandbox_exe.clone(), + )?, + )) + .await, + ); let (transport_event_tx, mut transport_event_rx) = mpsc::channel::(CHANNEL_CAPACITY); let (outgoing_tx, mut outgoing_rx) = mpsc::channel::(CHANNEL_CAPACITY); diff --git a/codex-rs/core/src/connectors.rs b/codex-rs/core/src/connectors.rs index d06ea24609f7..7d325abb43b2 100644 --- a/codex-rs/core/src/connectors.rs +++ b/codex-rs/core/src/connectors.rs @@ -201,7 +201,7 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_options_and_status( config.codex_linux_sandbox_exe.clone(), )?; let environment_manager = - EnvironmentManager::new(EnvironmentManagerArgs::from_env(local_runtime_paths)); + EnvironmentManager::new(EnvironmentManagerArgs::new(local_runtime_paths)).await; list_accessible_connectors_from_mcp_tools_with_environment_manager( config, force_refetch, diff --git a/codex-rs/core/src/environment_selection.rs b/codex-rs/core/src/environment_selection.rs index 7f93ba384c5c..a33aae92b094 100644 --- a/codex-rs/core/src/environment_selection.rs +++ b/codex-rs/core/src/environment_selection.rs @@ -61,7 +61,6 @@ pub(crate) fn selected_primary_environment( #[cfg(test)] mod tests { - use codex_exec_server::EnvironmentManagerArgs; use codex_exec_server::ExecServerRuntimePaths; use codex_exec_server::REMOTE_ENVIRONMENT_ID; use codex_protocol::protocol::TurnEnvironmentSelection; @@ -81,10 +80,11 @@ mod tests { #[tokio::test] async fn default_thread_environment_selections_use_manager_default_id() { let cwd = AbsolutePathBuf::current_dir().expect("cwd"); - let manager = EnvironmentManager::new(EnvironmentManagerArgs { - exec_server_url: Some("ws://127.0.0.1:8765".to_string()), - local_runtime_paths: test_runtime_paths(), - }); + let manager = EnvironmentManager::create_for_tests( + Some("ws://127.0.0.1:8765".to_string()), + test_runtime_paths(), + ) + .await; assert_eq!( default_thread_environment_selections(&manager, &cwd), @@ -98,10 +98,7 @@ mod tests { #[tokio::test] async fn default_thread_environment_selections_empty_when_default_disabled() { let cwd = AbsolutePathBuf::current_dir().expect("cwd"); - let manager = EnvironmentManager::new(EnvironmentManagerArgs { - exec_server_url: Some("none".to_string()), - local_runtime_paths: test_runtime_paths(), - }); + let manager = EnvironmentManager::disabled_for_tests(test_runtime_paths()); assert_eq!( default_thread_environment_selections(&manager, &cwd), diff --git a/codex-rs/core/src/prompt_debug.rs b/codex-rs/core/src/prompt_debug.rs index f782c0d232e0..cc1203c2db38 100644 --- a/codex-rs/core/src/prompt_debug.rs +++ b/codex-rs/core/src/prompt_debug.rs @@ -45,9 +45,7 @@ pub async fn build_prompt_input( .features .enabled(Feature::DefaultModeRequestUserInput), }, - Arc::new(EnvironmentManager::new(EnvironmentManagerArgs::from_env( - local_runtime_paths, - ))), + Arc::new(EnvironmentManager::new(EnvironmentManagerArgs::new(local_runtime_paths)).await), /*analytics_events_client*/ None, ); let thread = thread_manager.start_thread(config).await?; diff --git a/codex-rs/core/src/thread_manager_tests.rs b/codex-rs/core/src/thread_manager_tests.rs index 9307927b036a..54d7ea4f7211 100644 --- a/codex-rs/core/src/thread_manager_tests.rs +++ b/codex-rs/core/src/thread_manager_tests.rs @@ -47,20 +47,6 @@ fn assistant_msg(text: &str) -> ResponseItem { } } -fn disabled_environment_manager_for_tests() -> Arc { - let runtime_paths = codex_exec_server::ExecServerRuntimePaths::new( - std::env::current_exe().expect("current exe path"), - /*codex_linux_sandbox_exe*/ None, - ) - .expect("runtime paths"); - Arc::new(codex_exec_server::EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs { - exec_server_url: Some("none".to_string()), - local_runtime_paths: runtime_paths, - }, - )) -} - fn contextual_user_interrupted_marker() -> ResponseItem { interrupted_turn_history_marker(InterruptedTurnHistoryMarker::ContextualUser) .expect("contextual-user interrupted marker should be enabled") @@ -307,11 +293,23 @@ async fn start_thread_accepts_explicit_environment_when_default_environment_is_d config.cwd = config.codex_home.abs(); std::fs::create_dir_all(&config.codex_home).expect("create codex home"); + let runtime_paths = codex_exec_server::ExecServerRuntimePaths::new( + std::env::current_exe().expect("current exe path"), + /*codex_linux_sandbox_exe*/ None, + ) + .expect("runtime paths"); + let environment_manager = Arc::new( + codex_exec_server::EnvironmentManager::create_for_tests( + Some("none".to_string()), + runtime_paths, + ) + .await, + ); let manager = ThreadManager::with_models_provider_and_home_for_tests( CodexAuth::from_api_key("dummy"), config.model_provider.clone(), config.codex_home.to_path_buf(), - disabled_environment_manager_for_tests(), + environment_manager, ); let thread = manager diff --git a/codex-rs/core/tests/common/test_codex.rs b/codex-rs/core/tests/common/test_codex.rs index d77d55956a63..0fcab92f45c0 100644 --- a/codex-rs/core/tests/common/test_codex.rs +++ b/codex-rs/core/tests/common/test_codex.rs @@ -371,15 +371,17 @@ impl TestCodexBuilder { .exec_server_url .clone() .or_else(|| test_env.exec_server_url().map(str::to_owned)); - let environment_manager = Arc::new(codex_exec_server::EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs { + let local_runtime_paths = codex_exec_server::ExecServerRuntimePaths::new( + std::env::current_exe()?, + /*codex_linux_sandbox_exe*/ None, + )?; + let environment_manager = Arc::new( + codex_exec_server::EnvironmentManager::create_for_tests( exec_server_url, - local_runtime_paths: codex_exec_server::ExecServerRuntimePaths::new( - std::env::current_exe()?, - /*codex_linux_sandbox_exe*/ None, - )?, - }, - )); + local_runtime_paths, + ) + .await, + ); let file_system = test_env.environment().get_filesystem(); let mut workspace_setups = vec![]; swap(&mut self.workspace_setups, &mut workspace_setups); diff --git a/codex-rs/core/tests/suite/skills.rs b/codex-rs/core/tests/suite/skills.rs index 1edaefb783f9..8c02e2eed7a4 100644 --- a/codex-rs/core/tests/suite/skills.rs +++ b/codex-rs/core/tests/suite/skills.rs @@ -237,14 +237,11 @@ async fn list_skills_skips_cwd_roots_when_environment_disabled() -> Result<()> { codex_core::test_support::auth_manager_from_auth(CodexAuth::from_api_key("dummy")), SessionSource::Exec, CollaborationModesConfig::default(), - Arc::new(EnvironmentManager::new( - codex_exec_server::EnvironmentManagerArgs { - exec_server_url: Some("none".to_string()), - local_runtime_paths: ExecServerRuntimePaths::new( - std::env::current_exe()?, - /*codex_linux_sandbox_exe*/ None, - )?, - }, + Arc::new(EnvironmentManager::disabled_for_tests( + ExecServerRuntimePaths::new( + std::env::current_exe()?, + /*codex_linux_sandbox_exe*/ None, + )?, )), /*analytics_events_client*/ None, ); diff --git a/codex-rs/exec-server/src/environment.rs b/codex-rs/exec-server/src/environment.rs index b934ec8d933e..855989dafbc2 100644 --- a/codex-rs/exec-server/src/environment.rs +++ b/codex-rs/exec-server/src/environment.rs @@ -7,6 +7,9 @@ use crate::ExecutorFileSystem; use crate::HttpClient; use crate::client::LazyRemoteExecServerClient; use crate::client::http_client::ReqwestHttpClient; +use crate::environment_provider::DefaultEnvironmentProvider; +use crate::environment_provider::EnvironmentProvider; +use crate::environment_provider::normalize_exec_server_url; use crate::local_file_system::LocalFileSystem; use crate::local_process::LocalProcess; use crate::process::ExecBackend; @@ -17,15 +20,13 @@ pub const CODEX_EXEC_SERVER_URL_ENV_VAR: &str = "CODEX_EXEC_SERVER_URL"; /// Owns the execution/filesystem environments available to the Codex runtime. /// -/// `EnvironmentManager` is a shared registry for concrete environments. It -/// always creates a local environment under [`LOCAL_ENVIRONMENT_ID`]. When -/// `CODEX_EXEC_SERVER_URL` is set to a websocket URL, it also creates a remote -/// environment under [`REMOTE_ENVIRONMENT_ID`] and makes that the default -/// environment. Otherwise the local environment is the default. +/// `EnvironmentManager` is a shared registry for concrete environments. Its +/// default constructor preserves the legacy `CODEX_EXEC_SERVER_URL` behavior +/// while provider-based construction accepts a provider-supplied snapshot. /// /// Setting `CODEX_EXEC_SERVER_URL=none` disables environment access by leaving -/// the default environment unset while still keeping the local environment -/// available for internal callers by id. Callers use +/// the default environment unset while still keeping an explicit local +/// environment available through `local_environment()`. Callers use /// `default_environment().is_some()` as the signal for model-facing /// shell/filesystem tool availability. /// @@ -36,6 +37,7 @@ pub const CODEX_EXEC_SERVER_URL_ENV_VAR: &str = "CODEX_EXEC_SERVER_URL"; pub struct EnvironmentManager { default_environment: Option, environments: HashMap>, + local_environment: Arc, } pub const LOCAL_ENVIRONMENT_ID: &str = "local"; @@ -43,21 +45,12 @@ pub const REMOTE_ENVIRONMENT_ID: &str = "remote"; #[derive(Clone, Debug)] pub struct EnvironmentManagerArgs { - pub exec_server_url: Option, pub local_runtime_paths: ExecServerRuntimePaths, } impl EnvironmentManagerArgs { pub fn new(local_runtime_paths: ExecServerRuntimePaths) -> Self { Self { - exec_server_url: None, - local_runtime_paths, - } - } - - pub fn from_env(local_runtime_paths: ExecServerRuntimePaths) -> Self { - Self { - exec_server_url: std::env::var(CODEX_EXEC_SERVER_URL_ENV_VAR).ok(), local_runtime_paths, } } @@ -72,39 +65,103 @@ impl EnvironmentManager { LOCAL_ENVIRONMENT_ID.to_string(), Arc::new(Environment::default_for_tests()), )]), + local_environment: Arc::new(Environment::default_for_tests()), } } - /// Builds a manager from the raw `CODEX_EXEC_SERVER_URL` value and local - /// runtime paths used when creating local filesystem helpers. - pub fn new(args: EnvironmentManagerArgs) -> Self { + /// Builds a test-only manager with environment access disabled. + pub fn disabled_for_tests(local_runtime_paths: ExecServerRuntimePaths) -> Self { + let mut manager = Self::from_environments(HashMap::new(), local_runtime_paths); + manager.default_environment = None; + manager + } + + /// Builds a test-only manager from a raw exec-server URL value. + pub async fn create_for_tests( + exec_server_url: Option, + local_runtime_paths: ExecServerRuntimePaths, + ) -> Self { + Self::from_default_provider_url(exec_server_url, local_runtime_paths).await + } + + /// Builds a manager from `CODEX_EXEC_SERVER_URL` and local runtime paths + /// used when creating local filesystem helpers. + pub async fn new(args: EnvironmentManagerArgs) -> Self { let EnvironmentManagerArgs { - exec_server_url, local_runtime_paths, } = args; - let (exec_server_url, environment_disabled) = normalize_exec_server_url(exec_server_url); - let mut environments = HashMap::from([( - LOCAL_ENVIRONMENT_ID.to_string(), - Arc::new(Environment::local(local_runtime_paths.clone())), - )]); - let default_environment = if environment_disabled { - None - } else { - match exec_server_url { - Some(exec_server_url) => { - environments.insert( - REMOTE_ENVIRONMENT_ID.to_string(), - Arc::new(Environment::remote(exec_server_url, local_runtime_paths)), - ); - Some(REMOTE_ENVIRONMENT_ID.to_string()) - } - None => Some(LOCAL_ENVIRONMENT_ID.to_string()), + let exec_server_url = std::env::var(CODEX_EXEC_SERVER_URL_ENV_VAR).ok(); + Self::from_default_provider_url(exec_server_url, local_runtime_paths).await + } + + async fn from_default_provider_url( + exec_server_url: Option, + local_runtime_paths: ExecServerRuntimePaths, + ) -> Self { + let environment_disabled = normalize_exec_server_url(exec_server_url.clone()).1; + let provider = DefaultEnvironmentProvider::new(exec_server_url); + let provider_environments = provider.environments(&local_runtime_paths); + let mut manager = Self::from_environments(provider_environments, local_runtime_paths); + if environment_disabled { + // TODO: Remove this legacy `CODEX_EXEC_SERVER_URL=none` crutch once + // environment attachment defaulting moves out of EnvironmentManager. + manager.default_environment = None; + } + manager + } + + /// Builds a manager from a provider-supplied startup snapshot. + pub async fn from_provider

( + provider: &P, + local_runtime_paths: ExecServerRuntimePaths, + ) -> Result + where + P: EnvironmentProvider + ?Sized, + { + Self::from_provider_environments( + provider.get_environments(&local_runtime_paths).await?, + local_runtime_paths, + ) + } + + fn from_provider_environments( + environments: HashMap, + local_runtime_paths: ExecServerRuntimePaths, + ) -> Result { + for id in environments.keys() { + if id.is_empty() { + return Err(ExecServerError::Protocol( + "environment id cannot be empty".to_string(), + )); } + } + + Ok(Self::from_environments(environments, local_runtime_paths)) + } + + fn from_environments( + environments: HashMap, + local_runtime_paths: ExecServerRuntimePaths, + ) -> Self { + // TODO: Stop deriving a default environment here once omitted + // environment attachment is owned by thread/session setup. + let default_environment = if environments.contains_key(REMOTE_ENVIRONMENT_ID) { + Some(REMOTE_ENVIRONMENT_ID.to_string()) + } else if environments.contains_key(LOCAL_ENVIRONMENT_ID) { + Some(LOCAL_ENVIRONMENT_ID.to_string()) + } else { + None }; + let local_environment = Arc::new(Environment::local(local_runtime_paths)); + let environments = environments + .into_iter() + .map(|(id, environment)| (id, Arc::new(environment))) + .collect(); Self { default_environment, environments, + local_environment, } } @@ -122,10 +179,7 @@ impl EnvironmentManager { /// Returns the local environment instance used for internal runtime work. pub fn local_environment(&self) -> Arc { - match self.get_environment(LOCAL_ENVIRONMENT_ID) { - Some(environment) => environment, - None => unreachable!("EnvironmentManager always has a local environment"), - } + Arc::clone(&self.local_environment) } /// Returns a named environment instance. @@ -204,7 +258,7 @@ impl Environment { }) } - fn local(local_runtime_paths: ExecServerRuntimePaths) -> Self { + pub(crate) fn local(local_runtime_paths: ExecServerRuntimePaths) -> Self { Self { exec_server_url: None, exec_backend: Arc::new(LocalProcess::default()), @@ -216,11 +270,7 @@ impl Environment { } } - fn remote(exec_server_url: String, local_runtime_paths: ExecServerRuntimePaths) -> Self { - Self::remote_inner(exec_server_url, Some(local_runtime_paths)) - } - - fn remote_inner( + pub(crate) fn remote_inner( exec_server_url: String, local_runtime_paths: Option, ) -> Self { @@ -264,20 +314,13 @@ impl Environment { } } -fn normalize_exec_server_url(exec_server_url: Option) -> (Option, bool) { - match exec_server_url.as_deref().map(str::trim) { - None | Some("") => (None, false), - Some(url) if url.eq_ignore_ascii_case("none") => (None, true), - Some(url) => (Some(url.to_string()), false), - } -} #[cfg(test)] mod tests { + use std::collections::HashMap; use std::sync::Arc; use super::Environment; use super::EnvironmentManager; - use super::EnvironmentManagerArgs; use super::LOCAL_ENVIRONMENT_ID; use super::REMOTE_ENVIRONMENT_ID; use crate::ExecServerRuntimePaths; @@ -303,10 +346,8 @@ mod tests { #[tokio::test] async fn environment_manager_normalizes_empty_url() { - let manager = EnvironmentManager::new(EnvironmentManagerArgs { - exec_server_url: Some(String::new()), - local_runtime_paths: test_runtime_paths(), - }); + let manager = + EnvironmentManager::create_for_tests(Some(String::new()), test_runtime_paths()).await; let environment = manager.default_environment().expect("default environment"); assert_eq!(manager.default_environment_id(), Some(LOCAL_ENVIRONMENT_ID)); @@ -321,29 +362,23 @@ mod tests { } #[tokio::test] - async fn environment_manager_treats_none_value_as_disabled() { - let manager = EnvironmentManager::new(EnvironmentManagerArgs { - exec_server_url: Some("none".to_string()), - local_runtime_paths: test_runtime_paths(), - }); + async fn disabled_environment_manager_has_no_default_but_keeps_explicit_local_environment() { + let manager = EnvironmentManager::disabled_for_tests(test_runtime_paths()); assert!(manager.default_environment().is_none()); assert_eq!(manager.default_environment_id(), None); - assert!( - !manager - .get_environment(LOCAL_ENVIRONMENT_ID) - .expect("local environment") - .is_remote() - ); + assert!(!manager.local_environment().is_remote()); + assert!(manager.get_environment(LOCAL_ENVIRONMENT_ID).is_none()); assert!(manager.get_environment(REMOTE_ENVIRONMENT_ID).is_none()); } #[tokio::test] async fn environment_manager_reports_remote_url() { - let manager = EnvironmentManager::new(EnvironmentManagerArgs { - exec_server_url: Some("ws://127.0.0.1:8765".to_string()), - local_runtime_paths: test_runtime_paths(), - }); + let manager = EnvironmentManager::create_for_tests( + Some("ws://127.0.0.1:8765".to_string()), + test_runtime_paths(), + ) + .await; let environment = manager.default_environment().expect("default environment"); assert_eq!( @@ -364,6 +399,7 @@ mod tests { .expect("local environment") .is_remote() ); + assert!(!manager.local_environment().is_remote()); } #[tokio::test] @@ -380,45 +416,99 @@ mod tests { )); } + #[tokio::test] + async fn environment_manager_builds_from_provider_environments() { + let manager = EnvironmentManager::from_environments( + HashMap::from([( + REMOTE_ENVIRONMENT_ID.to_string(), + Environment::create_for_tests(Some("ws://127.0.0.1:8765".to_string())) + .expect("remote environment"), + )]), + test_runtime_paths(), + ); + + assert_eq!( + manager.default_environment_id(), + Some(REMOTE_ENVIRONMENT_ID) + ); + assert!( + manager + .get_environment(REMOTE_ENVIRONMENT_ID) + .expect("remote environment") + .is_remote() + ); + assert!(manager.get_environment(LOCAL_ENVIRONMENT_ID).is_none()); + assert!(!manager.local_environment().is_remote()); + } + + #[tokio::test] + async fn environment_manager_rejects_empty_environment_id() { + let err = EnvironmentManager::from_provider_environments( + HashMap::from([("".to_string(), Environment::default_for_tests())]), + test_runtime_paths(), + ) + .expect_err("empty id should fail"); + + assert_eq!( + err.to_string(), + "exec-server protocol error: environment id cannot be empty" + ); + } + + #[tokio::test] + async fn environment_manager_uses_provider_supplied_local_environment() { + let manager = EnvironmentManager::create_for_tests( + /*exec_server_url*/ None, + test_runtime_paths(), + ) + .await; + + assert_eq!(manager.default_environment_id(), Some(LOCAL_ENVIRONMENT_ID)); + let provider_local = manager + .get_environment(LOCAL_ENVIRONMENT_ID) + .expect("provider local environment"); + assert!(!provider_local.is_remote()); + assert!(!manager.local_environment().is_remote()); + assert!(!Arc::ptr_eq(&provider_local, &manager.local_environment())); + } + #[tokio::test] async fn environment_manager_carries_local_runtime_paths() { let runtime_paths = test_runtime_paths(); - let manager = EnvironmentManager::new(EnvironmentManagerArgs { - exec_server_url: None, - local_runtime_paths: runtime_paths.clone(), - }); + let manager = EnvironmentManager::create_for_tests( + /*exec_server_url*/ None, + runtime_paths.clone(), + ) + .await; let environment = manager.default_environment().expect("default environment"); assert_eq!(environment.local_runtime_paths(), Some(&runtime_paths)); - let manager = EnvironmentManager::new(EnvironmentManagerArgs { - exec_server_url: environment.exec_server_url().map(str::to_owned), - local_runtime_paths: environment + let manager = EnvironmentManager::create_for_tests( + environment.exec_server_url().map(str::to_owned), + environment .local_runtime_paths() .expect("local runtime paths") .clone(), - }); + ) + .await; let environment = manager.default_environment().expect("default environment"); assert_eq!(environment.local_runtime_paths(), Some(&runtime_paths)); } #[tokio::test] async fn disabled_environment_manager_has_no_default_environment() { - let manager = EnvironmentManager::new(EnvironmentManagerArgs { - exec_server_url: Some("none".to_string()), - local_runtime_paths: test_runtime_paths(), - }); + let manager = EnvironmentManager::disabled_for_tests(test_runtime_paths()); assert!(manager.default_environment().is_none()); assert_eq!(manager.default_environment_id(), None); } #[tokio::test] - async fn environment_manager_keeps_local_lookup_when_default_disabled() { - let manager = EnvironmentManager::new(EnvironmentManagerArgs { - exec_server_url: Some("none".to_string()), - local_runtime_paths: test_runtime_paths(), - }); + async fn environment_manager_keeps_default_provider_local_lookup_when_default_disabled() { + let manager = + EnvironmentManager::create_for_tests(Some("none".to_string()), test_runtime_paths()) + .await; assert!(manager.default_environment().is_none()); assert_eq!(manager.default_environment_id(), None); diff --git a/codex-rs/exec-server/src/environment_provider.rs b/codex-rs/exec-server/src/environment_provider.rs new file mode 100644 index 000000000000..7c8db07e85e5 --- /dev/null +++ b/codex-rs/exec-server/src/environment_provider.rs @@ -0,0 +1,172 @@ +use std::collections::HashMap; + +use async_trait::async_trait; + +use crate::Environment; +use crate::ExecServerError; +use crate::ExecServerRuntimePaths; +use crate::environment::CODEX_EXEC_SERVER_URL_ENV_VAR; +use crate::environment::LOCAL_ENVIRONMENT_ID; +use crate::environment::REMOTE_ENVIRONMENT_ID; + +/// Lists the concrete environments available to Codex. +/// +/// Implementations should return the provider-owned startup snapshot that +/// `EnvironmentManager` will cache. Providers that want the local environment to +/// be addressable by id should include it explicitly in the returned map. +#[async_trait] +pub trait EnvironmentProvider: Send + Sync { + /// Returns the environments available for a new manager. + async fn get_environments( + &self, + local_runtime_paths: &ExecServerRuntimePaths, + ) -> Result, ExecServerError>; +} + +/// Default provider backed by `CODEX_EXEC_SERVER_URL`. +#[derive(Clone, Debug)] +pub struct DefaultEnvironmentProvider { + exec_server_url: Option, +} + +impl DefaultEnvironmentProvider { + /// Builds a provider from an already-read raw `CODEX_EXEC_SERVER_URL` value. + pub fn new(exec_server_url: Option) -> Self { + Self { exec_server_url } + } + + /// Builds a provider by reading `CODEX_EXEC_SERVER_URL`. + pub fn from_env() -> Self { + Self::new(std::env::var(CODEX_EXEC_SERVER_URL_ENV_VAR).ok()) + } + + pub(crate) fn environments( + &self, + local_runtime_paths: &ExecServerRuntimePaths, + ) -> HashMap { + let mut environments = HashMap::from([( + LOCAL_ENVIRONMENT_ID.to_string(), + Environment::local(local_runtime_paths.clone()), + )]); + let exec_server_url = normalize_exec_server_url(self.exec_server_url.clone()).0; + + if let Some(exec_server_url) = exec_server_url { + environments.insert( + REMOTE_ENVIRONMENT_ID.to_string(), + Environment::remote_inner(exec_server_url, Some(local_runtime_paths.clone())), + ); + } + + environments + } +} + +#[async_trait] +impl EnvironmentProvider for DefaultEnvironmentProvider { + async fn get_environments( + &self, + local_runtime_paths: &ExecServerRuntimePaths, + ) -> Result, ExecServerError> { + Ok(self.environments(local_runtime_paths)) + } +} + +pub(crate) fn normalize_exec_server_url(exec_server_url: Option) -> (Option, bool) { + match exec_server_url.as_deref().map(str::trim) { + None | Some("") => (None, false), + Some(url) if url.eq_ignore_ascii_case("none") => (None, true), + Some(url) => (Some(url.to_string()), false), + } +} + +#[cfg(test)] +mod tests { + use pretty_assertions::assert_eq; + + use super::*; + use crate::ExecServerRuntimePaths; + + fn test_runtime_paths() -> ExecServerRuntimePaths { + ExecServerRuntimePaths::new( + std::env::current_exe().expect("current exe"), + /*codex_linux_sandbox_exe*/ None, + ) + .expect("runtime paths") + } + + #[tokio::test] + async fn default_provider_returns_local_environment_when_url_is_missing() { + let provider = DefaultEnvironmentProvider::new(/*exec_server_url*/ None); + let runtime_paths = test_runtime_paths(); + let environments = provider + .get_environments(&runtime_paths) + .await + .expect("environments"); + + assert!(!environments[LOCAL_ENVIRONMENT_ID].is_remote()); + assert_eq!( + environments[LOCAL_ENVIRONMENT_ID].local_runtime_paths(), + Some(&runtime_paths) + ); + assert!(!environments.contains_key(REMOTE_ENVIRONMENT_ID)); + } + + #[tokio::test] + async fn default_provider_returns_local_environment_when_url_is_empty() { + let provider = DefaultEnvironmentProvider::new(Some(String::new())); + let runtime_paths = test_runtime_paths(); + let environments = provider + .get_environments(&runtime_paths) + .await + .expect("environments"); + + assert!(!environments[LOCAL_ENVIRONMENT_ID].is_remote()); + assert!(!environments.contains_key(REMOTE_ENVIRONMENT_ID)); + } + + #[tokio::test] + async fn default_provider_returns_local_environment_for_none_value() { + let provider = DefaultEnvironmentProvider::new(Some("none".to_string())); + let runtime_paths = test_runtime_paths(); + let environments = provider + .get_environments(&runtime_paths) + .await + .expect("environments"); + + assert!(!environments[LOCAL_ENVIRONMENT_ID].is_remote()); + assert!(!environments.contains_key(REMOTE_ENVIRONMENT_ID)); + } + + #[tokio::test] + async fn default_provider_adds_remote_environment_for_websocket_url() { + let provider = DefaultEnvironmentProvider::new(Some("ws://127.0.0.1:8765".to_string())); + let runtime_paths = test_runtime_paths(); + let environments = provider + .get_environments(&runtime_paths) + .await + .expect("environments"); + + assert!(!environments[LOCAL_ENVIRONMENT_ID].is_remote()); + let remote_environment = &environments[REMOTE_ENVIRONMENT_ID]; + assert!(remote_environment.is_remote()); + assert_eq!( + remote_environment.exec_server_url(), + Some("ws://127.0.0.1:8765") + ); + } + + #[tokio::test] + async fn default_provider_normalizes_exec_server_url() { + let provider = DefaultEnvironmentProvider::new(Some(" ws://127.0.0.1:8765 ".to_string())); + let runtime_paths = test_runtime_paths(); + let environments = provider + .get_environments(&runtime_paths) + .await + .expect("environments"); + + assert_eq!( + environments[REMOTE_ENVIRONMENT_ID].exec_server_url(), + Some("ws://127.0.0.1:8765") + ); + } +} diff --git a/codex-rs/exec-server/src/lib.rs b/codex-rs/exec-server/src/lib.rs index e739b056443a..1550653d94b8 100644 --- a/codex-rs/exec-server/src/lib.rs +++ b/codex-rs/exec-server/src/lib.rs @@ -2,6 +2,7 @@ mod client; mod client_api; mod connection; mod environment; +mod environment_provider; mod fs_helper; mod fs_helper_main; mod fs_sandbox; @@ -38,6 +39,8 @@ pub use environment::EnvironmentManager; pub use environment::EnvironmentManagerArgs; pub use environment::LOCAL_ENVIRONMENT_ID; pub use environment::REMOTE_ENVIRONMENT_ID; +pub use environment_provider::DefaultEnvironmentProvider; +pub use environment_provider::EnvironmentProvider; pub use fs_helper::CODEX_FS_HELPER_ARG1; pub use fs_helper_main::main as run_fs_helper_main; pub use local_file_system::LOCAL_FS; diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index d4044a728d86..e6fe5abe0cd3 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -502,9 +502,9 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result cloud_requirements: run_cloud_requirements, feedback: CodexFeedback::new(), log_db: None, - environment_manager: std::sync::Arc::new(EnvironmentManager::new( - EnvironmentManagerArgs::from_env(local_runtime_paths), - )), + environment_manager: std::sync::Arc::new( + EnvironmentManager::new(EnvironmentManagerArgs::new(local_runtime_paths)).await, + ), config_warnings, session_source: SessionSource::Exec, enable_codex_api_key_env: true, diff --git a/codex-rs/mcp-server/src/lib.rs b/codex-rs/mcp-server/src/lib.rs index 95fc4341ee78..bb54ffcc53f8 100644 --- a/codex-rs/mcp-server/src/lib.rs +++ b/codex-rs/mcp-server/src/lib.rs @@ -60,12 +60,15 @@ pub async fn run_main( arg0_paths: Arg0DispatchPaths, cli_config_overrides: CliConfigOverrides, ) -> IoResult<()> { - let environment_manager = Arc::new(EnvironmentManager::new(EnvironmentManagerArgs::from_env( - ExecServerRuntimePaths::from_optional_paths( - arg0_paths.codex_self_exe.clone(), - arg0_paths.codex_linux_sandbox_exe.clone(), - )?, - ))); + let environment_manager = Arc::new( + EnvironmentManager::new(EnvironmentManagerArgs::new( + ExecServerRuntimePaths::from_optional_paths( + arg0_paths.codex_self_exe.clone(), + arg0_paths.codex_linux_sandbox_exe.clone(), + )?, + )) + .await, + ); // Parse CLI overrides once and derive the base Config eagerly so later // components do not need to work with raw TOML values. let cli_kv_overrides = cli_config_overrides.parse_overrides().map_err(|e| { diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index d09876739858..9ef4c4994157 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -745,12 +745,15 @@ pub async fn run_main( } }; - let environment_manager = Arc::new(EnvironmentManager::new(EnvironmentManagerArgs::from_env( - ExecServerRuntimePaths::from_optional_paths( - arg0_paths.codex_self_exe.clone(), - arg0_paths.codex_linux_sandbox_exe.clone(), - )?, - ))); + let environment_manager = Arc::new( + EnvironmentManager::new(EnvironmentManagerArgs::new( + ExecServerRuntimePaths::from_optional_paths( + arg0_paths.codex_self_exe.clone(), + arg0_paths.codex_linux_sandbox_exe.clone(), + )?, + )) + .await, + ); let cwd = cli.cwd.clone(); let config_cwd = config_cwd_for_app_server_target(cwd.as_deref(), &app_server_target, &environment_manager)?; @@ -2017,14 +2020,14 @@ mod tests { Path::new("/definitely/not/local/to/this/test") }; let target = AppServerTarget::Embedded; - let environment_manager = - EnvironmentManager::new(codex_exec_server::EnvironmentManagerArgs { - exec_server_url: Some("ws://127.0.0.1:8765".to_string()), - local_runtime_paths: ExecServerRuntimePaths::new( - std::env::current_exe().expect("current exe"), - /*codex_linux_sandbox_exe*/ None, - )?, - }); + let environment_manager = EnvironmentManager::create_for_tests( + Some("ws://127.0.0.1:8765".to_string()), + ExecServerRuntimePaths::new( + std::env::current_exe().expect("current exe"), + /*codex_linux_sandbox_exe*/ None, + )?, + ) + .await; let config_cwd = config_cwd_for_app_server_target(Some(remote_only_cwd), &target, &environment_manager)?;