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

Make default environment lookup infallible

Browse Source 
Return concrete default and local environments from EnvironmentManager now that the manager always installs local and default entries. Keep arbitrary ID lookup optional.

Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
starr-openai
2026-04-17 15:09:41 -07:00
parent 194b9d8571
commit e589fddaec
7 changed files with 34 additions and 47 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
codex-rs/app-server-client/src/lib.rs
View File

@@ -2006,7 +2006,7 @@ mod tests {
runtime_args
.environment_manager
.default_environment()
.is_some_and(|environment| environment.is_remote())
.is_remote()
);
}

9
codex-rs/app-server/src/codex_message_processor.rs
View File

@@ -5698,8 +5698,7 @@ impl CodexMessageProcessor {
let environment = self
.thread_manager
.environment_manager()
.default_environment()
.unwrap_or_else(|| Arc::new(codex_exec_server::Environment::default()));
.default_environment();
// Status listing has no turn cwd. This fallback is used only
// by executor-backed stdio MCPs whose config omits `cwd`.
McpRuntimeEnvironment::new(environment, config.cwd.to_path_buf())
@@ -5856,8 +5855,7 @@ impl CodexMessageProcessor {
let environment = self
.thread_manager
.environment_manager()
.default_environment()
.unwrap_or_else(|| Arc::new(codex_exec_server::Environment::default()));
.default_environment();
// Resource reads without a thread have no turn cwd. This fallback
// is used only by executor-backed stdio MCPs whose config omits `cwd`.
McpRuntimeEnvironment::new(environment, config.cwd.to_path_buf())
@@ -6451,7 +6449,8 @@ impl CodexMessageProcessor {
.thread_manager
.environment_manager()
.default_environment()
.map(|environment| environment.get_filesystem());
.get_filesystem();
let fs = Some(fs);
let mut data = Vec::new();
for cwd in cwds {
let extra_roots = extra_roots_by_cwd

6
codex-rs/core/src/session/mod.rs
View File

@@ -461,9 +461,7 @@ impl Codex {
let (tx_event, rx_event) = async_channel::unbounded();
let environment = environment_manager.default_environment();
let fs = environment
.as_ref()
.map(|environment| environment.get_filesystem());
let fs = Some(environment.get_filesystem());
let plugin_outcome = plugins_manager.plugins_for_config(&config).await;
let effective_skill_roots = plugin_outcome.effective_skill_roots();
let skills_input = skills_load_input_from_config(&config, effective_skill_roots);
@@ -513,7 +511,7 @@ impl Codex {
}
let user_instructions = AgentsMdManager::new(&config)
.user_instructions(environment.as_deref())
.user_instructions(Some(environment.as_ref()))
.await;
let exec_policy = if crate::guardian::is_guardian_reviewer_source(&session_source) {

2
codex-rs/core/src/session/session.rs
View File

@@ -698,7 +698,7 @@ impl Session {
config.js_repl_node_path.clone(),
),
environment_manager,
environment,
environment: Some(environment),
allows_agent_environment_access,
};
services

6
codex-rs/core/src/thread_manager.rs
View File

@@ -923,8 +923,8 @@ impl ThreadManagerState {
user_shell_override: Option<crate::shell::Shell>,
) -> CodexResult<NewThread> {
let environment = self.environment_manager.default_environment();
let watch_registration = match environment.as_ref() {
Some(environment) if !environment.is_remote() => {
let watch_registration = match environment.is_remote() {
false => {
self.skills_watcher
.register_config(
&config,
@@ -934,7 +934,7 @@ impl ThreadManagerState {
)
.await
}
Some(_) | None => crate::file_watcher::WatchRegistration::default(),
true => crate::file_watcher::WatchRegistration::default(),
};
let CodexSpawnOk {
codex, thread_id, ..

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

@@ -23,7 +23,7 @@ pub const CODEX_EXEC_SERVER_URL_ENV_VAR: &str = "CODEX_EXEC_SERVER_URL";
/// separately tracking whether model-facing tools may access environments.
#[derive(Debug)]
pub struct EnvironmentManager {
default_environment: Option<String>,
default_environment: String,
environment_disabled_for_agent: bool,
environments: HashMap<String, Arc<Environment>>,
}
@@ -124,9 +124,9 @@ impl EnvironmentManager {
.expect("valid remote environment"),
),
);
Some(REMOTE_ENVIRONMENT_ID.to_string())
REMOTE_ENVIRONMENT_ID.to_string()
}
None => Some(LOCAL_ENVIRONMENT_ID.to_string()),
None => LOCAL_ENVIRONMENT_ID.to_string(),
};
Self {
@@ -139,22 +139,19 @@ impl EnvironmentManager {
/// Returns true when model-facing tools may access an environment.
pub fn allows_agent_environment_access(&self) -> bool {
!self.environment_disabled_for_agent
&& self
.default_environment
.as_deref()
.is_some_and(|environment_id| self.environments.contains_key(environment_id))
&& self.environments.contains_key(&self.default_environment)
}
/// Returns the default environment instance.
pub fn default_environment(&self) -> Option<Arc<Environment>> {
self.default_environment
.as_deref()
.and_then(|environment_id| self.get_environment(environment_id))
pub fn default_environment(&self) -> Arc<Environment> {
self.get_environment(&self.default_environment)
.expect("default environment exists")
}
/// Returns the local environment instance.
pub fn local_environment(&self) -> Option<Arc<Environment>> {
pub fn local_environment(&self) -> Arc<Environment> {
self.get_environment(LOCAL_ENVIRONMENT_ID)
.expect("local environment exists")
}
/// Returns a named environment instance.
@@ -297,10 +294,10 @@ mod tests {
local_runtime_paths: None,
});
let environment = manager.default_environment().expect("local environment");
let environment = manager.default_environment();
assert!(!environment.is_remote());
assert!(manager.allows_agent_environment_access());
assert!(manager.local_environment().is_some());
assert!(!manager.local_environment().is_remote());
assert!(manager.get_environment(REMOTE_ENVIRONMENT_ID).is_none());
}
@@ -312,8 +309,8 @@ mod tests {
});
assert!(!manager.allows_agent_environment_access());
assert!(manager.default_environment().is_some());
assert!(manager.local_environment().is_some());
assert!(!manager.default_environment().is_remote());
assert!(!manager.local_environment().is_remote());
assert!(manager.get_environment(REMOTE_ENVIRONMENT_ID).is_none());
}
@@ -324,16 +321,11 @@ mod tests {
local_runtime_paths: None,
});
let environment = manager.default_environment().expect("remote environment");
let environment = manager.default_environment();
assert!(environment.is_remote());
assert!(manager.allows_agent_environment_access());
assert_eq!(environment.exec_server_url(), Some("ws://127.0.0.1:8765"));
assert!(
!manager
.local_environment()
.expect("local environment")
.is_remote()
);
assert!(!manager.local_environment().is_remote());
assert_eq!(
manager
.get_environment(REMOTE_ENVIRONMENT_ID)
@@ -347,8 +339,8 @@ mod tests {
async fn environment_manager_default_environment_caches_environment() {
let manager = EnvironmentManager::new(EnvironmentManagerArgs::default());
let first = manager.default_environment().expect("local environment");
let second = manager.default_environment().expect("local environment");
let first = manager.default_environment();
let second = manager.default_environment();
assert!(Arc::ptr_eq(&first, &second));
}
@@ -365,14 +357,14 @@ mod tests {
local_runtime_paths: Some(runtime_paths.clone()),
});
let environment = manager.default_environment().expect("local environment");
let environment = manager.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.local_runtime_paths().cloned(),
});
let environment = manager.default_environment().expect("local environment");
let environment = manager.default_environment();
assert_eq!(environment.local_runtime_paths(), Some(&runtime_paths));
}
@@ -383,7 +375,7 @@ mod tests {
local_runtime_paths: None,
});
assert!(manager.default_environment().is_some());
assert!(!manager.default_environment().is_remote());
assert!(!manager.allows_agent_environment_access());
}
@@ -394,9 +386,9 @@ mod tests {
local_runtime_paths: None,
});
assert!(manager.default_environment().is_some());
assert!(!manager.default_environment().is_remote());
assert!(!manager.allows_agent_environment_access());
assert!(manager.local_environment().is_some());
assert!(!manager.local_environment().is_remote());
assert!(manager.get_environment(REMOTE_ENVIRONMENT_ID).is_none());
}

4
codex-rs/tui/src/lib.rs
View File

@@ -625,9 +625,7 @@ fn config_cwd_for_app_server_target(
app_server_target: &AppServerTarget,
environment_manager: &EnvironmentManager,
) -> std::io::Result<Option<AbsolutePathBuf>> {
if environment_manager
.default_environment()
.is_some_and(|environment| environment.is_remote())
if environment_manager.default_environment().is_remote()
|| matches!(app_server_target, AppServerTarget::Remote { .. })
{
return Ok(None);