mirror of
https://github.com/openai/codex.git
synced 2026-11-12 21:20:34 +00:00
persist comparable environment context
This commit is contained in:
@@ -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<String, EnvironmentState>,
|
||||
#[serde(skip)]
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
current_date: Option<String>,
|
||||
#[serde(skip)]
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
timezone: Option<String>,
|
||||
#[serde(skip)]
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
network: Option<NetworkContext>,
|
||||
#[serde(skip)]
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
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 {
|
||||
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<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);
|
||||
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
|
||||
|
||||
@@ -55,7 +55,6 @@ fn environment_state(
|
||||
network,
|
||||
filesystem: None,
|
||||
subagents,
|
||||
turn_context_values_comparable: true,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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<String>,
|
||||
workspace_roots: IndexMap<String, bool>,
|
||||
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<FileSystemSandboxEntry>,
|
||||
entries: IndexMap<String, FileSystemSandboxEntry>,
|
||||
glob_scan_max_depth: Option<usize>,
|
||||
},
|
||||
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 = "<filesystem>".to_string();
|
||||
if !self.workspace_roots.is_empty() {
|
||||
rendered.push_str("<workspace_roots>");
|
||||
for root in &self.workspace_roots {
|
||||
for root in self.workspace_roots.keys() {
|
||||
push_text_element(&mut rendered, "root", root);
|
||||
}
|
||||
rendered.push_str("</workspace_roots>");
|
||||
@@ -76,15 +80,15 @@ impl From<ManagedFileSystemPermissions> 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("</file_system>");
|
||||
@@ -186,9 +190,13 @@ fn render_special_path_with_subpath(base: &str, subpath: &Option<PathBuf>) -> St
|
||||
}
|
||||
}
|
||||
|
||||
fn dedupe_file_system_entries(entries: &mut Vec<FileSystemSandboxEntry>) {
|
||||
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<String>,
|
||||
denied_domains: Vec<String>,
|
||||
domains: IndexMap<String, NetworkDomainAccess>,
|
||||
}
|
||||
|
||||
#[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<String>, denied_domains: Vec<String>) -> 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 = "<network enabled=\"true\">".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("</network>");
|
||||
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!("</{name}>"));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user