From 0a7006cebc7bfce19345019c60b6fe36901e3104 Mon Sep 17 00:00:00 2001 From: starr-openai Date: Tue, 5 May 2026 13:58:49 -0700 Subject: [PATCH] Simplify environment provider defaults Co-authored-by: Codex --- codex-rs/app-server-client/src/lib.rs | 17 ++--- codex-rs/app-server/src/lib.rs | 15 ++-- codex-rs/core/src/connectors.rs | 2 +- codex-rs/core/src/environment_selection.rs | 3 +- codex-rs/core/src/prompt_debug.rs | 4 +- codex-rs/core/src/thread_manager_tests.rs | 11 ++- codex-rs/core/tests/common/test_codex.rs | 8 +-- codex-rs/exec-server/src/environment.rs | 84 +++++++++++++--------- codex-rs/exec/src/lib.rs | 6 +- codex-rs/mcp-server/src/lib.rs | 15 ++-- codex-rs/thread-manager-sample/src/main.rs | 9 +-- codex-rs/tui/src/lib.rs | 18 ++--- 12 files changed, 93 insertions(+), 99 deletions(-) diff --git a/codex-rs/app-server-client/src/lib.rs b/codex-rs/app-server-client/src/lib.rs index 6e10cef255..e9b514bf5f 100644 --- a/codex-rs/app-server-client/src/lib.rs +++ b/codex-rs/app-server-client/src/lib.rs @@ -2092,17 +2092,14 @@ 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::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"), + 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, ) - .await, - ); + .expect("runtime paths"), + )); 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 2efdfef52e..f27261728e 100644 --- a/codex-rs/app-server/src/lib.rs +++ b/codex-rs/app-server/src/lib.rs @@ -420,15 +420,12 @@ pub async fn run_main_with_transport_options( auth: AppServerWebsocketAuthSettings, runtime_options: AppServerRuntimeOptions, ) -> IoResult<()> { - 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 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(), + )?, + ))); 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 0bd53a50ee..3c7ef672be 100644 --- a/codex-rs/core/src/connectors.rs +++ b/codex-rs/core/src/connectors.rs @@ -202,7 +202,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::new(local_runtime_paths)).await; + EnvironmentManager::new(EnvironmentManagerArgs::new(local_runtime_paths)); 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 b4bd9cbe89..f14361bdb5 100644 --- a/codex-rs/core/src/environment_selection.rs +++ b/codex-rs/core/src/environment_selection.rs @@ -106,8 +106,7 @@ mod tests { 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), diff --git a/codex-rs/core/src/prompt_debug.rs b/codex-rs/core/src/prompt_debug.rs index 1b0c75230c..145e372981 100644 --- a/codex-rs/core/src/prompt_debug.rs +++ b/codex-rs/core/src/prompt_debug.rs @@ -48,7 +48,9 @@ pub async fn build_prompt_input( &config, Arc::clone(&auth_manager), SessionSource::Exec, - Arc::new(EnvironmentManager::new(EnvironmentManagerArgs::new(local_runtime_paths)).await), + Arc::new(EnvironmentManager::new(EnvironmentManagerArgs::new( + local_runtime_paths, + ))), /*analytics_events_client*/ None, state_db, thread_store, diff --git a/codex-rs/core/src/thread_manager_tests.rs b/codex-rs/core/src/thread_manager_tests.rs index 0f6afa05a6..7ad54565ff 100644 --- a/codex-rs/core/src/thread_manager_tests.rs +++ b/codex-rs/core/src/thread_manager_tests.rs @@ -318,13 +318,10 @@ async fn start_thread_accepts_explicit_environment_when_default_environment_is_d /*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 environment_manager = Arc::new(codex_exec_server::EnvironmentManager::create_for_tests( + Some("none".to_string()), + runtime_paths, + )); let manager = ThreadManager::with_models_provider_and_home_for_tests( CodexAuth::from_api_key("dummy"), config.model_provider.clone(), diff --git a/codex-rs/core/tests/common/test_codex.rs b/codex-rs/core/tests/common/test_codex.rs index 1728f5bc5c..5399d05292 100644 --- a/codex-rs/core/tests/common/test_codex.rs +++ b/codex-rs/core/tests/common/test_codex.rs @@ -391,13 +391,11 @@ impl TestCodexBuilder { std::env::current_exe()?, /*codex_linux_sandbox_exe*/ None, )?; - let environment_manager = Arc::new( - codex_exec_server::EnvironmentManager::create_for_tests( + let environment_manager = + Arc::new(codex_exec_server::EnvironmentManager::create_for_tests( exec_server_url, 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/exec-server/src/environment.rs b/codex-rs/exec-server/src/environment.rs index d7867c9732..524314337e 100644 --- a/codex-rs/exec-server/src/environment.rs +++ b/codex-rs/exec-server/src/environment.rs @@ -13,6 +13,7 @@ use crate::environment_provider::EnvironmentDefault; use crate::environment_provider::EnvironmentProvider; use crate::environment_provider::EnvironmentProviderSnapshot; use crate::environment_provider::normalize_exec_server_url; +use crate::environment_toml::environment_provider_from_codex_home; use crate::local_file_system::LocalFileSystem; use crate::local_process::LocalProcess; use crate::process::ExecBackend; @@ -23,9 +24,10 @@ 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. Its -/// default constructor preserves the legacy `CODEX_EXEC_SERVER_URL` behavior -/// while provider-based construction accepts a provider-supplied snapshot. +/// `EnvironmentManager` is a shared registry for concrete environments. +/// `from_codex_home` preserves the legacy `CODEX_EXEC_SERVER_URL` behavior when +/// no `environments.toml` file is present, 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 an explicit local @@ -46,19 +48,6 @@ pub struct EnvironmentManager { pub const LOCAL_ENVIRONMENT_ID: &str = "local"; pub const REMOTE_ENVIRONMENT_ID: &str = "remote"; -#[derive(Clone, Debug)] -pub struct EnvironmentManagerArgs { - pub local_runtime_paths: ExecServerRuntimePaths, -} - -impl EnvironmentManagerArgs { - pub fn new(local_runtime_paths: ExecServerRuntimePaths) -> Self { - Self { - local_runtime_paths, - } - } -} - impl EnvironmentManager { /// Builds a test-only manager without configured sandbox helper paths. pub fn default_for_tests() -> Self { @@ -89,14 +78,28 @@ impl EnvironmentManager { 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 { - local_runtime_paths, - } = args; - 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 + /// Builds a manager from `CODEX_HOME` and local runtime paths used when + /// creating local filesystem helpers. + /// + /// If `CODEX_HOME/environments.toml` is present, it defines the configured + /// environments. Otherwise this preserves the legacy + /// `CODEX_EXEC_SERVER_URL` behavior. Callers that ignore user config + /// should use [`Self::from_env`] instead. + pub async fn from_codex_home( + codex_home: impl AsRef, + local_runtime_paths: ExecServerRuntimePaths, + ) -> Result { + let provider = environment_provider_from_codex_home(codex_home.as_ref())?; + Self::from_provider(provider.as_ref(), local_runtime_paths).await + } + + /// Builds a manager from the legacy environment-variable provider without + /// reading user config files from `CODEX_HOME`. + pub async fn from_env( + local_runtime_paths: ExecServerRuntimePaths, + ) -> Result { + let provider = DefaultEnvironmentProvider::from_env(); + Self::from_provider(&provider, local_runtime_paths).await } async fn from_default_provider_url( @@ -193,7 +196,7 @@ impl EnvironmentManager { /// paths used by filesystem helpers. #[derive(Clone)] pub struct Environment { - exec_server_url: Option, + remote_transport: Option, exec_backend: Arc, filesystem: Arc, http_client: Arc, @@ -204,7 +207,7 @@ impl Environment { /// Builds a test-only local environment without configured sandbox helper paths. pub fn default_for_tests() -> Self { Self { - exec_server_url: None, + remote_transport: None, exec_backend: Arc::new(LocalProcess::default()), filesystem: Arc::new(LocalFileSystem::unsandboxed()), http_client: Arc::new(ReqwestHttpClient), @@ -216,7 +219,7 @@ impl Environment { impl std::fmt::Debug for Environment { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("Environment") - .field("exec_server_url", &self.exec_server_url) + .field("exec_server_url", &self.exec_server_url()) .finish_non_exhaustive() } } @@ -259,7 +262,7 @@ impl Environment { pub(crate) fn local(local_runtime_paths: ExecServerRuntimePaths) -> Self { Self { - exec_server_url: None, + remote_transport: None, exec_backend: Arc::new(LocalProcess::default()), filesystem: Arc::new(LocalFileSystem::with_runtime_paths( local_runtime_paths.clone(), @@ -273,15 +276,23 @@ impl Environment { exec_server_url: String, local_runtime_paths: Option, ) -> Self { - let client = LazyRemoteExecServerClient::new(ExecServerTransportParams::WebSocketUrl( - exec_server_url.clone(), - )); + Self::remote_with_transport( + ExecServerTransportParams::WebSocketUrl(exec_server_url), + local_runtime_paths, + ) + } + + pub(crate) fn remote_with_transport( + transport_params: ExecServerTransportParams, + local_runtime_paths: Option, + ) -> Self { + let client = LazyRemoteExecServerClient::new(transport_params.clone()); let exec_backend: Arc = Arc::new(RemoteProcess::new(client.clone())); let filesystem: Arc = Arc::new(RemoteFileSystem::new(client.clone())); Self { - exec_server_url: Some(exec_server_url), + remote_transport: Some(transport_params), exec_backend, filesystem, http_client: Arc::new(client), @@ -290,12 +301,15 @@ impl Environment { } pub fn is_remote(&self) -> bool { - self.exec_server_url.is_some() + self.remote_transport.is_some() } /// Returns the remote exec-server URL when this environment is remote. - pub fn exec_server_url(&self) -> Option<&str> { - self.exec_server_url.as_deref() + pub(crate) fn exec_server_url(&self) -> Option<&str> { + match self.remote_transport.as_ref() { + Some(ExecServerTransportParams::WebSocketUrl(url)) => Some(url.as_str()), + Some(ExecServerTransportParams::StdioCommand(_)) | None => None, + } } pub fn local_runtime_paths(&self) -> Option<&ExecServerRuntimePaths> { diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index b035a19517..a8fba18f4a 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -518,9 +518,9 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result feedback: CodexFeedback::new(), log_db: None, state_db: state_db.clone(), - environment_manager: std::sync::Arc::new( - EnvironmentManager::new(EnvironmentManagerArgs::new(local_runtime_paths)).await, - ), + environment_manager: std::sync::Arc::new(EnvironmentManager::new( + EnvironmentManagerArgs::new(local_runtime_paths), + )), 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 2fd94e586a..63ddd0c3e5 100644 --- a/codex-rs/mcp-server/src/lib.rs +++ b/codex-rs/mcp-server/src/lib.rs @@ -61,15 +61,12 @@ pub async fn run_main( arg0_paths: Arg0DispatchPaths, cli_config_overrides: CliConfigOverrides, ) -> IoResult<()> { - 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 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(), + )?, + ))); // 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/thread-manager-sample/src/main.rs b/codex-rs/thread-manager-sample/src/main.rs index 4ad937f9af..3190f119bb 100644 --- a/codex-rs/thread-manager-sample/src/main.rs +++ b/codex-rs/thread-manager-sample/src/main.rs @@ -20,7 +20,6 @@ use codex_core_api::Config; use codex_core_api::ConfigLayerStack; use codex_core_api::Constrained; use codex_core_api::EnvironmentManager; -use codex_core_api::EnvironmentManagerArgs; use codex_core_api::EventMsg; use codex_core_api::ExecServerRuntimePaths; use codex_core_api::Features; @@ -58,7 +57,6 @@ use codex_core_api::built_in_model_providers; use codex_core_api::find_codex_home; use codex_core_api::init_state_db_from_config; use codex_core_api::item_event_to_server_notification; -use codex_core_api::resolve_installation_id; use codex_core_api::set_default_originator; use codex_core_api::thread_store_from_config; @@ -118,9 +116,9 @@ async fn run_main(arg0_paths: Arg0DispatchPaths) -> anyhow::Result<()> { }; let thread_store = thread_store_from_config(&config, state_db.clone()); let agent_graph_store = agent_graph_store_from_state_db(state_db.clone()); - let environment_manager = - Arc::new(EnvironmentManager::new(EnvironmentManagerArgs::new(local_runtime_paths)).await); - let installation_id = resolve_installation_id(&config.codex_home).await?; + let environment_manager = Arc::new( + EnvironmentManager::from_codex_home(config.codex_home.clone(), local_runtime_paths).await?, + ); let thread_manager = ThreadManager::new( &config, auth_manager, @@ -130,7 +128,6 @@ async fn run_main(arg0_paths: Arg0DispatchPaths) -> anyhow::Result<()> { state_db, Arc::clone(&thread_store), agent_graph_store, - installation_id, ); let NewThread { diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index b2e92a19b4..5bcc6ffa26 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -761,15 +761,12 @@ pub async fn run_main( } }; - 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 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(), + )?, + ))); let cwd = cli.cwd.clone(); let config_cwd = config_cwd_for_app_server_target(cwd.as_deref(), &app_server_target, &environment_manager)?; @@ -2141,8 +2138,7 @@ mod tests { 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)?;