Preserve environment configuration ownership (#38678)

## Why

Environment attachments can either inherit configuration from their thread or
provide their own. Later thread setting updates must refresh inherited
configuration without overwriting attachment-owned permissions and capability
roots.

## What changed

- Resolve each attachment's configuration when it is selected and retain
  whether it came from the thread or the attachment owner.
- Apply subsequent thread configuration updates only to thread-owned
  attachments, while preserving ownership across snapshots and child threads.
- Keep the resolved configuration with the environment selection so runtime
  consumers use a single canonical value.

## Testing

- Extend remote-environment coverage to verify that thread-owned permissions
  follow thread updates while owner-provided read-only permissions remain in
  effect.
- Cover configuration inheritance, attachment replacement, and owner-configured
  capability roots.

GitOrigin-RevId: fe70c4be5f151432b69bf4b141e316faa89036ae
This commit is contained in:
sayan-oai authored and copyberry committed 2026-08-15 00:43:44 +00:00
1 parent 233739e76a
commit 22bf16a37e
23 files changed
+327 -176

No files matched your search

+9 -8
View File
@@ -2,6 +2,7 @@ use super::*;
use crate::config::ConfigBuilder; use crate::config::ConfigBuilder;
use crate::config::PermissionProfileSnapshot; use crate::config::PermissionProfileSnapshot;
use crate::context::ContextualUserFragment; use crate::context::ContextualUserFragment;
use crate::environment_selection::EnvironmentConfigOrigin;
use crate::environment_selection::TurnEnvironmentSnapshot; use crate::environment_selection::TurnEnvironmentSnapshot;
use crate::environment_selection::TurnEnvironmentState; use crate::environment_selection::TurnEnvironmentState;
use crate::session::turn_context::TurnEnvironment; use crate::session::turn_context::TurnEnvironment;
@@ -338,20 +339,20 @@ fn resolved_local_environments<const N: usize>(
environment_id: environment_id.to_string(), environment_id: environment_id.to_string(),
cwd: PathUri::from_abs_path(&cwd), cwd: PathUri::from_abs_path(&cwd),
workspace_roots: Vec::new(), workspace_roots: Vec::new(),
config: EnvironmentConfigState::FromThread, config: EnvironmentConfigState::Ready(EnvironmentConfig {
},
Arc::new(
Environment::create_for_tests(/*exec_server_url*/ None)
.expect("local environment"),
),
/*shell*/ None,
EnvironmentConfig {
allow_login_shell: true, allow_login_shell: true,
permission_profile: PermissionProfileSnapshot::legacy( permission_profile: PermissionProfileSnapshot::legacy(
PermissionProfile::read_only(), PermissionProfile::read_only(),
), ),
selected_capability_roots: Vec::new(), selected_capability_roots: Vec::new(),
}),
}, },
EnvironmentConfigOrigin::Thread,
Arc::new(
Environment::create_for_tests(/*exec_server_url*/ None)
.expect("local environment"),
),
/*shell*/ None,
)) ))
}) })
.collect(), .collect(),
+114 -61
View File
@@ -34,6 +34,26 @@ use crate::session::turn_context::TurnEnvironment;
use crate::shell::Shell; use crate::shell::Shell;
use crate::shell_snapshot::ShellSnapshot; use crate::shell_snapshot::ShellSnapshot;
/// Records whether a normalized config should follow later thread setting updates.
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub(crate) enum EnvironmentConfigOrigin {
Thread,
Owner,
}
impl EnvironmentConfigOrigin {
/// Reconstructs the input form so another attachment boundary preserves config ownership.
pub(crate) fn into_input_selection(
self,
mut selection: TurnEnvironmentSelection,
) -> TurnEnvironmentSelection {
if self == Self::Thread {
selection.config = EnvironmentConfigState::FromThread;
}
selection
}
}
pub(crate) fn default_thread_environment_selections( pub(crate) fn default_thread_environment_selections(
environment_manager: &EnvironmentManager, environment_manager: &EnvironmentManager,
cwd: &AbsolutePathBuf, cwd: &AbsolutePathBuf,
@@ -66,7 +86,7 @@ struct ResolvedEnvironment {
#[derive(Clone)] #[derive(Clone)]
struct SelectedTurnEnvironment { struct SelectedTurnEnvironment {
selection: TurnEnvironmentSelection, selection: TurnEnvironmentSelection,
config: EnvironmentConfig, config_origin: EnvironmentConfigOrigin,
environment: Arc<Environment>, environment: Arc<Environment>,
// Selection clones share one listener; the final handle drop aborts it. // Selection clones share one listener; the final handle drop aborts it.
connection_events_task: Option<Arc<AbortOnDropHandle<()>>>, connection_events_task: Option<Arc<AbortOnDropHandle<()>>>,
@@ -76,24 +96,33 @@ struct SelectedTurnEnvironment {
#[derive(Clone)] #[derive(Clone)]
pub(crate) struct StartingTurnEnvironment { pub(crate) struct StartingTurnEnvironment {
pub(crate) selection: TurnEnvironmentSelection, pub(crate) selection: TurnEnvironmentSelection,
config: EnvironmentConfig, config_origin: EnvironmentConfigOrigin,
resolution: TurnEnvironmentResolution, resolution: TurnEnvironmentResolution,
} }
impl SelectedTurnEnvironment { /// Resolves config once at the attachment boundary and records who owns future updates.
fn apply_configuration( fn resolve_selection_config(
&mut self, mut selection: TurnEnvironmentSelection,
selection_config: EnvironmentConfigState,
thread_config: &EnvironmentConfig, thread_config: &EnvironmentConfig,
) { ) -> (TurnEnvironmentSelection, EnvironmentConfigOrigin) {
self.config = match &selection_config { let (config, origin) = match selection.config {
EnvironmentConfigState::FromThread => thread_config.clone(), EnvironmentConfigState::FromThread => {
EnvironmentConfigState::Ready(config) => config.clone(), (thread_config.clone(), EnvironmentConfigOrigin::Thread)
}
EnvironmentConfigState::Ready(config) => (config, EnvironmentConfigOrigin::Owner),
EnvironmentConfigState::Pending => { EnvironmentConfigState::Pending => {
unreachable!("pending environment configuration is not supported yet") unreachable!("pending environment configuration is not supported yet")
} }
}; };
self.selection.config = selection_config; selection.config = EnvironmentConfigState::Ready(config);
(selection, origin)
}
impl SelectedTurnEnvironment {
fn refresh_thread_config(&mut self, config: &EnvironmentConfig) {
if self.config_origin == EnvironmentConfigOrigin::Thread {
self.selection.config = EnvironmentConfigState::Ready(config.clone());
}
} }
} }
@@ -139,9 +168,9 @@ impl ThreadEnvironments {
let TurnEnvironmentState::Ready(environment) = environment else { let TurnEnvironmentState::Ready(environment) = environment else {
return None; return None;
}; };
let selection = environment.selection(); let selection = environment.selection;
let config_origin = environment.config_origin;
let selected_environment = Arc::clone(&environment.environment); let selected_environment = Arc::clone(&environment.environment);
let inherited_config = environment.config;
let resolution: TurnEnvironmentResolution = let resolution: TurnEnvironmentResolution =
futures::future::ready(Ok(ResolvedEnvironment { futures::future::ready(Ok(ResolvedEnvironment {
environment: environment.environment, environment: environment.environment,
@@ -152,17 +181,13 @@ impl ThreadEnvironments {
.shared(); .shared();
let mut inherited_environment = SelectedTurnEnvironment { let mut inherited_environment = SelectedTurnEnvironment {
selection, selection,
config: inherited_config, config_origin,
environment: selected_environment, environment: selected_environment,
connection_events_task: None, connection_events_task: None,
resolution, resolution,
}; };
// Child threads get their own settings, but inherit any // Child threads re-infer thread-owned config while preserving owner config.
// environment-owned policy that was already installed. inherited_environment.refresh_thread_config(&thread_environment_config);
inherited_environment.apply_configuration(
inherited_environment.selection.config.clone(),
&thread_environment_config,
);
Some(inherited_environment) Some(inherited_environment)
}) })
.collect(); .collect();
@@ -188,6 +213,8 @@ impl ThreadEnvironments {
if !seen_environment_ids.insert(selected_environment.environment_id.as_str()) { if !seen_environment_ids.insert(selected_environment.environment_id.as_str()) {
continue; continue;
} }
let (selected_environment, config_origin) =
resolve_selection_config(selected_environment.clone(), thread_environment_config);
if let Some(environment) = previous.iter().find(|environment| { if let Some(environment) = previous.iter().find(|environment| {
let previous = &environment.selection; let previous = &environment.selection;
previous.environment_id == selected_environment.environment_id previous.environment_id == selected_environment.environment_id
@@ -196,10 +223,8 @@ impl ThreadEnvironments {
}) && !matches!(environment.resolution.clone().now_or_never(), Some(Err(_))) }) && !matches!(environment.resolution.clone().now_or_never(), Some(Err(_)))
{ {
let mut environment = environment.clone(); let mut environment = environment.clone();
environment.apply_configuration( environment.selection = selected_environment;
selected_environment.config.clone(), environment.config_origin = config_origin;
thread_environment_config,
);
next.push(environment); next.push(environment);
continue; continue;
} }
@@ -235,17 +260,13 @@ impl ThreadEnvironments {
.remote_handle(); .remote_handle();
drop(tokio::spawn(resolution_task)); drop(tokio::spawn(resolution_task));
let resolution = resolution.boxed().shared(); let resolution = resolution.boxed().shared();
let mut selected = SelectedTurnEnvironment { let selected = SelectedTurnEnvironment {
selection: selected_environment.clone(), selection: selected_environment,
config: thread_environment_config.clone(), config_origin,
environment, environment,
connection_events_task, connection_events_task,
resolution, resolution,
}; };
selected.apply_configuration(
selected_environment.config.clone(),
thread_environment_config,
);
next.push(selected); next.push(selected);
} }
let removed_connection_tasks = previous let removed_connection_tasks = previous
@@ -267,11 +288,16 @@ impl ThreadEnvironments {
} }
} }
/// Projects canonical selections back to caller input form without losing config ownership.
pub(crate) fn selections(&self) -> Vec<TurnEnvironmentSelection> { pub(crate) fn selections(&self) -> Vec<TurnEnvironmentSelection> {
self.environments self.environments
.load() .load()
.iter() .iter()
.map(|environment| environment.selection.clone()) .map(|environment| {
environment
.config_origin
.into_input_selection(environment.selection.clone())
})
.collect() .collect()
} }
@@ -299,14 +325,15 @@ impl ThreadEnvironments {
.unwrap_or_default() .unwrap_or_default()
} }
pub(crate) fn update_environment_configs(&self, config: &EnvironmentConfig) { /// Refreshes attachments whose configuration is inferred from the thread.
pub(crate) fn update_thread_config(&self, config: &EnvironmentConfig) {
let environments = self let environments = self
.environments .environments
.load() .load()
.iter() .iter()
.map(|environment| { .map(|environment| {
let mut environment = environment.clone(); let mut environment = environment.clone();
environment.apply_configuration(environment.selection.config.clone(), config); environment.refresh_thread_config(config);
environment environment
}) })
.collect(); .collect();
@@ -331,8 +358,8 @@ impl ThreadEnvironments {
))); )));
}; };
let thread_config = environment.config.clone(); environment.selection.config = EnvironmentConfigState::Ready(config);
environment.apply_configuration(EnvironmentConfigState::Ready(config), &thread_config); environment.config_origin = EnvironmentConfigOrigin::Owner;
self.environments.store(Arc::new(environments)); self.environments.store(Arc::new(environments));
Ok(()) Ok(())
} }
@@ -346,8 +373,10 @@ impl ThreadEnvironments {
let environments = self.environments.load(); let environments = self.environments.load();
let mut selected_capability_roots = thread_selected_capability_roots.to_vec(); let mut selected_capability_roots = thread_selected_capability_roots.to_vec();
for environment in environments.iter() { for environment in environments.iter() {
selected_capability_roots let EnvironmentConfigState::Ready(config) = &environment.selection.config else {
.extend(environment.config.selected_capability_roots.iter().cloned()); unreachable!("selected environments always carry resolved configuration")
};
selected_capability_roots.extend(config.selected_capability_roots.iter().cloned());
} }
let mut seen_root_ids = HashSet::with_capacity(selected_capability_roots.len()); let mut seen_root_ids = HashSet::with_capacity(selected_capability_roots.len());
selected_capability_roots.retain(|root| seen_root_ids.insert(root.id.clone())); selected_capability_roots.retain(|root| seen_root_ids.insert(root.id.clone()));
@@ -491,7 +520,7 @@ impl ThreadEnvironments {
if let Some(environment) = TurnEnvironmentState::from_resolution( if let Some(environment) = TurnEnvironmentState::from_resolution(
StartingTurnEnvironment { StartingTurnEnvironment {
selection: environment.selection.clone(), selection: environment.selection.clone(),
config: environment.config.clone(), config_origin: environment.config_origin,
resolution: environment.resolution.clone(), resolution: environment.resolution.clone(),
}, },
resolved, resolved,
@@ -522,9 +551,9 @@ impl TurnEnvironmentState {
Some(Ok(environment)) => { Some(Ok(environment)) => {
let mut turn_environment = TurnEnvironment::new( let mut turn_environment = TurnEnvironment::new(
starting.selection, starting.selection,
starting.config_origin,
environment.environment, environment.environment,
environment.shell, environment.shell,
starting.config,
); );
turn_environment.shell_snapshot = environment.shell_snapshot; turn_environment.shell_snapshot = environment.shell_snapshot;
Some(Self::Ready(turn_environment)) Some(Self::Ready(turn_environment))
@@ -897,7 +926,7 @@ url = "ws://127.0.0.1:8765"
let environment = snapshot.primary().expect("local environment"); let environment = snapshot.primary().expect("local environment");
assert_eq!(environment.shell.as_ref(), Some(&local_shell)); assert_eq!(environment.shell.as_ref(), Some(&local_shell));
assert_eq!(environment.config, expected_config); assert_eq!(environment.config(), &expected_config);
} }
#[tokio::test] #[tokio::test]
@@ -1107,6 +1136,10 @@ url = "ws://127.0.0.1:8765"
turn_environments.update_selections(&[remote.clone(), local.clone()], &expected_config); turn_environments.update_selections(&[remote.clone(), local.clone()], &expected_config);
let starting = turn_environments.snapshot().await; let starting = turn_environments.snapshot().await;
let resolved_remote = TurnEnvironmentSelection {
config: EnvironmentConfigState::Ready(expected_config.clone()),
..remote.clone()
};
assert_eq!( assert_eq!(
starting starting
.turn_environments() .turn_environments()
@@ -1117,7 +1150,7 @@ url = "ws://127.0.0.1:8765"
assert_eq!( assert_eq!(
starting starting
.turn_environments() .turn_environments()
.map(|environment| environment.config.clone()) .map(|environment| environment.config().clone())
.collect::<Vec<_>>(), .collect::<Vec<_>>(),
vec![expected_config.clone()] vec![expected_config.clone()]
); );
@@ -1126,13 +1159,13 @@ url = "ws://127.0.0.1:8765"
.starting() .starting()
.map(|environment| environment.selection.clone()) .map(|environment| environment.selection.clone())
.collect::<Vec<_>>(), .collect::<Vec<_>>(),
vec![remote.clone()] vec![resolved_remote.clone()]
); );
assert_eq!(starting.to_selections(), vec![local.clone()]); assert_eq!(starting.to_selections(), vec![local.clone()]);
assert!(starting.single_local_environment().is_none()); assert!(starting.single_local_environment().is_none());
let next_config = test_environment_config(); let next_config = test_environment_config();
turn_environments.update_environment_configs(&next_config); turn_environments.update_thread_config(&next_config);
let next_starting = turn_environments.snapshot().await; let next_starting = turn_environments.snapshot().await;
let server = tokio::spawn(serve_environment_info(listener)); let server = tokio::spawn(serve_environment_info(listener));
@@ -1161,7 +1194,7 @@ url = "ws://127.0.0.1:8765"
assert_eq!( assert_eq!(
attached attached
.turn_environments() .turn_environments()
.map(|environment| environment.config.clone()) .map(|environment| environment.config().clone())
.collect::<Vec<_>>(), .collect::<Vec<_>>(),
vec![expected_config.clone(), expected_config] vec![expected_config.clone(), expected_config]
); );
@@ -1170,7 +1203,7 @@ url = "ws://127.0.0.1:8765"
next_starting next_starting
.refresh_readiness() .refresh_readiness()
.turn_environments() .turn_environments()
.map(|environment| environment.config.clone()) .map(|environment| environment.config().clone())
.collect::<Vec<_>>(), .collect::<Vec<_>>(),
vec![next_config.clone(), next_config] vec![next_config.clone(), next_config]
); );
@@ -1240,7 +1273,7 @@ url = "ws://127.0.0.1:8765"
allow_login_shell: false, allow_login_shell: false,
..test_environment_config() ..test_environment_config()
}; };
environments.update_environment_configs(&next_config); environments.update_thread_config(&next_config);
assert!( assert!(
failed_resolution.ptr_eq(&environments.environments.load()[0].resolution), failed_resolution.ptr_eq(&environments.environments.load()[0].resolution),
"updating environment config must not retry a failed environment" "updating environment config must not retry a failed environment"
@@ -1252,7 +1285,14 @@ url = "ws://127.0.0.1:8765"
.starting() .starting()
.next() .next()
.expect("expected the replacement environment to be starting"); .expect("expected the replacement environment to be starting");
assert_eq!(replacement.selection, selection); assert_eq!(
replacement.selection,
TurnEnvironmentSelection {
config: EnvironmentConfigState::Ready(next_config),
..selection.clone()
}
);
assert_eq!(environments.selections(), vec![selection]);
assert!(!failed_resolution.ptr_eq(&replacement.resolution)); assert!(!failed_resolution.ptr_eq(&replacement.resolution));
} }
@@ -1373,10 +1413,13 @@ url = "ws://127.0.0.1:8765"
.expect("inherited environment"), .expect("inherited environment"),
); );
let inherited = TurnEnvironment::new( let inherited = TurnEnvironment::new(
selection.clone(), TurnEnvironmentSelection {
config: EnvironmentConfigState::Ready(test_environment_config()),
..selection.clone()
},
EnvironmentConfigOrigin::Thread,
Arc::clone(&inherited_environment), Arc::clone(&inherited_environment),
/*shell*/ None, /*shell*/ None,
test_environment_config(),
); );
let manager = Arc::new(environment_manager_without_environments()); let manager = Arc::new(environment_manager_without_environments());
manager manager
@@ -1411,7 +1454,7 @@ url = "ws://127.0.0.1:8765"
let inherited = snapshot.primary().expect("inherited environment"); let inherited = snapshot.primary().expect("inherited environment");
assert!(Arc::ptr_eq(&inherited.environment, &inherited_environment)); assert!(Arc::ptr_eq(&inherited.environment, &inherited_environment));
assert_eq!(inherited.config, child_config); assert_eq!(inherited.config(), &child_config);
} }
#[tokio::test] #[tokio::test]
@@ -1448,6 +1491,16 @@ url = "ws://127.0.0.1:8765"
), ),
..test_environment_config() ..test_environment_config()
}; };
parent.update_thread_config(&child_thread_config);
assert_eq!(
parent
.snapshot()
.await
.primary()
.expect("owner-configured environment")
.config(),
&parent_owner_config
);
let child = ThreadEnvironments::new( let child = ThreadEnvironments::new(
manager, manager,
crate::shell::default_user_shell(), crate::shell::default_user_shell(),
@@ -1471,10 +1524,10 @@ url = "ws://127.0.0.1:8765"
cleared_snapshot cleared_snapshot
.primary() .primary()
.expect("environment with cleared roots") .expect("environment with cleared roots")
.config, .config(),
cleared_parent_owner_config &cleared_parent_owner_config
); );
assert_eq!(child_environment.config, parent_owner_config); assert_eq!(child_environment.config(), &parent_owner_config);
let changed_selection = TurnEnvironmentSelection { let changed_selection = TurnEnvironmentSelection {
cwd: PathUri::from_abs_path(&cwd.join("changed")), cwd: PathUri::from_abs_path(&cwd.join("changed")),
@@ -1484,7 +1537,7 @@ url = "ws://127.0.0.1:8765"
parent.update_selections(std::slice::from_ref(&changed_selection), &new_thread_config); parent.update_selections(std::slice::from_ref(&changed_selection), &new_thread_config);
let changed_snapshot = parent.snapshot().await; let changed_snapshot = parent.snapshot().await;
let changed_environment = changed_snapshot.primary().expect("changed environment"); let changed_environment = changed_snapshot.primary().expect("changed environment");
assert_eq!(changed_environment.config, new_thread_config); assert_eq!(changed_environment.config(), &new_thread_config);
} }
#[tokio::test] #[tokio::test]
@@ -1514,11 +1567,11 @@ url = "ws://127.0.0.1:8765"
environment_id: REMOTE_ENVIRONMENT_ID.to_string(), environment_id: REMOTE_ENVIRONMENT_ID.to_string(),
cwd: remote_cwd_uri.clone(), cwd: remote_cwd_uri.clone(),
workspace_roots: Vec::new(), workspace_roots: Vec::new(),
config: EnvironmentConfigState::FromThread, config: EnvironmentConfigState::Ready(test_environment_config()),
}, },
EnvironmentConfigOrigin::Thread,
remote_environment.clone(), remote_environment.clone(),
/*shell*/ None, /*shell*/ None,
test_environment_config(),
))], ))],
}; };
let multiple = TurnEnvironmentSnapshot { let multiple = TurnEnvironmentSnapshot {
@@ -1528,11 +1581,11 @@ url = "ws://127.0.0.1:8765"
environment_id: REMOTE_ENVIRONMENT_ID.to_string(), environment_id: REMOTE_ENVIRONMENT_ID.to_string(),
cwd: remote_cwd_uri, cwd: remote_cwd_uri,
workspace_roots: Vec::new(), workspace_roots: Vec::new(),
config: EnvironmentConfigState::FromThread, config: EnvironmentConfigState::Ready(test_environment_config()),
}, },
EnvironmentConfigOrigin::Thread,
remote_environment, remote_environment,
/*shell*/ None, /*shell*/ None,
test_environment_config(),
)), )),
TurnEnvironmentState::Ready(local.primary().expect("local environment").clone()), TurnEnvironmentState::Ready(local.primary().expect("local environment").clone()),
], ],
@@ -1555,9 +1608,9 @@ url = "ws://127.0.0.1:8765"
environment_id: LOCAL_ENVIRONMENT_ID.to_string(), environment_id: LOCAL_ENVIRONMENT_ID.to_string(),
cwd: PathUri::from_abs_path(&cwd), cwd: PathUri::from_abs_path(&cwd),
workspace_roots: Vec::new(), workspace_roots: Vec::new(),
config: EnvironmentConfigState::FromThread, config: EnvironmentConfigState::Ready(test_environment_config()),
}, },
config: test_environment_config(), config_origin: EnvironmentConfigOrigin::Thread,
resolution: futures::future::pending().boxed().shared(), resolution: futures::future::pending().boxed().shared(),
})], })],
}; };
+12 -8
View File
@@ -1057,18 +1057,22 @@ async fn run_review_on_session(
.permissions .permissions
.permission_profile_state() .permission_profile_state()
.snapshot(); .snapshot();
let mut parent_turn_environments = params.parent_context.environments().to_selections(); // Guardian must receive read-only permissions for every inherited environment.
// FromThread inherits the Guardian profile below. Once Core normalizes FromThread let parent_turn_environments = params
// to Ready(config), every selection will carry explicit config and this conditional can be .parent_context
// removed. .environments()
for selection in &mut parent_turn_environments { .turn_environments()
if let EnvironmentConfigState::Ready(config) = &mut selection.config { .map(|environment| {
let mut selection = environment.selection();
let mut config = environment.config().clone();
config.permission_profile = config.permission_profile =
PermissionProfileSnapshot::legacy(read_only_guardian_permission_profile( PermissionProfileSnapshot::legacy(read_only_guardian_permission_profile(
config.permission_profile.permission_profile(), config.permission_profile.permission_profile(),
)); ));
} selection.config = EnvironmentConfigState::Ready(config);
} selection
})
.collect();
// TODO(anp): Migrate guardian review thread settings to a PathUri fallback cwd so foreign // TODO(anp): Migrate guardian review thread settings to a PathUri fallback cwd so foreign
// parent environments do not fall back to the host-native config cwd. // parent environments do not fall back to the host-native config cwd.
let parent_turn_legacy_fallback_cwd = params let parent_turn_legacy_fallback_cwd = params
+1 -1
View File
@@ -628,7 +628,7 @@ async fn build_guardian_prompt_includes_parent_turn_denied_reads() -> anyhow::Re
let TurnEnvironmentState::Ready(environment) = &mut turn.environments.environments[0] else { let TurnEnvironmentState::Ready(environment) = &mut turn.environments.environments[0] else {
panic!("parent environment should be ready"); panic!("parent environment should be ready");
}; };
environment.config.permission_profile = environment.config_mut().permission_profile =
PermissionProfileSnapshot::legacy(environment_permission_profile); PermissionProfileSnapshot::legacy(environment_permission_profile);
environment.selection.workspace_roots = vec![ environment.selection.workspace_roots = vec![
PathUri::from_abs_path(&workspace_root), PathUri::from_abs_path(&workspace_root),
+1 -1
View File
@@ -367,7 +367,7 @@ mod tests {
.primary() .primary()
.expect("ready primary environment"); .expect("ready primary environment");
let selection = environment.selection(); let selection = environment.selection();
let environment_config = environment.config.clone(); let environment_config = environment.config().clone();
let environments = crate::environment_selection::ThreadEnvironments::new( let environments = crate::environment_selection::ThreadEnvironments::new(
session.services.turn_environments.environment_manager(), session.services.turn_environments.environment_manager(),
crate::shell::default_user_shell(), crate::shell::default_user_shell(),
+6 -5
View File
@@ -1,6 +1,7 @@
use super::*; use super::*;
use crate::config::ConfigBuilder; use crate::config::ConfigBuilder;
use crate::config::ManagedFeatures; use crate::config::ManagedFeatures;
use crate::environment_selection::EnvironmentConfigOrigin;
use crate::environment_selection::TurnEnvironmentState; use crate::environment_selection::TurnEnvironmentState;
use crate::session::step_context::StepContext; use crate::session::step_context::StepContext;
use crate::session::tests::make_session_and_context; use crate::session::tests::make_session_and_context;
@@ -1112,11 +1113,7 @@ async fn mcp_sandbox_cwd_uses_matching_server_environment_uri() -> anyhow::Resul
environment_id: "remote".to_string(), environment_id: "remote".to_string(),
cwd: secondary_cwd.clone(), cwd: secondary_cwd.clone(),
workspace_roots: Vec::new(), workspace_roots: Vec::new(),
config: EnvironmentConfigState::FromThread, config: EnvironmentConfigState::Ready(EnvironmentConfig {
},
environment,
/*shell*/ None,
EnvironmentConfig {
allow_login_shell: true, allow_login_shell: true,
permission_profile: turn_context permission_profile: turn_context
.config .config
@@ -1124,7 +1121,11 @@ async fn mcp_sandbox_cwd_uses_matching_server_environment_uri() -> anyhow::Resul
.permission_profile_state() .permission_profile_state()
.snapshot(), .snapshot(),
selected_capability_roots: Vec::new(), selected_capability_roots: Vec::new(),
}),
}, },
EnvironmentConfigOrigin::Thread,
environment,
/*shell*/ None,
))); )));
let step_context = StepContext::for_test(Arc::new(turn_context)); let step_context = StepContext::for_test(Arc::new(turn_context));
+1 -1
View File
@@ -448,7 +448,7 @@ impl Session {
.chain( .chain(
environments environments
.turn_environments() .turn_environments()
.flat_map(|environment| environment.config.selected_capability_roots.clone()), .flat_map(|environment| environment.config().selected_capability_roots.clone()),
) )
.enumerate() .enumerate()
{ {
+1 -1
View File
@@ -1596,7 +1596,7 @@ impl Session {
} else if state.session_configuration.turn_environment_config() != environment_config { } else if state.session_configuration.turn_environment_config() != environment_config {
self.services self.services
.turn_environments .turn_environments
.update_environment_configs(&environment_config); .update_thread_config(&environment_config);
} }
state.session_configuration = updated; state.session_configuration = updated;
let new_config = notify_config_contributors let new_config = notify_config_contributors
+27 -21
View File
@@ -7,6 +7,7 @@ use crate::config::ConfigOverrides;
use crate::config::test_config; use crate::config::test_config;
use crate::context::ContextualUserFragment; use crate::context::ContextualUserFragment;
use crate::context::TurnAborted; use crate::context::TurnAborted;
use crate::environment_selection::EnvironmentConfigOrigin;
use crate::environment_selection::ThreadEnvironments; use crate::environment_selection::ThreadEnvironments;
use crate::environment_selection::TurnEnvironmentState; use crate::environment_selection::TurnEnvironmentState;
use crate::function_tool::FunctionCallError; use crate::function_tool::FunctionCallError;
@@ -5450,7 +5451,7 @@ async fn permission_profile_updates_apply_to_next_turn_environment() {
.environments .environments
.primary() .primary()
.expect("active turn environment") .expect("active turn environment")
.config .config()
.clone(); .clone();
let profile_root = active_turn.config.cwd.join("profile-root"); let profile_root = active_turn.config.cwd.join("profile-root");
let active_profile = ActivePermissionProfile::read_only(); let active_profile = ActivePermissionProfile::read_only();
@@ -5485,14 +5486,14 @@ async fn permission_profile_updates_apply_to_next_turn_environment() {
vec![profile_root], vec![profile_root],
); );
assert_eq!(next_environment.config, expected_environment_config); assert_eq!(next_environment.config(), &expected_environment_config);
assert_eq!( assert_eq!(
active_turn active_turn
.environments .environments
.primary() .primary()
.expect("active turn environment") .expect("active turn environment")
.config, .config(),
active_environment_config &active_environment_config
); );
} }
} }
@@ -6639,17 +6640,18 @@ async fn request_permissions_tool_resolves_relative_paths_against_selected_envir
.primary() .primary()
.expect("primary environment") .expect("primary environment")
.clone(); .clone();
let current_environment_config = current_environment.config().clone();
turn_context_mut.environments.environments[0] = turn_context_mut.environments.environments[0] =
TurnEnvironmentState::Ready(TurnEnvironment::new( TurnEnvironmentState::Ready(TurnEnvironment::new(
TurnEnvironmentSelection { TurnEnvironmentSelection {
environment_id: "remote".to_string(), environment_id: "remote".to_string(),
cwd: PathUri::from_abs_path(&environment_cwd), cwd: PathUri::from_abs_path(&environment_cwd),
workspace_roots: Vec::new(), workspace_roots: Vec::new(),
config: EnvironmentConfigState::FromThread, config: EnvironmentConfigState::Ready(current_environment_config),
}, },
current_environment.config_origin,
current_environment.environment, current_environment.environment,
current_environment.shell, current_environment.shell,
current_environment.config,
)); ));
let call_id = "call-1".to_string(); let call_id = "call-1".to_string();
@@ -7237,6 +7239,7 @@ async fn primary_environment_uses_first_turn_environment() {
#[allow(deprecated)] #[allow(deprecated)]
let second_cwd = turn_context.cwd.join("second"); let second_cwd = turn_context.cwd.join("second");
let second_cwd_uri = codex_utils_path_uri::PathUri::from_abs_path(&second_cwd); let second_cwd_uri = codex_utils_path_uri::PathUri::from_abs_path(&second_cwd);
let first_environment_config = first_environment.config().clone();
turn_context turn_context
.environments .environments
.environments .environments
@@ -7245,11 +7248,11 @@ async fn primary_environment_uses_first_turn_environment() {
environment_id: "second".to_string(), environment_id: "second".to_string(),
cwd: second_cwd_uri.clone(), cwd: second_cwd_uri.clone(),
workspace_roots: Vec::new(), workspace_roots: Vec::new(),
config: EnvironmentConfigState::FromThread, config: EnvironmentConfigState::Ready(first_environment_config),
}, },
first_environment.config_origin,
Arc::clone(&first_environment.environment), Arc::clone(&first_environment.environment),
/*shell*/ None, /*shell*/ None,
first_environment.config.clone(),
))); )));
assert_eq!( assert_eq!(
@@ -8624,7 +8627,7 @@ async fn conflicting_ready_environment_root_ids_keep_first_location() {
environment_id, environment_id,
.. ..
} = &selected_root.location; } = &selected_root.location;
let mut environment_config = local_environment.config.clone(); let mut environment_config = local_environment.config().clone();
environment_config.selected_capability_roots = vec![selected_root.clone()]; environment_config.selected_capability_roots = vec![selected_root.clone()];
turn_environments.push(TurnEnvironment::new( turn_environments.push(TurnEnvironment::new(
TurnEnvironmentSelection { TurnEnvironmentSelection {
@@ -8633,12 +8636,12 @@ async fn conflicting_ready_environment_root_ids_keep_first_location() {
workspace_roots: local_environment.workspace_roots().to_vec(), workspace_roots: local_environment.workspace_roots().to_vec(),
config: EnvironmentConfigState::Ready(environment_config.clone()), config: EnvironmentConfigState::Ready(environment_config.clone()),
}, },
EnvironmentConfigOrigin::Owner,
Arc::new( Arc::new(
codex_exec_server::Environment::create_for_tests(/*exec_server_url*/ None) codex_exec_server::Environment::create_for_tests(/*exec_server_url*/ None)
.expect("create test environment"), .expect("create test environment"),
), ),
local_environment.shell.clone(), local_environment.shell.clone(),
environment_config,
)); ));
} }
let environments = TurnEnvironmentSnapshot { let environments = TurnEnvironmentSnapshot {
@@ -8691,7 +8694,7 @@ async fn capability_discovery_uses_environment_permission_profile() {
access: FileSystemAccessMode::Deny, access: FileSystemAccessMode::Deny,
missing_path_behavior: None, missing_path_behavior: None,
}); });
environment.config.permission_profile = environment.config_mut().permission_profile =
PermissionProfileSnapshot::legacy(PermissionProfile::from_runtime_permissions( PermissionProfileSnapshot::legacy(PermissionProfile::from_runtime_permissions(
&file_system_policy, &file_system_policy,
NetworkSandboxPolicy::Restricted, NetworkSandboxPolicy::Restricted,
@@ -8888,17 +8891,18 @@ async fn record_context_updates_emits_environment_item_for_cwd_changes() {
.primary() .primary()
.expect("primary environment") .expect("primary environment")
.clone(); .clone();
let environment_config = environment.config().clone();
current_context.environments.environments[0] = current_context.environments.environments[0] =
TurnEnvironmentState::Ready(TurnEnvironment::new( TurnEnvironmentState::Ready(TurnEnvironment::new(
TurnEnvironmentSelection { TurnEnvironmentSelection {
environment_id: environment.selection.environment_id, environment_id: environment.selection.environment_id,
cwd: PathUri::from_abs_path(&cwd), cwd: PathUri::from_abs_path(&cwd),
workspace_roots: Vec::new(), workspace_roots: Vec::new(),
config: EnvironmentConfigState::FromThread, config: EnvironmentConfigState::Ready(environment_config),
}, },
environment.config_origin,
environment.environment, environment.environment,
environment.shell, environment.shell,
environment.config,
)); ));
let update_items = let update_items =
@@ -8927,7 +8931,7 @@ async fn record_context_updates_use_environment_permission_profile_and_workspace
.primary() .primary()
.expect("primary environment") .expect("primary environment")
.clone(); .clone();
previous_environment.config.permission_profile = previous_environment.config_mut().permission_profile =
PermissionProfileSnapshot::legacy(PermissionProfile::Disabled); PermissionProfileSnapshot::legacy(PermissionProfile::Disabled);
previous_context.environments.environments[0] = previous_context.environments.environments[0] =
TurnEnvironmentState::Ready(previous_environment); TurnEnvironmentState::Ready(previous_environment);
@@ -8945,7 +8949,7 @@ async fn record_context_updates_use_environment_permission_profile_and_workspace
.clone(); .clone();
let cwd = environment.cwd().clone(); let cwd = environment.cwd().clone();
let workspace_root = current_context.config.cwd.join("selected-workspace"); let workspace_root = current_context.config.cwd.join("selected-workspace");
let mut environment_config = environment.config; let mut environment_config = environment.config().clone();
environment_config.permission_profile = environment_config.permission_profile =
PermissionProfileSnapshot::legacy(PermissionProfile::workspace_write()); PermissionProfileSnapshot::legacy(PermissionProfile::workspace_write());
current_context.environments.environments[0] = current_context.environments.environments[0] =
@@ -8954,11 +8958,11 @@ async fn record_context_updates_use_environment_permission_profile_and_workspace
environment_id: environment.selection.environment_id, environment_id: environment.selection.environment_id,
cwd, cwd,
workspace_roots: vec![PathUri::from_abs_path(&workspace_root)], workspace_roots: vec![PathUri::from_abs_path(&workspace_root)],
config: EnvironmentConfigState::FromThread, config: EnvironmentConfigState::Ready(environment_config),
}, },
environment.config_origin,
environment.environment, environment.environment,
environment.shell, environment.shell,
environment_config,
)); ));
let update_items = let update_items =
@@ -9024,17 +9028,18 @@ async fn record_context_updates_omits_environment_item_when_disabled() {
.primary() .primary()
.expect("primary environment") .expect("primary environment")
.clone(); .clone();
let environment_config = environment.config().clone();
current_context.environments.environments[0] = current_context.environments.environments[0] =
TurnEnvironmentState::Ready(TurnEnvironment::new( TurnEnvironmentState::Ready(TurnEnvironment::new(
TurnEnvironmentSelection { TurnEnvironmentSelection {
environment_id: environment.selection.environment_id, environment_id: environment.selection.environment_id,
cwd: PathUri::from_abs_path(&test_path_buf("/new-repo").abs()), cwd: PathUri::from_abs_path(&test_path_buf("/new-repo").abs()),
workspace_roots: Vec::new(), workspace_roots: Vec::new(),
config: EnvironmentConfigState::FromThread, config: EnvironmentConfigState::Ready(environment_config),
}, },
environment.config_origin,
environment.environment, environment.environment,
environment.shell, environment.shell,
environment.config,
)); ));
let update_items = let update_items =
@@ -9497,16 +9502,17 @@ async fn turn_context_item_stores_local_cwd() {
.expect("primary environment") .expect("primary environment")
.clone(); .clone();
let cwd = PathUri::parse("file:///C:/windows").expect("Windows cwd URI"); let cwd = PathUri::parse("file:///C:/windows").expect("Windows cwd URI");
let environment_config = environment.config().clone();
turn_context.environments.environments[0] = TurnEnvironmentState::Ready(TurnEnvironment::new( turn_context.environments.environments[0] = TurnEnvironmentState::Ready(TurnEnvironment::new(
TurnEnvironmentSelection { TurnEnvironmentSelection {
environment_id: "remote".to_string(), environment_id: "remote".to_string(),
cwd, cwd,
workspace_roots: Vec::new(), workspace_roots: Vec::new(),
config: EnvironmentConfigState::FromThread, config: EnvironmentConfigState::Ready(environment_config),
}, },
environment.config_origin,
environment.environment, environment.environment,
environment.shell, environment.shell,
environment.config,
)); ));
#[allow(deprecated)] #[allow(deprecated)]
@@ -349,7 +349,7 @@ async fn guardian_allows_shell_command_additional_permissions_requests_past_poli
else { else {
panic!("primary environment should be ready"); panic!("primary environment should be ready");
}; };
environment.config.permission_profile = environment.config_mut().permission_profile =
config.permissions.permission_profile_state().snapshot(); config.permissions.permission_profile_state().snapshot();
config.codex_linux_sandbox_exe = codex_linux_sandbox_exe_or_skip!(); config.codex_linux_sandbox_exe = codex_linux_sandbox_exe_or_skip!();
config config
@@ -471,7 +471,7 @@ async fn strict_auto_review_turn_grant_forces_guardian_for_shell_command_policy_
else { else {
panic!("primary environment should be ready"); panic!("primary environment should be ready");
}; };
environment.config.permission_profile = environment.config_mut().permission_profile =
config.permissions.permission_profile_state().snapshot(); config.permissions.permission_profile_state().snapshot();
config.approvals_reviewer = ApprovalsReviewer::User; config.approvals_reviewer = ApprovalsReviewer::User;
config.model_provider.base_url = Some(format!("{}/v1", server.uri())); config.model_provider.base_url = Some(format!("{}/v1", server.uri()));
+28 -8
View File
@@ -1,4 +1,5 @@
use super::*; use super::*;
use crate::environment_selection::EnvironmentConfigOrigin;
use crate::environment_selection::TurnEnvironmentSnapshot; use crate::environment_selection::TurnEnvironmentSnapshot;
use crate::exec_policy::AllowPrefixRules; use crate::exec_policy::AllowPrefixRules;
use crate::shell_snapshot::ShellSnapshotFile; use crate::shell_snapshot::ShellSnapshotFile;
@@ -15,6 +16,7 @@ use codex_protocol::models::AdditionalPermissionProfile;
use codex_protocol::openai_models::MODEL_SPECIALTY_CYBER; use codex_protocol::openai_models::MODEL_SPECIALTY_CYBER;
use codex_protocol::openai_models::ModelInfo; use codex_protocol::openai_models::ModelInfo;
use codex_protocol::protocol::EnvironmentConfig; use codex_protocol::protocol::EnvironmentConfig;
use codex_protocol::protocol::EnvironmentConfigState;
use codex_protocol::protocol::ErrorEvent; use codex_protocol::protocol::ErrorEvent;
use codex_protocol::protocol::MultiAgentVersion; use codex_protocol::protocol::MultiAgentVersion;
use codex_protocol::protocol::ThreadHistoryMode; use codex_protocol::protocol::ThreadHistoryMode;
@@ -34,28 +36,44 @@ pub(crate) type ShellSnapshotTask = Shared<BoxFuture<'static, Option<Arc<ShellSn
#[derive(Clone)] #[derive(Clone)]
pub(crate) struct TurnEnvironment { pub(crate) struct TurnEnvironment {
pub(crate) selection: TurnEnvironmentSelection, pub(crate) selection: TurnEnvironmentSelection,
pub(crate) config_origin: EnvironmentConfigOrigin,
pub(crate) environment: Arc<Environment>, pub(crate) environment: Arc<Environment>,
pub(crate) shell: Option<shell::Shell>, pub(crate) shell: Option<shell::Shell>,
pub(crate) config: EnvironmentConfig,
pub(crate) shell_snapshot: ShellSnapshotTask, pub(crate) shell_snapshot: ShellSnapshotTask,
} }
impl TurnEnvironment { impl TurnEnvironment {
pub(crate) fn new( pub(crate) fn new(
selection: TurnEnvironmentSelection, selection: TurnEnvironmentSelection,
config_origin: EnvironmentConfigOrigin,
environment: Arc<Environment>, environment: Arc<Environment>,
shell: Option<shell::Shell>, shell: Option<shell::Shell>,
config: EnvironmentConfig,
) -> Self { ) -> Self {
debug_assert!(matches!(selection.config, EnvironmentConfigState::Ready(_)));
Self { Self {
selection, selection,
config_origin,
environment, environment,
shell, shell,
config,
shell_snapshot: futures::future::ready(None).boxed().shared(), shell_snapshot: futures::future::ready(None).boxed().shared(),
} }
} }
pub(crate) fn config(&self) -> &EnvironmentConfig {
let EnvironmentConfigState::Ready(config) = &self.selection.config else {
unreachable!("ready turn environments always carry resolved configuration")
};
config
}
#[cfg(test)]
pub(crate) fn config_mut(&mut self) -> &mut EnvironmentConfig {
let EnvironmentConfigState::Ready(config) = &mut self.selection.config else {
unreachable!("ready turn environments always carry resolved configuration")
};
config
}
pub(crate) fn shell_snapshot(&self, cwd: &AbsolutePathBuf) -> Option<AbsolutePathBuf> { pub(crate) fn shell_snapshot(&self, cwd: &AbsolutePathBuf) -> Option<AbsolutePathBuf> {
if self.selection.cwd != PathUri::from_abs_path(cwd) { if self.selection.cwd != PathUri::from_abs_path(cwd) {
return None; return None;
@@ -75,11 +93,11 @@ impl TurnEnvironment {
} }
pub(crate) fn permission_profile(&self) -> &PermissionProfile { pub(crate) fn permission_profile(&self) -> &PermissionProfile {
self.config.permission_profile.permission_profile() self.config().permission_profile.permission_profile()
} }
pub(crate) fn active_permission_profile(&self) -> Option<ActivePermissionProfile> { pub(crate) fn active_permission_profile(&self) -> Option<ActivePermissionProfile> {
self.config.permission_profile.active_permission_profile() self.config().permission_profile.active_permission_profile()
} }
pub(crate) fn permission_profile_with_workspace_roots(&self) -> PermissionProfile { pub(crate) fn permission_profile_with_workspace_roots(&self) -> PermissionProfile {
@@ -94,7 +112,8 @@ impl TurnEnvironment {
} }
pub(crate) fn selection(&self) -> TurnEnvironmentSelection { pub(crate) fn selection(&self) -> TurnEnvironmentSelection {
self.selection.clone() self.config_origin
.into_input_selection(self.selection.clone())
} }
} }
@@ -106,7 +125,8 @@ impl std::fmt::Debug for TurnEnvironment {
.field("cwd", &self.selection.cwd) .field("cwd", &self.selection.cwd)
.field("workspace_roots", &self.selection.workspace_roots) .field("workspace_roots", &self.selection.workspace_roots)
.field("shell", &self.shell) .field("shell", &self.shell)
.field("config", &self.config) .field("config", self.config())
.field("config_origin", &self.config_origin)
.finish_non_exhaustive() .finish_non_exhaustive()
} }
} }
@@ -690,7 +710,7 @@ impl Session {
{ {
self.services self.services
.turn_environments .turn_environments
.update_environment_configs(&environment_config); .update_thread_config(&environment_config);
} }
if mcp_inputs_changed { if mcp_inputs_changed {
self.mark_mcp_runtime_dirty(); self.mark_mcp_runtime_dirty();
@@ -2298,7 +2298,7 @@ async fn spawn_agent_reapplies_runtime_sandbox_after_role_config() {
else { else {
panic!("parent environment should be ready"); panic!("parent environment should be ready");
}; };
environment.config.permission_profile = environment.config_mut().permission_profile =
PermissionProfileSnapshot::legacy(expected_permission_profile.clone()); PermissionProfileSnapshot::legacy(expected_permission_profile.clone());
assert_ne!( assert_ne!(
role_config.permissions.effective_permission_profile(), role_config.permissions.effective_permission_profile(),
@@ -4572,7 +4572,7 @@ async fn build_agent_resume_config_clears_base_instructions() {
else { else {
panic!("parent environment should be ready"); panic!("parent environment should be ready");
}; };
environment.config.permission_profile = environment.config_mut().permission_profile =
PermissionProfileSnapshot::legacy(environment_permission_profile.clone()); PermissionProfileSnapshot::legacy(environment_permission_profile.clone());
let config = let config =
@@ -101,8 +101,10 @@ impl ShellCommandHandler {
.shell .shell
.as_ref() .as_ref()
.unwrap_or(session_shell.as_ref()); .unwrap_or(session_shell.as_ref());
let use_login_shell = let use_login_shell = Self::resolve_use_login_shell(
Self::resolve_use_login_shell(params.login, turn_environment.config.allow_login_shell)?; params.login,
turn_environment.config().allow_login_shell,
)?;
let command = Self::base_command(shell, &params.command, use_login_shell); let command = Self::base_command(shell, &params.command, use_login_shell);
let mut env = create_env( let mut env = create_env(
+11 -10
View File
@@ -6,6 +6,7 @@ use codex_protocol::models::ShellCommandToolCallParams;
use pretty_assertions::assert_eq; use pretty_assertions::assert_eq;
use crate::config::PermissionProfileSnapshot; use crate::config::PermissionProfileSnapshot;
use crate::environment_selection::EnvironmentConfigOrigin;
use crate::exec_env::CODEX_PERMISSION_PROFILE_ENV_VAR; use crate::exec_env::CODEX_PERMISSION_PROFILE_ENV_VAR;
use crate::exec_env::create_env; use crate::exec_env::create_env;
use crate::exec_env::inject_permission_profile_env; use crate::exec_env::inject_permission_profile_env;
@@ -109,8 +110,16 @@ async fn shell_command_handler_to_exec_params_uses_selected_environment() {
environment_id: "selected-environment".to_string(), environment_id: "selected-environment".to_string(),
cwd: PathUri::from_abs_path(&selected_cwd), cwd: PathUri::from_abs_path(&selected_cwd),
workspace_roots: Vec::new(), workspace_roots: Vec::new(),
config: EnvironmentConfigState::FromThread, config: EnvironmentConfigState::Ready(EnvironmentConfig {
allow_login_shell: true,
permission_profile: PermissionProfileSnapshot::active(
permission_profile,
active_permission_profile.clone(),
),
selected_capability_roots: Vec::new(),
}),
}, },
EnvironmentConfigOrigin::Thread,
Arc::clone( Arc::clone(
&turn_context &turn_context
.environments .environments
@@ -119,14 +128,6 @@ async fn shell_command_handler_to_exec_params_uses_selected_environment() {
.environment, .environment,
), ),
Some(selected_shell), Some(selected_shell),
EnvironmentConfig {
allow_login_shell: true,
permission_profile: PermissionProfileSnapshot::active(
permission_profile,
active_permission_profile.clone(),
),
selected_capability_roots: Vec::new(),
},
); );
let mut expected_env = create_env( let mut expected_env = create_env(
&turn_context.config.permissions.shell_environment_policy, &turn_context.config.permissions.shell_environment_policy,
@@ -210,7 +211,7 @@ async fn shell_command_handler_defaults_to_non_login_when_disallowed() {
.primary() .primary()
.expect("primary environment") .expect("primary environment")
.clone(); .clone();
turn_environment.config.allow_login_shell = false; turn_environment.config_mut().allow_login_shell = false;
let cwd = turn_environment let cwd = turn_environment
.cwd() .cwd()
.to_abs_path() .to_abs_path()
@@ -233,7 +233,7 @@ impl ExecCommandHandler {
&args, &args,
shell, shell,
&shell_mode, &shell_mode,
turn_environment.config.allow_login_shell, turn_environment.config().allow_login_shell,
) )
.map_err(FunctionCallError::RespondToModel)?; .map_err(FunctionCallError::RespondToModel)?;
let command = resolved_command.command; let command = resolved_command.command;
@@ -183,7 +183,7 @@ async fn exec_command_rejects_login_when_selected_environment_disallows_it() {
else { else {
panic!("primary environment should be ready"); panic!("primary environment should be ready");
}; };
environment.config.allow_login_shell = false; environment.config_mut().allow_login_shell = false;
let turn = Arc::new(turn); let turn = Arc::new(turn);
let invocation = ToolInvocation { let invocation = ToolInvocation {
@@ -290,9 +290,9 @@ mod tests {
selection.workspace_roots.clear(); selection.workspace_roots.clear();
turn.environments.environments[0] = TurnEnvironmentState::Ready(TurnEnvironment::new( turn.environments.environments[0] = TurnEnvironmentState::Ready(TurnEnvironment::new(
selection, selection,
current.config_origin,
current.environment, current.environment,
current.shell, current.shell,
current.config,
)); ));
} }
@@ -358,7 +358,7 @@ mod tests {
else { else {
panic!("primary environment should be ready"); panic!("primary environment should be ready");
}; };
environment.config.permission_profile = environment.config_mut().permission_profile =
PermissionProfileSnapshot::legacy(PermissionProfile::read_only()); PermissionProfileSnapshot::legacy(PermissionProfile::read_only());
let turn = Arc::new(turn); let turn = Arc::new(turn);
@@ -430,7 +430,7 @@ mod tests {
else { else {
panic!("primary environment should be ready"); panic!("primary environment should be ready");
}; };
environment.config.permission_profile = environment.config_mut().permission_profile =
PermissionProfileSnapshot::legacy(PermissionProfile::Disabled); PermissionProfileSnapshot::legacy(PermissionProfile::Disabled);
let turn = Arc::new(turn); let turn = Arc::new(turn);
@@ -467,7 +467,7 @@ mod tests {
else { else {
panic!("primary environment should be ready"); panic!("primary environment should be ready");
}; };
environment.config.permission_profile = environment.config_mut().permission_profile =
PermissionProfileSnapshot::legacy(PermissionProfile::Disabled); PermissionProfileSnapshot::legacy(PermissionProfile::Disabled);
let turn = Arc::new(turn); let turn = Arc::new(turn);
@@ -1,5 +1,6 @@
use super::*; use super::*;
use crate::config::PermissionProfileSnapshot; use crate::config::PermissionProfileSnapshot;
use crate::environment_selection::EnvironmentConfigOrigin;
use crate::tools::sandboxing::SandboxAttempt; use crate::tools::sandboxing::SandboxAttempt;
use codex_protocol::config_types::WindowsSandboxLevel; use codex_protocol::config_types::WindowsSandboxLevel;
use codex_protocol::models::AdditionalPermissionProfile; use codex_protocol::models::AdditionalPermissionProfile;
@@ -24,15 +25,17 @@ fn test_turn_environment(environment_id: &str) -> crate::session::turn_context::
environment_id: environment_id.to_string(), environment_id: environment_id.to_string(),
cwd: PathUri::from_abs_path(&std::env::temp_dir().abs()), cwd: PathUri::from_abs_path(&std::env::temp_dir().abs()),
workspace_roots: Vec::new(), workspace_roots: Vec::new(),
config: EnvironmentConfigState::FromThread, config: EnvironmentConfigState::Ready(EnvironmentConfig {
allow_login_shell: true,
permission_profile: PermissionProfileSnapshot::legacy(
PermissionProfile::read_only(),
),
selected_capability_roots: Vec::new(),
}),
}, },
EnvironmentConfigOrigin::Thread,
std::sync::Arc::new(codex_exec_server::Environment::default_for_tests()), std::sync::Arc::new(codex_exec_server::Environment::default_for_tests()),
/*shell*/ None, /*shell*/ None,
EnvironmentConfig {
allow_login_shell: true,
permission_profile: PermissionProfileSnapshot::legacy(PermissionProfile::read_only()),
selected_capability_roots: Vec::new(),
},
) )
} }
@@ -1,5 +1,6 @@
use super::*; use super::*;
use crate::config::PermissionProfileSnapshot; use crate::config::PermissionProfileSnapshot;
use crate::environment_selection::EnvironmentConfigOrigin;
use crate::tools::approvals::ApprovalCacheKey; use crate::tools::approvals::ApprovalCacheKey;
use codex_exec_server::Environment; use codex_exec_server::Environment;
use codex_protocol::models::PermissionProfile; use codex_protocol::models::PermissionProfile;
@@ -21,17 +22,17 @@ async fn approval_key_uses_path_uri_and_includes_environment_id() {
environment_id: "remote".to_string(), environment_id: "remote".to_string(),
cwd: PathUri::from_abs_path(&cwd), cwd: PathUri::from_abs_path(&cwd),
workspace_roots: Vec::new(), workspace_roots: Vec::new(),
config: EnvironmentConfigState::FromThread, config: EnvironmentConfigState::Ready(EnvironmentConfig {
},
Arc::new(Environment::default_for_tests()),
/*shell*/ None,
EnvironmentConfig {
allow_login_shell: true, allow_login_shell: true,
permission_profile: PermissionProfileSnapshot::legacy( permission_profile: PermissionProfileSnapshot::legacy(
PermissionProfile::read_only(), PermissionProfile::read_only(),
), ),
selected_capability_roots: Vec::new(), selected_capability_roots: Vec::new(),
}),
}, },
EnvironmentConfigOrigin::Thread,
Arc::new(Environment::default_for_tests()),
/*shell*/ None,
), ),
shell_type: None, shell_type: None,
hook_command: "echo hello".to_string(), hook_command: "echo hello".to_string(),
@@ -514,6 +514,7 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecAttempt> for UnifiedExecRunt
mod tests { mod tests {
use super::*; use super::*;
use crate::config::PermissionProfileSnapshot; use crate::config::PermissionProfileSnapshot;
use crate::environment_selection::EnvironmentConfigOrigin;
use crate::exec::DEFAULT_EXEC_COMMAND_TIMEOUT_MS; use crate::exec::DEFAULT_EXEC_COMMAND_TIMEOUT_MS;
use crate::tools::sandboxing::ToolRuntime; use crate::tools::sandboxing::ToolRuntime;
use codex_exec_server::Environment; use codex_exec_server::Environment;
@@ -536,17 +537,17 @@ mod tests {
environment_id: LOCAL_ENVIRONMENT_ID.to_string(), environment_id: LOCAL_ENVIRONMENT_ID.to_string(),
cwd, cwd,
workspace_roots: Vec::new(), workspace_roots: Vec::new(),
config: EnvironmentConfigState::FromThread, config: EnvironmentConfigState::Ready(EnvironmentConfig {
},
Arc::new(Environment::default_for_tests()),
/*shell*/ None,
EnvironmentConfig {
allow_login_shell: true, allow_login_shell: true,
permission_profile: PermissionProfileSnapshot::legacy( permission_profile: PermissionProfileSnapshot::legacy(
PermissionProfile::read_only(), PermissionProfile::read_only(),
), ),
selected_capability_roots: Vec::new(), selected_capability_roots: Vec::new(),
}),
}, },
EnvironmentConfigOrigin::Thread,
Arc::new(Environment::default_for_tests()),
/*shell*/ None,
) )
} }
+1 -1
View File
@@ -954,7 +954,7 @@ fn tool_environment_mode(environments: &TurnEnvironmentSnapshot) -> ToolEnvironm
fn any_environment_allows_login_shell(environments: &TurnEnvironmentSnapshot) -> bool { fn any_environment_allows_login_shell(environments: &TurnEnvironmentSnapshot) -> bool {
environments environments
.turn_environments() .turn_environments()
.any(|environment| environment.config.allow_login_shell) .any(|environment| environment.config().allow_login_shell)
} }
#[instrument(level = "trace", skip_all)] #[instrument(level = "trace", skip_all)]
+13 -11
View File
@@ -699,7 +699,7 @@ async fn login_shell_parameter_follows_selected_environment() {
else { else {
panic!("primary environment should be ready"); panic!("primary environment should be ready");
}; };
environment.config.allow_login_shell = allow_login_shell; environment.config_mut().allow_login_shell = allow_login_shell;
if guardian { if guardian {
turn.session_source = codex_protocol::protocol::SessionSource::SubAgent( turn.session_source = codex_protocol::protocol::SessionSource::SubAgent(
codex_protocol::protocol::SubAgentSource::Other( codex_protocol::protocol::SubAgentSource::Other(
@@ -731,7 +731,7 @@ async fn login_shell_parameter_is_available_when_any_environment_allows_it() {
let TurnEnvironmentState::Ready(environment) = environment else { let TurnEnvironmentState::Ready(environment) = environment else {
panic!("environment should be ready"); panic!("environment should be ready");
}; };
environment.config.allow_login_shell = index == 1; environment.config_mut().allow_login_shell = index == 1;
} }
}) })
.await; .await;
@@ -908,15 +908,7 @@ async fn zsh_fork_unified_exec_keeps_shell_parameter_when_remote_environment_ava
environment_id: "remote".to_string(), environment_id: "remote".to_string(),
cwd: remote_cwd, cwd: remote_cwd,
workspace_roots: Vec::new(), workspace_roots: Vec::new(),
config: EnvironmentConfigState::FromThread, config: EnvironmentConfigState::Ready(
},
Arc::new(
codex_exec_server::Environment::create_for_tests(Some(
"ws://127.0.0.1:1/remote-exec-server".to_string(),
))
.expect("remote test environment"),
),
/*shell*/ None,
codex_protocol::protocol::EnvironmentConfig { codex_protocol::protocol::EnvironmentConfig {
allow_login_shell: true, allow_login_shell: true,
permission_profile: turn permission_profile: turn
@@ -927,6 +919,16 @@ async fn zsh_fork_unified_exec_keeps_shell_parameter_when_remote_environment_ava
selected_capability_roots: Vec::new(), selected_capability_roots: Vec::new(),
}, },
), ),
},
crate::environment_selection::EnvironmentConfigOrigin::Thread,
Arc::new(
codex_exec_server::Environment::create_for_tests(Some(
"ws://127.0.0.1:1/remote-exec-server".to_string(),
))
.expect("remote test environment"),
),
/*shell*/ None,
),
)); ));
}) })
.await; .await;
+67 -11
View File
@@ -437,8 +437,9 @@ async fn explicit_remote_shell_runs_in_remote_cwd() -> Result<()> {
} }
#[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn ready_environment_permissions_override_thread_permissions() -> Result<()> { async fn environment_permissions_follow_configuration_ownership() -> Result<()> {
const CALL_ID: &str = "attachment-specific-permissions"; const THREAD_CONFIG_CALL_ID: &str = "thread-config-permissions";
const OWNER_CONFIG_CALL_ID: &str = "owner-config-permissions";
const FILE_NAME: &str = "attachment-read-only-marker.txt"; const FILE_NAME: &str = "attachment-read-only-marker.txt";
skip_if_target_windows!( skip_if_target_windows!(
@@ -459,13 +460,8 @@ async fn ready_environment_permissions_override_thread_permissions() -> Result<(
.expect("thread should allow workspace writes"); .expect("thread should allow workspace writes");
}); });
let test = builder.build_with_auto_env(&server).await?; let test = builder.build_with_auto_env(&server).await?;
let mut selection = test.executor_environment().selection().clone(); let selection = test.executor_environment().selection().clone();
let marker = selection.cwd.join(FILE_NAME)?; let marker = selection.cwd.join(FILE_NAME)?;
selection.config = EnvironmentConfigState::Ready(EnvironmentConfig {
allow_login_shell: test.config.permissions.allow_login_shell,
permission_profile: PermissionProfileSnapshot::legacy(PermissionProfile::read_only()),
selected_capability_roots: Vec::new(),
});
let (shell, command) = match test_target_os() { let (shell, command) = match test_target_os() {
TestTargetOs::Linux => ( TestTargetOs::Linux => (
@@ -499,7 +495,7 @@ async fn ready_environment_permissions_override_thread_permissions() -> Result<(
vec![ vec![
sse(vec![ sse(vec![
ev_response_created("resp-1"), ev_response_created("resp-1"),
ev_function_call(CALL_ID, "exec_command", &arguments), ev_function_call(THREAD_CONFIG_CALL_ID, "exec_command", &arguments),
ev_completed("resp-1"), ev_completed("resp-1"),
]), ]),
sse(vec![ sse(vec![
@@ -507,17 +503,77 @@ async fn ready_environment_permissions_override_thread_permissions() -> Result<(
ev_assistant_message("msg-1", "done"), ev_assistant_message("msg-1", "done"),
ev_completed("resp-2"), ev_completed("resp-2"),
]), ]),
sse(vec![
ev_response_created("resp-3"),
ev_function_call(OWNER_CONFIG_CALL_ID, "exec_command", &arguments),
ev_completed("resp-3"),
]),
sse(vec![
ev_response_created("resp-4"),
ev_assistant_message("msg-2", "done"),
ev_completed("resp-4"),
]),
], ],
) )
.await; .await;
test.submit_turn_with_environments("try to write a file", Some(vec![selection])) submit_thread_settings(
&test.codex,
ThreadSettingsOverrides {
permission_profile: Some(PermissionProfile::read_only()),
..Default::default()
},
)
.await?;
test.submit_text_turn("try to write a file with thread-owned permissions")
.await?; .await?;
let output = response_mock let output = response_mock
.last_request() .last_request()
.context("model should receive the command output")? .context("model should receive the command output")?
.function_call_output_text(CALL_ID) .function_call_output_text(THREAD_CONFIG_CALL_ID)
.context("shell tool result should be present")?;
assert!(
output.contains("WRITE_DENIED"),
"unexpected output: {output}"
);
assert!(!output.contains("WRITE_SUCCEEDED"));
submit_thread_settings(
&test.codex,
ThreadSettingsOverrides {
environments: Some(TurnEnvironmentSelections::new(
test.config.cwd.clone(),
vec![TurnEnvironmentSelection {
config: EnvironmentConfigState::Ready(EnvironmentConfig {
allow_login_shell: test.config.permissions.allow_login_shell,
permission_profile: PermissionProfileSnapshot::legacy(
PermissionProfile::read_only(),
),
selected_capability_roots: Vec::new(),
}),
..selection
}],
)),
..Default::default()
},
)
.await?;
submit_thread_settings(
&test.codex,
ThreadSettingsOverrides {
permission_profile: Some(PermissionProfile::workspace_write()),
..Default::default()
},
)
.await?;
test.submit_text_turn("try to write a file with owner-provided permissions")
.await?;
let output = response_mock
.last_request()
.context("model should receive the command output")?
.function_call_output_text(OWNER_CONFIG_CALL_ID)
.context("shell tool result should be present")?; .context("shell tool result should be present")?;
assert!( assert!(
output.contains("WRITE_DENIED"), output.contains("WRITE_DENIED"),