From e2058d91cd6244828ad147f632f43f2b60825bc4 Mon Sep 17 00:00:00 2001 From: pakrym-oai Date: Sat, 20 Jun 2026 12:13:01 -0700 Subject: [PATCH] persist comparable environment context --- .../src/context/world_state/environment.rs | 31 +++--- .../world_state/environment_render_tests.rs | 1 - .../world_state/environment_support.rs | 96 +++++++++++++------ .../context/world_state/environment_tests.rs | 26 +++-- 4 files changed, 96 insertions(+), 58 deletions(-) diff --git a/codex-rs/core/src/context/world_state/environment.rs b/codex-rs/core/src/context/world_state/environment.rs index aa069d7a28..5d06e3b79e 100644 --- a/codex-rs/core/src/context/world_state/environment.rs +++ b/codex-rs/core/src/context/world_state/environment.rs @@ -13,30 +13,29 @@ use serde::Deserialize; use serde::Serialize; use std::collections::BTreeMap; -/// Live environment values; only identifiers and working directories persist. +/// Environment values visible to the model. #[derive(Clone, Debug, Default, Serialize, Deserialize)] -#[serde(transparent)] pub(crate) struct EnvironmentsState { environments: BTreeMap, - #[serde(skip)] + #[serde(default, skip_serializing_if = "Option::is_none")] current_date: Option, - #[serde(skip)] + #[serde(default, skip_serializing_if = "Option::is_none")] timezone: Option, - #[serde(skip)] + #[serde(default, skip_serializing_if = "Option::is_none")] network: Option, - #[serde(skip)] + #[serde(default, skip_serializing_if = "Option::is_none")] filesystem: Option, #[serde(skip)] subagents: Option, - /// 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 { fn eq(&self, other: &Self) -> bool { self.environments == other.environments + && self.current_date == other.current_date + && self.timezone == other.timezone + && self.network == other.network + && self.filesystem == other.filesystem } } @@ -71,7 +70,6 @@ impl EnvironmentsState { &turn_context.config.effective_workspace_roots(), )), subagents: None, - turn_context_values_comparable: true, }; for environment in &turn_context.environments.starting { state @@ -106,7 +104,6 @@ impl EnvironmentsState { &workspace_roots_from_turn_context_item(turn_context_item), )), subagents: None, - turn_context_values_comparable: true, } } @@ -146,12 +143,10 @@ impl WorldStateSection for EnvironmentsState { fn render_diff(&self, previous: &Self) -> Option { 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); + let turn_context_values_changed = 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 diff --git a/codex-rs/core/src/context/world_state/environment_render_tests.rs b/codex-rs/core/src/context/world_state/environment_render_tests.rs index adf5392d89..e3c0468065 100644 --- a/codex-rs/core/src/context/world_state/environment_render_tests.rs +++ b/codex-rs/core/src/context/world_state/environment_render_tests.rs @@ -55,7 +55,6 @@ fn environment_state( network, filesystem: None, subagents, - turn_context_values_comparable: true, } } diff --git a/codex-rs/core/src/context/world_state/environment_support.rs b/codex-rs/core/src/context/world_state/environment_support.rs index 7d00790501..93382508e2 100644 --- a/codex-rs/core/src/context/world_state/environment_support.rs +++ b/codex-rs/core/src/context/world_state/environment_support.rs @@ -5,26 +5,30 @@ use codex_protocol::permissions::FileSystemPath; use codex_protocol::permissions::FileSystemSandboxEntry; use codex_protocol::permissions::FileSystemSpecialPath; use codex_utils_absolute_path::AbsolutePathBuf; -use std::collections::HashSet; +use indexmap::IndexMap; +use serde::Deserialize; +use serde::Serialize; use std::path::PathBuf; -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub(crate) struct FileSystemContext { - workspace_roots: Vec, + workspace_roots: IndexMap, permission_profile: FileSystemPermissionProfileContext, } -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] enum FileSystemPermissionProfileContext { Managed(ManagedFileSystemContext), Disabled, External, } -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] enum ManagedFileSystemContext { Restricted { - entries: Vec, + entries: IndexMap, glob_scan_max_depth: Option, }, Unrestricted, @@ -40,7 +44,7 @@ impl FileSystemContext { .materialize_project_roots_with_workspace_roots(workspace_roots); let workspace_roots = workspace_roots .iter() - .map(|root| root.to_string_lossy().into_owned()) + .map(|root| (root.to_string_lossy().into_owned(), true)) .collect(); let permission_profile = match permission_profile { PermissionProfile::Managed { file_system, .. } => { @@ -61,7 +65,7 @@ impl FileSystemContext { let mut rendered = "".to_string(); if !self.workspace_roots.is_empty() { rendered.push_str(""); - for root in &self.workspace_roots { + for root in self.workspace_roots.keys() { push_text_element(&mut rendered, "root", root); } rendered.push_str(""); @@ -76,15 +80,15 @@ impl From for ManagedFileSystemContext { fn from(file_system: ManagedFileSystemPermissions) -> Self { match file_system { ManagedFileSystemPermissions::Restricted { - mut entries, + entries, glob_scan_max_depth, - } => { - dedupe_file_system_entries(&mut entries); - Self::Restricted { - entries, - glob_scan_max_depth: glob_scan_max_depth.map(usize::from), - } - } + } => Self::Restricted { + entries: entries + .into_iter() + .map(|entry| (file_system_entry_key(&entry), entry)) + .collect(), + glob_scan_max_depth: glob_scan_max_depth.map(usize::from), + }, ManagedFileSystemPermissions::Unrestricted => Self::Unrestricted, } } @@ -129,7 +133,7 @@ impl ManagedFileSystemContext { rendered.push_str(&format!(" glob_scan_max_depth=\"{glob_scan_max_depth}\"")); } rendered.push('>'); - for entry in entries { + for entry in entries.values() { render_file_system_entry(rendered, entry); } rendered.push_str(""); @@ -186,9 +190,13 @@ fn render_special_path_with_subpath(base: &str, subpath: &Option) -> St } } -fn dedupe_file_system_entries(entries: &mut Vec) { - let mut seen = HashSet::new(); - entries.retain(|entry| seen.insert(entry.clone())); +fn file_system_entry_key(entry: &FileSystemSandboxEntry) -> String { + let path = match &entry.path { + FileSystemPath::Path { path } => format!("path:{}", path.to_string_lossy()), + FileSystemPath::GlobPattern { pattern } => format!("glob:{pattern}"), + FileSystemPath::Special { value } => format!("special:{}", render_special_path(value)), + }; + format!("{}:{path}", entry.access) } fn push_text_element(rendered: &mut String, name: &str, value: &str) { @@ -210,35 +218,61 @@ pub(super) fn push_xml_escaped_text(rendered: &mut String, value: &str) { } } -#[derive(Debug, Clone, PartialEq, Eq, Default)] +#[derive(Debug, Clone, PartialEq, Eq, Default, Serialize, Deserialize)] pub(crate) struct NetworkContext { - allowed_domains: Vec, - denied_domains: Vec, + domains: IndexMap, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +enum NetworkDomainAccess { + Allow, + Deny, } impl NetworkContext { pub(crate) fn new(allowed_domains: Vec, denied_domains: Vec) -> Self { Self { - allowed_domains, - denied_domains, + domains: allowed_domains + .into_iter() + .map(|domain| (domain, NetworkDomainAccess::Allow)) + .chain( + denied_domains + .into_iter() + .map(|domain| (domain, NetworkDomainAccess::Deny)), + ) + .collect(), } } pub(super) fn render(&self) -> String { let mut rendered = "".to_string(); - Self::push_rendered_domain_element(&mut rendered, "allowed", &self.allowed_domains); - Self::push_rendered_domain_element(&mut rendered, "denied", &self.denied_domains); + self.push_rendered_domain_element(&mut rendered, "allowed", NetworkDomainAccess::Allow); + self.push_rendered_domain_element(&mut rendered, "denied", NetworkDomainAccess::Deny); rendered.push_str(""); rendered } - fn push_rendered_domain_element(rendered_network: &mut String, name: &str, domains: &[String]) { - if domains.is_empty() { + fn push_rendered_domain_element( + &self, + rendered_network: &mut String, + name: &str, + access: NetworkDomainAccess, + ) { + let mut domains = self + .domains + .iter() + .filter_map(|(domain, current_access)| (*current_access == access).then_some(domain)); + let Some(first) = domains.next() else { return; - } + }; rendered_network.push_str(&format!("<{name}>")); - rendered_network.push_str(&domains.join(",")); + rendered_network.push_str(first); + for domain in domains { + rendered_network.push(','); + rendered_network.push_str(domain); + } rendered_network.push_str(&format!("")); } } diff --git a/codex-rs/core/src/context/world_state/environment_tests.rs b/codex-rs/core/src/context/world_state/environment_tests.rs index 3d5b798595..5941fb2b5c 100644 --- a/codex-rs/core/src/context/world_state/environment_tests.rs +++ b/codex-rs/core/src/context/world_state/environment_tests.rs @@ -30,7 +30,6 @@ 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(); @@ -132,7 +131,6 @@ fn persisted_turn_context_values_render_a_diff() -> Result<()> { &PermissionProfile::Disabled, &[], )), - turn_context_values_comparable: true, ..environments.clone() }); let mut current = WorldState::default(); @@ -149,7 +147,6 @@ fn persisted_turn_context_values_render_a_diff() -> Result<()> { }, &[], )), - turn_context_values_comparable: true, ..environments }); @@ -206,17 +203,30 @@ 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)?; assert_eq!( json!({ - "devbox": { - "cwd": "file:///workspace", + "environments": { + "devbox": { + "cwd": "file:///workspace", + }, + "laptop": { + "cwd": "file:///repo", + }, }, - "laptop": { - "cwd": "file:///repo", + "current_date": "2026-06-20", + "timezone": "America/Los_Angeles", + "network": { + "domains": { + "api.example.com": "allow", + "blocked.example.com": "deny", + }, + }, + "filesystem": { + "workspace_roots": {}, + "permission_profile": "disabled", }, }), stored,