diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index 27458e3165..f0fd46fc2b 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -1570,6 +1570,20 @@ impl App { } } } + AppEvent::FetchPermissionProfiles { request_id, thread_cwd } => { + if self.chat_widget.permission_popup_request_is_current(request_id) { + crate::permission_discovery::fetch( + app_server, + request_id, + self.chat_widget.config_ref(), + thread_cwd.as_deref(), + self.app_event_tx.clone(), + ); + } + } + AppEvent::PermissionProfilesLoaded { request_id, result } => { + self.chat_widget.on_permission_profiles_loaded(request_id, result); + } AppEvent::FetchModels { request_id } => { if self.chat_widget.model_popup_request_is_current(request_id) { app_server.fetch_models(request_id, self.app_event_tx.clone()); @@ -2494,9 +2508,6 @@ impl App { )); } } - AppEvent::OpenApprovalsPopup => { - self.chat_widget.open_approvals_popup(); - } AppEvent::OpenAgentsOverview => { self.open_agents_overview(app_server); } @@ -2732,8 +2743,12 @@ impl App { self.chat_widget.add_error_message(err); } } - AppEvent::OpenPermissionsPopup => { - self.chat_widget.open_permissions_popup(); + AppEvent::OpenPermissionsPopup | AppEvent::OpenApprovalsPopup => { + if app_server.uses_remote_workspace() { + self.chat_widget.request_permission_profiles(); + } else { + self.chat_widget.open_approvals_popup(); + } } AppEvent::OpenReviewBranchPicker(cwd) => { self.chat_widget.show_review_branch_picker(&cwd).await; diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index 50512ba28b..4ff72819fa 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -1026,6 +1026,15 @@ pub(crate) enum AppEvent { result: Result, String>, }, + FetchPermissionProfiles { + request_id: uuid::Uuid, + thread_cwd: Option, + }, + PermissionProfilesLoaded { + request_id: uuid::Uuid, + result: Result, + }, + /// Open the reasoning selection popup after picking a model. OpenReasoningPopup { model: ModelPreset, diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 14a2b8b3e2..9c81378d97 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -388,6 +388,7 @@ mod model_popup_state; mod model_popups; mod notifications; use self::notifications::Notification; +mod permission_discovery; mod permission_popups; mod permission_shortcuts; mod permissions_menu; @@ -577,6 +578,8 @@ pub(crate) struct ChatWidget { has_codex_backend_auth: bool, model_catalog: Arc, model_popup_request_id: Option, + permission_popup_request_id: Option, + permission_profiles_menu_opened: bool, model_popup_model_ids: Vec, session_telemetry: SessionTelemetry, session_header: SessionHeader, diff --git a/codex-rs/tui/src/chatwidget/constructor.rs b/codex-rs/tui/src/chatwidget/constructor.rs index 81e927f5bd..8ebc5f6509 100644 --- a/codex-rs/tui/src/chatwidget/constructor.rs +++ b/codex-rs/tui/src/chatwidget/constructor.rs @@ -125,6 +125,8 @@ impl ChatWidget { has_codex_backend_auth, model_catalog, model_popup_request_id: None, + permission_popup_request_id: None, + permission_profiles_menu_opened: false, model_popup_model_ids: Vec::new(), session_telemetry, session_header: SessionHeader::new(header_model), diff --git a/codex-rs/tui/src/chatwidget/permission_discovery.rs b/codex-rs/tui/src/chatwidget/permission_discovery.rs new file mode 100644 index 0000000000..120d2c0595 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/permission_discovery.rs @@ -0,0 +1,75 @@ +//! Keep named-profile discovery responsive and discard replies after closing or changing scope. + +use super::*; +use crate::permission_discovery::PermissionDiscovery; + +pub(super) const VIEW_ID: &str = "permission-profiles"; + +impl ChatWidget { + pub(crate) fn request_permission_profiles(&mut self) { + self.invalidate_permission_discovery(); + let request_id = uuid::Uuid::new_v4(); + self.permission_popup_request_id = Some(request_id); + self.bottom_pane.show_selection_view(SelectionViewParams { + view_id: Some(VIEW_ID), + title: Some("Update Model Permissions".to_string()), + items: vec![SelectionItem { + name: "Loading permission profiles…".to_string(), + is_disabled: true, + ..Default::default() + }], + footer_hint: Some(standard_popup_hint_line()), + ..Default::default() + }); + self.app_event_tx.send(AppEvent::FetchPermissionProfiles { + request_id, + thread_cwd: self.thread_id.and(self.current_cwd.clone()), + }); + } + + pub(super) fn invalidate_permission_discovery(&mut self) { + self.permission_popup_request_id = None; + self.bottom_pane.dismiss_view_by_id(VIEW_ID); + } + + pub(crate) fn permission_popup_request_is_current(&self, request_id: uuid::Uuid) -> bool { + self.permission_popup_request_id == Some(request_id) + } + + pub(crate) fn on_permission_profiles_loaded( + &mut self, + request_id: uuid::Uuid, + result: Result, + ) { + if !self.permission_popup_request_is_current(request_id) { + return; + } + self.permission_popup_request_id = None; + if !self.bottom_pane.dismiss_active_view_if_id(VIEW_ID) { + self.bottom_pane.dismiss_view_by_id(VIEW_ID); + return; + } + match result { + Ok(discovery) if !discovery.explicit_profile_mode => { + self.open_legacy_permissions_popup() + } + Ok(discovery) => { + self.permission_profiles_menu_opened = true; + self.open_permission_profiles_popup(discovery); + } + Err(message) => self.bottom_pane.show_selection_view(SelectionViewParams { + view_id: Some(VIEW_ID), + title: Some("Update Model Permissions".to_string()), + subtitle: Some(message), + items: vec![SelectionItem { + name: "Retry".to_string(), + actions: vec![Box::new(|tx| tx.send(AppEvent::OpenPermissionsPopup))], + dismiss_on_select: true, + ..Default::default() + }], + footer_hint: Some(standard_popup_hint_line()), + ..Default::default() + }), + } + } +} diff --git a/codex-rs/tui/src/chatwidget/permission_popups.rs b/codex-rs/tui/src/chatwidget/permission_popups.rs index 93e85db009..09ab54a351 100644 --- a/codex-rs/tui/src/chatwidget/permission_popups.rs +++ b/codex-rs/tui/src/chatwidget/permission_popups.rs @@ -15,11 +15,22 @@ impl ChatWidget { /// Open a popup to choose the permissions mode. pub(crate) fn open_permissions_popup(&mut self) { - if self.config.explicit_permission_profile_mode { - self.open_permission_profiles_popup(); + if self.config.explicit_permission_profile_mode + || self.permission_profiles_menu_opened + || self + .config + .permissions + .active_permission_profile() + .is_some_and(|profile| !profile.id.starts_with(':')) + { + self.request_permission_profiles(); return; } + self.open_legacy_permissions_popup(); + } + + pub(super) fn open_legacy_permissions_popup(&mut self) { let include_read_only = cfg!(target_os = "windows"); let current_approval = AskForApproval::from(self.config.permissions.approval_policy.value()); diff --git a/codex-rs/tui/src/chatwidget/permissions_menu.rs b/codex-rs/tui/src/chatwidget/permissions_menu.rs index 49251f8736..80bf23d5a6 100644 --- a/codex-rs/tui/src/chatwidget/permissions_menu.rs +++ b/codex-rs/tui/src/chatwidget/permissions_menu.rs @@ -1,4 +1,7 @@ +//! Named permission choices from the server catalog, using the existing selection actions. + use super::*; +use crate::permission_discovery::PermissionDiscovery; use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_DANGER_FULL_ACCESS; pub(crate) fn auto_review_available(config: &Config) -> bool { @@ -58,7 +61,7 @@ impl ChatWidget { }) } - pub(super) fn open_permission_profiles_popup(&mut self) { + pub(super) fn open_permission_profiles_popup(&mut self, discovery: PermissionDiscovery) { let active_profile_id = self .config .permissions @@ -85,6 +88,7 @@ impl ChatWidget { }; let mut items = vec![ self.builtin_permission_mode_selection_item( + &discovery, default, ":workspace", default @@ -96,6 +100,7 @@ impl ChatWidget { ]; if self.config.features.enabled(Feature::GuardianApproval) { items.push(self.builtin_permission_mode_selection_item( + &discovery, default, ":workspace", AUTO_REVIEW_DESCRIPTION.to_string(), @@ -104,6 +109,7 @@ impl ChatWidget { )); } items.push(self.builtin_permission_mode_selection_item( + &discovery, full_access, BUILT_IN_PERMISSION_PROFILE_DANGER_FULL_ACCESS, full_access.description.to_string(), @@ -111,6 +117,7 @@ impl ChatWidget { ApprovalsReviewer::User, )); items.push(self.builtin_permission_mode_selection_item( + &discovery, read_only, ":read-only", read_only.description.to_string(), @@ -118,9 +125,10 @@ impl ChatWidget { ApprovalsReviewer::User, )); items.extend( - self.config - .custom_permission_profiles + discovery + .profiles .iter() + .filter(|profile| !profile.id.starts_with(':')) .map(|profile| { Self::permission_profile_selection_item( &profile.id, @@ -130,12 +138,36 @@ impl ChatWidget { .as_deref() .unwrap_or("Configured permission profile."), active_profile_id.as_deref(), - profile.allowed, + discovery.disabled_reason( + &profile.id, + /*approval*/ None, + /*reviewer*/ None, + ), ) }), ); + if discovery.profiles.is_empty() { + items.clear(); + } + if let Some(id) = active_profile_id.as_deref() + && !discovery.profiles.iter().any(|profile| profile.id == id) + && !items.iter().any(|item| item.is_current) + { + items.push(Self::permission_profile_selection_item( + id, + id, + "Current permission profile.", + Some(id), + Some("Not available on this server.".to_string()), + )); + } self.bottom_pane.show_selection_view(SelectionViewParams { + view_id: Some(super::permission_discovery::VIEW_ID), + subtitle: discovery + .profiles + .is_empty() + .then(|| "No permission profiles returned by the server.".to_string()), title: Some("Update Model Permissions".to_string()), footer_hint: Some(standard_popup_hint_line()), items, @@ -146,6 +178,7 @@ impl ChatWidget { fn builtin_permission_mode_selection_item( &self, + discovery: &PermissionDiscovery, preset: &ApprovalPreset, id: &str, description: String, @@ -186,7 +219,9 @@ impl ChatWidget { /*return_to_permissions*/ true, ), dismiss_on_select: true, - disabled_reason: self.permission_mode_disabled_reason(preset, approval_policy), + disabled_reason: discovery + .disabled_reason(id, Some(approval_policy), Some(approvals_reviewer.into())) + .or_else(|| self.permission_mode_disabled_reason(preset, approval_policy)), ..Default::default() } } @@ -196,7 +231,7 @@ impl ChatWidget { id: &str, description: &str, active_profile_id: Option<&str>, - allowed: bool, + disabled_reason: Option, ) -> SelectionItem { let id_for_action = id.to_string(); let selection = PermissionProfileSelection { @@ -211,7 +246,7 @@ impl ChatWidget { is_current: active_profile_id == Some(id), actions: Self::permission_profile_selection_actions(selection), dismiss_on_select: true, - disabled_reason: (!allowed).then(|| "Disabled by requirements.".to_string()), + disabled_reason, ..Default::default() } } diff --git a/codex-rs/tui/src/chatwidget/session_flow.rs b/codex-rs/tui/src/chatwidget/session_flow.rs index adfbf5625d..89e9b988f1 100644 --- a/codex-rs/tui/src/chatwidget/session_flow.rs +++ b/codex-rs/tui/src/chatwidget/session_flow.rs @@ -9,6 +9,8 @@ impl ChatWidget { display: SessionConfiguredDisplay, fork_parent_title: Option, ) { + self.invalidate_permission_discovery(); + self.permission_profiles_menu_opened = false; self.transcript.reset_copy_history(); let history_metadata = session.message_history.unwrap_or_default(); self.bottom_pane.set_history_metadata( diff --git a/codex-rs/tui/src/chatwidget/settings.rs b/codex-rs/tui/src/chatwidget/settings.rs index 89461c5051..cd6b08ab32 100644 --- a/codex-rs/tui/src/chatwidget/settings.rs +++ b/codex-rs/tui/src/chatwidget/settings.rs @@ -217,6 +217,7 @@ impl ChatWidget { // Account-update notifications are the identity boundary. The visible account fields can // be identical across two accounts, so always invalidate account-scoped requests and data. self.model_popup_request_id = None; + self.invalidate_permission_discovery(); self.invalidate_connector_scope(); self.clear_pending_token_activity_refreshes(); self.clear_pending_rate_limit_reset_requests(); @@ -466,6 +467,7 @@ impl ChatWidget { } fn apply_thread_settings(&mut self, mut settings: ThreadSettings) { + self.invalidate_permission_discovery(); let cwd_changed = self.config.cwd != settings.cwd; self.apply_thread_settings_cwd(settings.cwd.clone()); self.config.model_provider_id = settings.model_provider.clone(); diff --git a/codex-rs/tui/src/chatwidget/slash_dispatch.rs b/codex-rs/tui/src/chatwidget/slash_dispatch.rs index 08ccdfd66c..8ec4614cd1 100644 --- a/codex-rs/tui/src/chatwidget/slash_dispatch.rs +++ b/codex-rs/tui/src/chatwidget/slash_dispatch.rs @@ -336,7 +336,11 @@ impl ChatWidget { self.app_event_tx.send(AppEvent::OpenAgentPicker); } SlashCommand::Permissions => { - self.open_permissions_popup(); + if self.remote_connection.is_some() { + self.app_event_tx.send(AppEvent::OpenPermissionsPopup); + } else { + self.open_permissions_popup(); + } self.defer_input_until_settings_applied(); } SlashCommand::Vim => { diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__permission_discovery_empty.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__permission_discovery_empty.snap new file mode 100644 index 0000000000..a685885463 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__permission_discovery_empty.snap @@ -0,0 +1,11 @@ +--- +source: tui/src/chatwidget/tests/permissions.rs +expression: "render_bottom_popup(&chat, 100)" +--- + Update Model Permissions + No permission profiles returned by the server. + +› current-profile (current) (disabled) Current permission profile. (disabled: Not available on + this server.) + + Press enter to confirm or esc to go back diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__permission_discovery_failure.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__permission_discovery_failure.snap new file mode 100644 index 0000000000..e1ceadaf84 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__permission_discovery_failure.snap @@ -0,0 +1,10 @@ +--- +source: tui/src/chatwidget/tests/permissions.rs +expression: "render_bottom_popup(&chat, 80)" +--- + Update Model Permissions + Failed to load permissions: disconnected + +› 1. Retry + + Press enter to confirm or esc to go back diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__permission_discovery_loading.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__permission_discovery_loading.snap new file mode 100644 index 0000000000..8de53e84f7 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__permission_discovery_loading.snap @@ -0,0 +1,9 @@ +--- +source: tui/src/chatwidget/tests/permissions.rs +expression: "render_bottom_popup(&chat, 80)" +--- + Update Model Permissions + +› Loading permission profiles… + + Press enter to confirm or esc to go back diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__permission_discovery_remote.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__permission_discovery_remote.snap new file mode 100644 index 0000000000..151d454af2 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__permission_discovery_remote.snap @@ -0,0 +1,20 @@ +--- +source: tui/src/chatwidget/tests/permissions.rs +assertion_line: 50 +expression: "render_bottom_popup(&chat, 110)" +--- + Update Model Permissions + +› 1. Ask for approval (current) Codex can read and edit files in the current workspace, and run commands. + Approval is required to access the internet or edit other files. + Approve for me (disabled) Only ask for actions detected as potentially unsafe. (disabled: Disabled by + requirements.) + Full Access (disabled) Codex can edit files outside this workspace and access the internet without + asking for approval. Exercise caution when using. (disabled: Disabled by + requirements.) + Read Only (disabled) Codex can read files in the current workspace. Approval is required to edit + files or access the internet. (disabled: Disabled by requirements.) + server-only (disabled) Description from the connected server. (disabled: Selecting custom profiles + from a remote server is not supported yet.) + + Press enter to confirm or esc to go back diff --git a/codex-rs/tui/src/chatwidget/tests/app_server.rs b/codex-rs/tui/src/chatwidget/tests/app_server.rs index a690b7750a..40a76f5466 100644 --- a/codex-rs/tui/src/chatwidget/tests/app_server.rs +++ b/codex-rs/tui/src/chatwidget/tests/app_server.rs @@ -1842,3 +1842,27 @@ async fn live_app_server_thread_closed_requests_immediate_exit() { assert_matches!(rx.try_recv(), Ok(AppEvent::Exit(ExitMode::Immediate))); } + +#[tokio::test] +async fn permission_discovery_invalidates_on_thread_settings_and_uses_updated_cwd() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + let thread_id = ThreadId::new(); + chat.thread_id = Some(thread_id); + chat.config.explicit_permission_profile_mode = true; + for complete in [false, true] { + chat.open_permissions_popup(); + let request_id = chat.permission_popup_request_id.unwrap(); + if complete { + chat.on_permission_profiles_loaded(request_id, Err("disconnected".to_string())); + } + chat.on_thread_settings_updated(thread_settings_for_test("gpt-5.2", thread_id)); + chat.on_permission_profiles_loaded(request_id, Err("stale".to_string())); + assert!(!chat.bottom_pane.has_active_view()); + } + while rx.try_recv().is_ok() {} + chat.open_permissions_popup(); + let AppEvent::FetchPermissionProfiles { thread_cwd, .. } = rx.try_recv().unwrap() else { + panic!("expected discovery") + }; + assert_eq!(thread_cwd, Some(test_path_buf("/tmp/thread-settings"))); +} diff --git a/codex-rs/tui/src/chatwidget/tests/permissions.rs b/codex-rs/tui/src/chatwidget/tests/permissions.rs index 8397347be1..e1a66ed204 100644 --- a/codex-rs/tui/src/chatwidget/tests/permissions.rs +++ b/codex-rs/tui/src/chatwidget/tests/permissions.rs @@ -1,5 +1,6 @@ use super::*; use crate::legacy_core::config::PermissionProfileCatalogEntry; +use crate::permission_discovery::PermissionDiscovery as Discovery; use codex_protocol::models::ActivePermissionProfile; use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_DANGER_FULL_ACCESS; use codex_protocol::models::ManagedFileSystemPermissions; @@ -10,6 +11,127 @@ use codex_protocol::permissions::FileSystemSpecialPath; use codex_protocol::permissions::NetworkSandboxPolicy; use pretty_assertions::assert_eq; +#[tokio::test] +async fn permission_discovery_uses_server_catalog_and_keeps_remote_custom_selection_disabled() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + chat.config.explicit_permission_profile_mode = false; + chat.set_permission_profile_from_session_snapshot(PermissionProfileSnapshot::active( + PermissionProfile::workspace_write(), + ActivePermissionProfile::new(":workspace"), + )) + .unwrap(); + let mut discovery = Discovery::local(&chat.config); + discovery.remote = true; + discovery + .profiles + .push(codex_app_server_protocol::PermissionProfileSummary { + id: "server-only".to_string(), + description: Some("Description from the connected server.".to_string()), + allowed: true, + }); + discovery.requirements = Some( + serde_json::from_value(serde_json::json!({ + "allowedApprovalsReviewers": ["user"], + "allowedPermissionProfiles": {":workspace": true, "server-only": true} + })) + .unwrap(), + ); + chat.request_permission_profiles(); + let request_id = chat.permission_popup_request_id.unwrap(); + rx.try_recv().unwrap(); + assert_chatwidget_snapshot!( + "permission_discovery_loading", + render_bottom_popup(&chat, /*width*/ 80) + ); + chat.on_permission_profiles_loaded(request_id, Ok(discovery)); + assert_chatwidget_snapshot!( + "permission_discovery_remote", + render_bottom_popup(&chat, /*width*/ 110) + ); + // Server-only alternatives remain visible, but cannot enter the local mutation path. + chat.handle_key_event(KeyEvent::from(KeyCode::Char('5'))); + assert!(rx.try_recv().is_err()); + chat.handle_key_event(KeyEvent::from(KeyCode::Esc)); + chat.open_permissions_popup(); + rx.try_recv().unwrap(); + let request_id = chat.permission_popup_request_id.unwrap(); + let mut legacy = Discovery::local(&chat.config); + legacy.explicit_profile_mode = false; + chat.on_permission_profiles_loaded(request_id, Ok(legacy)); + let actual = render_bottom_popup(&chat, /*width*/ 110); + chat.open_legacy_permissions_popup(); + assert_eq!(actual, render_bottom_popup(&chat, /*width*/ 110)); +} + +#[tokio::test] +async fn permission_discovery_discards_stale_results_and_preserves_covering_modal() { + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + chat.config.explicit_permission_profile_mode = true; + chat.open_permissions_popup(); + let first = chat.permission_popup_request_id.unwrap(); + chat.handle_key_event(KeyEvent::from(KeyCode::Esc)); + chat.on_permission_profiles_loaded(first, Ok(Discovery::local(&chat.config))); + assert!(!chat.bottom_pane.has_active_view()); + chat.open_permissions_popup(); + let second = chat.permission_popup_request_id.unwrap(); + chat.on_permission_profiles_loaded(first, Ok(Discovery::local(&chat.config))); + assert!(chat.permission_popup_request_is_current(second)); + chat.update_account_state( + /*status_account_display*/ None, /*plan_type*/ None, + /*has_chatgpt_account*/ false, /*has_codex_backend_auth*/ false, + ); + chat.on_permission_profiles_loaded(second, Ok(Discovery::local(&chat.config))); + assert!(!chat.bottom_pane.has_active_view()); + + chat.open_permissions_popup(); + let request_id = chat.permission_popup_request_id.unwrap(); + chat.bottom_pane.show_selection_view(SelectionViewParams { + view_id: Some("other-modal"), + title: Some("Other modal".to_string()), + ..Default::default() + }); + chat.on_permission_profiles_loaded(request_id, Ok(Discovery::local(&chat.config))); + assert_eq!(chat.bottom_pane.active_view_id(), Some("other-modal")); + chat.handle_key_event(KeyEvent::from(KeyCode::Esc)); + assert!(!chat.bottom_pane.has_active_view()); +} + +#[tokio::test] +async fn permission_discovery_failure_can_retry_and_empty_catalog_keeps_current_profile() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + chat.config.explicit_permission_profile_mode = true; + chat.set_permission_profile_from_session_snapshot(PermissionProfileSnapshot::active( + PermissionProfile::workspace_write(), + ActivePermissionProfile::new("current-profile"), + )) + .unwrap(); + chat.open_permissions_popup(); + let request_id = chat.permission_popup_request_id.unwrap(); + rx.try_recv().unwrap(); + chat.on_permission_profiles_loaded( + request_id, + Err("Failed to load permissions: disconnected".to_string()), + ); + assert_chatwidget_snapshot!( + "permission_discovery_failure", + render_bottom_popup(&chat, /*width*/ 80) + ); + chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); + assert!(matches!(rx.try_recv(), Ok(AppEvent::OpenPermissionsPopup))); + chat.open_permissions_popup(); + let request_id = chat.permission_popup_request_id.unwrap(); + rx.try_recv().unwrap(); + let mut discovery = Discovery::local(&chat.config); + discovery.profiles.clear(); + chat.on_permission_profiles_loaded(request_id, Ok(discovery)); + assert_chatwidget_snapshot!( + "permission_discovery_empty", + render_bottom_popup(&chat, /*width*/ 100) + ); + chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); + assert!(rx.try_recv().is_err()); +} + fn app_server_workspace_write_profile(extra_root: AbsolutePathBuf) -> PermissionProfile { PermissionProfile::Managed { network: NetworkSandboxPolicy::Restricted, @@ -108,7 +230,7 @@ async fn profile_permissions_selection_popup_snapshot() { )) .expect("set active profile"); - chat.open_permissions_popup(); + chat.open_permission_profiles_popup(Discovery::local(&chat.config)); assert_chatwidget_snapshot!( "profile_permissions_selection_popup", @@ -128,7 +250,7 @@ async fn profile_permissions_selection_popup_with_disallowed_full_access_snapsho ..Default::default() }); - chat.open_permissions_popup(); + chat.open_permission_profiles_popup(Discovery::local(&chat.config)); assert_chatwidget_snapshot!( "profile_permissions_selection_popup_with_disallowed_full_access", @@ -160,7 +282,7 @@ async fn profile_permissions_selection_popup_with_custom_profiles_snapshot() { )) .expect("set active profile"); - chat.open_permissions_popup(); + chat.open_permission_profiles_popup(Discovery::local(&chat.config)); assert_chatwidget_snapshot!( "profile_permissions_selection_popup_with_custom_profiles", @@ -184,7 +306,7 @@ async fn profile_permissions_selection_emits_named_profile_event_only() { )) .expect("set active profile"); - chat.open_permissions_popup(); + chat.open_permission_profiles_popup(Discovery::local(&chat.config)); chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); let events = std::iter::from_fn(|| rx.try_recv().ok()).collect::>(); @@ -217,7 +339,7 @@ async fn profile_permissions_selection_emits_active_custom_profile() { )) .expect("set active profile"); - chat.open_permissions_popup(); + chat.open_permission_profiles_popup(Discovery::local(&chat.config)); chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); let events = std::iter::from_fn(|| rx.try_recv().ok()).collect::>(); @@ -249,7 +371,7 @@ async fn profile_permissions_selection_emits_auto_review_mode_event() { )) .expect("set active profile"); - chat.open_permissions_popup(); + chat.open_permission_profiles_popup(Discovery::local(&chat.config)); chat.handle_key_event(KeyEvent::from(KeyCode::Down)); chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); @@ -273,7 +395,7 @@ async fn profile_permissions_full_access_always_opens_confirmation() { chat.set_feature_enabled(Feature::GuardianApproval, /*enabled*/ false); chat.local_settings.notices.hide_full_access_warning = Some(true); - chat.open_permissions_popup(); + chat.open_permission_profiles_popup(Discovery::local(&chat.config)); chat.handle_key_event(KeyEvent::from(KeyCode::Up)); chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 1a14ef58bc..cb44bf73a5 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -120,6 +120,7 @@ mod color; mod config_update; pub(crate) mod custom_terminal; mod experimental_features; +mod permission_discovery; mod pets; pub use custom_terminal::Terminal; mod auto_review_denials; diff --git a/codex-rs/tui/src/permission_discovery.rs b/codex-rs/tui/src/permission_discovery.rs new file mode 100644 index 0000000000..f25ec548ec --- /dev/null +++ b/codex-rs/tui/src/permission_discovery.rs @@ -0,0 +1,220 @@ +//! Bounded discovery for permission pickers; fixed request IDs bound abandoned RPCs. +//! This never applies a profile; +//! remote custom profiles remain display-only until selection stops rebuilding local config. + +use crate::app_event::AppEvent; +use crate::app_event_sender::AppEventSender; +use crate::app_server_session::AppServerSession; +use crate::app_server_session::ThreadParamsMode; +use crate::legacy_core::config::Config; +use codex_app_server_client::TypedRequestError; +use codex_app_server_protocol::ApprovalsReviewer; +use codex_app_server_protocol::AskForApproval; +use codex_app_server_protocol::ClientRequest; +use codex_app_server_protocol::ConfigReadParams; +use codex_app_server_protocol::ConfigReadResponse; +use codex_app_server_protocol::ConfigRequirements; +use codex_app_server_protocol::ConfigRequirementsReadResponse; +use codex_app_server_protocol::PermissionProfileListParams; +use codex_app_server_protocol::PermissionProfileListResponse; +use codex_app_server_protocol::PermissionProfileSummary; +use codex_app_server_protocol::RequestId; +use codex_utils_approval_presets::builtin_approval_presets; +use std::collections::HashSet; +use std::time::Duration; +use uuid::Uuid; + +#[derive(Debug)] +pub(crate) struct PermissionDiscovery { + pub(crate) profiles: Vec, + pub(crate) requirements: Option, + pub(crate) remote: bool, + pub(crate) explicit_profile_mode: bool, +} + +impl PermissionDiscovery { + pub(crate) fn local(config: &Config) -> Self { + let mut profiles = builtin_approval_presets() + .into_iter() + .map(|preset| PermissionProfileSummary { + id: preset.active_permission_profile.id, + description: None, + allowed: true, + }) + .collect::>(); + profiles.extend(config.custom_permission_profiles.iter().map(|profile| { + PermissionProfileSummary { + id: profile.id.clone(), + description: profile.description.clone(), + allowed: profile.allowed, + } + })); + Self { + profiles, + requirements: None, + remote: false, + explicit_profile_mode: true, + } + } + + pub(crate) fn disabled_reason( + &self, + id: &str, + approval: Option, + reviewer: Option, + ) -> Option { + let Some(profile) = self.profiles.iter().find(|profile| profile.id == id) else { + return Some("Not available on this server.".to_string()); + }; + let requirements = self.requirements.as_ref(); + if !profile.allowed + || requirements + .and_then(|r| r.allowed_permission_profiles.as_ref()) + .is_some_and(|allowed| allowed.get(id) != Some(&true)) + || requirements + .and_then(|r| r.allowed_approval_policies.as_ref()) + .is_some_and(|allowed| approval.is_some_and(|value| !allowed.contains(&value))) + || requirements + .and_then(|r| r.allowed_approvals_reviewers.as_ref()) + .is_some_and(|allowed| reviewer.is_some_and(|value| !allowed.contains(&value))) + { + return Some("Disabled by requirements.".to_string()); + } + (self.remote && !id.starts_with(':')).then(|| { + "Selecting custom profiles from a remote server is not supported yet.".to_string() + }) + } +} + +pub(crate) fn fetch( + app_server: &AppServerSession, + request_id: Uuid, + config: &Config, + thread_cwd: Option<&std::path::Path>, + tx: AppEventSender, +) { + let handle = app_server.request_handle(); + let mode = app_server.thread_params_mode(); + let cwd = thread_cwd + .or_else(|| match mode { + ThreadParamsMode::Embedded => Some(config.cwd.as_path()), + ThreadParamsMode::Remote => app_server.remote_cwd_override(), + }) + .map(|cwd| cwd.to_string_lossy().into_owned()); + // The daemon's catalog cannot see permission definitions from this invocation. + let local_discovery = (!app_server.uses_embedded_app_server() + && mode == ThreadParamsMode::Embedded + && config.config_layer_stack.layers_low_to_high().any(|layer| { + matches!(layer.name, codex_config::ConfigLayerSource::SessionFlags) + && layer.config.get("permissions").is_some() + })) + .then(|| PermissionDiscovery::local(config)); + let active_custom_profile = config + .permissions + .active_permission_profile() + .is_some_and(|profile| !profile.id.starts_with(':')); + tokio::spawn(async move { + let request = async { + if let Some(discovery) = local_discovery { + return Ok(discovery); + } + if mode == ThreadParamsMode::Remote && !active_custom_profile { + let config: ConfigReadResponse = handle + .request_typed(ClientRequest::ConfigRead { + request_id: RequestId::String("tui-permission-config".to_string()), + params: ConfigReadParams { + include_layers: false, + cwd: cwd.clone(), + }, + }) + .await + .map_err(discovery_error)?; + if !config + .config + .additional + .get("default_permissions") + .is_some_and(serde_json::Value::is_string) + { + return Ok(PermissionDiscovery { + profiles: Vec::new(), + requirements: None, + remote: true, + explicit_profile_mode: false, + }); + } + } + let requirements: ConfigRequirementsReadResponse = handle + .request_typed(ClientRequest::ConfigRequirementsRead { + request_id: RequestId::String("tui-permission-requirements".to_string()), + params: None, + }) + .await + .map_err(discovery_error)?; + let mut profiles = Vec::new(); + let mut cursor = None; + let mut cursors = HashSet::new(); + for _ in 0..10 { + let response: PermissionProfileListResponse = handle + .request_typed(ClientRequest::PermissionProfileList { + request_id: RequestId::String("tui-permission-profiles".to_string()), + params: PermissionProfileListParams { + cursor, + limit: Some(100), + cwd: cwd.clone(), + }, + }) + .await + .map_err(discovery_error)?; + if response.data.len() > 100 { + break; + } + profiles.extend(response.data); + cursor = response.next_cursor; + let Some(next) = cursor.as_ref() else { + let mut ids = HashSet::new(); + if !profiles.iter().all(|profile| ids.insert(&profile.id)) { + return Err( + "The server returned duplicate permission profiles.".to_string() + ); + } + return Ok(PermissionDiscovery { + profiles, + requirements: requirements.requirements, + remote: mode == ThreadParamsMode::Remote, + explicit_profile_mode: true, + }); + }; + if !cursors.insert(next.clone()) { + break; + } + } + Err( + "Permission discovery exceeded its pagination limit. Try /permissions again." + .to_string(), + ) + }; + let result = tokio::time::timeout(Duration::from_secs(10), request) + .await + .unwrap_or_else(|_| { + Err("Permission discovery timed out. Try /permissions again.".to_string()) + }); + tx.send(AppEvent::PermissionProfilesLoaded { request_id, result }); + }); +} + +fn discovery_error(error: TypedRequestError) -> String { + if let TypedRequestError::Server { source, .. } = &error + && (source.code == -32601 + || (source.code == -32600 + && (source.message.contains("permissionProfile/list") + || source.message.contains("configRequirements/read") + || source.message.contains("config/read")))) + { + return "This server does not support permission discovery. Upgrade the Codex server to use this menu.".to_string(); + } + format!("Failed to load permissions: {error}") +} + +#[cfg(test)] +#[path = "permission_discovery_tests.rs"] +mod tests; diff --git a/codex-rs/tui/src/permission_discovery_tests.rs b/codex-rs/tui/src/permission_discovery_tests.rs new file mode 100644 index 0000000000..27f29bf0df --- /dev/null +++ b/codex-rs/tui/src/permission_discovery_tests.rs @@ -0,0 +1,200 @@ +use super::*; +use crate::legacy_core::config::ConfigBuilder; +use futures::SinkExt; +use futures::StreamExt; +use pretty_assertions::assert_eq; +use serde_json::Value; +use serde_json::json; +use tokio_tungstenite::tungstenite::Message; + +#[tokio::test] +async fn discovery_preserves_config_scope_and_bounds_server_requests() { + let home = tempfile::tempdir().unwrap(); + for case in [ + "local", + "session-only", + "remote", + "remote-default", + "thread", + "legacy", + "empty", + "unsupported", + "config-error", + "cycle", + "limit", + "timeout", + ] { + let mode = if matches!(case, "local" | "session-only" | "timeout") { + ThreadParamsMode::Embedded + } else { + ThreadParamsMode::Remote + }; + let overrides = if case == "session-only" { + vec![ + ( + "permissions.session-only.extends".into(), + ":workspace".into(), + ), + ("default_permissions".into(), "session-only".into()), + ] + } else { + Vec::new() + }; + let config = ConfigBuilder::default() + .codex_home(home.path().to_path_buf()) + .cli_overrides(overrides) + .build() + .await + .unwrap(); + let thread_cwd = (case == "thread").then_some(std::path::Path::new("/thread")); + let remote_cwd = (case != "remote-default").then_some("/server"); + let cwd = thread_cwd.map(|path| path.to_str().unwrap()).or_else(|| { + if mode == ThreadParamsMode::Embedded { + config.cwd.to_str() + } else { + remote_cwd + } + }); + let data = json!([ + {"id": ":workspace", "description": null, "allowed": true}, + {"id": "remote-profile", "description": "Server description", "allowed": true} + ]); + let requirements = json!({"allowedPermissionProfiles": {"remote-profile": true}}); + let mut replies = vec![json!({"result": {"requirements": requirements}})]; + replies.extend(match case { + "empty" => vec![json!({"result": {"data": [], "nextCursor": null}})], + "unsupported" => vec![json!({"error": {"code": -32601, "message": "Unknown method"}})], + "config-error" => { + vec![json!({"error": {"code": -32603, "message": "Invalid project config"}})] + } + "cycle" => vec![json!({"result": {"data": [], "nextCursor": "repeat"}}); 2], + "limit" => (0..10) + .map(|i| json!({"result": {"data": [], "nextCursor": i.to_string()}})) + .collect(), + _ => vec![ + json!({"result": {"data": [data[0]], "nextCursor": "second"}}), + json!({"result": {"data": [data[1]], "nextCursor": null}}), + ], + }); + if mode == ThreadParamsMode::Remote { + replies.insert( + /*index*/ 0, + json!({"result": {"config": { + "default_permissions": (case != "legacy").then_some(":workspace") + }, "origins": {}}}), + ); + } else if case == "timeout" { + replies = vec![Value::Null]; + } + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let websocket_url = format!("ws://{}", listener.local_addr().unwrap()); + let server = tokio::spawn(async move { + let (stream, _) = listener.accept().await.unwrap(); + let mut socket = tokio_tungstenite::accept_async(stream).await.unwrap(); + let mut replies = replies.into_iter(); + let mut requests = Vec::new(); + while let Some(Ok(Message::Text(text))) = socket.next().await { + let request: Value = serde_json::from_str(&text).unwrap(); + if request["id"].is_null() { + continue; + } + let mut response = if request["method"] == "initialize" { + json!({"result": {"userAgent": "permission-discovery-test", "platformFamily": "unix", "platformOs": "linux"}}) + } else { + requests.push(request.clone()); + replies.next().expect("unexpected discovery request") + }; + if response.is_null() { + continue; + } + response["id"] = request["id"].clone(); + socket + .send(Message::Text(response.to_string().into())) + .await + .unwrap(); + } + requests + }); + let client = crate::connect_remote_app_server(crate::RemoteAppServerEndpoint::WebSocket { + websocket_url, + auth_token: None, + }) + .await + .unwrap(); + let session = AppServerSession::new(client, mode) + .with_remote_cwd_override(remote_cwd.map(std::path::PathBuf::from)); + let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel(); + if case == "timeout" { + tokio::time::pause(); + } // Keep real time for the socket handshake. + for attempt in 0..if case == "timeout" { 4 } else { 1 } { + fetch( + &session, + Uuid::new_v4(), + &config, + thread_cwd, + AppEventSender::new(tx.clone()), + ); + let AppEvent::PermissionProfilesLoaded { result, .. } = rx.recv().await.unwrap() else { + panic!("expected discovery result") + }; + let error = match case { + "unsupported" => Some("Upgrade"), + "config-error" => Some("Invalid project config"), + "cycle" | "limit" => Some("pagination limit"), + "timeout" if attempt == 0 => Some("timed out"), + "timeout" => Some("duplicate"), + _ => None, + }; + if let Some(error) = error { + assert!(result.unwrap_err().contains(error)); + continue; + } + let discovery = result.unwrap(); + assert_eq!(discovery.explicit_profile_mode, case != "legacy"); + if case == "session-only" { + assert!( + discovery + .profiles + .iter() + .any(|profile| profile.id == "session-only" && profile.allowed) + ); + } else if matches!(case, "empty" | "legacy") { + assert!(discovery.profiles.is_empty()); + } else { + assert_eq!(serde_json::to_value(&discovery.profiles).unwrap(), data); + assert_eq!( + discovery.requirements, + Some(serde_json::from_value(requirements.clone()).unwrap()) + ); + } + } + session.shutdown().await.unwrap(); + let requests = server.await.unwrap(); + match case { + "session-only" => assert!(requests.is_empty()), + "timeout" | "legacy" => assert_eq!(requests.len(), 1), + "local" | "remote" | "remote-default" | "thread" => { + let pages: Vec<_> = requests + .iter() + .filter(|request| request["method"] == "permissionProfile/list") + .map(|request| request["params"].clone()) + .collect(); + assert_eq!( + pages, + vec![ + json!({"cwd": cwd, "limit": 100, "cursor": null}), + json!({"cwd": cwd, "limit": 100, "cursor": "second"}) + ] + ); + if mode == ThreadParamsMode::Remote { + assert_eq!(requests[0]["params"]["cwd"], json!(cwd)); + } + } + _ => {} + } + if case == "timeout" { + tokio::time::resume(); + } + } +}