clone/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-05-16 09:12:54 +00:00
Code Issues Packages Projects Releases Wiki Activity

Return provider environment snapshots

Browse Source 
Make environment providers return the environment map and default id together. This keeps provider-owned startup state in one boundary and removes the separate default callback over a map.

Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
starr-openai
2026-05-04 12:46:29 -07:00
parent 93f68577ed
commit 7a8bed96eb
2 changed files with 109 additions and 106 deletions
Download Patch File Download Diff File Expand all files Collapse all files

130
codex-rs/exec-server/src/environment.rs
View File

@@ -10,6 +10,7 @@ use crate::client::http_client::ReqwestHttpClient;
use crate::client_api::ExecServerTransportParams;
use crate::environment_provider::DefaultEnvironmentProvider;
use crate::environment_provider::EnvironmentProvider;
use crate::environment_provider::EnvironmentProviderSnapshot;
use crate::environment_provider::normalize_exec_server_url;
use crate::local_file_system::LocalFileSystem;
use crate::local_process::LocalProcess;
@@ -72,11 +73,11 @@ impl EnvironmentManager {
/// Builds a test-only manager with environment access disabled.
pub fn disabled_for_tests(local_runtime_paths: ExecServerRuntimePaths) -> Self {
match Self::from_environments(
HashMap::new(),
local_runtime_paths,
/*default_environment*/ None,
) {
let snapshot = EnvironmentProviderSnapshot {
environments: HashMap::new(),
default_environment_id: None,
};
match Self::from_provider_snapshot(snapshot, local_runtime_paths) {
Ok(manager) => manager,
Err(err) => panic!("disabled test environment manager: {err}"),
}
@@ -105,13 +106,8 @@ impl EnvironmentManager {
local_runtime_paths: ExecServerRuntimePaths,
) -> Self {
let provider = DefaultEnvironmentProvider::new(exec_server_url);
let provider_environments = provider.environments(&local_runtime_paths);
let default_environment = provider.default_environment_id(&provider_environments);
match Self::from_environments(
provider_environments,
local_runtime_paths,
default_environment,
) {
let snapshot = provider.snapshot(&local_runtime_paths);
match Self::from_provider_snapshot(snapshot, local_runtime_paths) {
Ok(manager) => manager,
Err(err) => panic!("default provider should create valid environments: {err}"),
}
@@ -125,16 +121,20 @@ impl EnvironmentManager {
where
P: EnvironmentProvider + ?Sized,
{
let environments = provider.get_environments(&local_runtime_paths).await?;
let default_environment = provider.default_environment_id(&environments);
Self::from_provider_environments(environments, local_runtime_paths, default_environment)
let snapshot = provider
.get_environment_snapshot(&local_runtime_paths)
.await?;
Self::from_provider_snapshot(snapshot, local_runtime_paths)
}
fn from_provider_environments(
environments: HashMap<String, Environment>,
fn from_provider_snapshot(
snapshot: EnvironmentProviderSnapshot,
local_runtime_paths: ExecServerRuntimePaths,
default_environment: Option<String>,
) -> Result<Self, ExecServerError> {
let EnvironmentProviderSnapshot {
environments,
default_environment_id,
} = snapshot;
for id in environments.keys() {
if id.is_empty() {
return Err(ExecServerError::Protocol(
@@ -143,15 +143,7 @@ impl EnvironmentManager {
}
}
Self::from_environments(environments, local_runtime_paths, default_environment)
}
fn from_environments(
environments: HashMap<String, Environment>,
local_runtime_paths: ExecServerRuntimePaths,
default_environment: Option<String>,
) -> Result<Self, ExecServerError> {
if let Some(environment_id) = default_environment.as_ref()
if let Some(environment_id) = default_environment_id.as_ref()
&& !environments.contains_key(environment_id)
{
return Err(ExecServerError::Protocol(format!(
@@ -165,7 +157,7 @@ impl EnvironmentManager {
.collect();
Ok(Self {
default_environment,
default_environment: default_environment_id,
environments,
local_environment,
})
@@ -441,15 +433,17 @@ 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"),
)]),
async fn environment_manager_builds_from_provider_snapshot() {
let manager = EnvironmentManager::from_provider_snapshot(
EnvironmentProviderSnapshot {
environments: HashMap::from([(
REMOTE_ENVIRONMENT_ID.to_string(),
Environment::create_for_tests(Some("ws://127.0.0.1:8765".to_string()))
.expect("remote environment"),
)]),
default_environment_id: Some(REMOTE_ENVIRONMENT_ID.to_string()),
},
test_runtime_paths(),
Some(REMOTE_ENVIRONMENT_ID.to_string()),
)
.expect("environment manager");
@@ -469,10 +463,12 @@ mod tests {
#[tokio::test]
async fn environment_manager_rejects_empty_environment_id() {
let err = EnvironmentManager::from_provider_environments(
HashMap::from([("".to_string(), Environment::default_for_tests())]),
let err = EnvironmentManager::from_provider_snapshot(
EnvironmentProviderSnapshot {
environments: HashMap::from([("".to_string(), Environment::default_for_tests())]),
default_environment_id: None,
},
test_runtime_paths(),
/*default_environment*/ None,
)
.expect_err("empty id should fail");
@@ -484,20 +480,22 @@ mod tests {
#[tokio::test]
async fn environment_manager_uses_explicit_provider_default() {
let manager = EnvironmentManager::from_provider_environments(
HashMap::from([
(
LOCAL_ENVIRONMENT_ID.to_string(),
Environment::default_for_tests(),
),
(
"devbox".to_string(),
Environment::create_for_tests(Some("ws://127.0.0.1:8765".to_string()))
.expect("remote environment"),
),
]),
let manager = EnvironmentManager::from_provider_snapshot(
EnvironmentProviderSnapshot {
environments: HashMap::from([
(
LOCAL_ENVIRONMENT_ID.to_string(),
Environment::default_for_tests(),
),
(
"devbox".to_string(),
Environment::create_for_tests(Some("ws://127.0.0.1:8765".to_string()))
.expect("remote environment"),
),
]),
default_environment_id: Some("devbox".to_string()),
},
test_runtime_paths(),
Some("devbox".to_string()),
)
.expect("manager");
@@ -507,13 +505,15 @@ mod tests {
#[tokio::test]
async fn environment_manager_disables_provider_default() {
let manager = EnvironmentManager::from_provider_environments(
HashMap::from([(
LOCAL_ENVIRONMENT_ID.to_string(),
Environment::default_for_tests(),
)]),
let manager = EnvironmentManager::from_provider_snapshot(
EnvironmentProviderSnapshot {
environments: HashMap::from([(
LOCAL_ENVIRONMENT_ID.to_string(),
Environment::default_for_tests(),
)]),
default_environment_id: None,
},
test_runtime_paths(),
/*default_environment*/ None,
)
.expect("manager");
@@ -524,13 +524,15 @@ mod tests {
#[tokio::test]
async fn environment_manager_rejects_unknown_provider_default() {
let err = EnvironmentManager::from_provider_environments(
HashMap::from([(
LOCAL_ENVIRONMENT_ID.to_string(),
Environment::default_for_tests(),
)]),
let err = EnvironmentManager::from_provider_snapshot(
EnvironmentProviderSnapshot {
environments: HashMap::from([(
LOCAL_ENVIRONMENT_ID.to_string(),
Environment::default_for_tests(),
)]),
default_environment_id: Some("missing".to_string()),
},
test_runtime_paths(),
Some("missing".to_string()),
)
.expect_err("unknown default should fail");

85
codex-rs/exec-server/src/environment_provider.rs
View File

@@ -17,17 +17,15 @@ use crate::environment::REMOTE_ENVIRONMENT_ID;
#[async_trait]
pub trait EnvironmentProvider: Send + Sync {
/// Returns the environments available for a new manager.
async fn get_environments(
async fn get_environment_snapshot(
&self,
local_runtime_paths: &ExecServerRuntimePaths,
) -> Result<HashMap<String, Environment>, ExecServerError>;
) -> Result<EnvironmentProviderSnapshot, ExecServerError>;
}
fn default_environment_id(
&self,
environments: &HashMap<String, Environment>,
) -> Option<String> {
derived_default_environment_id(environments)
}
pub struct EnvironmentProviderSnapshot {
pub environments: HashMap<String, Environment>,
pub default_environment_id: Option<String>,
}
/// Default provider backed by `CODEX_EXEC_SERVER_URL`.
@@ -47,15 +45,15 @@ impl DefaultEnvironmentProvider {
Self::new(std::env::var(CODEX_EXEC_SERVER_URL_ENV_VAR).ok())
}
pub(crate) fn environments(
pub(crate) fn snapshot(
&self,
local_runtime_paths: &ExecServerRuntimePaths,
) -> HashMap<String, Environment> {
) -> EnvironmentProviderSnapshot {
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;
let (exec_server_url, disabled) = normalize_exec_server_url(self.exec_server_url.clone());
if let Some(exec_server_url) = exec_server_url {
environments.insert(
@@ -64,28 +62,26 @@ impl DefaultEnvironmentProvider {
);
}
environments
let default_environment_id = if disabled {
None
} else {
derived_default_environment_id(&environments)
};
EnvironmentProviderSnapshot {
environments,
default_environment_id,
}
}
}
#[async_trait]
impl EnvironmentProvider for DefaultEnvironmentProvider {
async fn get_environments(
async fn get_environment_snapshot(
&self,
local_runtime_paths: &ExecServerRuntimePaths,
) -> Result<HashMap<String, Environment>, ExecServerError> {
Ok(self.environments(local_runtime_paths))
}
fn default_environment_id(
&self,
environments: &HashMap<String, Environment>,
) -> Option<String> {
if normalize_exec_server_url(self.exec_server_url.clone()).1 {
None
} else {
derived_default_environment_id(environments)
}
) -> Result<EnvironmentProviderSnapshot, ExecServerError> {
Ok(self.snapshot(local_runtime_paths))
}
}
@@ -126,10 +122,11 @@ mod tests {
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)
let snapshot = provider
.get_environment_snapshot(&runtime_paths)
.await
.expect("environments");
.expect("environment snapshot");
let environments = snapshot.environments;
assert!(!environments[LOCAL_ENVIRONMENT_ID].is_remote());
assert_eq!(
@@ -143,10 +140,11 @@ mod tests {
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)
let snapshot = provider
.get_environment_snapshot(&runtime_paths)
.await
.expect("environments");
.expect("environment snapshot");
let environments = snapshot.environments;
assert!(!environments[LOCAL_ENVIRONMENT_ID].is_remote());
assert!(!environments.contains_key(REMOTE_ENVIRONMENT_ID));
@@ -156,24 +154,26 @@ mod tests {
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)
let snapshot = provider
.get_environment_snapshot(&runtime_paths)
.await
.expect("environments");
.expect("environment snapshot");
let environments = snapshot.environments;
assert!(!environments[LOCAL_ENVIRONMENT_ID].is_remote());
assert!(!environments.contains_key(REMOTE_ENVIRONMENT_ID));
assert_eq!(provider.default_environment_id(&environments), None);
assert_eq!(snapshot.default_environment_id, None);
}
#[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)
let snapshot = provider
.get_environment_snapshot(&runtime_paths)
.await
.expect("environments");
.expect("environment snapshot");
let environments = snapshot.environments;
assert!(!environments[LOCAL_ENVIRONMENT_ID].is_remote());
let remote_environment = &environments[REMOTE_ENVIRONMENT_ID];
@@ -188,10 +188,11 @@ mod tests {
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)
let snapshot = provider
.get_environment_snapshot(&runtime_paths)
.await
.expect("environments");
.expect("environment snapshot");
let environments = snapshot.environments;
assert_eq!(
environments[REMOTE_ENVIRONMENT_ID].exec_server_url(),