mirror of
https://github.com/openai/codex.git
synced 2026-09-08 15:50:34 +00:00
fix network context removal
This commit is contained in:
@@ -18,6 +18,7 @@ pub(crate) struct EnvironmentContext {
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Default)]
|
||||
pub(crate) struct NetworkContext {
|
||||
enabled: bool,
|
||||
allowed_domains: Vec<String>,
|
||||
denied_domains: Vec<String>,
|
||||
}
|
||||
@@ -25,10 +26,19 @@ pub(crate) struct NetworkContext {
|
||||
impl NetworkContext {
|
||||
pub(crate) fn new(allowed_domains: Vec<String>, denied_domains: Vec<String>) -> 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>{timezone}</timezone>"));
|
||||
}
|
||||
match &self.network {
|
||||
Some(network) => {
|
||||
Some(network) if network.enabled => {
|
||||
lines.push(" <network enabled=\"true\">".to_string());
|
||||
for allowed in &network.allowed_domains {
|
||||
lines.push(format!(" <allowed>{allowed}</allowed>"));
|
||||
@@ -190,6 +203,9 @@ impl ContextualUserFragment for EnvironmentContext {
|
||||
}
|
||||
lines.push(" </network>".to_string());
|
||||
}
|
||||
Some(_) => {
|
||||
lines.push(" <network enabled=\"false\" />".to_string());
|
||||
}
|
||||
None => {
|
||||
// TODO(mbolin): Include this line if it helps the model.
|
||||
// lines.push(" <network enabled=\"false\" />".to_string());
|
||||
|
||||
@@ -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#"<environment_context>
|
||||
<cwd>{}</cwd>
|
||||
<shell>bash</shell>
|
||||
<current_date>2026-02-26</current_date>
|
||||
<timezone>America/Los_Angeles</timezone>
|
||||
<network enabled="false" />
|
||||
</environment_context>"#,
|
||||
test_path_buf("/repo").display()
|
||||
);
|
||||
|
||||
assert_eq!(context.render(), expected);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn serialize_read_only_environment_context() {
|
||||
let context = EnvironmentContext::new(
|
||||
|
||||
@@ -4764,6 +4764,80 @@ async fn build_settings_update_items_emits_environment_item_for_network_changes(
|
||||
assert!(environment_update.contains("<denied>blocked.example.com</denied>"));
|
||||
}
|
||||
|
||||
#[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("<environment_context>"))
|
||||
.expect("environment update item should be emitted");
|
||||
assert!(environment_update.contains("<network enabled=\"false\" />"));
|
||||
assert!(!environment_update.contains("<allowed>api.example.com</allowed>"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn build_settings_update_items_emits_environment_item_for_time_changes() {
|
||||
let (session, previous_context) = make_session_and_context().await;
|
||||
|
||||
Reference in New Issue
Block a user