From 341550c27570129dfbd170a0b589f3806d20eb6f Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Mon, 27 Apr 2026 22:49:58 -0700 Subject: [PATCH] permissions: store thread sessions as profiles (#19776) ## Why After thread sessions have a required `PermissionProfile`, the TUI no longer needs to cache a separate legacy `SandboxPolicy` in `ThreadSessionState`. Keeping the legacy field would reintroduce two permission authorities in the session cache and make later replay/switching logic easier to get wrong. This PR keeps legacy app-server compatibility at the ingestion boundary: old `sandbox` response values are still accepted, but they are immediately converted to a cwd-anchored profile. ## What Changed - Removes `ThreadSessionState.sandbox_policy`. - Updates active-session permission syncing to write only the current `PermissionProfile`. - Updates thread-read/replay/test fixtures to use profiles as the cached session permission source. - Leaves legacy `sandbox` fields in app-server request/response protocol paths unchanged; those are compatibility boundaries and are converted before entering cached TUI state. ## Verification - `cargo test -p codex-tui thread_session_state::tests --lib` - `cargo test -p codex-tui inactive_thread_started_notification_initializes_replay_session --lib` - `cargo test -p codex-tui thread_events --lib` - `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/19776). * #19900 * #19899 * __->__ #19776 --- codex-rs/tui/src/app/tests.rs | 9 +------ codex-rs/tui/src/app/thread_events.rs | 2 -- codex-rs/tui/src/app/thread_session_state.rs | 26 +------------------- codex-rs/tui/src/app_server_session.rs | 4 --- 4 files changed, 2 insertions(+), 39 deletions(-) diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index 6c82dac63..9477030bc 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -2211,7 +2211,6 @@ async fn inactive_thread_approval_bubbles_into_active_view() -> Result<()> { /*capacity*/ 1, ThreadSessionState { approval_policy: AskForApproval::OnRequest, - sandbox_policy: SandboxPolicy::new_workspace_write_policy(), permission_profile: PermissionProfile::workspace_write(), rollout_path: Some(test_path_buf("/tmp/agent-rollout.jsonl")), ..test_thread_session(agent_thread_id, test_path_buf("/tmp/agent")) @@ -2371,7 +2370,6 @@ async fn side_defers_subagent_approval_overlay_until_side_exits() -> Result<()> /*capacity*/ 4, ThreadSessionState { approval_policy: AskForApproval::OnRequest, - sandbox_policy: SandboxPolicy::new_workspace_write_policy(), permission_profile: PermissionProfile::workspace_write(), rollout_path: Some(test_path_buf("/tmp/agent-rollout.jsonl")), ..test_thread_session(agent_thread_id, test_path_buf("/tmp/agent")) @@ -2663,7 +2661,6 @@ async fn inactive_thread_approval_badge_clears_after_turn_completion_notificatio /*capacity*/ 4, ThreadSessionState { approval_policy: AskForApproval::OnRequest, - sandbox_policy: SandboxPolicy::new_workspace_write_policy(), permission_profile: PermissionProfile::workspace_write(), rollout_path: Some(test_path_buf("/tmp/agent-rollout.jsonl")), ..test_thread_session(agent_thread_id, test_path_buf("/tmp/agent")) @@ -2717,7 +2714,6 @@ async fn inactive_thread_started_notification_initializes_replay_session() -> Re ThreadId::from_string("00000000-0000-0000-0000-000000000202").expect("valid thread"); let primary_session = ThreadSessionState { approval_policy: AskForApproval::OnRequest, - sandbox_policy: SandboxPolicy::new_workspace_write_policy(), permission_profile: PermissionProfile::workspace_write(), ..test_thread_session(main_thread_id, test_path_buf("/tmp/main")) }; @@ -2742,7 +2738,7 @@ async fn inactive_thread_started_notification_initializes_replay_session() -> Re current_date: None, timezone: None, approval_policy: primary_session.approval_policy, - sandbox_policy: primary_session.sandbox_policy.clone(), + sandbox_policy: SandboxPolicy::new_workspace_write_policy(), permission_profile: None, network: None, file_system_sandbox_policy: None, @@ -2830,7 +2826,6 @@ async fn inactive_thread_started_notification_preserves_primary_model_when_path_ ThreadId::from_string("00000000-0000-0000-0000-000000000302").expect("valid thread"); let primary_session = ThreadSessionState { approval_policy: AskForApproval::OnRequest, - sandbox_policy: SandboxPolicy::new_workspace_write_policy(), permission_profile: PermissionProfile::workspace_write(), ..test_thread_session(main_thread_id, test_path_buf("/tmp/main")) }; @@ -2899,7 +2894,6 @@ async fn thread_read_session_state_does_not_reuse_primary_permission_profile() { ThreadId::from_string("00000000-0000-0000-0000-000000000402").expect("valid thread"); let primary_session = ThreadSessionState { approval_policy: AskForApproval::OnRequest, - sandbox_policy: SandboxPolicy::new_workspace_write_policy(), permission_profile: PermissionProfile::workspace_write(), ..test_thread_session(main_thread_id, test_path_buf("/tmp/main")) }; @@ -3803,7 +3797,6 @@ fn test_thread_session(thread_id: ThreadId, cwd: PathBuf) -> ThreadSessionState service_tier: None, approval_policy: AskForApproval::Never, approvals_reviewer: ApprovalsReviewer::User, - sandbox_policy: SandboxPolicy::new_read_only_policy(), permission_profile: PermissionProfile::read_only(), cwd: cwd.abs(), instruction_source_paths: Vec::new(), diff --git a/codex-rs/tui/src/app/thread_events.rs b/codex-rs/tui/src/app/thread_events.rs index d73c39dfe..10fc370f5 100644 --- a/codex-rs/tui/src/app/thread_events.rs +++ b/codex-rs/tui/src/app/thread_events.rs @@ -335,7 +335,6 @@ mod tests { use codex_config::types::ApprovalsReviewer; use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::AskForApproval; - use codex_protocol::protocol::SandboxPolicy; use pretty_assertions::assert_eq; use std::path::PathBuf; @@ -350,7 +349,6 @@ mod tests { service_tier: None, approval_policy: AskForApproval::Never, approvals_reviewer: ApprovalsReviewer::User, - sandbox_policy: SandboxPolicy::new_read_only_policy(), permission_profile: PermissionProfile::read_only(), cwd: cwd.abs(), instruction_source_paths: Vec::new(), diff --git a/codex-rs/tui/src/app/thread_session_state.rs b/codex-rs/tui/src/app/thread_session_state.rs index a0ba5bdc3..0ff1cec26 100644 --- a/codex-rs/tui/src/app/thread_session_state.rs +++ b/codex-rs/tui/src/app/thread_session_state.rs @@ -4,7 +4,6 @@ use crate::read_session_model; use codex_app_server_protocol::Thread; use codex_protocol::ThreadId; use codex_protocol::models::PermissionProfile; -use codex_protocol::protocol::SandboxPolicy; impl App { pub(super) async fn sync_active_thread_permission_settings_to_cached_session(&mut self) { @@ -14,10 +13,6 @@ impl App { let approval_policy = self.config.permissions.approval_policy.value(); let approvals_reviewer = self.config.approvals_reviewer; - let sandbox_policy = self - .config - .permissions - .legacy_sandbox_policy(self.config.cwd.as_path()); let permission_profile = self .chat_widget .config_ref() @@ -26,7 +21,6 @@ impl App { 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(); }; @@ -49,7 +43,6 @@ impl App { thread_id: ThreadId, thread: &Thread, ) -> ThreadSessionState { - let sandbox_policy = self.active_legacy_sandbox_policy_for_cwd(thread.cwd.as_path()); let permission_profile = self.active_permission_profile(); let mut session = self .primary_session_configured @@ -64,7 +57,6 @@ impl App { service_tier: self.chat_widget.current_service_tier(), approval_policy: self.config.permissions.approval_policy.value(), approvals_reviewer: self.config.approvals_reviewer, - sandbox_policy: sandbox_policy.clone(), permission_profile: permission_profile.clone(), cwd: thread.cwd.clone(), instruction_source_paths: Vec::new(), @@ -78,7 +70,6 @@ impl App { session.thread_name = thread.name.clone(); session.model_provider_id = thread.model_provider.clone(); session.cwd = thread.cwd.clone(); - session.sandbox_policy = sandbox_policy; session.permission_profile = permission_profile; session.instruction_source_paths = Vec::new(); session.rollout_path = thread.path.clone(); @@ -100,13 +91,6 @@ impl App { .permissions .permission_profile() } - - fn active_legacy_sandbox_policy_for_cwd(&self, cwd: &std::path::Path) -> SandboxPolicy { - self.chat_widget - .config_ref() - .permissions - .legacy_sandbox_policy(cwd) - } } #[cfg(test)] @@ -141,7 +125,6 @@ mod tests { service_tier: None, approval_policy: AskForApproval::Never, approvals_reviewer: ApprovalsReviewer::User, - sandbox_policy: SandboxPolicy::new_read_only_policy(), permission_profile: PermissionProfile::read_only(), cwd: cwd.abs(), instruction_source_paths: Vec::new(), @@ -163,7 +146,7 @@ mod tests { let main_session = test_thread_session(main_thread_id, test_path_buf("/tmp/main")); let side_session = ThreadSessionState { approval_policy: AskForApproval::OnRequest, - sandbox_policy: SandboxPolicy::new_workspace_write_policy(), + permission_profile: PermissionProfile::workspace_write(), ..test_thread_session(side_thread_id, test_path_buf("/tmp/side")) }; @@ -215,7 +198,6 @@ mod tests { let expected_main_session = ThreadSessionState { approval_policy: AskForApproval::OnRequest, approvals_reviewer: ApprovalsReviewer::AutoReview, - sandbox_policy: expected_sandbox_policy, permission_profile: expected_permission_profile, ..main_session }; @@ -348,17 +330,11 @@ mod tests { .session_state_for_thread_read(read_thread_id, &read_thread) .await; - let expected_sandbox_policy = app - .chat_widget - .config_ref() - .permissions - .legacy_sandbox_policy(read_thread.cwd.as_path()); let expected_permission_profile = app .chat_widget .config_ref() .permissions .permission_profile(); - assert_eq!(session.sandbox_policy, expected_sandbox_policy); assert_eq!(session.permission_profile, expected_permission_profile); assert_ne!( session.permission_profile, diff --git a/codex-rs/tui/src/app_server_session.rs b/codex-rs/tui/src/app_server_session.rs index 5a33896dd..0d38449db 100644 --- a/codex-rs/tui/src/app_server_session.rs +++ b/codex-rs/tui/src/app_server_session.rs @@ -156,9 +156,6 @@ pub(crate) struct ThreadSessionState { pub(crate) service_tier: Option, pub(crate) approval_policy: AskForApproval, pub(crate) approvals_reviewer: codex_protocol::config_types::ApprovalsReviewer, - /// Legacy sandbox projection kept only for compatibility fields that have - /// not migrated to `PermissionProfile` yet. - pub(crate) sandbox_policy: SandboxPolicy, /// Canonical active permissions for this session. Legacy app-server /// responses are converted to a profile at ingestion time using the /// response cwd so cached sessions do not reinterpret cwd-bound grants. @@ -1420,7 +1417,6 @@ async fn thread_session_state_from_thread_response( service_tier, approval_policy, approvals_reviewer, - sandbox_policy, permission_profile, cwd, instruction_source_paths,