From a780939ec7e533ae2229446d81368cb2d5fee488 Mon Sep 17 00:00:00 2001 From: Ruslan Nigmatullin Date: Fri, 8 May 2026 16:19:06 +0000 Subject: [PATCH] [exec-server] Make remote environment settings configurable --- codex-rs/exec-server/src/client_transport.rs | 63 +++++++++++++- codex-rs/exec-server/src/environment.rs | 8 +- .../exec-server/src/environment_provider.rs | 84 +++++++++++++++++-- codex-rs/exec-server/src/lib.rs | 3 + 4 files changed, 147 insertions(+), 11 deletions(-) diff --git a/codex-rs/exec-server/src/client_transport.rs b/codex-rs/exec-server/src/client_transport.rs index 3fccfa25c5..50394b627b 100644 --- a/codex-rs/exec-server/src/client_transport.rs +++ b/codex-rs/exec-server/src/client_transport.rs @@ -19,18 +19,24 @@ use crate::connection::JsonRpcConnection; const ENVIRONMENT_CLIENT_NAME: &str = "codex-environment"; const ENVIRONMENT_CONNECT_TIMEOUT: Duration = Duration::from_secs(5); const ENVIRONMENT_INITIALIZE_TIMEOUT: Duration = Duration::from_secs(5); +pub const CODEX_EXEC_SERVER_CONNECT_TIMEOUT_MS_ENV_VAR: &str = + "CODEX_EXEC_SERVER_CONNECT_TIMEOUT_MS"; +pub const CODEX_EXEC_SERVER_INITIALIZE_TIMEOUT_MS_ENV_VAR: &str = + "CODEX_EXEC_SERVER_INITIALIZE_TIMEOUT_MS"; impl ExecServerClient { pub(crate) async fn connect_for_transport( transport_params: crate::client_api::ExecServerTransportParams, ) -> Result { + let connect_timeout = environment_connect_timeout(); + let initialize_timeout = environment_initialize_timeout(); match transport_params { crate::client_api::ExecServerTransportParams::WebSocketUrl(websocket_url) => { Self::connect_websocket(RemoteExecServerConnectArgs { websocket_url, client_name: ENVIRONMENT_CLIENT_NAME.to_string(), - connect_timeout: ENVIRONMENT_CONNECT_TIMEOUT, - initialize_timeout: ENVIRONMENT_INITIALIZE_TIMEOUT, + connect_timeout, + initialize_timeout, resume_session_id: None, }) .await @@ -39,7 +45,7 @@ impl ExecServerClient { Self::connect_stdio_command(StdioExecServerConnectArgs { command, client_name: ENVIRONMENT_CLIENT_NAME.to_string(), - initialize_timeout: ENVIRONMENT_INITIALIZE_TIMEOUT, + initialize_timeout, resume_session_id: None, }) .await @@ -125,3 +131,54 @@ fn stdio_command_process(stdio_command: &StdioExecServerCommand) -> Command { command.process_group(0); command } + +fn environment_connect_timeout() -> Duration { + timeout_from_env_var( + CODEX_EXEC_SERVER_CONNECT_TIMEOUT_MS_ENV_VAR, + ENVIRONMENT_CONNECT_TIMEOUT, + ) +} + +fn environment_initialize_timeout() -> Duration { + timeout_from_env_var( + CODEX_EXEC_SERVER_INITIALIZE_TIMEOUT_MS_ENV_VAR, + ENVIRONMENT_INITIALIZE_TIMEOUT, + ) +} + +fn timeout_from_env_var(env_var: &str, default: Duration) -> Duration { + timeout_from_env_value(std::env::var(env_var).ok().as_deref(), default) +} + +fn timeout_from_env_value(value: Option<&str>, default: Duration) -> Duration { + value + .map(str::trim) + .and_then(|value| value.parse::().ok()) + .map(Duration::from_millis) + .unwrap_or(default) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn timeout_from_env_value_uses_milliseconds() { + assert_eq!( + timeout_from_env_value(Some("1234"), Duration::from_secs(5)), + Duration::from_millis(1234) + ); + } + + #[test] + fn timeout_from_env_value_uses_default_for_missing_or_invalid_values() { + let default = Duration::from_secs(5); + + assert_eq!(timeout_from_env_value(/*value*/ None, default), default); + assert_eq!(timeout_from_env_value(Some(""), default), default); + assert_eq!( + timeout_from_env_value(Some("not-a-number"), default), + default + ); + } +} diff --git a/codex-rs/exec-server/src/environment.rs b/codex-rs/exec-server/src/environment.rs index d13ba6d3bc..a655d9ac8a 100644 --- a/codex-rs/exec-server/src/environment.rs +++ b/codex-rs/exec-server/src/environment.rs @@ -21,6 +21,7 @@ use crate::remote_file_system::RemoteFileSystem; use crate::remote_process::RemoteProcess; pub const CODEX_EXEC_SERVER_URL_ENV_VAR: &str = "CODEX_EXEC_SERVER_URL"; +pub const CODEX_EXEC_SERVER_ENVIRONMENT_ID_ENV_VAR: &str = "CODEX_EXEC_SERVER_ENVIRONMENT_ID"; /// Owns the execution/filesystem environments available to the Codex runtime. /// @@ -96,8 +97,11 @@ impl EnvironmentManager { 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 + let provider = DefaultEnvironmentProvider::from_env(); + match Self::from_provider(&provider, local_runtime_paths).await { + Ok(manager) => manager, + Err(err) => panic!("default provider should create valid environments: {err}"), + } } /// Builds a manager from `CODEX_HOME` and local runtime paths used when diff --git a/codex-rs/exec-server/src/environment_provider.rs b/codex-rs/exec-server/src/environment_provider.rs index 0e4bcc5191..a0bb7d9b91 100644 --- a/codex-rs/exec-server/src/environment_provider.rs +++ b/codex-rs/exec-server/src/environment_provider.rs @@ -5,6 +5,7 @@ use async_trait::async_trait; use crate::Environment; use crate::ExecServerError; use crate::ExecServerRuntimePaths; +use crate::environment::CODEX_EXEC_SERVER_ENVIRONMENT_ID_ENV_VAR; use crate::environment::CODEX_EXEC_SERVER_URL_ENV_VAR; use crate::environment::LOCAL_ENVIRONMENT_ID; use crate::environment::REMOTE_ENVIRONMENT_ID; @@ -40,17 +41,33 @@ pub enum EnvironmentDefault { #[derive(Clone, Debug)] pub struct DefaultEnvironmentProvider { exec_server_url: Option, + remote_environment_id: String, } 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 } + Self { + exec_server_url, + remote_environment_id: REMOTE_ENVIRONMENT_ID.to_string(), + } } - /// Builds a provider by reading `CODEX_EXEC_SERVER_URL`. + /// Builds a provider by reading `CODEX_EXEC_SERVER_URL` and its companion settings. pub fn from_env() -> Self { - Self::new(std::env::var(CODEX_EXEC_SERVER_URL_ENV_VAR).ok()) + Self { + exec_server_url: std::env::var(CODEX_EXEC_SERVER_URL_ENV_VAR).ok(), + remote_environment_id: normalize_remote_environment_id( + std::env::var(CODEX_EXEC_SERVER_ENVIRONMENT_ID_ENV_VAR).ok(), + ), + } + } + + #[cfg(test)] + fn with_remote_environment_id(mut self, remote_environment_id: impl Into) -> Self { + self.remote_environment_id = + normalize_remote_environment_id(Some(remote_environment_id.into())); + self } pub(crate) fn snapshot_inner( @@ -62,18 +79,19 @@ impl DefaultEnvironmentProvider { Environment::local(local_runtime_paths.clone()), )]); let (exec_server_url, disabled) = normalize_exec_server_url(self.exec_server_url.clone()); + let has_remote_environment = exec_server_url.is_some(); if let Some(exec_server_url) = exec_server_url { environments.insert( - REMOTE_ENVIRONMENT_ID.to_string(), + self.remote_environment_id.clone(), Environment::remote_inner(exec_server_url, Some(local_runtime_paths.clone())), ); } let default = if disabled { EnvironmentDefault::Disabled - } else if environments.contains_key(REMOTE_ENVIRONMENT_ID) { - EnvironmentDefault::EnvironmentId(REMOTE_ENVIRONMENT_ID.to_string()) + } else if has_remote_environment { + EnvironmentDefault::EnvironmentId(self.remote_environment_id.clone()) } else { EnvironmentDefault::EnvironmentId(LOCAL_ENVIRONMENT_ID.to_string()) }; @@ -103,6 +121,13 @@ pub(crate) fn normalize_exec_server_url(exec_server_url: Option) -> (Opt } } +fn normalize_remote_environment_id(remote_environment_id: Option) -> String { + match remote_environment_id.as_deref().map(str::trim) { + Some("") | None => REMOTE_ENVIRONMENT_ID.to_string(), + Some(remote_environment_id) => remote_environment_id.to_string(), + } +} + #[cfg(test)] mod tests { use pretty_assertions::assert_eq; @@ -210,4 +235,51 @@ mod tests { Some("ws://127.0.0.1:8765") ); } + + #[tokio::test] + async fn default_provider_uses_custom_remote_environment_id() { + let provider = DefaultEnvironmentProvider::new(Some("ws://127.0.0.1:8765".to_string())) + .with_remote_environment_id("devbox"); + let runtime_paths = test_runtime_paths(); + let snapshot = provider + .snapshot(&runtime_paths) + .await + .expect("environments"); + + assert!(!snapshot.environments.contains_key(REMOTE_ENVIRONMENT_ID)); + assert!(snapshot.environments["devbox"].is_remote()); + assert_eq!( + snapshot.default, + EnvironmentDefault::EnvironmentId("devbox".to_string()) + ); + } + + #[tokio::test] + async fn default_provider_custom_remote_environment_id_can_replace_local() { + let provider = DefaultEnvironmentProvider::new(Some("ws://127.0.0.1:8765".to_string())) + .with_remote_environment_id(LOCAL_ENVIRONMENT_ID); + let runtime_paths = test_runtime_paths(); + let snapshot = provider + .snapshot(&runtime_paths) + .await + .expect("environments"); + + assert!(snapshot.environments[LOCAL_ENVIRONMENT_ID].is_remote()); + assert_eq!( + snapshot.default, + EnvironmentDefault::EnvironmentId(LOCAL_ENVIRONMENT_ID.to_string()) + ); + } + + #[test] + fn remote_environment_id_normalization_uses_default_for_empty_values() { + assert_eq!( + normalize_remote_environment_id(Some(" ".to_string())), + REMOTE_ENVIRONMENT_ID + ); + assert_eq!( + normalize_remote_environment_id(/*remote_environment_id*/ None), + REMOTE_ENVIRONMENT_ID + ); + } } diff --git a/codex-rs/exec-server/src/lib.rs b/codex-rs/exec-server/src/lib.rs index 85de8258f2..9b022de15a 100644 --- a/codex-rs/exec-server/src/lib.rs +++ b/codex-rs/exec-server/src/lib.rs @@ -28,6 +28,8 @@ pub use client::http_client::ReqwestHttpClient; pub use client_api::ExecServerClientConnectOptions; pub use client_api::HttpClient; pub use client_api::RemoteExecServerConnectArgs; +pub use client_transport::CODEX_EXEC_SERVER_CONNECT_TIMEOUT_MS_ENV_VAR; +pub use client_transport::CODEX_EXEC_SERVER_INITIALIZE_TIMEOUT_MS_ENV_VAR; pub use codex_file_system::CopyOptions; pub use codex_file_system::CreateDirectoryOptions; pub use codex_file_system::ExecutorFileSystem; @@ -36,6 +38,7 @@ pub use codex_file_system::FileSystemResult; pub use codex_file_system::FileSystemSandboxContext; pub use codex_file_system::ReadDirectoryEntry; pub use codex_file_system::RemoveOptions; +pub use environment::CODEX_EXEC_SERVER_ENVIRONMENT_ID_ENV_VAR; pub use environment::CODEX_EXEC_SERVER_URL_ENV_VAR; pub use environment::Environment; pub use environment::EnvironmentManager;