diff --git a/codex-rs/app-server/tests/suite/v2/turn_start.rs b/codex-rs/app-server/tests/suite/v2/turn_start.rs index 586fcef87f..524b795b81 100644 --- a/codex-rs/app-server/tests/suite/v2/turn_start.rs +++ b/codex-rs/app-server/tests/suite/v2/turn_start.rs @@ -1991,7 +1991,7 @@ async fn turn_start_updates_sandbox_and_cwd_between_turns_v2() -> Result<()> { } #[tokio::test] -async fn turn_start_resolves_sticky_thread_environments_and_turn_overrides() -> Result<()> { +async fn turn_start_resolves_sticky_thread_local_environment_and_turn_overrides() -> Result<()> { let tmp = TempDir::new()?; let codex_home = tmp.path().join("codex_home"); std::fs::create_dir(&codex_home)?; @@ -2000,12 +2000,16 @@ async fn turn_start_resolves_sticky_thread_environments_and_turn_overrides() -> let server = create_mock_responses_server_repeating_assistant("done").await; create_config_toml(&codex_home, &server.uri(), "never", &BTreeMap::default())?; + std::fs::write( + codex_home.join("environments.toml"), + r#" +[[environments]] +id = "remote" +url = "ws://127.0.0.1:1" +"#, + )?; - let mut mcp = McpProcess::new_with_env( - &codex_home, - &[("CODEX_EXEC_SERVER_URL", Some("http://127.0.0.1:1"))], - ) - .await?; + let mut mcp = McpProcess::new(&codex_home).await?; timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; for case in [ @@ -2024,16 +2028,6 @@ async fn turn_start_resolves_sticky_thread_environments_and_turn_overrides() -> sticky: Some(&["local"]), turn: None, }, - EnvironmentSelectionCase { - name: "sticky_remote_turn_unset", - sticky: Some(&["remote"]), - turn: None, - }, - EnvironmentSelectionCase { - name: "sticky_local_remote_turn_unset", - sticky: Some(&["local", "remote"]), - turn: None, - }, EnvironmentSelectionCase { name: "sticky_local_turn_empty", sticky: Some(&["local"]), @@ -2044,21 +2038,6 @@ async fn turn_start_resolves_sticky_thread_environments_and_turn_overrides() -> sticky: Some(&[]), turn: Some(&["local"]), }, - EnvironmentSelectionCase { - name: "sticky_local_turn_remote", - sticky: Some(&["local"]), - turn: Some(&["remote"]), - }, - EnvironmentSelectionCase { - name: "sticky_remote_turn_local", - sticky: Some(&["remote"]), - turn: Some(&["local"]), - }, - EnvironmentSelectionCase { - name: "sticky_unset_turn_local_remote", - sticky: None, - turn: Some(&["local", "remote"]), - }, ] { run_environment_selection_case(&mut mcp, &workspace, case).await?; } diff --git a/codex-rs/core/src/environment_selection.rs b/codex-rs/core/src/environment_selection.rs index 640d813243..89808c27ee 100644 --- a/codex-rs/core/src/environment_selection.rs +++ b/codex-rs/core/src/environment_selection.rs @@ -85,6 +85,7 @@ 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; @@ -109,15 +110,41 @@ mod tests { ) .await; + assert_eq!( + default_thread_environment_selections(&manager, &cwd), + vec![TurnEnvironmentSelection { + environment_id: REMOTE_ENVIRONMENT_ID.to_string(), + cwd, + }] + ); + } + + #[tokio::test] + async fn toml_default_thread_environment_selections_include_local_and_remote() { + let temp_dir = tempfile::tempdir().expect("tempdir"); + std::fs::write( + temp_dir.path().join("environments.toml"), + r#" +[[environments]] +id = "remote" +url = "ws://127.0.0.1:8765" +"#, + ) + .expect("write environments.toml"); + let cwd = AbsolutePathBuf::current_dir().expect("cwd"); + let manager = EnvironmentManager::from_codex_home(temp_dir.path(), test_runtime_paths()) + .await + .expect("environment manager"); + assert_eq!( default_thread_environment_selections(&manager, &cwd), vec![ TurnEnvironmentSelection { - environment_id: REMOTE_ENVIRONMENT_ID.to_string(), + environment_id: LOCAL_ENVIRONMENT_ID.to_string(), cwd: cwd.clone(), }, TurnEnvironmentSelection { - environment_id: "local".to_string(), + 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 3bc5c77841..21fa03ad7f 100644 --- a/codex-rs/core/src/thread_manager_tests.rs +++ b/codex-rs/core/src/thread_manager_tests.rs @@ -293,7 +293,7 @@ async fn shutdown_all_threads_bounded_submits_shutdown_to_every_thread() { } #[tokio::test] -async fn start_thread_accepts_explicit_environment_when_default_environment_is_disabled() { +async fn start_thread_rejects_explicit_local_environment_when_default_provider_is_disabled() { let temp_dir = tempdir().expect("tempdir"); let mut config = test_config().await; config.codex_home = temp_dir.path().join("codex-home").abs(); @@ -319,7 +319,7 @@ async fn start_thread_accepts_explicit_environment_when_default_environment_is_d environment_manager, ); - let thread = manager + let result = manager .start_thread_with_options(StartThreadOptions { config: config.clone(), initial_history: InitialHistory::New, @@ -334,10 +334,14 @@ async fn start_thread_accepts_explicit_environment_when_default_environment_is_d cwd: config.cwd.clone(), }], }) - .await - .expect("explicit sticky environment should resolve by id"); + .await; + let err = match result { + Ok(_) => panic!("explicit local environment should not resolve when provider is disabled"), + Err(err) => err, + }; - assert_eq!(manager.list_thread_ids().await, vec![thread.thread_id]); + assert_eq!(err.to_string(), "unknown turn environment id `local`"); + assert!(manager.list_thread_ids().await.is_empty()); } #[tokio::test] diff --git a/codex-rs/exec-server/src/environment.rs b/codex-rs/exec-server/src/environment.rs index be83393bb5..b8983cd84c 100644 --- a/codex-rs/exec-server/src/environment.rs +++ b/codex-rs/exec-server/src/environment.rs @@ -142,10 +142,7 @@ impl EnvironmentManager { where P: EnvironmentProvider + ?Sized, { - Self::from_provider_snapshot( - provider.snapshot(&local_runtime_paths).await?, - local_runtime_paths, - ) + Self::from_provider_snapshot(provider.snapshot().await?, local_runtime_paths) } fn from_provider_snapshot( @@ -155,14 +152,28 @@ impl EnvironmentManager { let EnvironmentProviderSnapshot { environments, default, + include_local, } = snapshot; - let mut environment_map = HashMap::with_capacity(environments.len()); + let mut environment_map = + HashMap::with_capacity(environments.len() + usize::from(include_local)); + let local_environment = Arc::new(Environment::local(local_runtime_paths)); + if include_local { + environment_map.insert( + LOCAL_ENVIRONMENT_ID.to_string(), + Arc::clone(&local_environment), + ); + } for (id, environment) in environments { if id.is_empty() { return Err(ExecServerError::Protocol( "environment id cannot be empty".to_string(), )); } + if id == LOCAL_ENVIRONMENT_ID { + return Err(ExecServerError::Protocol(format!( + "environment id `{LOCAL_ENVIRONMENT_ID}` is reserved for EnvironmentManager" + ))); + } if environment_map .insert(id.clone(), Arc::new(environment)) .is_some() @@ -183,8 +194,6 @@ impl EnvironmentManager { Some(environment_id) } }; - let local_environment = Arc::new(Environment::local(local_runtime_paths)); - Ok(Self { default_environment, environments: environment_map, @@ -399,10 +408,7 @@ mod tests { #[async_trait::async_trait] impl EnvironmentProvider for TestEnvironmentProvider { - async fn snapshot( - &self, - _local_runtime_paths: &ExecServerRuntimePaths, - ) -> Result { + async fn snapshot(&self) -> Result { Ok(self.snapshot.clone()) } } @@ -431,14 +437,15 @@ mod tests { let environment = manager.default_environment().expect("default environment"); assert_eq!(manager.default_environment_id(), Some(LOCAL_ENVIRONMENT_ID)); - assert!(!environment.is_remote()); - assert!( - !manager + assert!(Arc::ptr_eq( + &environment, + &manager .get_environment(LOCAL_ENVIRONMENT_ID) .expect("local environment") - .is_remote() - ); + )); + assert!(Arc::ptr_eq(&environment, &manager.local_environment())); assert!(manager.get_environment(REMOTE_ENVIRONMENT_ID).is_none()); + assert!(!environment.is_remote()); } #[tokio::test] @@ -473,12 +480,7 @@ mod tests { .get_environment(REMOTE_ENVIRONMENT_ID) .expect("remote environment") )); - assert!( - !manager - .get_environment(LOCAL_ENVIRONMENT_ID) - .expect("local environment") - .is_remote() - ); + assert!(manager.get_environment(LOCAL_ENVIRONMENT_ID).is_none()); assert!(!manager.local_environment().is_remote()); } @@ -506,6 +508,7 @@ mod tests { .expect("remote environment"), )], default: EnvironmentDefault::EnvironmentId(REMOTE_ENVIRONMENT_ID.to_string()), + include_local: false, }, }; let manager = EnvironmentManager::from_provider(&provider, test_runtime_paths()) @@ -532,6 +535,7 @@ mod tests { snapshot: EnvironmentProviderSnapshot { environments: vec![("".to_string(), Environment::default_for_tests())], default: EnvironmentDefault::Disabled, + include_local: false, }, }; let err = EnvironmentManager::from_provider(&provider, test_runtime_paths()) @@ -544,22 +548,39 @@ mod tests { ); } + #[tokio::test] + async fn environment_manager_rejects_provider_supplied_local_environment() { + let provider = TestEnvironmentProvider { + snapshot: EnvironmentProviderSnapshot { + environments: vec![( + LOCAL_ENVIRONMENT_ID.to_string(), + Environment::default_for_tests(), + )], + default: EnvironmentDefault::Disabled, + include_local: false, + }, + }; + let err = EnvironmentManager::from_provider(&provider, test_runtime_paths()) + .await + .expect_err("local id should fail"); + + assert_eq!( + err.to_string(), + "exec-server protocol error: environment id `local` is reserved for EnvironmentManager" + ); + } + #[tokio::test] async fn environment_manager_uses_explicit_provider_default() { let provider = TestEnvironmentProvider { snapshot: EnvironmentProviderSnapshot { - environments: vec![ - ( - 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"), - ), - ], + environments: vec![( + "devbox".to_string(), + Environment::create_for_tests(Some("ws://127.0.0.1:8765".to_string())) + .expect("remote environment"), + )], default: EnvironmentDefault::EnvironmentId("devbox".to_string()), + include_local: true, }, }; let manager = EnvironmentManager::from_provider(&provider, test_runtime_paths()) @@ -579,10 +600,12 @@ mod tests { let provider = TestEnvironmentProvider { snapshot: EnvironmentProviderSnapshot { environments: vec![( - 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: EnvironmentDefault::Disabled, + include_local: true, }, }; let manager = EnvironmentManager::from_provider(&provider, test_runtime_paths()) @@ -591,7 +614,12 @@ mod tests { assert_eq!(manager.default_environment_id(), None); assert!(manager.default_environment().is_none()); - assert!(manager.get_environment(LOCAL_ENVIRONMENT_ID).is_some()); + assert!(Arc::ptr_eq( + &manager + .get_environment(LOCAL_ENVIRONMENT_ID) + .expect("local environment"), + &manager.local_environment() + )); } #[tokio::test] @@ -599,10 +627,12 @@ mod tests { let provider = TestEnvironmentProvider { snapshot: EnvironmentProviderSnapshot { environments: vec![( - 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: EnvironmentDefault::EnvironmentId("missing".to_string()), + include_local: true, }, }; let err = EnvironmentManager::from_provider(&provider, test_runtime_paths()) @@ -616,20 +646,23 @@ mod tests { } #[tokio::test] - async fn environment_manager_uses_provider_supplied_local_environment() { + async fn environment_manager_includes_local_for_default_provider_without_url() { let manager = EnvironmentManager::create_for_tests( /*exec_server_url*/ None, test_runtime_paths(), ) .await; + let environment = manager.default_environment().expect("default environment"); assert_eq!(manager.default_environment_id(), Some(LOCAL_ENVIRONMENT_ID)); - let provider_local = manager - .get_environment(LOCAL_ENVIRONMENT_ID) - .expect("provider local environment"); - assert!(!provider_local.is_remote()); - assert!(!manager.local_environment().is_remote()); - assert!(!Arc::ptr_eq(&provider_local, &manager.local_environment())); + assert!(Arc::ptr_eq( + &environment, + &manager + .get_environment(LOCAL_ENVIRONMENT_ID) + .expect("local environment") + )); + assert!(Arc::ptr_eq(&environment, &manager.local_environment())); + assert!(!environment.is_remote()); } #[tokio::test] @@ -641,7 +674,7 @@ mod tests { ) .await; - let environment = manager.default_environment().expect("default environment"); + let environment = manager.local_environment(); assert_eq!(environment.local_runtime_paths(), Some(&runtime_paths)); let manager = EnvironmentManager::create_for_tests( @@ -652,7 +685,7 @@ mod tests { .clone(), ) .await; - let environment = manager.default_environment().expect("default environment"); + let environment = manager.local_environment(); assert_eq!(environment.local_runtime_paths(), Some(&runtime_paths)); } @@ -665,20 +698,16 @@ mod tests { } #[tokio::test] - async fn environment_manager_keeps_default_provider_local_lookup_when_default_disabled() { + async fn environment_manager_omits_default_provider_local_lookup_when_default_disabled() { let manager = EnvironmentManager::create_for_tests(Some("none".to_string()), test_runtime_paths()) .await; assert!(manager.default_environment().is_none()); assert_eq!(manager.default_environment_id(), None); - assert!( - !manager - .get_environment(LOCAL_ENVIRONMENT_ID) - .expect("local environment") - .is_remote() - ); + assert!(manager.get_environment(LOCAL_ENVIRONMENT_ID).is_none()); assert!(manager.get_environment(REMOTE_ENVIRONMENT_ID).is_none()); + assert!(!manager.local_environment().is_remote()); } #[tokio::test] diff --git a/codex-rs/exec-server/src/environment_provider.rs b/codex-rs/exec-server/src/environment_provider.rs index bced67db55..7e132ee2b4 100644 --- a/codex-rs/exec-server/src/environment_provider.rs +++ b/codex-rs/exec-server/src/environment_provider.rs @@ -2,7 +2,6 @@ use async_trait::async_trait; use crate::Environment; use crate::ExecServerError; -use crate::ExecServerRuntimePaths; use crate::environment::CODEX_EXEC_SERVER_URL_ENV_VAR; use crate::environment::LOCAL_ENVIRONMENT_ID; use crate::environment::REMOTE_ENVIRONMENT_ID; @@ -11,21 +10,20 @@ use crate::environment::REMOTE_ENVIRONMENT_ID; /// /// Implementations own a startup snapshot containing both the available /// environment list in configured order and the default environment -/// selection. Providers that want the local environment to be addressable by -/// id should include it explicitly in the returned list. +/// selection. Providers should only return provider-owned remote environments; +/// `include_local` controls whether `EnvironmentManager` should add the local +/// environment to the snapshot. #[async_trait] pub trait EnvironmentProvider: Send + Sync { /// Returns the provider-owned environment startup snapshot. - async fn snapshot( - &self, - local_runtime_paths: &ExecServerRuntimePaths, - ) -> Result; + async fn snapshot(&self) -> Result; } #[derive(Clone, Debug)] pub struct EnvironmentProviderSnapshot { pub environments: Vec<(String, Environment)>, pub default: EnvironmentDefault, + pub include_local: bool, } #[derive(Clone, Debug, PartialEq, Eq)] @@ -51,26 +49,21 @@ impl DefaultEnvironmentProvider { Self::new(std::env::var(CODEX_EXEC_SERVER_URL_ENV_VAR).ok()) } - pub(crate) fn snapshot_inner( - &self, - local_runtime_paths: &ExecServerRuntimePaths, - ) -> EnvironmentProviderSnapshot { - let mut environments = vec![( - LOCAL_ENVIRONMENT_ID.to_string(), - Environment::local(local_runtime_paths.clone()), - )]; + pub(crate) fn snapshot_inner(&self) -> EnvironmentProviderSnapshot { + let mut environments = Vec::new(); let (exec_server_url, disabled) = normalize_exec_server_url(self.exec_server_url.clone()); if let Some(exec_server_url) = exec_server_url { environments.push(( REMOTE_ENVIRONMENT_ID.to_string(), - Environment::remote_inner(exec_server_url, Some(local_runtime_paths.clone())), + Environment::remote_inner(exec_server_url, /*local_runtime_paths*/ None), )); } let has_remote = environments .iter() .any(|(id, _environment)| id == REMOTE_ENVIRONMENT_ID); + let include_local = !disabled && !has_remote; let default = if disabled { EnvironmentDefault::Disabled } else if has_remote { @@ -82,17 +75,15 @@ impl DefaultEnvironmentProvider { EnvironmentProviderSnapshot { environments, default, + include_local, } } } #[async_trait] impl EnvironmentProvider for DefaultEnvironmentProvider { - async fn snapshot( - &self, - local_runtime_paths: &ExecServerRuntimePaths, - ) -> Result { - Ok(self.snapshot_inner(local_runtime_paths)) + async fn snapshot(&self) -> Result { + Ok(self.snapshot_inner()) } } @@ -111,35 +102,20 @@ mod tests { use pretty_assertions::assert_eq; use super::*; - use crate::ExecServerRuntimePaths; - - fn test_runtime_paths() -> ExecServerRuntimePaths { - ExecServerRuntimePaths::new( - std::env::current_exe().expect("current exe"), - /*codex_linux_sandbox_exe*/ None, - ) - .expect("runtime paths") - } #[tokio::test] - async fn default_provider_returns_local_environment_when_url_is_missing() { + async fn default_provider_requests_local_environment_when_url_is_missing() { let provider = DefaultEnvironmentProvider::new(/*exec_server_url*/ None); - let runtime_paths = test_runtime_paths(); - let snapshot = provider - .snapshot(&runtime_paths) - .await - .expect("environments"); + let snapshot = provider.snapshot().await.expect("environments"); let EnvironmentProviderSnapshot { environments, default, + include_local, } = snapshot; let environments: HashMap<_, _> = environments.into_iter().collect(); - assert!(!environments[LOCAL_ENVIRONMENT_ID].is_remote()); - assert_eq!( - environments[LOCAL_ENVIRONMENT_ID].local_runtime_paths(), - Some(&runtime_paths) - ); + assert!(include_local); + assert!(!environments.contains_key(LOCAL_ENVIRONMENT_ID)); assert!(!environments.contains_key(REMOTE_ENVIRONMENT_ID)); assert_eq!( default, @@ -148,20 +124,18 @@ mod tests { } #[tokio::test] - async fn default_provider_returns_local_environment_when_url_is_empty() { + async fn default_provider_requests_local_environment_when_url_is_empty() { let provider = DefaultEnvironmentProvider::new(Some(String::new())); - let runtime_paths = test_runtime_paths(); - let snapshot = provider - .snapshot(&runtime_paths) - .await - .expect("environments"); + let snapshot = provider.snapshot().await.expect("environments"); let EnvironmentProviderSnapshot { environments, default, + include_local, } = snapshot; let environments: HashMap<_, _> = environments.into_iter().collect(); - assert!(!environments[LOCAL_ENVIRONMENT_ID].is_remote()); + assert!(include_local); + assert!(!environments.contains_key(LOCAL_ENVIRONMENT_ID)); assert!(!environments.contains_key(REMOTE_ENVIRONMENT_ID)); assert_eq!( default, @@ -170,20 +144,18 @@ mod tests { } #[tokio::test] - async fn default_provider_returns_local_environment_for_none_value() { + async fn default_provider_omits_local_environment_for_none_value() { let provider = DefaultEnvironmentProvider::new(Some("none".to_string())); - let runtime_paths = test_runtime_paths(); - let snapshot = provider - .snapshot(&runtime_paths) - .await - .expect("environments"); + let snapshot = provider.snapshot().await.expect("environments"); let EnvironmentProviderSnapshot { environments, default, + include_local, } = snapshot; let environments: HashMap<_, _> = environments.into_iter().collect(); - assert!(!environments[LOCAL_ENVIRONMENT_ID].is_remote()); + assert!(!include_local); + assert!(!environments.contains_key(LOCAL_ENVIRONMENT_ID)); assert!(!environments.contains_key(REMOTE_ENVIRONMENT_ID)); assert_eq!(default, EnvironmentDefault::Disabled); } @@ -191,18 +163,16 @@ mod tests { #[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 snapshot = provider - .snapshot(&runtime_paths) - .await - .expect("environments"); + let snapshot = provider.snapshot().await.expect("environments"); let EnvironmentProviderSnapshot { environments, default, + include_local, } = snapshot; let environments: HashMap<_, _> = environments.into_iter().collect(); - assert!(!environments[LOCAL_ENVIRONMENT_ID].is_remote()); + assert!(!include_local); + assert!(!environments.contains_key(LOCAL_ENVIRONMENT_ID)); let remote_environment = &environments[REMOTE_ENVIRONMENT_ID]; assert!(remote_environment.is_remote()); assert_eq!( @@ -218,11 +188,7 @@ mod tests { #[tokio::test] 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 snapshot = provider - .snapshot(&runtime_paths) - .await - .expect("environments"); + let snapshot = provider.snapshot().await.expect("environments"); let environments: HashMap<_, _> = snapshot.environments.into_iter().collect(); assert_eq!( diff --git a/codex-rs/exec-server/src/environment_toml.rs b/codex-rs/exec-server/src/environment_toml.rs index a1f328377a..2f5fd97790 100644 --- a/codex-rs/exec-server/src/environment_toml.rs +++ b/codex-rs/exec-server/src/environment_toml.rs @@ -11,7 +11,6 @@ use crate::DefaultEnvironmentProvider; use crate::Environment; use crate::EnvironmentProvider; use crate::ExecServerError; -use crate::ExecServerRuntimePaths; use crate::client_api::ExecServerTransportParams; use crate::client_api::StdioExecServerCommand; use crate::environment::LOCAL_ENVIRONMENT_ID; @@ -78,21 +77,14 @@ impl TomlEnvironmentProvider { #[async_trait] impl EnvironmentProvider for TomlEnvironmentProvider { - async fn snapshot( - &self, - local_runtime_paths: &ExecServerRuntimePaths, - ) -> Result { - let mut environments = Vec::with_capacity(self.environments.len() + 1); - environments.push(( - LOCAL_ENVIRONMENT_ID.to_string(), - Environment::local(local_runtime_paths.clone()), - )); + async fn snapshot(&self) -> Result { + let mut environments = Vec::with_capacity(self.environments.len()); for (id, transport_params) in &self.environments { environments.push(( id.clone(), Environment::remote_with_transport( transport_params.clone(), - Some(local_runtime_paths.clone()), + /*local_runtime_paths*/ None, ), )); } @@ -100,6 +92,7 @@ impl EnvironmentProvider for TomlEnvironmentProvider { Ok(EnvironmentProviderSnapshot { environments, default: self.default.clone(), + include_local: true, }) } } @@ -292,16 +285,8 @@ mod tests { use super::*; - fn test_runtime_paths() -> ExecServerRuntimePaths { - ExecServerRuntimePaths::new( - std::env::current_exe().expect("current exe"), - /*codex_linux_sandbox_exe*/ None, - ) - .expect("runtime paths") - } - #[tokio::test] - async fn toml_provider_adds_implicit_local_and_configured_environments() { + async fn toml_provider_includes_local_and_adds_configured_environments() { let provider = TomlEnvironmentProvider::new(EnvironmentsToml { default: Some("ssh-dev".to_string()), environments: vec![ @@ -326,27 +311,22 @@ mod tests { ], }) .expect("provider"); - let runtime_paths = test_runtime_paths(); - let snapshot = provider - .snapshot(&runtime_paths) - .await - .expect("environments"); + let snapshot = provider.snapshot().await.expect("environments"); let EnvironmentProviderSnapshot { environments, default, + include_local, } = 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"] - ); + assert_eq!(environment_ids, vec!["devbox", "ssh-dev"]); let environments: HashMap<_, _> = environments.into_iter().collect(); - assert!(!environments[LOCAL_ENVIRONMENT_ID].is_remote()); + assert!(include_local); + assert!(!environments.contains_key(LOCAL_ENVIRONMENT_ID)); assert_eq!( environments["devbox"].exec_server_url(), Some("ws://127.0.0.1:8765") @@ -362,11 +342,9 @@ mod tests { #[tokio::test] async fn toml_provider_default_omitted_selects_local() { let provider = TomlEnvironmentProvider::new(EnvironmentsToml::default()).expect("provider"); - let snapshot = provider - .snapshot(&test_runtime_paths()) - .await - .expect("environments"); + let snapshot = provider.snapshot().await.expect("environments"); + assert!(snapshot.include_local); assert_eq!( snapshot.default, EnvironmentDefault::EnvironmentId(LOCAL_ENVIRONMENT_ID.to_string()) @@ -380,11 +358,9 @@ mod tests { environments: Vec::new(), }) .expect("provider"); - let snapshot = provider - .snapshot(&test_runtime_paths()) - .await - .expect("environments"); + let snapshot = provider.snapshot().await.expect("environments"); + assert!(snapshot.include_local); assert_eq!(snapshot.default, EnvironmentDefault::Disabled); } @@ -681,17 +657,15 @@ default = "none" let provider = environment_provider_from_codex_home(codex_home.path()).expect("environment provider"); - let snapshot = provider - .snapshot(&test_runtime_paths()) - .await - .expect("environments"); + let snapshot = provider.snapshot().await.expect("environments"); let environment_ids: Vec<_> = snapshot .environments .into_iter() .map(|(id, _environment)| id) .collect(); - assert!(environment_ids.contains(&LOCAL_ENVIRONMENT_ID.to_string())); + assert!(snapshot.include_local); + assert!(!environment_ids.contains(&LOCAL_ENVIRONMENT_ID.to_string())); assert_eq!(snapshot.default, EnvironmentDefault::Disabled); } @@ -702,16 +676,18 @@ default = "none" let provider = environment_provider_from_codex_home(codex_home.path()).expect("environment provider"); - let snapshot = provider - .snapshot(&test_runtime_paths()) - .await - .expect("environments"); + let snapshot = provider.snapshot().await.expect("environments"); let environment_ids: Vec<_> = snapshot .environments .into_iter() .map(|(id, _environment)| id) .collect(); - assert!(environment_ids.contains(&LOCAL_ENVIRONMENT_ID.to_string())); + assert!(snapshot.include_local); + assert!(!environment_ids.contains(&LOCAL_ENVIRONMENT_ID.to_string())); + assert_eq!( + snapshot.default, + EnvironmentDefault::EnvironmentId(LOCAL_ENVIRONMENT_ID.to_string()) + ); } }