mirror of
https://github.com/openai/codex.git
synced 2026-05-17 09:43:19 +00:00
[exec-server] Make remote environment settings configurable
This commit is contained in:
@@ -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<Self, ExecServerError> {
|
||||
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::<u64>().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
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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<String>,
|
||||
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<String>) -> 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<String>) -> 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<String>) -> (Opt
|
||||
}
|
||||
}
|
||||
|
||||
fn normalize_remote_environment_id(remote_environment_id: Option<String>) -> 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
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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;
|
||||
|
||||
Reference in New Issue
Block a user