diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index a476a776df..08428f146a 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -1247,6 +1247,9 @@ "output_token_replay": { "type": "boolean" }, + "per_environment_context": { + "type": "boolean" + }, "personality": { "type": "boolean" }, @@ -8423,6 +8426,9 @@ "output_token_replay": { "type": "boolean" }, + "per_environment_context": { + "type": "boolean" + }, "personality": { "type": "boolean" }, diff --git a/codex-rs/core/src/context/world_state/environment.rs b/codex-rs/core/src/context/world_state/environment.rs index f9f8c49883..df3069b6a6 100644 --- a/codex-rs/core/src/context/world_state/environment.rs +++ b/codex-rs/core/src/context/world_state/environment.rs @@ -17,6 +17,7 @@ use codex_protocol::AgentPath; use codex_protocol::models::ContentItem; use codex_protocol::models::ContentItemKind; use codex_protocol::models::input_text; +use codex_protocol::protocol::NetworkAccess; use codex_utils_path_uri::PathUri; use serde::Deserialize; use serde::Serialize; @@ -36,6 +37,8 @@ static POWERSHELL_VERSIONS: LazyLock>>> = #[derive(Clone, Debug, Default)] pub(crate) struct EnvironmentsState { environments: BTreeMap, + // Keep each environment named and its rules grouped together, even when there is only one. + per_environment: bool, shell_version: Option, current_date: Option, timezone: Option, @@ -50,6 +53,10 @@ impl EnvironmentsState { environments: &TurnEnvironmentSnapshot, current_date: Option, ) -> Self { + let per_environment = turn_context + .config + .features + .enabled(Feature::PerEnvironmentContext); let shell_version = if turn_context .config .features @@ -63,7 +70,8 @@ impl EnvironmentsState { None }; Self { - environments: environment_states(environments), + environments: environment_states(environments, per_environment), + per_environment, shell_version, current_date, timezone: turn_context @@ -71,12 +79,15 @@ impl EnvironmentsState { .clone() .filter(|_| turn_context.config.include_environment_context_time), network: network_from_turn_context(turn_context), - filesystem: environments.primary().map(|environment| { - FileSystemContext::from_permission_profile( - environment.permission_profile(), - environment.workspace_roots(), - ) - }), + filesystem: environments + .primary() + .filter(|_| !per_environment) + .map(|environment| { + FileSystemContext::from_permission_profile( + environment.permission_profile(), + environment.workspace_roots(), + ) + }), subagents: None, } } @@ -88,6 +99,18 @@ impl EnvironmentsState { self } + fn rendered_network(&self) -> Option { + self.network.as_ref().map(|network| { + let network = network.render(); + if self.per_environment { + // This shared list does not include each environment's own domain restrictions. + format!("{network}") + } else { + network + } + }) + } + fn rendered_full(&self) -> RenderedEnvironments { RenderedEnvironments { updates: self @@ -97,14 +120,14 @@ impl EnvironmentsState { (id.clone(), EnvironmentUpdate::Current(environment.clone())) }) .collect(), - legacy_single: is_legacy_single(&self.environments), + legacy_single: !self.per_environment && is_legacy_single(&self.environments), include_primary: self.environments.len() > 1, shell_version: self.shell_version.clone(), shell_version_removed: false, current_date: self.current_date.clone(), current_date_removed: false, timezone: self.timezone.clone(), - network: self.network.clone(), + network: self.rendered_network(), filesystem: self.filesystem.clone(), subagents: self.subagents.clone(), } @@ -120,6 +143,7 @@ impl WorldStateSection for EnvironmentsState { previous: PreviousSectionState<'_, Self::Snapshot>, ) -> SectionTransition { let current = EnvironmentsSnapshot { + per_environment: self.per_environment, environments: self .environments .iter() @@ -132,6 +156,8 @@ impl WorldStateSection for EnvironmentsState { error: environment.error.clone(), shell: environment.shell.clone(), is_primary: self.environments.len() > 1 && environment.is_primary, + filesystem: environment.filesystem.clone(), + network_access: environment.network_access, }, ) }) @@ -139,7 +165,7 @@ impl WorldStateSection for EnvironmentsState { shell_version: self.shell_version.clone(), current_date: self.current_date.clone(), timezone: self.timezone.clone(), - network: self.network.as_ref().map(NetworkContext::render), + network: self.rendered_network(), filesystem: self.filesystem.as_ref().map(FileSystemContext::render), subagents: self.subagents.clone(), }; @@ -150,7 +176,9 @@ impl WorldStateSection for EnvironmentsState { }; let shell_version_added = current.shell_version.is_some() && previous.shell_version.is_none(); - let turn_context_values_changed = current.shell_version != previous.shell_version + let format_changed = current.per_environment != previous.per_environment; + let turn_context_values_changed = format_changed + || current.shell_version != previous.shell_version || current.current_date != previous.current_date || current.timezone != previous.timezone || current.network != previous.network @@ -163,7 +191,8 @@ impl WorldStateSection for EnvironmentsState { .filter(|(id, _)| { let environment = ¤t.environments[*id]; previous.environments.get(*id).is_none_or(|previous| { - multiple_environments != previous_multiple_environments + format_changed + || multiple_environments != previous_multiple_environments || (shell_version_added && previous.shell.is_none()) || !environment.has_same_diff_value(previous) }) @@ -177,7 +206,8 @@ impl WorldStateSection for EnvironmentsState { .filter(|id| !self.environments.contains_key(*id)) .map(|id| (id.clone(), EnvironmentUpdate::Unavailable)), ); - let legacy_single = is_legacy_single(&self.environments) + let legacy_single = !self.per_environment + && is_legacy_single(&self.environments) && updates .values() .all(|update| matches!(update, EnvironmentUpdate::Current(_))); @@ -193,7 +223,7 @@ impl WorldStateSection for EnvironmentsState { current_date_removed: self.current_date.is_none() && previous.current_date.is_some(), timezone: self.timezone.clone(), - network: self.network.clone(), + network: self.rendered_network(), filesystem: self.filesystem.clone(), subagents: self.subagents.clone(), }); @@ -237,7 +267,7 @@ pub(crate) struct RenderedEnvironments { current_date: Option, current_date_removed: bool, timezone: Option, - network: Option, + network: Option, filesystem: Option, pub(crate) subagents: Option, } @@ -312,7 +342,7 @@ impl ContextualUserFragment for RenderedEnvironments { push_optional_element(&mut rendered, "timezone", self.timezone.as_deref()); if let Some(network) = &self.network { rendered.push_str(" "); - rendered.push_str(&network.render()); + rendered.push_str(network); rendered.push('\n'); } if let Some(filesystem) = &self.filesystem { @@ -358,6 +388,15 @@ fn push_environment_values(rendered: &mut String, environment: &EnvironmentState push_xml_escaped_text(rendered, shell); rendered.push_str("\n"); } + if let Some(access) = environment.network_access { + rendered.push_str(indent); + rendered.push_str(&format!("\n")); + } + if let Some(filesystem) = &environment.filesystem { + rendered.push_str(indent); + rendered.push_str(filesystem); + rendered.push('\n'); + } } fn push_optional_element(rendered: &mut String, name: &str, value: Option<&str>) { @@ -380,11 +419,15 @@ struct EnvironmentState { shell: Option, error: Option, is_primary: bool, + filesystem: Option, + network_access: Option, } #[derive(Default, Deserialize, Serialize)] pub(crate) struct EnvironmentsSnapshot { environments: BTreeMap, + #[serde(default, skip_serializing_if = "std::ops::Not::not")] + per_environment: bool, #[serde(default, skip_serializing_if = "Option::is_none")] shell_version: Option, current_date: Option, @@ -403,6 +446,10 @@ struct EnvironmentSnapshot { error: Option, #[serde(default, skip_serializing_if = "std::ops::Not::not")] is_primary: bool, + #[serde(default, skip_serializing_if = "Option::is_none")] + filesystem: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + network_access: Option, } impl EnvironmentSnapshot { @@ -411,6 +458,8 @@ impl EnvironmentSnapshot { && self.status == other.status && self.error == other.error && self.is_primary == other.is_primary + && self.filesystem == other.filesystem + && self.network_access == other.network_access && self .shell .as_ref() @@ -467,7 +516,10 @@ async fn powershell_version(shell_path: &Path) -> Option { version } -fn environment_states(snapshot: &TurnEnvironmentSnapshot) -> BTreeMap { +fn environment_states( + snapshot: &TurnEnvironmentSnapshot, + per_environment: bool, +) -> BTreeMap { let mut environments = snapshot .turn_environments() .enumerate() @@ -483,6 +535,25 @@ fn environment_states(snapshot: &TurnEnvironmentSnapshot) -> BTreeMap BTreeMap BTreeMap) -> (String, Env error: None, shell: Some(shell.into()), is_primary: false, + filesystem: None, + network_access: None, }, ) } @@ -60,6 +62,7 @@ fn environment_state( .collect(); EnvironmentsState { environments, + per_environment: false, shell_version: None, current_date, timezone, @@ -110,17 +113,28 @@ fn serialize_environment_context_with_foreign_windows_cwd() { /*network*/ None, /*subagents*/ None, ); - context.filesystem = Some(FileSystemContext::from_permission_profile( - &PermissionProfile::Disabled, - &[PathUri::parse("file:///D:/workspace").expect("Windows workspace root URI")], - )); + context.per_environment = true; + let environment = context.environments.get_mut("remote").unwrap(); + environment.network_access = Some(NetworkAccess::Enabled); + environment.filesystem = Some( + FileSystemContext::from_permission_profile( + &PermissionProfile::Disabled, + &[PathUri::parse("file:///D:/workspace").expect("Windows workspace root URI")], + ) + .render(), + ); assert_eq!( context.render().expect_text(), r#" - C:\windows - powershell - D:\workspace + + + C:\windows + powershell + + D:\workspace + + "# ); } 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 9bdf4c4496..9eee9d4ad5 100644 --- a/codex-rs/core/src/context/world_state/environment_tests.rs +++ b/codex-rs/core/src/context/world_state/environment_tests.rs @@ -110,6 +110,8 @@ fn snapshots() -> Result<()> { error: None, shell: None, is_primary: false, + filesystem: None, + network_access: None, }, )] .into_iter() @@ -340,6 +342,8 @@ fn available(cwd: &str, shell: &str) -> Result { error: None, shell: Some(shell.to_string()), is_primary: false, + filesystem: None, + network_access: None, }) } @@ -357,6 +361,8 @@ fn starting(cwd: &str) -> Result { error: None, shell: None, is_primary: false, + filesystem: None, + network_access: None, }) } @@ -371,6 +377,8 @@ fn failure_context_is_escaped_incremental_and_cleared_on_recovery() -> Result<() error: Some("Repository & unavailable".to_string()), shell: None, is_primary: false, + filesystem: None, + network_access: None, }, )] .into_iter() @@ -440,7 +448,7 @@ fn failure_context_limits_total_detail_bytes_at_utf8_boundaries() { }) .collect(), }; - let states = environment_states(&snapshot); + let states = environment_states(&snapshot, /*per_environment*/ false); let details: Vec<_> = states .values() .map(|state| state.error.as_deref()) diff --git a/codex-rs/core/src/context/world_state/mod.rs b/codex-rs/core/src/context/world_state/mod.rs index 8781e37a50..481c20c57c 100644 --- a/codex-rs/core/src/context/world_state/mod.rs +++ b/codex-rs/core/src/context/world_state/mod.rs @@ -61,6 +61,7 @@ pub(crate) use model::ModelInstructionsState; pub(crate) use model_catalog::ModelCatalogState; pub(crate) use multi_agent_mode::MultiAgentModeState; pub(crate) use multi_agent_usage_hint::MultiAgentUsageHintState; +pub(crate) use permissions::PermissionDescription; pub(crate) use permissions::PermissionsState; pub(crate) use persistent_mode::PersistentModeState; pub(crate) use plugins_instructions::PluginsInstructionsState; diff --git a/codex-rs/core/src/context/world_state/permissions.rs b/codex-rs/core/src/context/world_state/permissions.rs index 555fc9df53..c12204a6fc 100644 --- a/codex-rs/core/src/context/world_state/permissions.rs +++ b/codex-rs/core/src/context/world_state/permissions.rs @@ -2,20 +2,27 @@ use super::PreviousSectionState; use super::SectionTransition; +use super::WorldStateContextFragment; use super::WorldStateHash; use super::WorldStateSection; use super::WorldStateUpdate; use crate::context::ApprovedCommandPrefixSaved; use crate::context::ContextualUserFragment; use crate::context::PermissionsInstructions; +use crate::context::environment_context::push_xml_escaped_text; use codex_execpolicy::Policy; +use codex_extension_api::RenderedWorldStateFragment; use codex_prompts::ApprovalPromptContext; +use codex_protocol::models::ContentItem; +use codex_protocol::models::ContentItemKind; use codex_protocol::models::PermissionProfile; use codex_protocol::models::format_allow_prefixes; +use codex_protocol::models::input_text; use codex_protocol::permissions::FileSystemSandboxPolicyContext; use codex_protocol::protocol::AskForApproval; use serde::Deserialize; use serde::Serialize; +use std::collections::BTreeMap; use std::collections::BTreeSet; use std::path::Path; @@ -24,12 +31,26 @@ use std::path::Path; pub(crate) struct PermissionsState { saved_prefix_heading: Option, snapshot: PermissionsSnapshot, + // None is unscoped text: the legacy description or the thread-default fallback. + descriptions: BTreeMap, PermissionDescription>, +} + +/// One permission description and its comparison value, excluding saved prefixes. +#[derive(Clone, Debug)] +pub(crate) struct PermissionDescription { instructions: PermissionsInstructions, + fingerprint: WorldStateHash, } #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] #[serde(untagged)] pub(crate) enum PermissionsSnapshot { + Scoped { + environments: BTreeMap, + #[serde(default, skip_serializing_if = "Option::is_none")] + thread_defaults: Option, + approved_command_prefixes: BTreeSet>, + }, Current { instructions: WorldStateHash, approved_command_prefixes: BTreeSet>, @@ -37,7 +58,7 @@ pub(crate) enum PermissionsSnapshot { Legacy(WorldStateHash), } -impl PermissionsState { +impl PermissionDescription { #[allow(clippy::too_many_arguments)] pub(crate) fn new( permission_profile: &PermissionProfile, @@ -63,16 +84,46 @@ impl PermissionsState { }; let instructions = build_instructions(exec_policy); let instructions_without_approved_prefixes = build_instructions(&Policy::empty()); - let snapshot = PermissionsSnapshot::Current { - instructions: WorldStateHash::from_fragment(&instructions_without_approved_prefixes), - approved_command_prefixes: exec_policy.get_allowed_prefixes().into_iter().collect(), + Self { + instructions, + fingerprint: WorldStateHash::from_fragment(&instructions_without_approved_prefixes), + } + } +} + +impl PermissionsState { + pub(crate) fn new( + descriptions: BTreeMap, PermissionDescription>, + exec_policy: &Policy, + approval_context: ApprovalPromptContext<'_>, + per_environment: bool, + ) -> Self { + let approved_command_prefixes = exec_policy.get_allowed_prefixes().into_iter().collect(); + let snapshot = if per_environment { + PermissionsSnapshot::Scoped { + environments: descriptions + .iter() + .filter_map(|(id, description)| { + Some((id.clone()?, description.fingerprint.clone())) + }) + .collect(), + thread_defaults: descriptions + .get(&None) + .map(|description| description.fingerprint.clone()), + approved_command_prefixes, + } + } else { + PermissionsSnapshot::Current { + instructions: descriptions[&None].fingerprint.clone(), + approved_command_prefixes, + } }; Self { saved_prefix_heading: approval_context .approved_command_prefix_saved_heading() .map(str::to_owned), snapshot, - instructions, + descriptions, } } } @@ -97,7 +148,95 @@ impl WorldStateSection for PermissionsState { &self, previous: PreviousSectionState<'_, Self::Snapshot>, ) -> SectionTransition { + if let PreviousSectionState::Known(previous) = &previous + && *previous == &self.snapshot + { + return (None, Vec::new()); + } let current = self.snapshot.clone(); + if let PermissionsSnapshot::Scoped { + approved_command_prefixes, + .. + } = &self.snapshot + { + let mut updates = Vec::new(); + let mut previous_descriptions = BTreeMap::new(); + let mut previous_prefixes = None; + if let PreviousSectionState::Known(PermissionsSnapshot::Scoped { + environments, + thread_defaults, + approved_command_prefixes, + }) = previous + { + previous_descriptions.extend( + environments + .iter() + .map(|(id, hash)| (Some(id.clone()), hash)), + ); + previous_descriptions.extend(thread_defaults.as_ref().map(|hash| (None, hash))); + previous_prefixes = Some(approved_command_prefixes); + } + // An environment can remain selected but stop being ready. Withdraw its old + // description even when the separate environment-context section is disabled. + for id in previous_descriptions + .keys() + .filter(|id| !self.descriptions.contains_key(*id)) + { + updates.push(scoped_permissions_update( + id.as_deref(), + /*instructions*/ None, + )); + } + // Switching models can make saved command approvals stop applying. + // We track these separately from each environment's other rules. + // Additions get a short notice; removals require the full current rules. + let prefixes_removed = previous_prefixes + .is_some_and(|prefixes| !prefixes.is_subset(approved_command_prefixes)); + for (id, description) in &self.descriptions { + if prefixes_removed + || previous_descriptions.get(id).copied() != Some(&description.fingerprint) + { + updates.push(scoped_permissions_update( + id.as_deref(), + Some(&description.instructions), + )); + } + } + // Saved prefixes apply to every environment, including those whose own + // description did not change. A removal instead refreshes the full text. + if !prefixes_removed + && let Some(previous_prefixes) = previous_prefixes + && previous_prefixes != approved_command_prefixes + { + let added = approved_command_prefixes + .difference(previous_prefixes) + .cloned() + .collect(); + if let Some(prefixes) = format_allow_prefixes(added) { + updates.push(WorldStateUpdate::fragment(ApprovedCommandPrefixSaved::new( + prefixes, + self.saved_prefix_heading.as_deref(), + ))); + } + } + return (Some(current), updates); + } + let instructions = &self.descriptions[&None].instructions; + if let PreviousSectionState::Known(PermissionsSnapshot::Scoped { + environments, + thread_defaults, + .. + }) = &previous + { + let mut updates = environments + .keys() + .map(|id| Some(id.as_str())) + .chain(thread_defaults.as_ref().map(|_| None)) + .map(|id| scoped_permissions_update(id, /*instructions*/ None)) + .collect::>(); + updates.push(WorldStateUpdate::fragment(instructions.clone())); + return (Some(current), updates); + } match (previous, &self.snapshot) { ( PreviousSectionState::Known(PermissionsSnapshot::Current { @@ -109,9 +248,6 @@ impl WorldStateSection for PermissionsState { approved_command_prefixes, }, ) if previous_instructions == instructions => { - if previous_prefixes == approved_command_prefixes { - return (None, Vec::new()); - } if previous_prefixes.is_subset(approved_command_prefixes) { let added_prefixes = approved_command_prefixes .difference(previous_prefixes) @@ -131,7 +267,7 @@ impl WorldStateSection for PermissionsState { ( PreviousSectionState::Known(PermissionsSnapshot::Legacy(previous)), PermissionsSnapshot::Current { .. }, - ) if previous == &WorldStateHash::from_fragment(&self.instructions) => { + ) if previous == &WorldStateHash::from_fragment(instructions) => { return (Some(current), Vec::new()); } _ => {} @@ -139,11 +275,46 @@ impl WorldStateSection for PermissionsState { ( Some(current), - vec![WorldStateUpdate::fragment(self.instructions.clone())], + vec![WorldStateUpdate::fragment(instructions.clone())], ) } } +/// Labels an environment's rules, or thread defaults when no environment ID is supplied. +/// Missing instructions withdraw the previous description for that scope. +fn scoped_permissions_update( + environment_id: Option<&str>, + instructions: Option<&PermissionsInstructions>, +) -> WorldStateUpdate { + let mut text = String::new(); + let end = if let Some(id) = environment_id { + text.push_str("\n"); + "" + } else { + text.push_str("\n"); + "" + }; + if let Some(instructions) = instructions { + let ContentItem::InputText { text: body } = instructions.body() else { + unreachable!("permission instructions are text") + }; + text.push_str(&body); + } else { + text.push_str("The previous permission description for this scope is no longer current.\n"); + } + text.push_str(end); + WorldStateUpdate::fragment(WorldStateContextFragment { + fragment: RenderedWorldStateFragment::new( + "developer", + PermissionsInstructions::type_markers(), + input_text(text), + ), + content_kind: ContentItemKind("permissions.instructions".to_string()), + }) +} + #[cfg(test)] #[path = "permissions_tests.rs"] mod tests; diff --git a/codex-rs/core/src/context/world_state/permissions_tests.rs b/codex-rs/core/src/context/world_state/permissions_tests.rs index de9f1949c4..472a1dc059 100644 --- a/codex-rs/core/src/context/world_state/permissions_tests.rs +++ b/codex-rs/core/src/context/world_state/permissions_tests.rs @@ -26,6 +26,32 @@ fn snapshots() { let read_only = permissions_state(PermissionProfile::read_only(), AskForApproval::OnRequest); let full_access = permissions_state(PermissionProfile::Disabled, AskForApproval::OnRequest); let never_ask = permissions_state(PermissionProfile::read_only(), AskForApproval::Never); + let approval_context = + ApprovalPromptContext::new(ApprovalsReviewer::User, ResolvedModelMessages::bundled()); + let scoped = PermissionsState::new( + BTreeMap::from([ + (Some("A".to_string()), read_only.descriptions[&None].clone()), + ( + Some("B".to_string()), + full_access.descriptions[&None].clone(), + ), + ]), + &Policy::empty(), + approval_context, + /*per_environment*/ true, + ); + let only_a = PermissionsState::new( + BTreeMap::from([(Some("A".to_string()), read_only.descriptions[&None].clone())]), + &Policy::empty(), + approval_context, + /*per_environment*/ true, + ); + let fallback = PermissionsState::new( + read_only.descriptions.clone(), + &Policy::empty(), + approval_context, + /*per_environment*/ true, + ); insta::assert_snapshot!(render_section_cases(&[ (Absent, Absent), @@ -34,6 +60,11 @@ fn snapshots() { (Known(&read_only), Known(&full_access)), (Known(&read_only), Known(&never_ask)), (Unknown, Known(&read_only)), + (Known(&read_only), Known(&only_a)), + (Known(&scoped), Known(&only_a)), + (Known(&only_a), Known(&only_a)), + (Known(&only_a), Known(&fallback)), + (Known(&only_a), Known(&read_only)), ])); } @@ -68,18 +99,24 @@ fn approved_prefix_is_rendered_without_reinjecting_permissions() { assert_ne!(without_snapshot, with_snapshot); assert_ne!( - without_approved_prefix.instructions.body().expect_text(), - with_approved_prefix.instructions.body().expect_text() + without_approved_prefix.descriptions[&None] + .instructions + .body() + .expect_text(), + with_approved_prefix.descriptions[&None] + .instructions + .body() + .expect_text() ); assert!( - !without_approved_prefix + !without_approved_prefix.descriptions[&None] .instructions .body() .expect_text() .contains(approved_prefix) ); assert!( - with_approved_prefix + with_approved_prefix.descriptions[&None] .instructions .body() .expect_text() @@ -92,7 +129,8 @@ fn approved_prefix_is_rendered_without_reinjecting_permissions() { assert!(!rendered_update.contains("")); let before_approval_permissions = - permissions_state(PermissionProfile::read_only(), AskForApproval::OnRequest) + permissions_state(PermissionProfile::read_only(), AskForApproval::OnRequest).descriptions + [&None] .instructions .render() .expect_text(); @@ -109,15 +147,26 @@ fn approved_prefix_is_rendered_without_reinjecting_permissions() { fn renders_only_newly_approved_prefixes() { use PreviousSectionState::Known; + let scoped = |policy: &Policy| { + let mut legacy = permissions_state_with_default_messages(policy); + let description = legacy.descriptions.remove(&None).unwrap(); + PermissionsState::new( + BTreeMap::from([(Some("A".to_string()), description)]), + policy, + ApprovalPromptContext::new(ApprovalsReviewer::User, ResolvedModelMessages::bundled()), + /*per_environment*/ true, + ) + }; + let mut exec_policy = Policy::empty(); exec_policy .add_prefix_rule(&["git".to_string(), "pull".to_string()], Decision::Allow) .expect("test prefix should be valid"); - let with_existing_prefix = permissions_state_with_default_messages(&exec_policy); + let with_existing_prefix = scoped(&exec_policy); exec_policy .add_prefix_rule(&["cargo".to_string(), "test".to_string()], Decision::Allow) .expect("test prefix should be valid"); - let with_new_prefix = permissions_state_with_default_messages(&exec_policy); + let with_new_prefix = scoped(&exec_policy); let existing_snapshot = with_existing_prefix .render_fragment_diff(PreviousSectionState::Absent) .0 @@ -156,15 +205,22 @@ fn legacy_snapshot_deserializes_and_only_suppresses_matching_full_permissions() .expect("test prefix should be valid"); let with_approved_prefix = permissions_state_with_default_messages(&exec_policy); let matching_legacy: WorldStateSnapshot = serde_json::from_value(json!({ - "permissions": WorldStateHash::from_fragment(&with_approved_prefix.instructions), + "permissions": WorldStateHash::from_fragment(&with_approved_prefix.descriptions[&None].instructions), })) .expect("legacy world-state snapshot should deserialize"); let stale_legacy: WorldStateSnapshot = serde_json::from_value(json!({ - "permissions": WorldStateHash::from_fragment(&without_approved_prefix.instructions), + "permissions": WorldStateHash::from_fragment(&without_approved_prefix.descriptions[&None].instructions), })) .expect("legacy world-state snapshot should deserialize"); - let expected_permissions = with_approved_prefix.instructions.render().expect_text(); - let retained = ContextualUserFragment::into(with_approved_prefix.instructions.clone()); + let expected_permissions = with_approved_prefix.descriptions[&None] + .instructions + .render() + .expect_text(); + let retained = ContextualUserFragment::into( + with_approved_prefix.descriptions[&None] + .instructions + .clone(), + ); let mut world_state = WorldState::default(); world_state.add_section(with_approved_prefix); @@ -210,14 +266,17 @@ fn removing_an_approved_prefix_renders_full_permissions() { assert_eq!( rendered, - without_approved_prefix.instructions.render().expect_text() + without_approved_prefix.descriptions[&None] + .instructions + .render() + .expect_text() ); } #[test] fn persisted_permissions_are_detected_inside_bundled_developer_messages() { let state = permissions_state(PermissionProfile::read_only(), AskForApproval::OnRequest); - let retained = ContextualUserFragment::into(state.instructions.clone()); + let retained = ContextualUserFragment::into(state.descriptions[&None].instructions.clone()); let mut world_state = super::super::WorldState::default(); world_state.add_section(state); let snapshot = world_state.render_full().0; @@ -280,28 +339,42 @@ fn permissions_state( ..Default::default() }); let model_messages = ResolvedModelMessages::from_model(&model); - PermissionsState::new( + let approval_context = ApprovalPromptContext::new(ApprovalsReviewer::User, model_messages); + let description = PermissionDescription::new( &permission_profile, approval_policy, - ApprovalPromptContext::new(ApprovalsReviewer::User, model_messages), + approval_context, &Policy::empty(), Path::new("/workspace"), /*paths*/ None, /*exec_permission_approvals_enabled*/ false, /*request_permissions_tool_enabled*/ false, + ); + PermissionsState::new( + BTreeMap::from([(None, description)]), + &Policy::empty(), + approval_context, + /*per_environment*/ false, ) } fn permissions_state_with_default_messages(exec_policy: &Policy) -> PermissionsState { let model_messages = ResolvedModelMessages::bundled(); - PermissionsState::new( + let approval_context = ApprovalPromptContext::new(ApprovalsReviewer::User, model_messages); + let description = PermissionDescription::new( &PermissionProfile::read_only(), AskForApproval::OnRequest, - ApprovalPromptContext::new(ApprovalsReviewer::User, model_messages), + approval_context, exec_policy, Path::new("/workspace"), /*paths*/ None, /*exec_permission_approvals_enabled*/ false, /*request_permissions_tool_enabled*/ false, + ); + PermissionsState::new( + BTreeMap::from([(None, description)]), + exec_policy, + approval_context, + /*per_environment*/ false, ) } diff --git a/codex-rs/core/src/context/world_state/snapshots/codex_core__context__world_state__permissions__tests__snapshots.snap b/codex-rs/core/src/context/world_state/snapshots/codex_core__context__world_state__permissions__tests__snapshots.snap index efe6728413..0fe8db47ef 100644 --- a/codex-rs/core/src/context/world_state/snapshots/codex_core__context__world_state__permissions__tests__snapshots.snap +++ b/codex-rs/core/src/context/world_state/snapshots/codex_core__context__world_state__permissions__tests__snapshots.snap @@ -1,6 +1,6 @@ --- source: core/src/context/world_state/permissions_tests.rs -expression: "render_section_cases(&[(Absent, Absent), (Absent, Known(&read_only)),\n(Known(&read_only), Known(&read_only)),\n(Known(&read_only), Known(&full_access)),\n(Known(&read_only), Known(&never_ask)), (Unknown, Known(&read_only)),])" +expression: "render_section_cases(&[(Absent, Absent), (Absent, Known(&read_only)),\n(Known(&read_only), Known(&read_only)),\n(Known(&read_only), Known(&full_access)),\n(Known(&read_only), Known(&never_ask)), (Unknown, Known(&read_only)),\n(Known(&read_only), Known(&only_a)), (Known(&scoped), Known(&only_a)),\n(Known(&only_a), Known(&only_a)), (Known(&only_a), Known(&fallback)),\n(Known(&only_a), Known(&read_only)),])" --- Absent -> Absent None @@ -31,3 +31,39 @@ Unknown -> {"approved_command_prefixes":[],"instructions":"0ccde536dd8b4cfebb1df Read only. Ask for approval. + +{"approved_command_prefixes":[],"instructions":"0ccde536dd8b4cfebb1df573f679dfe86b5718e3"} -> {"approved_command_prefixes":[],"environments":{"A":"0ccde536dd8b4cfebb1df573f679dfe86b5718e3"}} (role - developer) + + +Read only. +Ask for approval. + + +{"approved_command_prefixes":[],"environments":{"A":"0ccde536dd8b4cfebb1df573f679dfe86b5718e3","B":"7e17d9a0df61e5f0c032ed7f0bf1dda58c9d5e85"}} -> {"approved_command_prefixes":[],"environments":{"A":"0ccde536dd8b4cfebb1df573f679dfe86b5718e3"}} (role - developer) + +The previous permission description for this scope is no longer current. + + +{"approved_command_prefixes":[],"environments":{"A":"0ccde536dd8b4cfebb1df573f679dfe86b5718e3"}} -> {"approved_command_prefixes":[],"environments":{"A":"0ccde536dd8b4cfebb1df573f679dfe86b5718e3"}} +None + +{"approved_command_prefixes":[],"environments":{"A":"0ccde536dd8b4cfebb1df573f679dfe86b5718e3"}} -> {"approved_command_prefixes":[],"environments":{},"thread_defaults":"0ccde536dd8b4cfebb1df573f679dfe86b5718e3"} (role - developer) + +The previous permission description for this scope is no longer current. + + (role - developer) + + +Read only. +Ask for approval. + + +{"approved_command_prefixes":[],"environments":{"A":"0ccde536dd8b4cfebb1df573f679dfe86b5718e3"}} -> {"approved_command_prefixes":[],"instructions":"0ccde536dd8b4cfebb1df573f679dfe86b5718e3"} (role - developer) + +The previous permission description for this scope is no longer current. + + (role - developer) + +Read only. +Ask for approval. + diff --git a/codex-rs/core/src/session/world_state.rs b/codex-rs/core/src/session/world_state.rs index b0a8c7d44c..766e132a09 100644 --- a/codex-rs/core/src/session/world_state.rs +++ b/codex-rs/core/src/session/world_state.rs @@ -1,3 +1,4 @@ +use std::collections::BTreeMap; use std::sync::Arc; use super::session::Session; @@ -19,6 +20,7 @@ use crate::context::world_state::ModelCatalogState; use crate::context::world_state::ModelInstructionsState; use crate::context::world_state::MultiAgentModeState; use crate::context::world_state::MultiAgentUsageHintState; +use crate::context::world_state::PermissionDescription; use crate::context::world_state::PermissionsState; use crate::context::world_state::PersistentModeState; use crate::context::world_state::PluginsInstructionsState; @@ -267,36 +269,71 @@ impl Session { let approval_context = ApprovalPromptContext::new(settings.approvals_reviewer(), model_messages); if turn_context.config.include_permissions_instructions { - let environment = step_context.environments.primary(); - let sandbox = environment - .filter(|environment| environment.environment().is_remote()) - .map(|environment| { - environment.sandbox_context(/*additional_permissions*/ None) - }); - let paths = sandbox - .as_ref() - .map(FileSystemSandboxContext::policy_context); - let permission_profile = - turn_context.permission_profile_for_environments(&step_context.environments); - #[allow(deprecated)] - let cwd = environment - .and_then(|environment| environment.cwd().to_abs_path().ok()) - .unwrap_or_else(|| turn_context.cwd.clone()); + let per_environment = turn_context + .config + .features + .enabled(Feature::PerEnvironmentContext); + let mut environments = step_context + .environments + .turn_environments() + .map(Some) + .collect::>(); + if !per_environment { + environments.truncate(1); + } + // Until a computer is ready, describe the thread defaults without assigning + // those permissions to a computer whose configuration is still unknown. + if environments.is_empty() { + environments.push(None); + } + let mut descriptions = BTreeMap::new(); + for environment in environments { + let sandbox = environment + .filter(|environment| per_environment || environment.environment().is_remote()) + .map(|environment| { + environment.sandbox_context(/*additional_permissions*/ None) + }); + let paths = sandbox + .as_ref() + .map(FileSystemSandboxContext::policy_context); + let permission_profile = environment.map_or_else( + || turn_context.permission_profile_for_environments(&step_context.environments), + super::turn_context::TurnEnvironment::permission_profile_with_workspace_roots, + ); + #[allow(deprecated)] + let cwd = environment + .and_then(|environment| environment.cwd().to_abs_path().ok()) + .unwrap_or_else(|| turn_context.cwd.clone()); + let id = if per_environment { + environment.map(|environment| environment.selection.environment_id.clone()) + } else { + None + }; + descriptions.insert( + id, + PermissionDescription::new( + &permission_profile, + settings.approval_policy(), + approval_context, + exec_policy.as_ref(), + &cwd, + paths.as_ref(), + turn_context + .config + .features + .enabled(Feature::ExecPermissionApprovals), + turn_context + .config + .features + .enabled(Feature::RequestPermissionsTool), + ), + ); + } world_state.add_section(PermissionsState::new( - &permission_profile, - settings.approval_policy(), - approval_context, + descriptions, exec_policy.as_ref(), - &cwd, - paths.as_ref(), - turn_context - .config - .features - .enabled(Feature::ExecPermissionApprovals), - turn_context - .config - .features - .enabled(Feature::RequestPermissionsTool), + approval_context, + per_environment, )); } else { world_state.add_section(CompactPermissionsState::new( diff --git a/codex-rs/core/tests/suite/scenarios.rs b/codex-rs/core/tests/suite/scenarios.rs index db804fc0bd..820072d363 100644 --- a/codex-rs/core/tests/suite/scenarios.rs +++ b/codex-rs/core/tests/suite/scenarios.rs @@ -57,12 +57,15 @@ use codex_protocol::items::TurnItem; use codex_protocol::models::ContentItem; use codex_protocol::models::ImageDetail; use codex_protocol::models::ImageReference; +use codex_protocol::models::PermissionProfile; +use codex_protocol::models::PermissionProfileSnapshot; use codex_protocol::models::ResponseItem; use codex_protocol::openai_models::CodeModeToolMessages; use codex_protocol::openai_models::InputModality; use codex_protocol::openai_models::ReasoningEffort; use codex_protocol::openai_models::ToolMessage; use codex_protocol::openai_models::ToolMode; +use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::protocol::AdditionalContextEntry; use codex_protocol::protocol::AdditionalContextKind; use codex_protocol::protocol::EnvironmentConfigState; @@ -70,7 +73,9 @@ use codex_protocol::protocol::EventMsg; #[cfg(not(target_os = "windows"))] use codex_protocol::protocol::MultiAgentVersion; use codex_protocol::protocol::Op; +use codex_protocol::protocol::ThreadSettingsOverrides; use codex_protocol::protocol::TurnEnvironmentSelection; +use codex_protocol::protocol::TurnEnvironmentSelections; use codex_protocol::protocol::TurnSettingsUpdate; use codex_protocol::protocol::TurnSettingsUpdateOutcome; use codex_protocol::request_user_input::RequestUserInputAnswer; @@ -85,6 +90,7 @@ use codex_utils_path_uri::PathUri; use core_test_support::context_snapshot; use core_test_support::context_snapshot::ContextSnapshotOptions; use core_test_support::context_snapshot::SnapshotEntry; +use core_test_support::exec_server::ExecServerProcess; use core_test_support::responses::ev_assistant_message; use core_test_support::responses::ev_completed; use core_test_support::responses::ev_completed_with_tokens; @@ -1177,15 +1183,48 @@ async fn astra_switches_environments_for_the_rest_of_the_active_turn() -> Result .features .enable(Feature::DefaultModeRequestUserInput) .expect("enable request_user_input"); + config + .features + .enable(Feature::PerEnvironmentContext) + .expect("enable per-environment context"); }) .build_with_auto_env(&server) .await?; - let original = test.executor_environment().selection().clone(); + let executor = ExecServerProcess::start().await?; + let first_workspace = TempDir::new()?; + test.thread_manager + .environment_manager() + .upsert_environment( + "environment-a".to_string(), + executor.websocket_url.clone(), + /*connect_timeout*/ None, + )?; + let mut first = test.executor_environment().selection().clone(); + first.environment_id = "environment-a".to_string(); + first.cwd = PathUri::from_host_native_path(first_workspace.path())?; + first.workspace_roots = vec![first.cwd.clone()]; + let mut first_config = environment_config_for_selection(&test.config, &first); + first_config.permission_profile = + PermissionProfileSnapshot::legacy(PermissionProfile::read_only()); + first.config = EnvironmentConfigState::Ready(first_config); + + let mut original = test.executor_environment().selection().clone(); + let mut original_config = environment_config_for_selection(&test.config, &original); + original_config.permission_profile = + PermissionProfileSnapshot::legacy(PermissionProfile::workspace_write_with( + &[], + NetworkSandboxPolicy::Enabled, + /*exclude_tmpdir_env_var*/ true, + /*exclude_slash_tmp*/ true, + )); + original.config = EnvironmentConfigState::Ready(original_config); let mut other = original.clone(); other.cwd = original.cwd.join("other-workspace")?; other.workspace_roots = vec![other.cwd.clone()]; - other.config = - EnvironmentConfigState::Ready(environment_config_for_selection(&test.config, &other)); + let mut other_config = environment_config_for_selection(&test.config, &other); + other_config.permission_profile = + PermissionProfileSnapshot::legacy(PermissionProfile::read_only()); + other.config = EnvironmentConfigState::Ready(other_config); test.fs() .create_directory( &other.cwd, @@ -1198,9 +1237,21 @@ async fn astra_switches_environments_for_the_rest_of_the_active_turn() -> Result .await?; test.codex - .start_or_steer_turn(TurnInputRequest::user_input(vec![text( - "Ask before continuing in the other workspace.", - )])) + .start_or_steer_turn( + TurnInputRequest::user_input(vec![text( + "Ask before continuing in the other workspace.", + )]) + .with_thread_settings(ThreadSettingsOverrides { + environments: Some( + TurnEnvironmentSelections::new( + test.config.cwd.clone(), + vec![first.clone(), original], + ) + .into_requests(), + ), + ..Default::default() + }), + ) .await?; let request = wait_for_event_match(&test.codex, |event| match event { EventMsg::RequestUserInput(request) => Some(request.clone()), @@ -1212,7 +1263,7 @@ async fn astra_switches_environments_for_the_rest_of_the_active_turn() -> Result .submit(Op::TurnSettings { turn_id: request.turn_id.clone(), update: TurnSettingsUpdate { - environments: Some(vec![other.clone().into_request()]), + environments: Some(vec![first.into_request(), other.into_request()]), ..Default::default() }, reply, @@ -1243,14 +1294,14 @@ async fn astra_switches_environments_for_the_rest_of_the_active_turn() -> Result .await?; let requests = mock.requests(); - insta::assert_snapshot!( - "astra_active_environment_selection", - context_snapshot::format_request_history_snapshot( - "Astra starts in one workspace, switches for the next step of the active turn, and returns to the original workspace on the next turn.", - &requests, - &ContextSnapshotOptions::default(), - ) - ); + let snapshot = context_snapshot::format_request_history_snapshot( + "Astra sees two environments with different permissions. Only the second environment's workspace and permissions change for the next step; the original defaults return on the next turn.", + &requests, + &ContextSnapshotOptions::default(), + ) + .replace(r#" +02:message/developer[3]: + [01] + Filesystem sandboxing defines which files can be read or written. `sandbox_mode` is `read-only`: The san... permits reading files. Network access is restricted. [hash=be7da4425f89053c] # Escalation Requests Commands are run outside the sandbox if they are approved by the user, or match an existing rule that al...hell control operators, including but not limited to: [hash=d24ebf5fa4e4b9bb] - Pipes: | - - Logical operators: &&, || - + NEVER provide a prefix_rule if your command uses a heredoc or herestring. ### Examples @@ -56,8 +56,25 @@ Scenario: Astra starts in one workspace, switches for the next step of the activ - ["npm", "run", "dev"] - ["gh", "pr", "check"] - ["cargo", "test"] - - [02] # Collaboration Mode: Default + + [02] + + Filesystem sandboxing defines which files can be read or written. `sandbox_mode` is `workspace-write`: T...ctories requires approval. Network access is enabled. [hash=4bea877c3f99797a] + # Escalation Requests + + Commands are run outside the sandbox if they are approved by the user, or match an existing rule that al...hell control operators, including but not limited to: [hash=d24ebf5fa4e4b9bb] + + - Pipes: | + + + ### Examples + Good examples of prefixes: + - ["npm", "run", "dev"] + - ["gh", "pr", "check"] + - ["cargo", "test"] + The writable root is ``. + + [03] # Collaboration Mode: Default You are now in Default mode. Any previous instructions for other modes (e.g. Plan mode) are no longer active. @@ -95,11 +112,22 @@ Scenario: Astra starts in one workspace, switches for the next step of the activ Any earlier instruction enabling proactive multi-agent delegation no longer applies. D...elegation, or parallel agent work. [hash=2b6e037067b4390c] 05:message/user: - - + + + + + + + + + + [hash=e8c1de60307d0a3d] 06:message/user: Ask before continuing in the other workspace. @@ -107,24 +135,70 @@ Scenario: Astra starts in one workspace, switches for the next step of the activ -- request 2 (turn) -- 08:function_call/functions.request_user_input:{"questions":[{"header":"Continue","id":"continue","options":[{"description":"Continue working.","label"...o"}],"question":"Continue in the other workspace?"}]} [hash=028e565351844d96] 09:function_call_output:{"answers":{"continue":{"answers":["Yes"]}}} -10:message/user: +10:message/developer: + + + Filesystem sandboxing defines which files can be read or written. `sandbox_mode` is `read-only`: The san... permits reading files. Network access is restricted. [hash=be7da4425f89053c] + # Escalation Requests + + Commands are run outside the sandbox if they are approved by the user, or match an existing rule that al...hell control operators, including but not limited to: [hash=d24ebf5fa4e4b9bb] + + - Pipes: | + + NEVER provide a prefix_rule if your command uses a heredoc or herestring. + + ### Examples + Good examples of prefixes: + - ["npm", "run", "dev"] + - ["gh", "pr", "check"] + - ["cargo", "test"] + +11:message/user: - /other-workspace - + + + /other-workspace + + + /other-workspace [hash=018d5ca32808e0f8] + + - /other-workspace [hash=0be8191a0451377a] -- request 3 (turn) -- -11:message/assistant: +12:message/assistant: I continued in the other workspace. -12:message/user: +13:message/developer: + + + Filesystem sandboxing defines which files can be read or written. `sandbox_mode` is `workspace-write`: T...ctories requires approval. Network access is enabled. [hash=4bea877c3f99797a] + # Escalation Requests + + Commands are run outside the sandbox if they are approved by the user, or match an existing rule that al...hell control operators, including but not limited to: [hash=d24ebf5fa4e4b9bb] + + - Pipes: | + + + ### Examples + Good examples of prefixes: + - ["npm", "run", "dev"] + - ["gh", "pr", "check"] + - ["cargo", "test"] + The writable root is ``. + +14:message/user: - - + + + + + + [hash=e8c1de60307d0a3d] -13:message/user: +15:message/user: Which workspace are you using now? diff --git a/codex-rs/features/src/lib.rs b/codex-rs/features/src/lib.rs index ea19255aee..c37ba6f9db 100644 --- a/codex-rs/features/src/lib.rs +++ b/codex-rs/features/src/lib.rs @@ -214,6 +214,8 @@ pub enum Feature { ShellSnapshotV2, /// Allow turns to start while selected executors are still starting. DeferredExecutor, + /// Describe each selected environment's filesystem and permissions separately. + PerEnvironmentContext, /// Use the current working directory for turn diff display paths. CwdRelativeTurnDiffs, /// Enable runtime metrics snapshots via a manual reader. @@ -1276,6 +1278,12 @@ pub const FEATURES: &[FeatureSpec] = &[ stage: Stage::UnderDevelopment, default_enabled: false, }, + FeatureSpec { + id: Feature::PerEnvironmentContext, + key: "per_environment_context", + stage: Stage::UnderDevelopment, + default_enabled: false, + }, FeatureSpec { id: Feature::CwdRelativeTurnDiffs, key: "cwd_relative_turn_diffs",