mirror of
https://github.com/openai/codex.git
synced 2026-11-09 20:50:33 +00:00
Preserve executor path URIs in permission profile workspace roots (#45863)
## Why Executor profile roots can use path conventions that are not native to the current host. Converting them to host paths during configuration or turn reconstruction can reject or drop those roots, while case-insensitive comparison can hide Windows path spelling changes. ## What changed - Store profile roots as URI-backed `ProfileWorkspaceRoot` values throughout permission snapshots and thread settings, preserving spelling in equality and deduplication. - Keep effective workspace roots as `PathUri` values for permission materialization and status summaries. Convert Windows sandbox root hints to native paths only at native Windows sandbox boundaries, rejecting incompatible roots. - Omit the legacy rollout `workspace_roots` field when profile roots cannot be represented as host paths, retaining the compiled permission profile. ## Testing Add regression coverage for Windows and UNC root spelling changes, settings restoration and turn recording with foreign roots, executor-root status display, and backend-specific Windows root conversion. GitOrigin-RevId: 903c068fd74959bdd10e7cb1141aa42b953a59a4
This commit is contained in:
@@ -31,6 +31,7 @@ use codex_protocol::SessionId;
|
||||
use codex_protocol::capabilities::SelectedCapabilityRoot;
|
||||
use codex_protocol::config_types::ShellEnvironmentPolicy;
|
||||
use codex_protocol::mcp::ClientMcpExtensions;
|
||||
use codex_protocol::models::ProfileWorkspaceRoot;
|
||||
use codex_protocol::permissions::FileSystemPath;
|
||||
use codex_protocol::permissions::FileSystemSpecialPath;
|
||||
use codex_protocol::protocol::EnvironmentConfig;
|
||||
@@ -545,7 +546,7 @@ impl SessionConfiguration {
|
||||
&mut self,
|
||||
permission_profile: PermissionProfile,
|
||||
active_permission_profile: Option<ActivePermissionProfile>,
|
||||
profile_workspace_roots: Vec<AbsolutePathBuf>,
|
||||
profile_workspace_roots: Vec<ProfileWorkspaceRoot>,
|
||||
preserve_deny_reads_from: Option<&FileSystemSandboxPolicy>,
|
||||
) -> ConstraintResult<()> {
|
||||
let enforcement = permission_profile.enforcement();
|
||||
@@ -589,7 +590,7 @@ pub(crate) struct SessionSettingsUpdate {
|
||||
pub(crate) step_settings: StepSettingsUpdate,
|
||||
pub(crate) environments: Option<TurnEnvironmentSelections>,
|
||||
pub(crate) runtime_workspace_roots: Option<Vec<AbsolutePathBuf>>,
|
||||
pub(crate) profile_workspace_roots: Option<Vec<AbsolutePathBuf>>,
|
||||
pub(crate) profile_workspace_roots: Option<Vec<ProfileWorkspaceRoot>>,
|
||||
pub(crate) sandbox_policy: Option<SandboxPolicy>,
|
||||
pub(crate) permission_profile: Option<PermissionProfile>,
|
||||
pub(crate) active_permission_profile: Option<ActivePermissionProfile>,
|
||||
|
||||
@@ -5606,7 +5606,7 @@ async fn permission_profile_updates_apply_to_next_turn_environment() {
|
||||
let updates = SessionSettingsUpdate {
|
||||
permission_profile: Some(PermissionProfile::read_only()),
|
||||
active_permission_profile: Some(active_profile.clone()),
|
||||
profile_workspace_roots: Some(vec![profile_root.clone()]),
|
||||
profile_workspace_roots: Some(vec![profile_root.clone().into()]),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
@@ -5636,7 +5636,7 @@ async fn permission_profile_updates_apply_to_next_turn_environment() {
|
||||
PermissionProfileSnapshot::active_with_profile_workspace_roots(
|
||||
PermissionProfile::read_only(),
|
||||
active_profile,
|
||||
vec![profile_root],
|
||||
vec![profile_root.into()],
|
||||
);
|
||||
|
||||
assert_eq!(next_environment.config(), &expected_environment_config);
|
||||
|
||||
@@ -480,22 +480,22 @@ impl TurnContext {
|
||||
}
|
||||
|
||||
/// Combines the selected environment's workspace roots with its permission profile roots.
|
||||
pub(crate) fn effective_workspace_roots(&self) -> Vec<AbsolutePathBuf> {
|
||||
pub(crate) fn effective_workspace_roots(&self) -> Vec<PathUri> {
|
||||
let Some(environment) = self.environments.primary() else {
|
||||
return self.config.effective_workspace_roots();
|
||||
};
|
||||
|
||||
let mut workspace_roots = environment
|
||||
.workspace_roots()
|
||||
.iter()
|
||||
.filter_map(|root| root.to_abs_path().ok())
|
||||
.collect::<Vec<_>>();
|
||||
let mut workspace_roots = environment.workspace_roots().to_vec();
|
||||
for root in environment
|
||||
.config()
|
||||
.permission_profile
|
||||
.profile_workspace_roots()
|
||||
{
|
||||
if !workspace_roots.contains(root) {
|
||||
let root = root.as_uri();
|
||||
if !workspace_roots
|
||||
.iter()
|
||||
.any(|existing| existing.to_string() == root.to_string())
|
||||
{
|
||||
workspace_roots.push(root.clone());
|
||||
}
|
||||
}
|
||||
@@ -652,7 +652,31 @@ impl TurnContext {
|
||||
}
|
||||
|
||||
pub(crate) fn to_turn_context_item(&self) -> TurnContextItem {
|
||||
let workspace_roots = self.effective_workspace_roots();
|
||||
// The legacy rollout field still stores host-native paths. Keep its
|
||||
// runtime-root filtering and omit it for unrepresentable profile roots;
|
||||
// the authoritative permission profile retains its concrete entries.
|
||||
let profile_roots = self.environments.primary().map_or_else(
|
||||
|| self.config.permissions.profile_workspace_roots(),
|
||||
|environment| {
|
||||
environment
|
||||
.config()
|
||||
.permission_profile
|
||||
.profile_workspace_roots()
|
||||
},
|
||||
);
|
||||
let workspace_roots = if profile_roots
|
||||
.iter()
|
||||
.all(|root| root.as_uri().to_abs_path().is_ok())
|
||||
{
|
||||
let roots = self
|
||||
.effective_workspace_roots()
|
||||
.iter()
|
||||
.filter_map(|root| root.to_abs_path().ok())
|
||||
.collect::<Vec<_>>();
|
||||
(!roots.is_empty()).then_some(roots)
|
||||
} else {
|
||||
None
|
||||
};
|
||||
#[allow(deprecated)]
|
||||
let cwd = self.cwd.clone();
|
||||
TurnContextItem {
|
||||
@@ -660,7 +684,7 @@ impl TurnContext {
|
||||
root_turn_id: self.turn_metadata_state.root_turn_id(),
|
||||
disabled_plugin_ids: Some(self.disabled_plugin_ids.clone()),
|
||||
cwd,
|
||||
workspace_roots: (!workspace_roots.is_empty()).then_some(workspace_roots),
|
||||
workspace_roots,
|
||||
current_date: self.current_date.clone(),
|
||||
timezone: self.timezone.clone(),
|
||||
approval_policy: self.approval_policy(),
|
||||
|
||||
Reference in New Issue
Block a user