tui: sync session permission profiles (#18284)

## Why

Once `SessionConfigured` carries the active `PermissionProfile`, the TUI
must treat that as authoritative session state. Otherwise the widget can
keep stale local permission details after a session is configured or
resumed.

The TUI also keeps a local `Config` copy used for later operations, so
session-sourced profiles and subsequent local sandbox changes need to
keep the derived split runtime permissions in sync. Because this PR may
land before the follow-up user-turn profile plumbing, embedded
app-server turns also need a standalone path for carrying local runtime
sandbox overrides.

## What changed

- Sync the chat widget runtime filesystem/network permissions from
`SessionConfigured.permission_profile`, with the legacy `sandbox_policy`
as the fallback.
- Recompute split runtime permissions whenever the TUI applies or
carries forward a local sandbox-policy override.
- Mark feature-driven Auto-review sandbox changes as runtime sandbox
overrides so the standalone embedded turn-start profile path is used
even without the follow-up user-turn profile PR.
- Send a turn-start `permissionProfile` for embedded,
non-ExternalSandbox turns when the TUI has a runtime sandbox override;
remote and ExternalSandbox turns keep using the legacy sandbox field.
- Extend coverage for profile sync, local sandbox changes,
ExternalSandbox fallback, feature-driven sandbox overrides, and
turn-start permission override selection.

## Verification

- `cargo test -p codex-tui
update_feature_flags_enabling_guardian_selects_auto_review`
- `cargo test -p codex-tui
turn_start_permission_overrides_send_profiles_only_for_embedded_runtime_overrides`
- `cargo test -p codex-tui permission_settings_sync`
- `cargo test -p codex-tui
session_configured_external_sandbox_keeps_external_runtime_policy`
- `cargo test -p codex-tui
session_configured_syncs_widget_config_permissions_and_cwd`
- `just fix -p codex-tui`



---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/openai/codex/pull/18284).
* #18288
* #18287
* #18286
* #18285
* __->__ #18284
This commit is contained in:
Michael Bolin
2026-04-23 09:47:53 -07:00
committed by GitHub
Unverified
parent 1fda843fbc
commit 5c239ad748
7 changed files with 326 additions and 22 deletions
+25 -7
View File
@@ -72,13 +72,15 @@ impl App {
"Failed to carry forward approval policy override: {err}"
));
}
if let Some(policy) = self.runtime_sandbox_policy_override.as_ref()
&& let Err(err) = config.permissions.sandbox_policy.set(policy.clone())
{
tracing::warn!(%err, "failed to carry forward sandbox policy override");
self.chat_widget.add_error_message(format!(
"Failed to carry forward sandbox policy override: {err}"
));
if let Some(policy) = self.runtime_sandbox_policy_override.as_ref() {
if let Err(err) = config.permissions.sandbox_policy.set(policy.clone()) {
tracing::warn!(%err, "failed to carry forward sandbox policy override");
self.chat_widget.add_error_message(format!(
"Failed to carry forward sandbox policy override: {err}"
));
} else {
sync_runtime_permissions_from_legacy_sandbox_policy(config);
}
}
}
@@ -117,6 +119,7 @@ impl App {
.add_error_message(format!("{user_message_prefix}: {err}"));
return false;
}
sync_runtime_permissions_from_legacy_sandbox_policy(config);
true
}
@@ -306,6 +309,10 @@ impl App {
self.chat_widget
.add_error_message(format!("Failed to enable Auto-review: {err}"));
}
if sandbox_policy_override.is_some() {
self.runtime_sandbox_policy_override =
Some(self.config.permissions.sandbox_policy.get().clone());
}
if approval_policy_override.is_some()
|| approvals_reviewer_override.is_some()
@@ -536,6 +543,17 @@ impl App {
}
}
fn sync_runtime_permissions_from_legacy_sandbox_policy(config: &mut Config) {
let sandbox_policy = config.permissions.sandbox_policy.get();
config.permissions.file_system_sandbox_policy =
codex_protocol::permissions::FileSystemSandboxPolicy::from_legacy_sandbox_policy(
sandbox_policy,
&config.cwd,
);
config.permissions.network_sandbox_policy =
codex_protocol::permissions::NetworkSandboxPolicy::from(sandbox_policy);
}
#[cfg(test)]
mod tests {
use super::*;
+4 -1
View File
@@ -1643,7 +1643,10 @@ async fn update_feature_flags_enabling_guardian_selects_auto_review() -> Result<
auto_review.approvals_reviewer
);
assert_eq!(app.runtime_approval_policy_override, None);
assert_eq!(app.runtime_sandbox_policy_override, None);
assert_eq!(
app.runtime_sandbox_policy_override,
Some(auto_review.sandbox_policy.clone())
);
assert_eq!(
op_rx.try_recv(),
Ok(Op::OverrideTurnContext {
+9
View File
@@ -571,6 +571,14 @@ impl App {
}
}
if should_start_turn {
let permission_profile = if !app_server.is_remote()
&& self.runtime_sandbox_policy_override.is_some()
&& !matches!(sandbox_policy, SandboxPolicy::ExternalSandbox { .. })
{
Some(self.config.permissions.permission_profile())
} else {
None
};
app_server
.turn_start(
thread_id,
@@ -580,6 +588,7 @@ impl App {
approvals_reviewer
.unwrap_or(self.chat_widget.config_ref().approvals_reviewer),
sandbox_policy.clone(),
permission_profile,
model.to_string(),
effort,
*summary,
+95 -2
View File
@@ -3,6 +3,7 @@ use crate::app_server_session::ThreadSessionState;
use crate::read_session_model;
use codex_app_server_protocol::Thread;
use codex_protocol::ThreadId;
use codex_protocol::protocol::SandboxPolicy;
impl App {
pub(super) async fn sync_active_thread_permission_settings_to_cached_session(&mut self) {
@@ -13,10 +14,22 @@ impl App {
let approval_policy = self.config.permissions.approval_policy.value();
let approvals_reviewer = self.config.approvals_reviewer;
let sandbox_policy = self.config.permissions.sandbox_policy.get().clone();
let permission_profile = if matches!(sandbox_policy, SandboxPolicy::ExternalSandbox { .. })
{
None
} else {
Some(
self.chat_widget
.config_ref()
.permissions
.permission_profile(),
)
};
let update_session = |session: &mut ThreadSessionState| {
session.approval_policy = approval_policy;
session.approvals_reviewer = approvals_reviewer;
session.sandbox_policy = sandbox_policy.clone();
session.permission_profile = permission_profile.clone();
};
if self.primary_thread_id == Some(active_thread_id)
@@ -91,7 +104,14 @@ mod tests {
use crate::test_support::PathBufExt;
use crate::test_support::test_path_buf;
use codex_config::types::ApprovalsReviewer;
use codex_protocol::models::PermissionProfile;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::FileSystemAccessMode;
use codex_protocol::protocol::FileSystemPath;
use codex_protocol::protocol::FileSystemSandboxEntry;
use codex_protocol::protocol::FileSystemSandboxPolicy;
use codex_protocol::protocol::FileSystemSpecialPath;
use codex_protocol::protocol::NetworkSandboxPolicy;
use codex_protocol::protocol::SandboxPolicy;
use pretty_assertions::assert_eq;
use std::path::PathBuf;
@@ -157,8 +177,17 @@ mod tests {
app.config.permissions.approval_policy =
codex_config::Constrained::allow_any(AskForApproval::OnRequest);
app.config.approvals_reviewer = ApprovalsReviewer::AutoReview;
let expected_sandbox_policy = SandboxPolicy::new_workspace_write_policy();
let expected_permission_profile = PermissionProfile::from_legacy_sandbox_policy(
&expected_sandbox_policy,
&main_session.cwd,
);
app.chat_widget.handle_thread_session(main_session.clone());
app.chat_widget
.set_sandbox_policy(expected_sandbox_policy.clone())
.expect("set widget sandbox policy");
app.config.permissions.sandbox_policy =
codex_config::Constrained::allow_any(SandboxPolicy::new_workspace_write_policy());
codex_config::Constrained::allow_any(expected_sandbox_policy.clone());
app.sync_active_thread_permission_settings_to_cached_session()
.await;
@@ -166,7 +195,8 @@ mod tests {
let expected_main_session = ThreadSessionState {
approval_policy: AskForApproval::OnRequest,
approvals_reviewer: ApprovalsReviewer::AutoReview,
sandbox_policy: SandboxPolicy::new_workspace_write_policy(),
sandbox_policy: expected_sandbox_policy,
permission_profile: Some(expected_permission_profile),
..main_session
};
assert_eq!(
@@ -196,4 +226,67 @@ mod tests {
.clone();
assert_eq!(side_store_session, Some(side_session));
}
#[tokio::test]
async fn permission_settings_sync_preserves_active_profile_only_rules() {
let mut app = make_test_app().await;
let thread_id =
ThreadId::from_string("00000000-0000-0000-0000-000000000403").expect("valid thread");
let profile = PermissionProfile::from_runtime_permissions(
&FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::Root,
},
access: FileSystemAccessMode::Read,
},
FileSystemSandboxEntry {
path: FileSystemPath::GlobPattern {
pattern: "**/.env".to_string(),
},
access: FileSystemAccessMode::None,
},
]),
NetworkSandboxPolicy::Restricted,
);
let session = ThreadSessionState {
permission_profile: Some(profile.clone()),
..test_thread_session(thread_id, test_path_buf("/tmp/main"))
};
app.primary_thread_id = Some(thread_id);
app.active_thread_id = Some(thread_id);
app.primary_session_configured = Some(session.clone());
app.thread_event_channels.insert(
thread_id,
ThreadEventChannel::new_with_session(/*capacity*/ 4, session.clone(), Vec::new()),
);
app.chat_widget.handle_thread_session(session.clone());
app.config.permissions.approval_policy =
codex_config::Constrained::allow_any(AskForApproval::OnRequest);
app.sync_active_thread_permission_settings_to_cached_session()
.await;
let expected_session = ThreadSessionState {
approval_policy: AskForApproval::OnRequest,
permission_profile: Some(profile),
..session
};
assert_eq!(
app.primary_session_configured,
Some(expected_session.clone())
);
let store_session = app
.thread_event_channels
.get(&thread_id)
.expect("thread channel")
.store
.lock()
.await
.session
.clone();
assert_eq!(store_session, Some(expected_session));
}
}
+76 -11
View File
@@ -526,6 +526,7 @@ impl AppServerSession {
approval_policy: AskForApproval,
approvals_reviewer: codex_protocol::config_types::ApprovalsReviewer,
sandbox_policy: SandboxPolicy,
permission_profile: Option<PermissionProfile>,
model: String,
effort: Option<codex_protocol::openai_models::ReasoningEffort>,
summary: Option<codex_protocol::config_types::ReasoningSummary>,
@@ -535,13 +536,11 @@ impl AppServerSession {
output_schema: Option<serde_json::Value>,
) -> Result<TurnStartResponse> {
let request_id = self.next_request_id();
let sandbox_policy = if self.is_remote()
|| matches!(sandbox_policy, SandboxPolicy::ExternalSandbox { .. })
{
Some(sandbox_policy.into())
} else {
None
};
let (sandbox_policy, permission_profile) = turn_start_permission_overrides(
self.thread_params_mode(),
sandbox_policy,
permission_profile,
);
self.client
.request_typed(ClientRequest::TurnStart {
request_id,
@@ -553,11 +552,8 @@ impl AppServerSession {
cwd: Some(cwd),
approval_policy: Some(approval_policy.into()),
approvals_reviewer: Some(approvals_reviewer.into()),
// Embedded sessions already installed their full profile
// at thread start/resume/fork. Avoid sending a lossy
// legacy projection until user turns carry profiles.
sandbox_policy,
permission_profile: None,
permission_profile,
model: Some(model),
service_tier,
effort,
@@ -1046,6 +1042,26 @@ fn sandbox_mode_from_policy(
}
}
fn turn_start_permission_overrides(
mode: ThreadParamsMode,
sandbox_policy: SandboxPolicy,
permission_profile: Option<PermissionProfile>,
) -> (
Option<codex_app_server_protocol::SandboxPolicy>,
Option<codex_app_server_protocol::PermissionProfile>,
) {
let is_external_sandbox = matches!(&sandbox_policy, SandboxPolicy::ExternalSandbox { .. });
match (mode, is_external_sandbox, permission_profile) {
(ThreadParamsMode::Embedded, false, Some(permission_profile)) => {
(None, Some(permission_profile.into()))
}
(ThreadParamsMode::Embedded, false, None) => (None, None),
(ThreadParamsMode::Embedded, true, _) | (ThreadParamsMode::Remote, _, _) => {
(Some(sandbox_policy.into()), None)
}
}
}
fn permission_profile_override_from_config(
config: &Config,
thread_params_mode: ThreadParamsMode,
@@ -1530,6 +1546,55 @@ mod tests {
assert_eq!(fork.permission_profile, None);
}
#[test]
fn turn_start_permission_overrides_send_profiles_only_for_embedded_runtime_overrides() {
let cwd = test_path_buf("/tmp/project");
let workspace_write = SandboxPolicy::new_workspace_write_policy();
let workspace_write_profile =
PermissionProfile::from_legacy_sandbox_policy(&workspace_write, &cwd);
let (sandbox, profile) = turn_start_permission_overrides(
ThreadParamsMode::Embedded,
workspace_write.clone(),
Some(workspace_write_profile.clone()),
);
assert_eq!(sandbox, None);
assert_eq!(profile, Some(workspace_write_profile.into()));
let (sandbox, profile) = turn_start_permission_overrides(
ThreadParamsMode::Embedded,
workspace_write.clone(),
/*permission_profile*/ None,
);
assert_eq!(sandbox, None);
assert_eq!(profile, None);
let (sandbox, profile) = turn_start_permission_overrides(
ThreadParamsMode::Remote,
workspace_write.clone(),
Some(PermissionProfile::from_legacy_sandbox_policy(
&workspace_write,
&cwd,
)),
);
assert_eq!(sandbox, Some(workspace_write.into()));
assert_eq!(profile, None);
let external_sandbox = SandboxPolicy::ExternalSandbox {
network_access: codex_protocol::protocol::NetworkAccess::Restricted,
};
let (sandbox, profile) = turn_start_permission_overrides(
ThreadParamsMode::Embedded,
external_sandbox.clone(),
Some(PermissionProfile::from_legacy_sandbox_policy(
&external_sandbox,
&cwd,
)),
);
assert_eq!(sandbox, Some(external_sandbox.into()));
assert_eq!(profile, None);
}
#[tokio::test]
async fn thread_fork_params_forward_instruction_overrides() {
let temp_dir = tempfile::tempdir().expect("tempdir");
+24
View File
@@ -2119,6 +2119,20 @@ impl ChatWidget {
display: SessionConfiguredDisplay,
fork_parent_title: Option<String>,
) {
let (file_system_sandbox_policy, network_sandbox_policy) = match event
.permission_profile
.as_ref()
{
Some(permission_profile) => permission_profile.to_runtime_permissions(),
None => (
codex_protocol::permissions::FileSystemSandboxPolicy::from_legacy_sandbox_policy(
&event.sandbox_policy,
&event.cwd,
),
codex_protocol::permissions::NetworkSandboxPolicy::from(&event.sandbox_policy),
),
};
self.last_agent_markdown = None;
self.agent_turn_markdowns.clear();
self.visible_user_turn_count = 0;
@@ -2156,6 +2170,8 @@ impl ChatWidget {
self.config.permissions.sandbox_policy =
Constrained::allow_only(event.sandbox_policy.clone());
}
self.config.permissions.file_system_sandbox_policy = file_system_sandbox_policy;
self.config.permissions.network_sandbox_policy = network_sandbox_policy;
self.config.approvals_reviewer = event.approvals_reviewer;
self.status_line_project_root_name_cache = None;
let forked_from_id = event.forked_from_id;
@@ -9764,6 +9780,14 @@ impl ChatWidget {
#[cfg_attr(not(target_os = "windows"), allow(dead_code))]
pub(crate) fn set_sandbox_policy(&mut self, policy: SandboxPolicy) -> ConstraintResult<()> {
self.config.permissions.sandbox_policy.set(policy)?;
let sandbox_policy = self.config.permissions.sandbox_policy.get();
self.config.permissions.file_system_sandbox_policy =
codex_protocol::permissions::FileSystemSandboxPolicy::from_legacy_sandbox_policy(
sandbox_policy,
&self.config.cwd,
);
self.config.permissions.network_sandbox_policy =
codex_protocol::permissions::NetworkSandboxPolicy::from(sandbox_policy);
Ok(())
}
@@ -1,4 +1,12 @@
use super::*;
use codex_protocol::protocol::FileSystemAccessMode;
use codex_protocol::protocol::FileSystemPath;
use codex_protocol::protocol::FileSystemSandboxEntry;
use codex_protocol::protocol::FileSystemSandboxKind;
use codex_protocol::protocol::FileSystemSandboxPolicy;
use codex_protocol::protocol::FileSystemSpecialPath;
use codex_protocol::protocol::NetworkAccess;
use codex_protocol::protocol::NetworkSandboxPolicy;
use pretty_assertions::assert_eq;
#[tokio::test]
@@ -252,6 +260,25 @@ async fn session_configured_syncs_widget_config_permissions_and_cwd() {
let expected_sandbox = SandboxPolicy::new_read_only_policy();
let expected_cwd = test_path_buf("/home/user/sub-agent").abs();
let expected_file_system_policy = FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::Root,
},
access: FileSystemAccessMode::Read,
},
FileSystemSandboxEntry {
path: FileSystemPath::GlobPattern {
pattern: "**/.secret".to_string(),
},
access: FileSystemAccessMode::None,
},
]);
let expected_permission_profile =
codex_protocol::models::PermissionProfile::from_runtime_permissions(
&expected_file_system_policy,
NetworkSandboxPolicy::Restricted,
);
let configured = codex_protocol::protocol::SessionConfiguredEvent {
session_id: ThreadId::new(),
forked_from_id: None,
@@ -262,7 +289,7 @@ async fn session_configured_syncs_widget_config_permissions_and_cwd() {
approval_policy: AskForApproval::Never,
approvals_reviewer: ApprovalsReviewer::User,
sandbox_policy: expected_sandbox.clone(),
permission_profile: None,
permission_profile: Some(expected_permission_profile.clone()),
cwd: expected_cwd.clone(),
reasoning_effort: Some(ReasoningEffortConfig::default()),
history_log_id: 0,
@@ -285,7 +312,72 @@ async fn session_configured_syncs_widget_config_permissions_and_cwd() {
chat.config_ref().permissions.sandbox_policy.get(),
&expected_sandbox
);
assert_eq!(
chat.config_ref().permissions.permission_profile(),
expected_permission_profile
);
assert_eq!(&chat.config_ref().cwd, &expected_cwd);
let updated_sandbox = SandboxPolicy::new_workspace_write_policy();
chat.set_sandbox_policy(updated_sandbox.clone())
.expect("set sandbox policy");
assert_eq!(
chat.config_ref().permissions.permission_profile(),
codex_protocol::models::PermissionProfile::from_legacy_sandbox_policy(
&updated_sandbox,
&expected_cwd
),
"local sandbox changes should replace SessionConfigured profile-derived runtime permissions"
);
}
#[tokio::test]
async fn session_configured_external_sandbox_keeps_external_runtime_policy() {
let (mut chat, _rx, _ops) = make_chatwidget_manual(/*model_override*/ None).await;
let expected_sandbox = SandboxPolicy::ExternalSandbox {
network_access: NetworkAccess::Restricted,
};
let configured = codex_protocol::protocol::SessionConfiguredEvent {
session_id: ThreadId::new(),
forked_from_id: None,
thread_name: None,
model: "test-model".to_string(),
model_provider_id: "test-provider".to_string(),
service_tier: None,
approval_policy: AskForApproval::Never,
approvals_reviewer: ApprovalsReviewer::User,
sandbox_policy: expected_sandbox.clone(),
permission_profile: None,
cwd: test_path_buf("/home/user/external").abs(),
reasoning_effort: Some(ReasoningEffortConfig::default()),
history_log_id: 0,
history_entry_count: 0,
initial_messages: None,
network_proxy: None,
rollout_path: None,
};
chat.handle_codex_event(Event {
id: "session-configured".into(),
msg: EventMsg::SessionConfigured(configured),
});
assert_eq!(
chat.config_ref().permissions.sandbox_policy.get(),
&expected_sandbox
);
assert_eq!(
chat.config_ref()
.permissions
.file_system_sandbox_policy
.kind,
FileSystemSandboxKind::ExternalSandbox,
);
assert_eq!(
chat.config_ref().permissions.network_sandbox_policy,
NetworkSandboxPolicy::Restricted,
);
}
#[tokio::test]