mirror of
https://github.com/openai/codex.git
synced 2026-10-21 05:22:21 +00:00
Apply permission profile updates to future turn environments (#37031)
## What changed - Include the current permission profile in each turn environment's configuration. - Refresh retained environment configurations when session settings change, while preserving the configuration already captured by active turns. - Keep environment connection and shell snapshot resolution reusable across configuration updates. ## Testing - Cover permission profile updates applied through both session settings updates and per-turn updates. - Verify that pending environment resolutions use the configuration captured by their turn and that configuration-only updates do not retry failed environments. GitOrigin-RevId: 923694f0729b87a118d7635ade3a8ca9bcd4e7b1
This commit is contained in:
@@ -1,5 +1,6 @@
|
||||
use super::*;
|
||||
use crate::config::ConfigBuilder;
|
||||
use crate::config::PermissionProfileSnapshot;
|
||||
use crate::context::ContextualUserFragment;
|
||||
use crate::environment_selection::TurnEnvironmentSnapshot;
|
||||
use crate::environment_selection::TurnEnvironmentState;
|
||||
@@ -21,6 +22,7 @@ use codex_exec_server::ReadDirectoryEntry;
|
||||
use codex_exec_server::RemoveOptions;
|
||||
use codex_extension_api::UserInstructions;
|
||||
use codex_features::Feature;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use core_test_support::PathBufExt;
|
||||
@@ -340,6 +342,9 @@ fn resolved_local_environments<const N: usize>(
|
||||
/*shell*/ None,
|
||||
EnvironmentConfig {
|
||||
allow_login_shell: true,
|
||||
permission_profile: PermissionProfileSnapshot::legacy(
|
||||
PermissionProfile::read_only(),
|
||||
),
|
||||
},
|
||||
))
|
||||
})
|
||||
|
||||
@@ -271,6 +271,12 @@ impl PermissionProfileState {
|
||||
self.resolved_permission_profile.get().permission_profile()
|
||||
}
|
||||
|
||||
pub(crate) fn snapshot(&self) -> PermissionProfileSnapshot {
|
||||
PermissionProfileSnapshot {
|
||||
resolved_permission_profile: self.resolved_permission_profile.get().clone(),
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn active_permission_profile(&self) -> Option<ActivePermissionProfile> {
|
||||
self.resolved_permission_profile
|
||||
.get()
|
||||
|
||||
@@ -23,6 +23,7 @@ use futures::future::Shared;
|
||||
use tokio_util::task::AbortOnDropHandle;
|
||||
|
||||
use crate::session::turn_context::EnvironmentConfig;
|
||||
use crate::session::turn_context::ShellSnapshotTask;
|
||||
use crate::session::turn_context::TurnEnvironment;
|
||||
use crate::shell::Shell;
|
||||
use crate::shell_snapshot::ShellSnapshot;
|
||||
@@ -43,12 +44,23 @@ pub(crate) fn default_thread_environment_selections(
|
||||
.collect()
|
||||
}
|
||||
|
||||
type TurnEnvironmentResult = Result<TurnEnvironment, Arc<ExecServerError>>;
|
||||
type TurnEnvironmentResult = Result<ResolvedEnvironment, Arc<ExecServerError>>;
|
||||
type TurnEnvironmentResolution = Shared<BoxFuture<'static, TurnEnvironmentResult>>;
|
||||
|
||||
// Shared startup result used to build each turn's environment with its own config
|
||||
// without restarting the connection or shell resolution.
|
||||
#[derive(Clone)]
|
||||
struct ResolvedEnvironment {
|
||||
environment: Arc<Environment>,
|
||||
shell: Option<Shell>,
|
||||
shell_snapshot: ShellSnapshotTask,
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
struct SelectedTurnEnvironment {
|
||||
selection: TurnEnvironmentSelection,
|
||||
// Temporary: copied from thread settings until environments supply their own config.
|
||||
config: EnvironmentConfig,
|
||||
environment: Arc<Environment>,
|
||||
// Selection clones share one listener; the final handle drop aborts it.
|
||||
connection_events_task: Option<Arc<AbortOnDropHandle<()>>>,
|
||||
@@ -58,6 +70,7 @@ struct SelectedTurnEnvironment {
|
||||
#[derive(Clone)]
|
||||
pub(crate) struct StartingTurnEnvironment {
|
||||
pub(crate) selection: TurnEnvironmentSelection,
|
||||
config: EnvironmentConfig,
|
||||
resolution: TurnEnvironmentResolution,
|
||||
}
|
||||
|
||||
@@ -80,8 +93,6 @@ impl StartingTurnEnvironment {
|
||||
pub(crate) struct ThreadEnvironments {
|
||||
environment_manager: Arc<EnvironmentManager>,
|
||||
local_shell: Shell,
|
||||
// Temporary thread-wide config source until environments supply their own config.
|
||||
fallback_environment_config: EnvironmentConfig,
|
||||
shell_snapshot: ShellSnapshot,
|
||||
non_blocking_snapshots: bool,
|
||||
environments: ArcSwap<Vec<SelectedTurnEnvironment>>,
|
||||
@@ -92,7 +103,7 @@ impl ThreadEnvironments {
|
||||
pub(crate) fn new(
|
||||
environment_manager: Arc<EnvironmentManager>,
|
||||
local_shell: Shell,
|
||||
fallback_environment_config: EnvironmentConfig,
|
||||
thread_environment_config: EnvironmentConfig,
|
||||
shell_snapshot: ShellSnapshot,
|
||||
current: TurnEnvironmentSnapshot,
|
||||
non_blocking_snapshots: bool,
|
||||
@@ -102,17 +113,23 @@ impl ThreadEnvironments {
|
||||
.environments
|
||||
.into_iter()
|
||||
.filter_map(|environment| {
|
||||
let TurnEnvironmentState::Ready(mut environment) = environment else {
|
||||
let TurnEnvironmentState::Ready(environment) = environment else {
|
||||
return None;
|
||||
};
|
||||
// Child threads can have a different shell policy from their parent.
|
||||
environment.config = fallback_environment_config.clone();
|
||||
let selection = environment.selection();
|
||||
let selected_environment = Arc::clone(&environment.environment);
|
||||
let resolution: TurnEnvironmentResolution =
|
||||
futures::future::ready(Ok(environment)).boxed().shared();
|
||||
futures::future::ready(Ok(ResolvedEnvironment {
|
||||
environment: environment.environment,
|
||||
shell: environment.shell,
|
||||
shell_snapshot: environment.shell_snapshot,
|
||||
}))
|
||||
.boxed()
|
||||
.shared();
|
||||
Some(SelectedTurnEnvironment {
|
||||
selection,
|
||||
// Child threads can have different environment config from their parent.
|
||||
config: thread_environment_config.clone(),
|
||||
environment: selected_environment,
|
||||
connection_events_task: None,
|
||||
resolution,
|
||||
@@ -122,7 +139,6 @@ impl ThreadEnvironments {
|
||||
Self {
|
||||
environment_manager,
|
||||
local_shell,
|
||||
fallback_environment_config,
|
||||
shell_snapshot,
|
||||
non_blocking_snapshots,
|
||||
environments: ArcSwap::from_pointee(environments),
|
||||
@@ -130,7 +146,11 @@ impl ThreadEnvironments {
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn update_selections(&self, environments: &[TurnEnvironmentSelection]) {
|
||||
pub(crate) fn update_selections(
|
||||
&self,
|
||||
environments: &[TurnEnvironmentSelection],
|
||||
thread_environment_config: &EnvironmentConfig,
|
||||
) {
|
||||
let previous = self.environments.load();
|
||||
let mut seen_environment_ids = HashSet::with_capacity(environments.len());
|
||||
let mut next = Vec::with_capacity(environments.len());
|
||||
@@ -143,7 +163,9 @@ impl ThreadEnvironments {
|
||||
.find(|environment| environment.selection == *selected_environment)
|
||||
&& !matches!(environment.resolution.clone().now_or_never(), Some(Err(_)))
|
||||
{
|
||||
next.push(environment.clone());
|
||||
let mut environment = environment.clone();
|
||||
environment.config = thread_environment_config.clone();
|
||||
next.push(environment);
|
||||
continue;
|
||||
}
|
||||
|
||||
@@ -173,7 +195,6 @@ impl ThreadEnvironments {
|
||||
selected_environment.clone(),
|
||||
Arc::clone(&environment),
|
||||
self.local_shell.clone(),
|
||||
self.fallback_environment_config.clone(),
|
||||
self.shell_snapshot.clone(),
|
||||
)
|
||||
.remote_handle();
|
||||
@@ -181,6 +202,7 @@ impl ThreadEnvironments {
|
||||
let resolution = resolution.boxed().shared();
|
||||
next.push(SelectedTurnEnvironment {
|
||||
selection: selected_environment.clone(),
|
||||
config: thread_environment_config.clone(),
|
||||
environment,
|
||||
connection_events_task,
|
||||
resolution,
|
||||
@@ -205,6 +227,20 @@ impl ThreadEnvironments {
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn update_environment_configs(&self, config: &EnvironmentConfig) {
|
||||
let environments = self
|
||||
.environments
|
||||
.load()
|
||||
.iter()
|
||||
.map(|environment| {
|
||||
let mut environment = environment.clone();
|
||||
environment.config = config.clone();
|
||||
environment
|
||||
})
|
||||
.collect();
|
||||
self.environments.store(Arc::new(environments));
|
||||
}
|
||||
|
||||
fn spawn_connection_event_listener(
|
||||
environment: &Environment,
|
||||
environment_id: String,
|
||||
@@ -273,7 +309,6 @@ impl ThreadEnvironments {
|
||||
selection: TurnEnvironmentSelection,
|
||||
environment: Arc<Environment>,
|
||||
local_shell: Shell,
|
||||
config: EnvironmentConfig,
|
||||
shell_snapshot: ShellSnapshot,
|
||||
) -> BoxFuture<'static, TurnEnvironmentResult> {
|
||||
async move {
|
||||
@@ -303,21 +338,16 @@ impl ThreadEnvironments {
|
||||
} else {
|
||||
Some(local_shell)
|
||||
};
|
||||
let mut turn_environment = TurnEnvironment::new(
|
||||
selection.environment_id,
|
||||
environment,
|
||||
selection.cwd,
|
||||
selection.workspace_roots,
|
||||
shell,
|
||||
config,
|
||||
);
|
||||
let task = shell_snapshot
|
||||
.build(turn_environment.clone())
|
||||
.build(Arc::clone(&environment), selection.cwd, shell.clone())
|
||||
.boxed()
|
||||
.shared();
|
||||
drop(tokio::spawn(task.clone()));
|
||||
turn_environment.shell_snapshot = task;
|
||||
Ok(turn_environment)
|
||||
Ok(ResolvedEnvironment {
|
||||
environment,
|
||||
shell,
|
||||
shell_snapshot: task,
|
||||
})
|
||||
}
|
||||
.boxed()
|
||||
}
|
||||
@@ -335,6 +365,7 @@ impl ThreadEnvironments {
|
||||
if let Some(environment) = TurnEnvironmentState::from_resolution(
|
||||
StartingTurnEnvironment {
|
||||
selection: environment.selection.clone(),
|
||||
config: environment.config.clone(),
|
||||
resolution: environment.resolution.clone(),
|
||||
},
|
||||
resolved,
|
||||
@@ -362,7 +393,18 @@ impl TurnEnvironmentState {
|
||||
resolved: Option<TurnEnvironmentResult>,
|
||||
) -> Option<Self> {
|
||||
match resolved {
|
||||
Some(Ok(environment)) => Some(Self::Ready(environment)),
|
||||
Some(Ok(environment)) => {
|
||||
let mut turn_environment = TurnEnvironment::new(
|
||||
starting.selection.environment_id,
|
||||
environment.environment,
|
||||
starting.selection.cwd,
|
||||
starting.selection.workspace_roots,
|
||||
environment.shell,
|
||||
starting.config,
|
||||
);
|
||||
turn_environment.shell_snapshot = environment.shell_snapshot;
|
||||
Some(Self::Ready(turn_environment))
|
||||
}
|
||||
Some(Err(err)) => {
|
||||
tracing::debug!(
|
||||
environment_id = %starting.selection.environment_id,
|
||||
@@ -486,6 +528,7 @@ impl TurnEnvironmentSnapshot {
|
||||
mod tests {
|
||||
use std::time::Duration;
|
||||
|
||||
use crate::config::PermissionProfileSnapshot;
|
||||
use codex_exec_server::Environment;
|
||||
use codex_exec_server::ExecServerRuntimePaths;
|
||||
use codex_exec_server::LOCAL_ENVIRONMENT_ID;
|
||||
@@ -493,6 +536,8 @@ mod tests {
|
||||
use codex_exec_server_test_support::environment_manager_without_environments;
|
||||
use codex_http_client::HttpClientFactory;
|
||||
use codex_http_client::OutboundProxyPolicy;
|
||||
use codex_protocol::models::ActivePermissionProfile;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::protocol::TurnEnvironmentSelection;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
@@ -509,6 +554,13 @@ mod tests {
|
||||
|
||||
use super::*;
|
||||
|
||||
fn test_environment_config() -> EnvironmentConfig {
|
||||
EnvironmentConfig {
|
||||
allow_login_shell: true,
|
||||
permission_profile: PermissionProfileSnapshot::legacy(PermissionProfile::read_only()),
|
||||
}
|
||||
}
|
||||
|
||||
async fn resolve_turn_environments(
|
||||
environment_manager: Arc<EnvironmentManager>,
|
||||
selections: &[TurnEnvironmentSelection],
|
||||
@@ -516,14 +568,12 @@ mod tests {
|
||||
let turn_environments = Arc::new(ThreadEnvironments::new(
|
||||
environment_manager,
|
||||
crate::shell::default_user_shell(),
|
||||
EnvironmentConfig {
|
||||
allow_login_shell: true,
|
||||
},
|
||||
test_environment_config(),
|
||||
ShellSnapshot::disabled(),
|
||||
TurnEnvironmentSnapshot::default(),
|
||||
/*non_blocking_snapshots*/ false,
|
||||
));
|
||||
turn_environments.update_selections(selections);
|
||||
turn_environments.update_selections(selections, &test_environment_config());
|
||||
turn_environments.snapshot().await;
|
||||
turn_environments
|
||||
}
|
||||
@@ -668,27 +718,36 @@ url = "ws://127.0.0.1:8765"
|
||||
shell_type: crate::shell::ShellType::Zsh,
|
||||
shell_path: std::path::PathBuf::from("/configured/zsh"),
|
||||
};
|
||||
let expected_config = EnvironmentConfig {
|
||||
allow_login_shell: false,
|
||||
permission_profile: PermissionProfileSnapshot::active_with_profile_workspace_roots(
|
||||
PermissionProfile::read_only(),
|
||||
ActivePermissionProfile::read_only(),
|
||||
vec![cwd.join("profile-root")],
|
||||
),
|
||||
};
|
||||
let turn_environments = ThreadEnvironments::new(
|
||||
Arc::new(EnvironmentManager::default_for_tests()),
|
||||
local_shell.clone(),
|
||||
EnvironmentConfig {
|
||||
allow_login_shell: false,
|
||||
},
|
||||
expected_config.clone(),
|
||||
ShellSnapshot::disabled(),
|
||||
TurnEnvironmentSnapshot::default(),
|
||||
/*non_blocking_snapshots*/ false,
|
||||
);
|
||||
turn_environments.update_selections(&[TurnEnvironmentSelection {
|
||||
environment_id: LOCAL_ENVIRONMENT_ID.to_string(),
|
||||
cwd: PathUri::from_abs_path(&cwd),
|
||||
workspace_roots: Vec::new(),
|
||||
}]);
|
||||
turn_environments.update_selections(
|
||||
&[TurnEnvironmentSelection {
|
||||
environment_id: LOCAL_ENVIRONMENT_ID.to_string(),
|
||||
cwd: PathUri::from_abs_path(&cwd),
|
||||
workspace_roots: Vec::new(),
|
||||
}],
|
||||
&expected_config,
|
||||
);
|
||||
|
||||
let snapshot = turn_environments.snapshot().await;
|
||||
let environment = snapshot.primary().expect("local environment");
|
||||
|
||||
assert_eq!(environment.shell.as_ref(), Some(&local_shell));
|
||||
assert!(!environment.config.allow_login_shell);
|
||||
assert_eq!(environment.config, expected_config);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -810,14 +869,13 @@ url = "ws://127.0.0.1:8765"
|
||||
let environments = Arc::new(ThreadEnvironments::new(
|
||||
manager,
|
||||
crate::shell::default_user_shell(),
|
||||
EnvironmentConfig {
|
||||
allow_login_shell: true,
|
||||
},
|
||||
test_environment_config(),
|
||||
ShellSnapshot::disabled(),
|
||||
TurnEnvironmentSnapshot::default(),
|
||||
/*non_blocking_snapshots*/ false,
|
||||
));
|
||||
environments.update_selections(std::slice::from_ref(&selection));
|
||||
environments
|
||||
.update_selections(std::slice::from_ref(&selection), &test_environment_config());
|
||||
let snapshot_task = tokio::spawn({
|
||||
let environments = Arc::clone(&environments);
|
||||
async move { environments.snapshot().await }
|
||||
@@ -852,6 +910,14 @@ url = "ws://127.0.0.1:8765"
|
||||
.await,
|
||||
);
|
||||
let cwd = AbsolutePathBuf::current_dir().expect("cwd");
|
||||
let expected_config = EnvironmentConfig {
|
||||
allow_login_shell: false,
|
||||
permission_profile: PermissionProfileSnapshot::active_with_profile_workspace_roots(
|
||||
PermissionProfile::read_only(),
|
||||
ActivePermissionProfile::read_only(),
|
||||
vec![cwd.join("profile-root")],
|
||||
),
|
||||
};
|
||||
let cwd = PathUri::from_abs_path(&cwd);
|
||||
let remote = TurnEnvironmentSelection {
|
||||
environment_id: REMOTE_ENVIRONMENT_ID.to_string(),
|
||||
@@ -866,20 +932,19 @@ url = "ws://127.0.0.1:8765"
|
||||
let turn_environments = ThreadEnvironments::new(
|
||||
manager,
|
||||
crate::shell::default_user_shell(),
|
||||
EnvironmentConfig {
|
||||
allow_login_shell: true,
|
||||
},
|
||||
test_environment_config(),
|
||||
ShellSnapshot::disabled(),
|
||||
TurnEnvironmentSnapshot::default(),
|
||||
/*non_blocking_snapshots*/ true,
|
||||
);
|
||||
turn_environments.update_selections(std::slice::from_ref(&local));
|
||||
turn_environments
|
||||
.update_selections(std::slice::from_ref(&local), &test_environment_config());
|
||||
turn_environments.environments.load()[0]
|
||||
.resolution
|
||||
.clone()
|
||||
.await
|
||||
.expect("local environment should resolve");
|
||||
turn_environments.update_selections(&[remote.clone(), local.clone()]);
|
||||
turn_environments.update_selections(&[remote.clone(), local.clone()], &expected_config);
|
||||
|
||||
let starting = turn_environments.snapshot().await;
|
||||
assert_eq!(
|
||||
@@ -889,6 +954,13 @@ url = "ws://127.0.0.1:8765"
|
||||
.collect::<Vec<_>>(),
|
||||
vec![local.clone()]
|
||||
);
|
||||
assert_eq!(
|
||||
starting
|
||||
.turn_environments()
|
||||
.map(|environment| environment.config.clone())
|
||||
.collect::<Vec<_>>(),
|
||||
vec![expected_config.clone()]
|
||||
);
|
||||
assert_eq!(
|
||||
starting
|
||||
.starting()
|
||||
@@ -899,6 +971,10 @@ url = "ws://127.0.0.1:8765"
|
||||
assert_eq!(starting.to_selections(), vec![local.clone()]);
|
||||
assert!(starting.single_local_environment().is_none());
|
||||
|
||||
let next_config = test_environment_config();
|
||||
turn_environments.update_environment_configs(&next_config);
|
||||
let next_starting = turn_environments.snapshot().await;
|
||||
|
||||
let server = tokio::spawn(serve_environment_info(listener));
|
||||
timeout(
|
||||
std::time::Duration::from_secs(5),
|
||||
@@ -922,7 +998,22 @@ url = "ws://127.0.0.1:8765"
|
||||
.collect::<Vec<_>>(),
|
||||
vec![remote.clone(), local.clone()]
|
||||
);
|
||||
assert_eq!(
|
||||
attached
|
||||
.turn_environments()
|
||||
.map(|environment| environment.config.clone())
|
||||
.collect::<Vec<_>>(),
|
||||
vec![expected_config.clone(), expected_config]
|
||||
);
|
||||
assert_eq!(attached.to_selections(), vec![remote, local]);
|
||||
assert_eq!(
|
||||
next_starting
|
||||
.refresh_readiness()
|
||||
.turn_environments()
|
||||
.map(|environment| environment.config.clone())
|
||||
.collect::<Vec<_>>(),
|
||||
vec![next_config.clone(), next_config]
|
||||
);
|
||||
server.await.expect("server task");
|
||||
}
|
||||
|
||||
@@ -943,14 +1034,13 @@ url = "ws://127.0.0.1:8765"
|
||||
let environments = ThreadEnvironments::new(
|
||||
Arc::clone(&manager),
|
||||
crate::shell::default_user_shell(),
|
||||
EnvironmentConfig {
|
||||
allow_login_shell: true,
|
||||
},
|
||||
test_environment_config(),
|
||||
ShellSnapshot::disabled(),
|
||||
TurnEnvironmentSnapshot::default(),
|
||||
/*non_blocking_snapshots*/ true,
|
||||
);
|
||||
environments.update_selections(std::slice::from_ref(&selection));
|
||||
environments
|
||||
.update_selections(std::slice::from_ref(&selection), &test_environment_config());
|
||||
let failed_resolution = environments.environments.load()[0].resolution.clone();
|
||||
assert!(failed_resolution.clone().await.is_err());
|
||||
|
||||
@@ -964,7 +1054,16 @@ url = "ws://127.0.0.1:8765"
|
||||
/*connect_timeout*/ None,
|
||||
)
|
||||
.expect("replacement environment");
|
||||
environments.update_selections(std::slice::from_ref(&selection));
|
||||
let next_config = EnvironmentConfig {
|
||||
allow_login_shell: false,
|
||||
..test_environment_config()
|
||||
};
|
||||
environments.update_environment_configs(&next_config);
|
||||
assert!(
|
||||
failed_resolution.ptr_eq(&environments.environments.load()[0].resolution),
|
||||
"updating environment config must not retry a failed environment"
|
||||
);
|
||||
environments.update_selections(std::slice::from_ref(&selection), &next_config);
|
||||
|
||||
let replacement = environments.snapshot().await;
|
||||
let replacement = replacement
|
||||
@@ -1000,15 +1099,14 @@ url = "ws://127.0.0.1:8765"
|
||||
let environments = Arc::new(ThreadEnvironments::new(
|
||||
Arc::clone(&manager),
|
||||
crate::shell::default_user_shell(),
|
||||
EnvironmentConfig {
|
||||
allow_login_shell: true,
|
||||
},
|
||||
test_environment_config(),
|
||||
ShellSnapshot::disabled(),
|
||||
TurnEnvironmentSnapshot::default(),
|
||||
/*non_blocking_snapshots*/ true,
|
||||
));
|
||||
environments.start_connection_event_forwarding(tx_event);
|
||||
environments.update_selections(std::slice::from_ref(&selection));
|
||||
environments
|
||||
.update_selections(std::slice::from_ref(&selection), &test_environment_config());
|
||||
let initial_snapshot = environments.snapshot().await;
|
||||
let second_listener = TcpListener::bind("127.0.0.1:0")
|
||||
.await
|
||||
@@ -1026,12 +1124,16 @@ url = "ws://127.0.0.1:8765"
|
||||
)
|
||||
.expect("replace environment");
|
||||
|
||||
environments.update_selections(std::slice::from_ref(&selection));
|
||||
environments
|
||||
.update_selections(std::slice::from_ref(&selection), &test_environment_config());
|
||||
let reused_snapshot = environments.snapshot().await;
|
||||
environments.update_selections(&[TurnEnvironmentSelection {
|
||||
cwd: PathUri::from_abs_path(&cwd.join("changed")),
|
||||
..selection
|
||||
}]);
|
||||
environments.update_selections(
|
||||
&[TurnEnvironmentSelection {
|
||||
cwd: PathUri::from_abs_path(&cwd.join("changed")),
|
||||
..selection
|
||||
}],
|
||||
&test_environment_config(),
|
||||
);
|
||||
let changed_snapshot = environments.snapshot().await;
|
||||
|
||||
let initial = initial_snapshot
|
||||
@@ -1075,7 +1177,7 @@ url = "ws://127.0.0.1:8765"
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn inherited_environment_reuses_parent_handle_and_uses_child_login_policy() {
|
||||
async fn inherited_environment_reuses_parent_handle_and_uses_child_config() {
|
||||
let cwd = AbsolutePathBuf::current_dir().expect("cwd");
|
||||
let selection = TurnEnvironmentSelection {
|
||||
environment_id: REMOTE_ENVIRONMENT_ID.to_string(),
|
||||
@@ -1092,9 +1194,7 @@ url = "ws://127.0.0.1:8765"
|
||||
selection.cwd.clone(),
|
||||
Vec::new(),
|
||||
/*shell*/ None,
|
||||
EnvironmentConfig {
|
||||
allow_login_shell: true,
|
||||
},
|
||||
test_environment_config(),
|
||||
);
|
||||
let manager = Arc::new(environment_manager_without_environments());
|
||||
manager
|
||||
@@ -1104,12 +1204,18 @@ url = "ws://127.0.0.1:8765"
|
||||
/*connect_timeout*/ None,
|
||||
)
|
||||
.expect("replacement environment");
|
||||
let child_config = EnvironmentConfig {
|
||||
allow_login_shell: false,
|
||||
permission_profile: PermissionProfileSnapshot::active_with_profile_workspace_roots(
|
||||
PermissionProfile::read_only(),
|
||||
ActivePermissionProfile::read_only(),
|
||||
vec![cwd.join("child-profile-root")],
|
||||
),
|
||||
};
|
||||
let environments = ThreadEnvironments::new(
|
||||
manager,
|
||||
crate::shell::default_user_shell(),
|
||||
EnvironmentConfig {
|
||||
allow_login_shell: false,
|
||||
},
|
||||
child_config.clone(),
|
||||
ShellSnapshot::disabled(),
|
||||
TurnEnvironmentSnapshot {
|
||||
environments: vec![TurnEnvironmentState::Ready(inherited)],
|
||||
@@ -1117,12 +1223,12 @@ url = "ws://127.0.0.1:8765"
|
||||
/*non_blocking_snapshots*/ false,
|
||||
);
|
||||
|
||||
environments.update_selections(std::slice::from_ref(&selection));
|
||||
environments.update_selections(std::slice::from_ref(&selection), &child_config);
|
||||
let snapshot = environments.snapshot().await;
|
||||
|
||||
let inherited = snapshot.primary().expect("inherited environment");
|
||||
assert!(Arc::ptr_eq(&inherited.environment, &inherited_environment));
|
||||
assert!(!inherited.config.allow_login_shell);
|
||||
assert_eq!(inherited.config, child_config);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -1151,9 +1257,7 @@ url = "ws://127.0.0.1:8765"
|
||||
cwd_uri.clone(),
|
||||
Vec::new(),
|
||||
/*shell*/ None,
|
||||
EnvironmentConfig {
|
||||
allow_login_shell: true,
|
||||
},
|
||||
test_environment_config(),
|
||||
))],
|
||||
};
|
||||
let multiple = TurnEnvironmentSnapshot {
|
||||
@@ -1165,9 +1269,7 @@ url = "ws://127.0.0.1:8765"
|
||||
cwd_uri,
|
||||
Vec::new(),
|
||||
/*shell*/ None,
|
||||
EnvironmentConfig {
|
||||
allow_login_shell: true,
|
||||
},
|
||||
test_environment_config(),
|
||||
)),
|
||||
],
|
||||
};
|
||||
|
||||
@@ -217,7 +217,6 @@ mod tests {
|
||||
use super::*;
|
||||
use crate::environment_selection::TurnEnvironmentState;
|
||||
use crate::session::tests::make_session_and_context;
|
||||
use crate::session::turn_context::EnvironmentConfig;
|
||||
use crate::session::turn_context::TurnContext;
|
||||
use crate::session::turn_context::TurnEnvironment;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
@@ -316,22 +315,21 @@ mod tests {
|
||||
.await
|
||||
.expect("write local file");
|
||||
set_primary_environment_cwd(&mut turn_context, dir.path());
|
||||
let selection = turn_context
|
||||
let environment = turn_context
|
||||
.environments
|
||||
.primary()
|
||||
.expect("ready primary environment")
|
||||
.selection();
|
||||
.expect("ready primary environment");
|
||||
let selection = environment.selection();
|
||||
let environment_config = environment.config.clone();
|
||||
let environments = crate::environment_selection::ThreadEnvironments::new(
|
||||
session.services.turn_environments.environment_manager(),
|
||||
crate::shell::default_user_shell(),
|
||||
EnvironmentConfig {
|
||||
allow_login_shell: turn_context.config.permissions.allow_login_shell,
|
||||
},
|
||||
environment_config.clone(),
|
||||
crate::shell_snapshot::ShellSnapshot::disabled(),
|
||||
Default::default(),
|
||||
/*non_blocking_snapshots*/ true,
|
||||
);
|
||||
environments.update_selections(std::slice::from_ref(&selection));
|
||||
environments.update_selections(std::slice::from_ref(&selection), &environment_config);
|
||||
turn_context.environments = environments.snapshot().await;
|
||||
turn_context
|
||||
.environments
|
||||
|
||||
@@ -1104,6 +1104,11 @@ async fn mcp_sandbox_cwd_uses_matching_server_environment_uri() -> anyhow::Resul
|
||||
/*shell*/ None,
|
||||
EnvironmentConfig {
|
||||
allow_login_shell: true,
|
||||
permission_profile: turn_context
|
||||
.config
|
||||
.permissions
|
||||
.permission_profile_state()
|
||||
.snapshot(),
|
||||
},
|
||||
)));
|
||||
|
||||
|
||||
@@ -1531,10 +1531,15 @@ impl Session {
|
||||
let permission_profile_changed =
|
||||
previous_permission_profile != updated_permission_profile;
|
||||
let mcp_inputs_changed = state.session_configuration.mcp_inputs_differ(&updated);
|
||||
let environment_config = updated.environment_config();
|
||||
if updates.environments.is_some() {
|
||||
self.services
|
||||
.turn_environments
|
||||
.update_selections(updated.environment_selections());
|
||||
.update_selections(updated.environment_selections(), &environment_config);
|
||||
} else if state.session_configuration.environment_config() != environment_config {
|
||||
self.services
|
||||
.turn_environments
|
||||
.update_environment_configs(&environment_config);
|
||||
}
|
||||
state.session_configuration = updated;
|
||||
if mcp_inputs_changed {
|
||||
|
||||
@@ -156,6 +156,16 @@ impl SessionConfiguration {
|
||||
&self.permission_profile_state
|
||||
}
|
||||
|
||||
pub(super) fn environment_config(&self) -> EnvironmentConfig {
|
||||
EnvironmentConfig {
|
||||
allow_login_shell: self
|
||||
.original_config_do_not_use
|
||||
.permissions
|
||||
.allow_login_shell,
|
||||
permission_profile: self.permission_profile_state.snapshot(),
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) fn permission_profile(&self) -> PermissionProfile {
|
||||
self.permission_profile_state.permission_profile().clone()
|
||||
}
|
||||
@@ -943,14 +953,15 @@ impl Session {
|
||||
environment_manager,
|
||||
default_shell.clone(),
|
||||
// Temporary: preserve thread-level behavior until environments supply config.
|
||||
EnvironmentConfig {
|
||||
allow_login_shell: config.permissions.allow_login_shell,
|
||||
},
|
||||
session_configuration.environment_config(),
|
||||
shell_snapshot,
|
||||
inherited_environments.unwrap_or_default(),
|
||||
config.features.enabled(Feature::DeferredExecutor),
|
||||
));
|
||||
turn_environments.update_selections(session_configuration.environment_selections());
|
||||
turn_environments.update_selections(
|
||||
session_configuration.environment_selections(),
|
||||
&session_configuration.environment_config(),
|
||||
);
|
||||
let resolved_environments = turn_environments.snapshot().await;
|
||||
let agents_md_manager = Arc::new(AgentsMdManager::new(user_instructions));
|
||||
let plugin_skill_warmup = warm_plugins_and_skills_for_session_init(
|
||||
|
||||
@@ -1,5 +1,4 @@
|
||||
use super::mcp_refresh::McpRefresh;
|
||||
use super::turn_context::EnvironmentConfig;
|
||||
use super::turn_context::TurnEnvironment;
|
||||
use super::*;
|
||||
use crate::agents_md_manager::AgentsMdManager;
|
||||
@@ -4871,17 +4870,15 @@ async fn resolved_environments_for_configuration(
|
||||
let turn_environments = ThreadEnvironments::new(
|
||||
Arc::clone(&environment_manager),
|
||||
default_user_shell(),
|
||||
EnvironmentConfig {
|
||||
allow_login_shell: session_configuration
|
||||
.original_config_do_not_use
|
||||
.permissions
|
||||
.allow_login_shell,
|
||||
},
|
||||
session_configuration.environment_config(),
|
||||
ShellSnapshot::disabled(),
|
||||
TurnEnvironmentSnapshot::default(),
|
||||
/*non_blocking_snapshots*/ false,
|
||||
);
|
||||
turn_environments.update_selections(session_configuration.environment_selections());
|
||||
turn_environments.update_selections(
|
||||
session_configuration.environment_selections(),
|
||||
&session_configuration.environment_config(),
|
||||
);
|
||||
(environment_manager, turn_environments.snapshot().await)
|
||||
}
|
||||
|
||||
@@ -5363,6 +5360,61 @@ async fn session_update_settings_does_not_rewrite_sticky_environment_cwds() {
|
||||
assert_eq!(next_turn.config.cwd, turn_cwd);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn permission_profile_updates_apply_to_next_turn_environment() {
|
||||
for apply_on_turn_start in [false, true] {
|
||||
let (session, active_turn) = make_session_and_context().await;
|
||||
let active_environment_config = active_turn
|
||||
.environments
|
||||
.primary()
|
||||
.expect("active turn environment")
|
||||
.config
|
||||
.clone();
|
||||
let profile_root = active_turn.config.cwd.join("profile-root");
|
||||
let active_profile = ActivePermissionProfile::read_only();
|
||||
let updates = SessionSettingsUpdate {
|
||||
permission_profile: Some(PermissionProfile::read_only()),
|
||||
active_permission_profile: Some(active_profile.clone()),
|
||||
profile_workspace_roots: Some(vec![profile_root.clone()]),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
let next_turn = if apply_on_turn_start {
|
||||
session
|
||||
.new_turn_with_sub_id("permission-profile-update".to_string(), updates)
|
||||
.await
|
||||
.expect("turn permission profile update should succeed")
|
||||
} else {
|
||||
session
|
||||
.update_settings(updates)
|
||||
.await
|
||||
.expect("permission profile update should succeed");
|
||||
session.new_default_turn().await
|
||||
};
|
||||
let next_environment = next_turn
|
||||
.environments
|
||||
.primary()
|
||||
.expect("next turn environment");
|
||||
let mut expected_environment_config = active_environment_config.clone();
|
||||
expected_environment_config.permission_profile =
|
||||
PermissionProfileSnapshot::active_with_profile_workspace_roots(
|
||||
PermissionProfile::read_only(),
|
||||
active_profile,
|
||||
vec![profile_root],
|
||||
);
|
||||
|
||||
assert_eq!(next_environment.config, expected_environment_config);
|
||||
assert_eq!(
|
||||
active_turn
|
||||
.environments
|
||||
.primary()
|
||||
.expect("active turn environment")
|
||||
.config,
|
||||
active_environment_config
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn relative_cwd_update_without_environments_resolves_under_session_cwd() {
|
||||
let (session, _turn_context) = make_session_and_context().await;
|
||||
@@ -5681,9 +5733,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
|
||||
let turn_environments = Arc::new(ThreadEnvironments::new(
|
||||
environment_manager,
|
||||
default_user_shell(),
|
||||
EnvironmentConfig {
|
||||
allow_login_shell: config.permissions.allow_login_shell,
|
||||
},
|
||||
session_configuration.environment_config(),
|
||||
ShellSnapshot::disabled(),
|
||||
resolved_environments,
|
||||
/*non_blocking_snapshots*/ false,
|
||||
@@ -7077,14 +7127,18 @@ async fn turn_environments_set_primary_environment() {
|
||||
|
||||
#[tokio::test]
|
||||
async fn default_turn_does_not_overlay_legacy_fallback_cwd_onto_stored_thread_environments() {
|
||||
let (session, _turn_context, _rx) = make_session_and_context_with_rx().await;
|
||||
let (session, initial_turn, _rx) = make_session_and_context_with_rx().await;
|
||||
let session_cwd = session.get_config().await.cwd.clone();
|
||||
let selected_cwd =
|
||||
AbsolutePathBuf::try_from(session_cwd.as_path().join("selected")).expect("absolute path");
|
||||
session
|
||||
.services
|
||||
.turn_environments
|
||||
.update_selections(&[local(selected_cwd.clone())]);
|
||||
session.services.turn_environments.update_selections(
|
||||
&[local(selected_cwd.clone())],
|
||||
&initial_turn
|
||||
.environments
|
||||
.primary()
|
||||
.expect("initial turn environment")
|
||||
.config,
|
||||
);
|
||||
{
|
||||
let mut state = session.state.lock().await;
|
||||
state.session_configuration.environments.environments = vec![local(selected_cwd.clone())];
|
||||
@@ -7113,10 +7167,17 @@ async fn default_turn_does_not_overlay_legacy_fallback_cwd_onto_stored_thread_en
|
||||
|
||||
#[tokio::test]
|
||||
async fn default_turn_honors_empty_stored_thread_environments() {
|
||||
let (session, _turn_context, _rx) = make_session_and_context_with_rx().await;
|
||||
let (session, initial_turn, _rx) = make_session_and_context_with_rx().await;
|
||||
let session_cwd = session.get_config().await.cwd.clone();
|
||||
|
||||
session.services.turn_environments.update_selections(&[]);
|
||||
session.services.turn_environments.update_selections(
|
||||
&[],
|
||||
&initial_turn
|
||||
.environments
|
||||
.primary()
|
||||
.expect("initial turn environment")
|
||||
.config,
|
||||
);
|
||||
{
|
||||
let mut state = session.state.lock().await;
|
||||
state.session_configuration.environments.environments = Vec::new();
|
||||
@@ -7888,9 +7949,7 @@ where
|
||||
let turn_environments = Arc::new(ThreadEnvironments::new(
|
||||
environment_manager,
|
||||
default_user_shell(),
|
||||
EnvironmentConfig {
|
||||
allow_login_shell: config.permissions.allow_login_shell,
|
||||
},
|
||||
session_configuration.environment_config(),
|
||||
ShellSnapshot::disabled(),
|
||||
resolved_turn_environments.clone(),
|
||||
/*non_blocking_snapshots*/ false,
|
||||
|
||||
@@ -29,6 +29,7 @@ pub(crate) type ShellSnapshotTask = Shared<BoxFuture<'static, Option<Arc<ShellSn
|
||||
#[derive(Clone, Debug, PartialEq, Eq)]
|
||||
pub(crate) struct EnvironmentConfig {
|
||||
pub(crate) allow_login_shell: bool,
|
||||
pub(crate) permission_profile: PermissionProfileSnapshot,
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
@@ -611,10 +612,16 @@ impl Session {
|
||||
});
|
||||
let new_config = notify_config_contributors
|
||||
.then(|| Self::build_effective_session_config(&next));
|
||||
let environment_config = next.environment_config();
|
||||
if updates.environments.is_some() {
|
||||
self.services
|
||||
.turn_environments
|
||||
.update_selections(next.environment_selections());
|
||||
.update_selections(next.environment_selections(), &environment_config);
|
||||
} else if state.session_configuration.environment_config() != environment_config
|
||||
{
|
||||
self.services
|
||||
.turn_environments
|
||||
.update_environment_configs(&environment_config);
|
||||
}
|
||||
if mcp_inputs_changed {
|
||||
self.mark_mcp_runtime_dirty();
|
||||
|
||||
@@ -7,7 +7,6 @@ use std::time::SystemTime;
|
||||
|
||||
use crate::StateDbHandle;
|
||||
use crate::rollout::list::find_thread_path_by_id_str;
|
||||
use crate::session::turn_context::TurnEnvironment;
|
||||
use crate::shell::Shell;
|
||||
use crate::shell::ShellType;
|
||||
use crate::shell::get_shell;
|
||||
@@ -15,9 +14,11 @@ use anyhow::Context;
|
||||
use anyhow::Result;
|
||||
use anyhow::anyhow;
|
||||
use anyhow::bail;
|
||||
use codex_exec_server::Environment;
|
||||
use codex_otel::SessionTelemetry;
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use tokio::fs;
|
||||
use tokio::process::Command;
|
||||
use tokio::time::timeout;
|
||||
@@ -68,17 +69,19 @@ impl ShellSnapshot {
|
||||
|
||||
pub(crate) async fn build(
|
||||
self,
|
||||
environment: TurnEnvironment,
|
||||
environment: Arc<Environment>,
|
||||
cwd: PathUri,
|
||||
shell: Option<Shell>,
|
||||
) -> Option<Arc<ShellSnapshotFile>> {
|
||||
let config = self.config.as_ref()?;
|
||||
if environment.environment.is_remote() {
|
||||
if environment.is_remote() {
|
||||
return None;
|
||||
}
|
||||
|
||||
let shell = environment.shell.clone()?;
|
||||
let shell = shell?;
|
||||
// TODO(anp): Migrate shell snapshot creation to accept PathUri and defer native
|
||||
// conversion to the spawned shell process.
|
||||
let cwd = environment.cwd().to_abs_path().ok()?;
|
||||
let cwd = cwd.to_abs_path().ok()?;
|
||||
Self::build_for_cwd(Arc::clone(config), cwd, shell).await
|
||||
}
|
||||
|
||||
|
||||
@@ -114,6 +114,11 @@ async fn shell_command_handler_to_exec_params_uses_selected_environment() {
|
||||
Some(selected_shell),
|
||||
EnvironmentConfig {
|
||||
allow_login_shell: true,
|
||||
permission_profile: turn_context
|
||||
.config
|
||||
.permissions
|
||||
.permission_profile_state()
|
||||
.snapshot(),
|
||||
},
|
||||
);
|
||||
let mut expected_env = create_env(
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
use super::*;
|
||||
use crate::config::PermissionProfileSnapshot;
|
||||
use crate::session::turn_context::EnvironmentConfig;
|
||||
use crate::tools::sandboxing::SandboxAttempt;
|
||||
use codex_protocol::config_types::WindowsSandboxLevel;
|
||||
@@ -24,6 +25,7 @@ fn test_turn_environment(environment_id: &str) -> crate::session::turn_context::
|
||||
/*shell*/ None,
|
||||
EnvironmentConfig {
|
||||
allow_login_shell: true,
|
||||
permission_profile: PermissionProfileSnapshot::legacy(PermissionProfile::read_only()),
|
||||
},
|
||||
)
|
||||
}
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
use super::*;
|
||||
use crate::config::PermissionProfileSnapshot;
|
||||
use crate::session::turn_context::EnvironmentConfig;
|
||||
use codex_exec_server::Environment;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::sync::Arc;
|
||||
@@ -19,6 +21,9 @@ async fn approval_key_uses_path_uri_and_includes_environment_id() {
|
||||
/*shell*/ None,
|
||||
EnvironmentConfig {
|
||||
allow_login_shell: true,
|
||||
permission_profile: PermissionProfileSnapshot::legacy(
|
||||
PermissionProfile::read_only(),
|
||||
),
|
||||
},
|
||||
),
|
||||
shell_type: None,
|
||||
|
||||
@@ -550,11 +550,13 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::config::PermissionProfileSnapshot;
|
||||
use crate::exec::DEFAULT_EXEC_COMMAND_TIMEOUT_MS;
|
||||
use crate::session::turn_context::EnvironmentConfig;
|
||||
use crate::tools::sandboxing::ToolRuntime;
|
||||
use codex_exec_server::Environment;
|
||||
use codex_exec_server::LOCAL_ENVIRONMENT_ID;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_tools::ZshForkConfig;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
@@ -571,6 +573,9 @@ mod tests {
|
||||
/*shell*/ None,
|
||||
EnvironmentConfig {
|
||||
allow_login_shell: true,
|
||||
permission_profile: PermissionProfileSnapshot::legacy(
|
||||
PermissionProfile::read_only(),
|
||||
),
|
||||
},
|
||||
)
|
||||
}
|
||||
|
||||
@@ -912,6 +912,11 @@ async fn zsh_fork_unified_exec_keeps_shell_parameter_when_remote_environment_ava
|
||||
/*shell*/ None,
|
||||
crate::session::turn_context::EnvironmentConfig {
|
||||
allow_login_shell: true,
|
||||
permission_profile: turn
|
||||
.config
|
||||
.permissions
|
||||
.permission_profile_state()
|
||||
.snapshot(),
|
||||
},
|
||||
),
|
||||
));
|
||||
|
||||
Reference in New Issue
Block a user