permissions: require profiles in TUI thread state

bolinfest · bolinfest · commit 5ffd2158da63 · 2026-04-27T16:59:08.000-07:00
diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs
@@ -2212,7 +2212,7 @@ async fn inactive_thread_approval_bubbles_into_active_view() -> Result<()> {
             ThreadSessionState {
                 approval_policy: AskForApproval::OnRequest,
                 sandbox_policy: SandboxPolicy::new_workspace_write_policy(),
-                permission_profile: Some(PermissionProfile::workspace_write()),
+                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"))
             },
@@ -2372,7 +2372,7 @@ async fn side_defers_subagent_approval_overlay_until_side_exits() -> Result<()>
             ThreadSessionState {
                 approval_policy: AskForApproval::OnRequest,
                 sandbox_policy: SandboxPolicy::new_workspace_write_policy(),
-                permission_profile: Some(PermissionProfile::workspace_write()),
+                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"))
             },
@@ -2664,7 +2664,7 @@ async fn inactive_thread_approval_badge_clears_after_turn_completion_notificatio
             ThreadSessionState {
                 approval_policy: AskForApproval::OnRequest,
                 sandbox_policy: SandboxPolicy::new_workspace_write_policy(),
-                permission_profile: Some(PermissionProfile::workspace_write()),
+                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"))
             },
@@ -2718,7 +2718,7 @@ async fn inactive_thread_started_notification_initializes_replay_session() -> Re
     let primary_session = ThreadSessionState {
         approval_policy: AskForApproval::OnRequest,
         sandbox_policy: SandboxPolicy::new_workspace_write_policy(),
-        permission_profile: Some(PermissionProfile::workspace_write()),
+        permission_profile: PermissionProfile::workspace_write(),
         ..test_thread_session(main_thread_id, test_path_buf("/tmp/main"))
     };
 
@@ -2831,7 +2831,7 @@ async fn inactive_thread_started_notification_preserves_primary_model_when_path_
     let primary_session = ThreadSessionState {
         approval_policy: AskForApproval::OnRequest,
         sandbox_policy: SandboxPolicy::new_workspace_write_policy(),
-        permission_profile: Some(PermissionProfile::workspace_write()),
+        permission_profile: PermissionProfile::workspace_write(),
         ..test_thread_session(main_thread_id, test_path_buf("/tmp/main"))
     };
 
@@ -2900,7 +2900,7 @@ async fn thread_read_session_state_does_not_reuse_primary_permission_profile() {
     let primary_session = ThreadSessionState {
         approval_policy: AskForApproval::OnRequest,
         sandbox_policy: SandboxPolicy::new_workspace_write_policy(),
-        permission_profile: Some(PermissionProfile::workspace_write()),
+        permission_profile: PermissionProfile::workspace_write(),
         ..test_thread_session(main_thread_id, test_path_buf("/tmp/main"))
     };
     app.primary_session_configured = Some(primary_session);
@@ -2931,10 +2931,17 @@ async fn thread_read_session_state_does_not_reuse_primary_permission_profile() {
 
     assert_eq!(session.thread_id, read_thread_id);
     assert_eq!(session.cwd.as_path(), test_path_buf("/tmp/read").as_path());
+    let expected_permission_profile = PermissionProfile::from_legacy_sandbox_policy_for_cwd(
+        &app.config
+            .permissions
+            .legacy_sandbox_policy(thread.cwd.as_path()),
+        thread.cwd.as_path(),
+    );
     assert_eq!(
-        session.permission_profile, None,
-        "thread/read does not return an authoritative permission profile; reusing the primary \
-         session profile would reinterpret cwd-bound entries against the read thread cwd"
+        session.permission_profile, expected_permission_profile,
+        "thread/read does not return authoritative server permissions; the fallback profile must \
+         be rebuilt from local legacy settings against the read thread cwd rather than reusing \
+         the primary session profile"
     );
 }
 
@@ -3800,7 +3807,7 @@ fn test_thread_session(thread_id: ThreadId, cwd: PathBuf) -> ThreadSessionState
         approval_policy: AskForApproval::Never,
         approvals_reviewer: ApprovalsReviewer::User,
         sandbox_policy: SandboxPolicy::new_read_only_policy(),
-        permission_profile: Some(PermissionProfile::read_only()),
+        permission_profile: PermissionProfile::read_only(),
         cwd: cwd.abs(),
         instruction_source_paths: Vec::new(),
         reasoning_effort: None,
diff --git a/codex-rs/tui/src/app/thread_events.rs b/codex-rs/tui/src/app/thread_events.rs
@@ -351,7 +351,7 @@ mod tests {
             approval_policy: AskForApproval::Never,
             approvals_reviewer: ApprovalsReviewer::User,
             sandbox_policy: SandboxPolicy::new_read_only_policy(),
-            permission_profile: Some(PermissionProfile::read_only()),
+            permission_profile: PermissionProfile::read_only(),
             cwd: cwd.abs(),
             instruction_source_paths: Vec::new(),
             reasoning_effort: None,
diff --git a/codex-rs/tui/src/app/thread_session_state.rs b/codex-rs/tui/src/app/thread_session_state.rs
@@ -3,6 +3,8 @@ use crate::app_server_session::ThreadSessionState;
 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) {
@@ -16,12 +18,11 @@ impl App {
             .config
             .permissions
             .legacy_sandbox_policy(self.config.cwd.as_path());
-        let permission_profile = Some(
-            self.chat_widget
-                .config_ref()
-                .permissions
-                .permission_profile(),
-        );
+        let permission_profile = self
+            .chat_widget
+            .config_ref()
+            .permissions
+            .permission_profile();
         let update_session = |session: &mut ThreadSessionState| {
             session.approval_policy = approval_policy;
             session.approvals_reviewer = approvals_reviewer;
@@ -48,10 +49,8 @@ impl App {
         thread_id: ThreadId,
         thread: &Thread,
     ) -> ThreadSessionState {
-        let sandbox_policy = self
-            .config
-            .permissions
-            .legacy_sandbox_policy(self.config.cwd.as_path());
+        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
             .clone()
@@ -65,8 +64,8 @@ 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,
-                permission_profile: None,
+                sandbox_policy: sandbox_policy.clone(),
+                permission_profile: permission_profile.clone(),
                 cwd: thread.cwd.clone(),
                 instruction_source_paths: Vec::new(),
                 reasoning_effort: self.chat_widget.current_reasoning_effort(),
@@ -79,7 +78,8 @@ impl App {
         session.thread_name = thread.name.clone();
         session.model_provider_id = thread.model_provider.clone();
         session.cwd = thread.cwd.clone();
-        session.permission_profile = None;
+        session.sandbox_policy = sandbox_policy;
+        session.permission_profile = permission_profile;
         session.instruction_source_paths = Vec::new();
         session.rollout_path = thread.path.clone();
         if let Some(model) =
@@ -93,6 +93,20 @@ impl App {
         session.history_entry_count = 0;
         session
     }
+
+    fn active_permission_profile(&self) -> PermissionProfile {
+        self.chat_widget
+            .config_ref()
+            .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)]
@@ -128,7 +142,7 @@ mod tests {
             approval_policy: AskForApproval::Never,
             approvals_reviewer: ApprovalsReviewer::User,
             sandbox_policy: SandboxPolicy::new_read_only_policy(),
-            permission_profile: None,
+            permission_profile: PermissionProfile::read_only(),
             cwd: cwd.abs(),
             instruction_source_paths: Vec::new(),
             reasoning_effort: None,
@@ -202,7 +216,7 @@ mod tests {
             approval_policy: AskForApproval::OnRequest,
             approvals_reviewer: ApprovalsReviewer::AutoReview,
             sandbox_policy: expected_sandbox_policy,
-            permission_profile: Some(expected_permission_profile),
+            permission_profile: expected_permission_profile,
             ..main_session
         };
         assert_eq!(
@@ -256,7 +270,7 @@ mod tests {
             NetworkSandboxPolicy::Restricted,
         );
         let session = ThreadSessionState {
-            permission_profile: Some(profile.clone()),
+            permission_profile: profile.clone(),
             ..test_thread_session(thread_id, test_path_buf("/tmp/main"))
         };
 
@@ -276,7 +290,7 @@ mod tests {
 
         let expected_session = ThreadSessionState {
             approval_policy: AskForApproval::OnRequest,
-            permission_profile: Some(profile),
+            permission_profile: profile,
             ..session
         };
         assert_eq!(
@@ -295,4 +309,62 @@ mod tests {
             .clone();
         assert_eq!(store_session, Some(expected_session));
     }
+
+    #[tokio::test]
+    async fn thread_read_fallback_uses_active_permission_settings() {
+        let mut app = make_test_app().await;
+        let primary_thread_id =
+            ThreadId::from_string("00000000-0000-0000-0000-000000000404").expect("valid thread");
+        let read_thread_id =
+            ThreadId::from_string("00000000-0000-0000-0000-000000000405").expect("valid thread");
+        let primary_session = ThreadSessionState {
+            permission_profile: PermissionProfile::workspace_write(),
+            ..test_thread_session(primary_thread_id, test_path_buf("/tmp/primary"))
+        };
+        let read_thread = Thread {
+            id: read_thread_id.to_string(),
+            forked_from_id: None,
+            preview: "read thread".to_string(),
+            ephemeral: false,
+            model_provider: "read-provider".to_string(),
+            created_at: 1,
+            updated_at: 2,
+            status: codex_app_server_protocol::ThreadStatus::Idle,
+            path: None,
+            cwd: test_path_buf("/tmp/read").abs(),
+            cli_version: "0.0.0".to_string(),
+            source: codex_app_server_protocol::SessionSource::Unknown,
+            agent_nickname: None,
+            agent_role: None,
+            git_info: None,
+            name: Some("read thread".to_string()),
+            turns: Vec::new(),
+        };
+
+        app.primary_session_configured = Some(primary_session.clone());
+        app.chat_widget.handle_thread_session(primary_session);
+
+        let session = app
+            .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,
+            app.config.permissions.permission_profile(),
+            "thread/read fallback must use the active widget permissions rather than stale app \
+             config defaults"
+        );
+    }
 }
diff --git a/codex-rs/tui/src/app_server_session.rs b/codex-rs/tui/src/app_server_session.rs
@@ -156,13 +156,13 @@ pub(crate) struct ThreadSessionState {
     pub(crate) service_tier: Option<codex_protocol::config_types::ServiceTier>,
     pub(crate) approval_policy: AskForApproval,
     pub(crate) approvals_reviewer: codex_protocol::config_types::ApprovalsReviewer,
-    /// Legacy sandbox projection kept for compatibility. Use this only when
-    /// `permission_profile` is `None`.
+    /// Legacy sandbox projection kept only for compatibility fields that have
+    /// not migrated to `PermissionProfile` yet.
     pub(crate) sandbox_policy: SandboxPolicy,
-    /// Canonical active permissions when available. Consumers should prefer
-    /// this over `sandbox_policy`; `None` means the session only has a legacy
-    /// sandbox projection.
-    pub(crate) permission_profile: Option<PermissionProfile>,
+    /// 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.
+    pub(crate) permission_profile: PermissionProfile,
     pub(crate) cwd: AbsolutePathBuf,
     pub(crate) instruction_source_paths: Vec<AbsolutePathBuf>,
     pub(crate) reasoning_effort: Option<codex_protocol::openai_models::ReasoningEffort>,
@@ -1407,6 +1407,8 @@ async fn thread_session_state_from_thread_response(
         .map_err(|err| format!("forked_from_id is invalid: {err}"))?;
     let (history_log_id, history_entry_count) = message_history_metadata(config).await;
     let history_entry_count = u64::try_from(history_entry_count).unwrap_or(u64::MAX);
+    let permission_profile =
+        permission_profile_from_response_permissions(&sandbox_policy, permission_profile, &cwd);
 
     Ok(ThreadSessionState {
         thread_id,
@@ -1430,6 +1432,16 @@ async fn thread_session_state_from_thread_response(
     })
 }
 
+fn permission_profile_from_response_permissions(
+    sandbox_policy: &SandboxPolicy,
+    permission_profile: Option<PermissionProfile>,
+    cwd: &AbsolutePathBuf,
+) -> PermissionProfile {
+    permission_profile.unwrap_or_else(|| {
+        PermissionProfile::from_legacy_sandbox_policy_for_cwd(sandbox_policy, cwd.as_path())
+    })
+}
+
 pub(crate) fn app_server_rate_limit_snapshots_to_core(
     response: GetAccountRateLimitsResponse,
 ) -> Vec<RateLimitSnapshot> {
@@ -1770,7 +1782,11 @@ mod tests {
         );
         assert_eq!(
             started.session.permission_profile,
-            response.permission_profile.clone().map(Into::into)
+            response
+                .permission_profile
+                .clone()
+                .map(Into::into)
+                .expect("response includes profile")
         );
         assert_eq!(started.turns.len(), 1);
         assert_eq!(started.turns[0], response.thread.turns[0]);
diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs
@@ -1621,7 +1621,7 @@ fn thread_session_state_to_legacy_event(
         approval_policy: session.approval_policy,
         approvals_reviewer: session.approvals_reviewer,
         sandbox_policy: session.sandbox_policy,
-        permission_profile: session.permission_profile,
+        permission_profile: Some(session.permission_profile),
         cwd: session.cwd,
         reasoning_effort: session.reasoning_effort,
         history_log_id: session.history_log_id,
