From f69cd0bf996bfaa6f229a9182d83fcc65c3d9c22 Mon Sep 17 00:00:00 2001 From: starr-openai Date: Thu, 7 May 2026 15:41:12 -0700 Subject: [PATCH] Fix environment startup defaults and sample build Co-authored-by: Codex --- codex-rs/core/src/environment_selection.rs | 15 ++---- codex-rs/core/src/thread_manager_tests.rs | 22 ++++++++- codex-rs/exec-server/src/environment.rs | 46 +++++++++++-------- .../exec-server/src/environment_provider.rs | 10 ++++ codex-rs/exec-server/src/environment_toml.rs | 22 ++++----- codex-rs/thread-manager-sample/src/main.rs | 3 ++ 6 files changed, 74 insertions(+), 44 deletions(-) diff --git a/codex-rs/core/src/environment_selection.rs b/codex-rs/core/src/environment_selection.rs index 50371e86a0..2c8c5ffb95 100644 --- a/codex-rs/core/src/environment_selection.rs +++ b/codex-rs/core/src/environment_selection.rs @@ -85,7 +85,6 @@ pub(crate) fn resolve_environment_selections( #[cfg(test)] mod tests { use codex_exec_server::ExecServerRuntimePaths; - use codex_exec_server::LOCAL_ENVIRONMENT_ID; use codex_exec_server::REMOTE_ENVIRONMENT_ID; use codex_protocol::protocol::TurnEnvironmentSelection; use codex_utils_absolute_path::AbsolutePathBuf; @@ -111,16 +110,10 @@ mod tests { assert_eq!( default_thread_environment_selections(&manager, &cwd), - vec![ - TurnEnvironmentSelection { - environment_id: REMOTE_ENVIRONMENT_ID.to_string(), - cwd: cwd.clone(), - }, - TurnEnvironmentSelection { - environment_id: LOCAL_ENVIRONMENT_ID.to_string(), - cwd, - }, - ] + vec![TurnEnvironmentSelection { + environment_id: REMOTE_ENVIRONMENT_ID.to_string(), + cwd, + }] ); } diff --git a/codex-rs/core/src/thread_manager_tests.rs b/codex-rs/core/src/thread_manager_tests.rs index 9a16a88f69..bfe0bc4197 100644 --- a/codex-rs/core/src/thread_manager_tests.rs +++ b/codex-rs/core/src/thread_manager_tests.rs @@ -439,8 +439,26 @@ args = ["dev", "cd /tmp && true"] }) .expect("environment context prompt item"); assert!(environment_context.contains("")); - assert!(environment_context.contains("")); - assert!(environment_context.contains("")); + let cwd = thread.session_configured.cwd.display().to_string(); + let dev_entry = format!( + r#" + {cwd} + "# + ); + let local_entry = format!( + r#" + {cwd} + "# + ); + let dev_position = environment_context + .find(&dev_entry) + .expect("dev environment entry"); + let local_position = environment_context + .find(&local_entry) + .expect("local environment entry"); + assert!(dev_position < local_position); + assert!(!environment_context.contains("\n ")); + assert!(!environment_context.contains("\n ")); } #[tokio::test] diff --git a/codex-rs/exec-server/src/environment.rs b/codex-rs/exec-server/src/environment.rs index 60d20b3cdd..7c1d109c6d 100644 --- a/codex-rs/exec-server/src/environment.rs +++ b/codex-rs/exec-server/src/environment.rs @@ -41,7 +41,7 @@ pub const CODEX_EXEC_SERVER_URL_ENV_VAR: &str = "CODEX_EXEC_SERVER_URL"; #[derive(Debug)] pub struct EnvironmentManager { default_environment: Option, - configured_environment_ids: Vec, + startup_environment_ids: Vec, environments: HashMap>, local_environment: Arc, } @@ -54,7 +54,7 @@ impl EnvironmentManager { pub fn default_for_tests() -> Self { Self { default_environment: Some(LOCAL_ENVIRONMENT_ID.to_string()), - configured_environment_ids: vec![LOCAL_ENVIRONMENT_ID.to_string()], + startup_environment_ids: vec![LOCAL_ENVIRONMENT_ID.to_string()], environments: HashMap::from([( LOCAL_ENVIRONMENT_ID.to_string(), Arc::new(Environment::default_for_tests()), @@ -67,7 +67,7 @@ impl EnvironmentManager { pub fn disabled_for_tests(local_runtime_paths: ExecServerRuntimePaths) -> Self { Self { default_environment: None, - configured_environment_ids: Vec::new(), + startup_environment_ids: Vec::new(), environments: HashMap::new(), local_environment: Arc::new(Environment::local(local_runtime_paths)), } @@ -137,6 +137,7 @@ impl EnvironmentManager { let EnvironmentProviderSnapshot { environments, default, + include_all_environments_by_default, } = snapshot; let mut configured_environment_ids = Vec::with_capacity(environments.len()); let mut environment_map = HashMap::with_capacity(environments.len()); @@ -168,10 +169,25 @@ impl EnvironmentManager { } }; let local_environment = Arc::new(Environment::local(local_runtime_paths)); + let startup_environment_ids = match default_environment.as_ref() { + None => Vec::new(), + Some(default_environment_id) if include_all_environments_by_default => { + let mut environment_ids = Vec::with_capacity(configured_environment_ids.len()); + environment_ids.push(default_environment_id.clone()); + environment_ids.extend( + configured_environment_ids + .iter() + .filter(|environment_id| environment_id.as_str() != default_environment_id) + .cloned(), + ); + environment_ids + } + Some(default_environment_id) => vec![default_environment_id.clone()], + }; Ok(Self { default_environment, - configured_environment_ids, + startup_environment_ids, environments: environment_map, local_environment, }) @@ -191,22 +207,7 @@ impl EnvironmentManager { /// Returns the ordered environment ids used for new thread startup. pub fn default_environment_ids(&self) -> Vec { - let Some(default_environment_id) = self.default_environment.as_deref() else { - return Vec::new(); - }; - if self.configured_environment_ids.len() <= 1 { - return vec![default_environment_id.to_string()]; - } - - let mut environment_ids = Vec::with_capacity(self.configured_environment_ids.len()); - environment_ids.push(default_environment_id.to_string()); - environment_ids.extend( - self.configured_environment_ids - .iter() - .filter(|environment_id| environment_id.as_str() != default_environment_id) - .cloned(), - ); - environment_ids + self.startup_environment_ids.clone() } /// Returns the local environment instance used for internal runtime work. @@ -488,6 +489,7 @@ mod tests { .expect("remote environment"), )], default: EnvironmentDefault::EnvironmentId(REMOTE_ENVIRONMENT_ID.to_string()), + include_all_environments_by_default: false, }, }; let manager = EnvironmentManager::from_provider(&provider, test_runtime_paths()) @@ -514,6 +516,7 @@ mod tests { snapshot: EnvironmentProviderSnapshot { environments: vec![("".to_string(), Environment::default_for_tests())], default: EnvironmentDefault::Disabled, + include_all_environments_by_default: false, }, }; let err = EnvironmentManager::from_provider(&provider, test_runtime_paths()) @@ -542,6 +545,7 @@ mod tests { ), ], default: EnvironmentDefault::EnvironmentId("devbox".to_string()), + include_all_environments_by_default: true, }, }; let manager = EnvironmentManager::from_provider(&provider, test_runtime_paths()) @@ -565,6 +569,7 @@ mod tests { Environment::default_for_tests(), )], default: EnvironmentDefault::Disabled, + include_all_environments_by_default: true, }, }; let manager = EnvironmentManager::from_provider(&provider, test_runtime_paths()) @@ -585,6 +590,7 @@ mod tests { Environment::default_for_tests(), )], default: EnvironmentDefault::EnvironmentId("missing".to_string()), + include_all_environments_by_default: true, }, }; let err = EnvironmentManager::from_provider(&provider, test_runtime_paths()) diff --git a/codex-rs/exec-server/src/environment_provider.rs b/codex-rs/exec-server/src/environment_provider.rs index bced67db55..a337d7e1bc 100644 --- a/codex-rs/exec-server/src/environment_provider.rs +++ b/codex-rs/exec-server/src/environment_provider.rs @@ -26,6 +26,7 @@ pub trait EnvironmentProvider: Send + Sync { pub struct EnvironmentProviderSnapshot { pub environments: Vec<(String, Environment)>, pub default: EnvironmentDefault, + pub include_all_environments_by_default: bool, } #[derive(Clone, Debug, PartialEq, Eq)] @@ -82,6 +83,7 @@ impl DefaultEnvironmentProvider { EnvironmentProviderSnapshot { environments, default, + include_all_environments_by_default: false, } } } @@ -132,6 +134,7 @@ mod tests { let EnvironmentProviderSnapshot { environments, default, + include_all_environments_by_default, } = snapshot; let environments: HashMap<_, _> = environments.into_iter().collect(); @@ -145,6 +148,7 @@ mod tests { default, EnvironmentDefault::EnvironmentId(LOCAL_ENVIRONMENT_ID.to_string()) ); + assert!(!include_all_environments_by_default); } #[tokio::test] @@ -158,6 +162,7 @@ mod tests { let EnvironmentProviderSnapshot { environments, default, + include_all_environments_by_default, } = snapshot; let environments: HashMap<_, _> = environments.into_iter().collect(); @@ -167,6 +172,7 @@ mod tests { default, EnvironmentDefault::EnvironmentId(LOCAL_ENVIRONMENT_ID.to_string()) ); + assert!(!include_all_environments_by_default); } #[tokio::test] @@ -180,12 +186,14 @@ mod tests { let EnvironmentProviderSnapshot { environments, default, + include_all_environments_by_default, } = snapshot; let environments: HashMap<_, _> = environments.into_iter().collect(); assert!(!environments[LOCAL_ENVIRONMENT_ID].is_remote()); assert!(!environments.contains_key(REMOTE_ENVIRONMENT_ID)); assert_eq!(default, EnvironmentDefault::Disabled); + assert!(!include_all_environments_by_default); } #[tokio::test] @@ -199,6 +207,7 @@ mod tests { let EnvironmentProviderSnapshot { environments, default, + include_all_environments_by_default, } = snapshot; let environments: HashMap<_, _> = environments.into_iter().collect(); @@ -213,6 +222,7 @@ mod tests { default, EnvironmentDefault::EnvironmentId(REMOTE_ENVIRONMENT_ID.to_string()) ); + assert!(!include_all_environments_by_default); } #[tokio::test] diff --git a/codex-rs/exec-server/src/environment_toml.rs b/codex-rs/exec-server/src/environment_toml.rs index 5ad236c1a0..bd45bcf832 100644 --- a/codex-rs/exec-server/src/environment_toml.rs +++ b/codex-rs/exec-server/src/environment_toml.rs @@ -100,6 +100,7 @@ impl EnvironmentProvider for TomlEnvironmentProvider { Ok(EnvironmentProviderSnapshot { environments, default: self.default.clone(), + include_all_environments_by_default: true, }) } } @@ -302,15 +303,6 @@ mod tests { #[tokio::test] async fn toml_provider_adds_implicit_local_and_configured_environments() { - let ssh_transport = ExecServerTransportParams::StdioCommand(StdioExecServerCommand { - program: "ssh".to_string(), - args: vec![ - "dev".to_string(), - "codex exec-server --listen stdio".to_string(), - ], - env: HashMap::from([("CODEX_LOG".to_string(), "debug".to_string())]), - cwd: None, - }); let provider = TomlEnvironmentProvider::new(EnvironmentsToml { default: Some("ssh-dev".to_string()), environments: vec![ @@ -344,7 +336,16 @@ mod tests { let EnvironmentProviderSnapshot { environments, default, + include_all_environments_by_default, } = snapshot; + let environment_ids: Vec<_> = environments + .iter() + .map(|(id, _environment)| id.as_str()) + .collect(); + assert_eq!( + environment_ids, + vec![LOCAL_ENVIRONMENT_ID, "devbox", "ssh-dev"] + ); let environments: HashMap<_, _> = environments.into_iter().collect(); assert!(!environments[LOCAL_ENVIRONMENT_ID].is_remote()); @@ -352,14 +353,13 @@ mod tests { environments["devbox"].exec_server_url(), Some("ws://127.0.0.1:8765") ); - assert_eq!(provider.environments[1].0, "ssh-dev"); - assert_eq!(provider.environments[1].1, ssh_transport); assert!(environments["ssh-dev"].is_remote()); assert_eq!(environments["ssh-dev"].exec_server_url(), None); assert_eq!( default, EnvironmentDefault::EnvironmentId("ssh-dev".to_string()) ); + assert!(include_all_environments_by_default); } #[tokio::test] diff --git a/codex-rs/thread-manager-sample/src/main.rs b/codex-rs/thread-manager-sample/src/main.rs index 3190f119bb..83656f7e0d 100644 --- a/codex-rs/thread-manager-sample/src/main.rs +++ b/codex-rs/thread-manager-sample/src/main.rs @@ -57,6 +57,7 @@ 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; @@ -119,6 +120,7 @@ async fn run_main(arg0_paths: Arg0DispatchPaths) -> anyhow::Result<()> { let environment_manager = Arc::new( EnvironmentManager::from_codex_home(config.codex_home.clone(), local_runtime_paths).await?, ); + let installation_id = resolve_installation_id(&config.codex_home).await?; let thread_manager = ThreadManager::new( &config, auth_manager, @@ -128,6 +130,7 @@ async fn run_main(arg0_paths: Arg0DispatchPaths) -> anyhow::Result<()> { state_db, Arc::clone(&thread_store), agent_graph_store, + installation_id, ); let NewThread {