From 9bccdb87273c49de2e37cc6d1bf8c13554ade9df Mon Sep 17 00:00:00 2001 From: Thibault Sottiaux Date: Tue, 21 Apr 2026 13:31:36 -0700 Subject: [PATCH] fix network context removal --- .../core/src/context/environment_context.rs | 20 ++++- .../src/context/environment_context_tests.rs | 25 +++++++ codex-rs/core/src/session/tests.rs | 74 +++++++++++++++++++ 3 files changed, 117 insertions(+), 2 deletions(-) diff --git a/codex-rs/core/src/context/environment_context.rs b/codex-rs/core/src/context/environment_context.rs index c676bfb9d9..7ce71111f0 100644 --- a/codex-rs/core/src/context/environment_context.rs +++ b/codex-rs/core/src/context/environment_context.rs @@ -18,6 +18,7 @@ pub(crate) struct EnvironmentContext { #[derive(Debug, Clone, PartialEq, Eq, Default)] pub(crate) struct NetworkContext { + enabled: bool, allowed_domains: Vec, denied_domains: Vec, } @@ -25,10 +26,19 @@ pub(crate) struct NetworkContext { impl NetworkContext { pub(crate) fn new(allowed_domains: Vec, denied_domains: Vec) -> Self { Self { + enabled: true, allowed_domains, denied_domains, } } + + fn disabled() -> Self { + Self { + enabled: false, + allowed_domains: Vec::new(), + denied_domains: Vec::new(), + } + } } impl EnvironmentContext { @@ -79,7 +89,10 @@ impl EnvironmentContext { _ => None, }; let network = if before_network != after.network { - after.network.clone() + after + .network + .clone() + .or_else(|| Some(NetworkContext::disabled())) } else { before_network }; @@ -180,7 +193,7 @@ impl ContextualUserFragment for EnvironmentContext { lines.push(format!(" {timezone}")); } match &self.network { - Some(network) => { + Some(network) if network.enabled => { lines.push(" ".to_string()); for allowed in &network.allowed_domains { lines.push(format!(" {allowed}")); @@ -190,6 +203,9 @@ impl ContextualUserFragment for EnvironmentContext { } lines.push(" ".to_string()); } + Some(_) => { + lines.push(" ".to_string()); + } None => { // TODO(mbolin): Include this line if it helps the model. // lines.push(" ".to_string()); diff --git a/codex-rs/core/src/context/environment_context_tests.rs b/codex-rs/core/src/context/environment_context_tests.rs index 84f8c0d99f..2d9835d422 100644 --- a/codex-rs/core/src/context/environment_context_tests.rs +++ b/codex-rs/core/src/context/environment_context_tests.rs @@ -72,6 +72,31 @@ fn serialize_environment_context_with_network() { assert_eq!(context.render(), expected); } +#[test] +fn serialize_environment_context_with_disabled_network() { + let context = EnvironmentContext::new( + Some(test_path_buf("/repo")), + fake_shell_name(), + Some("2026-02-26".to_string()), + Some("America/Los_Angeles".to_string()), + Some(NetworkContext::disabled()), + /*subagents*/ None, + ); + + let expected = format!( + r#" + {} + bash + 2026-02-26 + America/Los_Angeles + +"#, + test_path_buf("/repo").display() + ); + + assert_eq!(context.render(), expected); +} + #[test] fn serialize_read_only_environment_context() { let context = EnvironmentContext::new( diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 97b6eba8b3..2bf5af6c4d 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -4764,6 +4764,80 @@ async fn build_settings_update_items_emits_environment_item_for_network_changes( assert!(environment_update.contains("blocked.example.com")); } +#[tokio::test] +async fn build_settings_update_items_marks_network_disabled_when_network_is_removed() { + let (session, mut previous_context) = make_session_and_context().await; + + let mut config = (*previous_context.config).clone(); + let mut requirements = config.config_layer_stack.requirements().clone(); + requirements.network = Some(Sourced::new( + NetworkConstraints { + domains: Some(NetworkDomainPermissionsToml { + entries: std::collections::BTreeMap::from([( + "api.example.com".to_string(), + NetworkDomainPermissionToml::Allow, + )]), + }), + ..Default::default() + }, + RequirementSource::CloudRequirements, + )); + let layers = config + .config_layer_stack + .get_layers( + ConfigLayerStackOrdering::LowestPrecedenceFirst, + /*include_disabled*/ true, + ) + .into_iter() + .cloned() + .collect(); + config.config_layer_stack = ConfigLayerStack::new( + layers, + requirements, + config.config_layer_stack.requirements_toml().clone(), + ) + .expect("rebuild previous config layer stack with network requirements"); + previous_context.config = Arc::new(config); + + let mut current_context = previous_context + .with_model( + previous_context.model_info.slug.clone(), + &session.services.models_manager, + ) + .await; + let mut config = (*current_context.config).clone(); + let mut requirements = config.config_layer_stack.requirements().clone(); + requirements.network = None; + let layers = config + .config_layer_stack + .get_layers( + ConfigLayerStackOrdering::LowestPrecedenceFirst, + /*include_disabled*/ true, + ) + .into_iter() + .cloned() + .collect(); + config.config_layer_stack = ConfigLayerStack::new( + layers, + requirements, + config.config_layer_stack.requirements_toml().clone(), + ) + .expect("rebuild current config layer stack without network requirements"); + current_context.config = Arc::new(config); + + let reference_context_item = previous_context.to_turn_context_item(); + let update_items = session + .build_settings_update_items(Some(&reference_context_item), ¤t_context) + .await; + + let environment_update = user_input_texts(&update_items) + .into_iter() + .find(|text| text.contains("")) + .expect("environment update item should be emitted"); + assert!(environment_update.contains("")); + assert!(!environment_update.contains("api.example.com")); +} + #[tokio::test] async fn build_settings_update_items_emits_environment_item_for_time_changes() { let (session, previous_context) = make_session_and_context().await;