mirror of
https://github.com/openai/codex.git
synced 2026-11-02 19:43:24 +00:00
preserve environment update comparisons
This commit is contained in:
@@ -28,6 +28,10 @@ pub(crate) struct EnvironmentsState {
|
||||
filesystem: Option<FileSystemContext>,
|
||||
#[serde(skip)]
|
||||
subagents: Option<String>,
|
||||
/// Whether the render-only values came from a `TurnContext` or persisted
|
||||
/// `TurnContextItem` and can participate in turn-to-turn comparison.
|
||||
#[serde(skip)]
|
||||
turn_context_values_comparable: bool,
|
||||
}
|
||||
|
||||
impl PartialEq for EnvironmentsState {
|
||||
@@ -67,6 +71,7 @@ impl EnvironmentsState {
|
||||
&turn_context.config.effective_workspace_roots(),
|
||||
)),
|
||||
subagents: None,
|
||||
turn_context_values_comparable: true,
|
||||
};
|
||||
for environment in &turn_context.environments.starting {
|
||||
state
|
||||
@@ -101,6 +106,7 @@ impl EnvironmentsState {
|
||||
&workspace_roots_from_turn_context_item(turn_context_item),
|
||||
)),
|
||||
subagents: None,
|
||||
turn_context_values_comparable: true,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -140,8 +146,15 @@ impl WorldStateSection for EnvironmentsState {
|
||||
fn render_diff(&self, previous: &Self) -> Option<ResponseItem> {
|
||||
let legacy_single =
|
||||
is_legacy_single(&self.environments) && previous.environments.len() <= 1;
|
||||
let turn_context_values_changed = self.turn_context_values_comparable
|
||||
&& previous.turn_context_values_comparable
|
||||
&& (self.current_date != previous.current_date
|
||||
|| self.timezone != previous.timezone
|
||||
|| self.network != previous.network
|
||||
|| self.filesystem != previous.filesystem);
|
||||
if legacy_single
|
||||
&& self.environments.values().next() == previous.environments.values().next()
|
||||
&& !turn_context_values_changed
|
||||
{
|
||||
return None;
|
||||
}
|
||||
@@ -160,7 +173,7 @@ impl WorldStateSection for EnvironmentsState {
|
||||
.map(|id| (id.clone(), EnvironmentUpdate::Unavailable)),
|
||||
);
|
||||
}
|
||||
(!updates.is_empty()).then(|| {
|
||||
(!updates.is_empty() || turn_context_values_changed).then(|| {
|
||||
ContextualUserFragment::into(RenderedEnvironments {
|
||||
updates,
|
||||
legacy_single,
|
||||
|
||||
@@ -55,6 +55,7 @@ fn environment_state(
|
||||
network,
|
||||
filesystem: None,
|
||||
subagents,
|
||||
turn_context_values_comparable: true,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -3,6 +3,7 @@ use crate::context::world_state::WorldState;
|
||||
use anyhow::Result;
|
||||
use codex_protocol::models::ContentItem;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::permissions::NetworkSandboxPolicy;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
|
||||
@@ -29,6 +30,7 @@ fn renders_full_environment_state() -> Result<()> {
|
||||
&[],
|
||||
)),
|
||||
subagents: Some("task_1: running\ntask_2: completed".to_string()),
|
||||
turn_context_values_comparable: true,
|
||||
};
|
||||
|
||||
let mut world_state = WorldState::default();
|
||||
@@ -111,7 +113,7 @@ fn renders_only_changed_environments() -> Result<()> {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unchanged_environments_do_not_render_a_diff() -> Result<()> {
|
||||
fn persisted_turn_context_values_render_a_diff() -> Result<()> {
|
||||
let environments = EnvironmentsState {
|
||||
environments: [("laptop".to_string(), available("file:///repo", "zsh")?)]
|
||||
.into_iter()
|
||||
@@ -130,7 +132,7 @@ fn unchanged_environments_do_not_render_a_diff() -> Result<()> {
|
||||
&PermissionProfile::Disabled,
|
||||
&[],
|
||||
)),
|
||||
subagents: Some("task_1: running".to_string()),
|
||||
turn_context_values_comparable: true,
|
||||
..environments.clone()
|
||||
});
|
||||
let mut current = WorldState::default();
|
||||
@@ -141,12 +143,27 @@ fn unchanged_environments_do_not_render_a_diff() -> Result<()> {
|
||||
vec!["new.example.com".to_string()],
|
||||
vec!["blocked.example.com".to_string()],
|
||||
)),
|
||||
filesystem: None,
|
||||
subagents: Some("task_1: completed".to_string()),
|
||||
filesystem: Some(FileSystemContext::from_permission_profile(
|
||||
&PermissionProfile::External {
|
||||
network: NetworkSandboxPolicy::Restricted,
|
||||
},
|
||||
&[],
|
||||
)),
|
||||
turn_context_values_comparable: true,
|
||||
..environments
|
||||
});
|
||||
|
||||
assert_eq!(Vec::<ResponseItem>::new(), current.render_diff(&previous));
|
||||
assert_eq!(
|
||||
vec![user_message(
|
||||
r#"<environment_context>
|
||||
<current_date>2026-06-20</current_date>
|
||||
<timezone>America/Los_Angeles</timezone>
|
||||
<network enabled="true"><allowed>new.example.com</allowed><denied>blocked.example.com</denied></network>
|
||||
<filesystem><permission_profile type="external"><file_system type="external" /></permission_profile></filesystem>
|
||||
</environment_context>"#,
|
||||
)],
|
||||
current.render_diff(&previous),
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -189,6 +206,7 @@ fn loaded_environment_state_produces_no_diff_with_live_state() -> Result<()> {
|
||||
&[],
|
||||
)),
|
||||
subagents: Some("task_1: running".to_string()),
|
||||
turn_context_values_comparable: true,
|
||||
};
|
||||
|
||||
let stored = serde_json::to_value(&live_state)?;
|
||||
|
||||
@@ -7325,6 +7325,68 @@ async fn spawn_task_does_not_update_previous_turn_settings_for_non_run_turn_task
|
||||
assert_eq!(sess.previous_turn_settings().await, None);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn build_settings_update_items_emits_environment_item_for_network_changes() {
|
||||
let (session, previous_context) = make_session_and_context().await;
|
||||
let previous_context = Arc::new(previous_context);
|
||||
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 = Some(Sourced::new(
|
||||
NetworkConstraints {
|
||||
domains: Some(NetworkDomainPermissionsToml {
|
||||
entries: std::collections::BTreeMap::from([
|
||||
(
|
||||
"api.example.com".to_string(),
|
||||
NetworkDomainPermissionToml::Allow,
|
||||
),
|
||||
(
|
||||
"blocked.example.com".to_string(),
|
||||
NetworkDomainPermissionToml::Deny,
|
||||
),
|
||||
]),
|
||||
}),
|
||||
..Default::default()
|
||||
},
|
||||
RequirementSource::LegacyManagedConfigTomlFromMdm,
|
||||
));
|
||||
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 config layer stack with 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=\"true\"><allowed>api.example.com</allowed><denied>blocked.example.com</denied></network>"
|
||||
));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn build_settings_update_items_emits_environment_item_for_cwd_changes() {
|
||||
let (session, previous_context) = make_session_and_context().await;
|
||||
@@ -7360,6 +7422,32 @@ async fn build_settings_update_items_emits_environment_item_for_cwd_changes() {
|
||||
assert!(!environment_update.contains("<environments>"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn build_settings_update_items_emits_environment_item_for_time_changes() {
|
||||
let (session, previous_context) = make_session_and_context().await;
|
||||
let previous_context = Arc::new(previous_context);
|
||||
let mut current_context = previous_context
|
||||
.with_model(
|
||||
previous_context.model_info.slug.clone(),
|
||||
&session.services.models_manager,
|
||||
)
|
||||
.await;
|
||||
current_context.current_date = Some("2026-02-27".to_string());
|
||||
current_context.timezone = Some("Europe/Berlin".to_string());
|
||||
|
||||
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("<current_date>2026-02-27</current_date>"));
|
||||
assert!(environment_update.contains("<timezone>Europe/Berlin</timezone>"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn build_settings_update_items_omits_environment_item_when_disabled() {
|
||||
let (session, previous_context) = make_session_and_context().await;
|
||||
|
||||
Reference in New Issue
Block a user