From b588812e8cc82f88a3141ab6bcdf654cbb18361d Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Wed, 30 Sep 2026 03:13:09 +0000 Subject: [PATCH] Use server-authoritative permissions in the TUI (#49472) ## Why Client permission definitions and constraints can differ from those on the connected app server. Accepted permission changes also need to survive task transitions before their settings notifications arrive. ## What changed - Discover permission profiles and availability from the connected server and apply selections through `thread/settings/update`, including custom profiles. - Adopt the server's effective permission settings instead of reapplying local definitions or constraints. - Retain accepted selections and effective approval settings across new tasks, forks, side conversations, directory changes, reconnects, and safety retries without blocking navigation on a settings notification. - Recognize the current built-in permission preset when the server supplies an unnamed profile. ## Testing Add regression coverage for conflicting client/server profile definitions, rejected and unchanged selections, consecutive requests with omitted approval settings, and permission carryover before confirmation. Extend discovery and menu tests to verify server-provided availability and unnamed-profile highlighting. GitOrigin-RevId: bcd767b58dc92dc4f945189078e51e6e5fce6d24 --- codex-rs/tui/src/app/agents_overview.rs | 11 + codex-rs/tui/src/app/agents_overview_new.rs | 7 + .../tui/src/app/agents_overview_threads.rs | 10 + codex-rs/tui/src/app/config_persistence.rs | 120 +++++--- codex-rs/tui/src/app/event_dispatch.rs | 5 +- codex-rs/tui/src/app/reconnect.rs | 9 +- codex-rs/tui/src/app/safety_buffering.rs | 10 +- codex-rs/tui/src/app/session_lifecycle.rs | 14 + codex-rs/tui/src/app/side.rs | 11 +- codex-rs/tui/src/app/tests.rs | 103 +++++-- .../src/app/tests/active_reconnect_tests.rs | 3 +- .../tests/background_task_defaults_tests.rs | 70 ++++- .../app/tests/permission_selection_tests.rs | 264 +++++++++++++++++- .../app/tests/permission_shortcuts_tests.rs | 6 +- .../tui/src/app/tests/safety_buffering.rs | 47 ++++ .../app/tests/session_lifecycle_requests.rs | 22 +- ...and_center_pending_server_permissions.snap | 11 - codex-rs/tui/src/app/thread_routing.rs | 57 ++-- codex-rs/tui/src/app/thread_settings.rs | 28 +- codex-rs/tui/src/app/working_directory.rs | 50 +++- codex-rs/tui/src/app_server_session.rs | 33 ++- .../rollout_history_tests.rs | 17 +- .../workspace_roots_tests.rs | 7 +- .../src/chatwidget/permission_discovery.rs | 3 - .../tui/src/chatwidget/permission_popups.rs | 3 +- .../tui/src/chatwidget/permissions_menu.rs | 21 +- codex-rs/tui/src/chatwidget/settings.rs | 12 +- ...s__permissions_unnamed_server_profile.snap | 15 + .../tui/src/chatwidget/tests/app_server.rs | 2 + .../tui/src/chatwidget/tests/permissions.rs | 36 ++- codex-rs/tui/src/permission_discovery.rs | 48 +--- .../tui/src/permission_discovery_tests.rs | 27 +- 32 files changed, 833 insertions(+), 249 deletions(-) delete mode 100644 codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__background_task_defaults_tests__command_center_pending_server_permissions.snap create mode 100644 codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__permissions_unnamed_server_profile.snap diff --git a/codex-rs/tui/src/app/agents_overview.rs b/codex-rs/tui/src/app/agents_overview.rs index 89db4ec2dd..6d6f80c923 100644 --- a/codex-rs/tui/src/app/agents_overview.rs +++ b/codex-rs/tui/src/app/agents_overview.rs @@ -59,6 +59,8 @@ pub(super) struct AgentsOverviewState { Arc>, /// Explicit permission-profile choices for new-session carryover, retained across navigation. pub(super) selected_permission_profiles: HashMap, + /// Accepted menu requests; unchanged settings may never produce a notification. + pub(super) requested_permission_profiles: HashMap, /// Keep new tasks subscribed and reusable until a first turn makes them resumable. pub(super) blank_sessions: HashMap, pub(super) input_states: HashMap, @@ -956,6 +958,15 @@ impl App { .active_permission_profile .as_ref() .is_some_and(|active| !active.id.starts_with(':')) + && self.chat_widget.thread_id().is_none_or(|thread_id| { + self.agents_overview + .selected_permission_profiles + .get(&thread_id) + != profile + .active_permission_profile + .as_ref() + .map(|active| &active.id) + }) && (!profile.matches_config(&config) || config.permissions.profile_workspace_roots() != self.config.permissions.profile_workspace_roots()) diff --git a/codex-rs/tui/src/app/agents_overview_new.rs b/codex-rs/tui/src/app/agents_overview_new.rs index d422598d5a..7f7fd04ffc 100644 --- a/codex-rs/tui/src/app/agents_overview_new.rs +++ b/codex-rs/tui/src/app/agents_overview_new.rs @@ -121,6 +121,13 @@ impl App { return Ok(AppRunControl::Continue); }; let selected_profile = self.chat_widget.thread_id().and_then(|thread_id| { + if let Some(selection) = self + .agents_overview + .requested_permission_profiles + .get(&thread_id) + { + return Some(selection.clone()); + } let source = self.chat_widget.config_ref(); let active = source.permissions.active_permission_profile()?; (self diff --git a/codex-rs/tui/src/app/agents_overview_threads.rs b/codex-rs/tui/src/app/agents_overview_threads.rs index fbe2ae575a..d8186f2022 100644 --- a/codex-rs/tui/src/app/agents_overview_threads.rs +++ b/codex-rs/tui/src/app/agents_overview_threads.rs @@ -119,6 +119,9 @@ impl App { self.agents_overview.threads.insert(thread_id, Some(thread)); } ServerNotification::ThreadArchived(_) | ServerNotification::ThreadDeleted(_) => { + self.agents_overview + .requested_permission_profiles + .remove(&thread_id); self.agents_overview .selected_permission_profiles .remove(&thread_id); @@ -132,6 +135,9 @@ impl App { self.agents_overview.removed_threads.remove(&thread_id); } ServerNotification::ThreadClosed(_) => { + self.agents_overview + .requested_permission_profiles + .remove(&thread_id); self.agents_overview.activity.remove(&thread_id); if let Some(usage) = self.agents_overview.usage.get_mut(&thread_id) { usage.tokens = None; @@ -160,6 +166,10 @@ impl App { } ServerNotification::ThreadSettingsUpdated(settings) => { if !self.pending_server_profiles.contains_key(&thread_id) + && !self + .agents_overview + .requested_permission_profiles + .contains_key(&thread_id) && self .agents_overview .selected_permission_profiles diff --git a/codex-rs/tui/src/app/config_persistence.rs b/codex-rs/tui/src/app/config_persistence.rs index 00d2794e32..d563aa2326 100644 --- a/codex-rs/tui/src/app/config_persistence.rs +++ b/codex-rs/tui/src/app/config_persistence.rs @@ -113,6 +113,7 @@ impl App { .await } + #[cfg(test)] pub(super) async fn rebuild_config_for_permission_profile( &self, profile_id: &str, @@ -136,6 +137,8 @@ impl App { .await } + // Local setup for startup fixtures; connected selections always use the server below. + #[cfg(test)] pub(super) async fn apply_permission_profile_selection( &mut self, selection: PermissionProfileSelection, @@ -266,25 +269,11 @@ impl App { pub(super) async fn select_permission_profile( &mut self, app_server: &mut AppServerSession, - selection: PermissionProfileSelection, + mut selection: PermissionProfileSelection, ) { if self.reject_pending_permission_change() { return; } - if (self.chat_widget.thread_id().is_none() && selection.profile_id.starts_with(':')) - || (app_server.thread_params_mode() - == crate::app_server_session::ThreadParamsMode::Embedded - && self - .config - .custom_permission_profiles - .iter() - .any(|profile| profile.id == selection.profile_id)) - { - if self.apply_permission_profile_selection(selection).await { - self.chat_widget.submit_initial_user_message_if_pending(); - } - return; - } let Some(thread_id) = self.chat_widget.thread_id() else { self.chat_widget .retain_input_after_failed_permission_selection(); @@ -293,21 +282,16 @@ impl App { ); return; }; - if !selection.profile_id.starts_with(':') - && self.chat_widget.is_user_turn_pending_or_running() - { - self.chat_widget - .retain_input_after_failed_permission_selection(); - self.chat_widget.add_error_message( - "Wait for the current turn to finish before changing permissions.".into(), - ); - return; - } let config = self.chat_widget.config_ref(); - if config - .permissions - .active_permission_profile() - .is_some_and(|profile| profile.id == selection.profile_id) + if !self + .agents_overview + .requested_permission_profiles + .contains_key(&thread_id) + && selection.profile_id.starts_with(':') + && config + .permissions + .active_permission_profile() + .is_some_and(|profile| profile.id == selection.profile_id) && selection .approval_policy .is_none_or(|policy| config.permissions.approval_policy.value() == policy.to_core()) @@ -330,10 +314,25 @@ impl App { }; match app_server.thread_settings_update(params).await { Ok(true) => { + // Omission preserves the active value on update, but uses defaults on start. + // Retain effective intent across consecutive requests before confirmation. + let previous = self + .agents_overview + .requested_permission_profiles + .get(&thread_id); + selection.approval_policy = selection + .approval_policy + .or_else(|| previous.and_then(|profile| profile.approval_policy)) + .or(Some(config.permissions.approval_policy.value().into())); + selection.approvals_reviewer = selection + .approvals_reviewer + .or_else(|| previous.and_then(|profile| profile.approvals_reviewer)) + .or(Some(config.approvals_reviewer)); self.agents_overview .selected_permission_profiles .insert(thread_id, selection.profile_id.clone()); - self.pending_server_profiles + self.agents_overview + .requested_permission_profiles .insert(thread_id, selection.clone()); self.chat_widget.add_info_message( format!( @@ -386,18 +385,35 @@ impl App { true } - pub(super) fn confirmed_server_profile( + pub(super) fn selected_server_profile( &self, thread_id: ThreadId, ) -> Option { if self.chat_widget.thread_id() != Some(thread_id) { return None; } + if let Some(selection) = self + .agents_overview + .requested_permission_profiles + .get(&thread_id) + { + return Some(selection.clone()); + } + if self.app_server_target.thread_params_mode() + == crate::app_server_session::ThreadParamsMode::Remote + { + return None; + } let config = self.chat_widget.config_ref(); let active = config.permissions.active_permission_profile()?; if active.id.starts_with(':') - || (self.app_server_target.thread_params_mode() - == crate::app_server_session::ThreadParamsMode::Embedded + || (self + .agents_overview + .selected_permission_profiles + .get(&thread_id) + != Some(&active.id) + && self.app_server_target.thread_params_mode() + == crate::app_server_session::ThreadParamsMode::Embedded && self .config .custom_permission_profiles @@ -414,6 +430,44 @@ impl App { }) } + /// Adopt authoritative widget settings before deriving overrides for subsequent turns. + pub(super) fn adopt_server_permissions(&mut self) { + self.chat_widget.set_permission_network(/*network*/ None); + self.config.permissions = self.chat_widget.config_ref().permissions.clone(); + self.config.approvals_reviewer = self.chat_widget.config_ref().approvals_reviewer; + self.runtime_approval_policy_override = Some(RuntimeApprovalPolicyOverride::Explicit( + self.config.permissions.approval_policy.value().into(), + )); + self.runtime_approvals_reviewer_override = if self + .config + .permissions + .active_permission_profile() + .is_some_and(|profile| profile.id.starts_with(':')) + { + Some(self.config.approvals_reviewer) + } else { + self.runtime_approvals_reviewer_override + .filter(|reviewer| *reviewer == self.config.approvals_reviewer) + }; + self.runtime_permission_profile_override = + Some(RuntimePermissionProfileOverride::from_config(&self.config)); + } + + pub(super) fn adopt_inherited_server_selection(&mut self) { + if let Some(thread_id) = self.chat_widget.thread_id() + && let Some(active) = self + .chat_widget + .config_ref() + .permissions + .active_permission_profile() + { + self.agents_overview + .selected_permission_profiles + .insert(thread_id, active.id); + } + self.adopt_server_permissions(); + } + pub(super) async fn refresh_in_memory_config_from_disk(&mut self) -> Result<()> { let mut config = self .rebuild_config_for_cwd(self.chat_widget.config_ref().cwd.to_path_buf()) diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index da47c0f7e9..01c9626029 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -533,7 +533,7 @@ impl App { fork_config.model = Some(self.chat_widget.current_model().to_string()); fork_config.model_reasoning_effort = self.chat_widget.current_reasoning_effort(); - let selected_profile = self.confirmed_server_profile(thread_id); + let selected_profile = self.selected_server_profile(thread_id); match app_server.fork_thread_at( &self.local_settings, fork_config, @@ -574,6 +574,9 @@ impl App { .await { Ok(()) => { + if selected_profile.is_some() { + self.adopt_inherited_server_selection(); + } // Keep local input without replacing the fork's running state. self.chat_widget.restore_reconnected_input(retained_input, &[]); if let Some(err) = name_error { diff --git a/codex-rs/tui/src/app/reconnect.rs b/codex-rs/tui/src/app/reconnect.rs index a0e8f9b1a5..0656ba6e38 100644 --- a/codex-rs/tui/src/app/reconnect.rs +++ b/codex-rs/tui/src/app/reconnect.rs @@ -327,8 +327,13 @@ impl App { self.pending_app_server_requests.clear(); // The displayed task was resumed above. Keep offscreen selections pending until those // tasks can be resumed from the server too; their old confirmations cannot arrive. - let pending_displayed_profile = - displayed.and_then(|id| self.pending_server_profiles.remove(&id)); + let pending_displayed_profile = displayed.and_then(|id| { + self.pending_server_profiles.remove(&id).or_else(|| { + self.agents_overview + .requested_permission_profiles + .remove(&id) + }) + }); if let Some(selection) = &pending_displayed_profile { self.runtime_approval_policy_override = None; if selection.approvals_reviewer.is_some() { diff --git a/codex-rs/tui/src/app/safety_buffering.rs b/codex-rs/tui/src/app/safety_buffering.rs index edab48f29b..705a0fe0e9 100644 --- a/codex-rs/tui/src/app/safety_buffering.rs +++ b/codex-rs/tui/src/app/safety_buffering.rs @@ -60,6 +60,8 @@ impl App { items, cwd, active_permission_profile, + approval_policy, + approvals_reviewer, model: turn_model, effort, collaboration_mode, @@ -180,7 +182,7 @@ impl App { let retry_display = ChatWidget::user_message_display_from_inputs(items); self.config = retry_config.clone(); - let selected_profile = self.confirmed_server_profile(thread_id); + let selected_profile = self.selected_server_profile(thread_id); let started = app_server .fork_thread_at( &self.local_settings, @@ -200,6 +202,9 @@ impl App { } }; let retry_thread_id = started.session.thread_id; + *approval_policy = started.session.approval_policy; + *approvals_reviewer = Some(started.session.approvals_reviewer); + *active_permission_profile = started.session.active_permission_profile.clone(); self.detach_current_thread_for_navigation(app_server, Some(retry_thread_id)) .await; @@ -216,6 +221,9 @@ impl App { return; } + if selected_profile.is_some() { + self.adopt_inherited_server_selection(); + } let failure_input_state = input_state.clone(); self.chat_widget.restore_thread_input_state( input_state, diff --git a/codex-rs/tui/src/app/session_lifecycle.rs b/codex-rs/tui/src/app/session_lifecycle.rs index 4723dec353..1516cba098 100644 --- a/codex-rs/tui/src/app/session_lifecycle.rs +++ b/codex-rs/tui/src/app/session_lifecycle.rs @@ -757,6 +757,20 @@ impl App { self.app_event_tx .send(AppEvent::ResetTranscriptForThreadSwitch); self.replay_thread_snapshot(snapshot, resume_restored_queue); + if let Some(thread_id) = self.chat_widget.thread_id() + && let Some(active) = self + .chat_widget + .config_ref() + .permissions + .active_permission_profile() + && self + .agents_overview + .selected_permission_profiles + .get(&thread_id) + == Some(&active.id) + { + self.adopt_server_permissions(); + } if external_writer { self.chat_widget.show_external_writer_thread(); } diff --git a/codex-rs/tui/src/app/side.rs b/codex-rs/tui/src/app/side.rs index f4006ba36a..e55aeb8892 100644 --- a/codex-rs/tui/src/app/side.rs +++ b/codex-rs/tui/src/app/side.rs @@ -722,8 +722,14 @@ impl App { .await; let fork_config = self.side_fork_config(); + let selected_profile = self.selected_server_profile(parent_thread_id); match app_server - .fork_side_thread(&self.local_settings, fork_config, parent_thread_id) + .fork_side_thread( + &self.local_settings, + fork_config, + parent_thread_id, + selected_profile.as_ref(), + ) .await { Ok(forked) => { @@ -779,6 +785,9 @@ impl App { return Ok(AppRunControl::Continue); } if self.active_thread_id == Some(child_thread_id) { + if selected_profile.is_some() { + self.adopt_inherited_server_selection(); + } if let Some(user_message) = user_message.take() { let _ = self .chat_widget diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index 8d54e52e55..34e83eab49 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -2862,6 +2862,20 @@ async fn handle_start_side_seeds_navigation_before_thread_started() -> Result<() while app_event_rx.try_recv().is_ok() {} let mut tui = crate::tui::test_support::make_test_tui()?; + app.runtime_permission_profile_override = Some(RuntimePermissionProfileOverride::from_config( + app.chat_widget.config_ref(), + )); + app.select_permission_profile( + &mut app_server, + PermissionProfileSelection { + profile_id: ":read-only".into(), + approval_policy: Some(AskForApproval::OnRequest), + approvals_reviewer: Some(ApprovalsReviewer::User), + display_label: "Read Only".into(), + }, + ) + .await; + let control = Box::pin(app.handle_start_side( &mut tui, &mut app_server, @@ -2903,6 +2917,53 @@ async fn handle_start_side_seeds_navigation_before_thread_started() -> Result<() } assert!(saw_thread_started); + assert_eq!( + app.config.permissions.permission_profile(), + &PermissionProfile::read_only() + ); + assert_eq!( + app.runtime_permission_profile_override, + Some(RuntimePermissionProfileOverride::from_config( + app.chat_widget.config_ref() + )) + ); + app.select_agent_thread(&mut tui, &mut app_server, parent_thread_id) + .await?; + app.select_permission_profile( + &mut app_server, + PermissionProfileSelection { + profile_id: ":workspace".into(), + approval_policy: Some(AskForApproval::OnRequest), + approvals_reviewer: Some(ApprovalsReviewer::User), + display_label: "Workspace".into(), + }, + ) + .await; + app.select_agent_thread(&mut tui, &mut app_server, side_thread_id) + .await?; + let settings = next_thread_settings_updated(&mut app_server, parent_thread_id).await; + app.enqueue_thread_notification( + parent_thread_id, + ServerNotification::ThreadSettingsUpdated(settings), + ) + .await?; + app.select_agent_thread(&mut tui, &mut app_server, parent_thread_id) + .await?; + assert_eq!( + app.config + .permissions + .active_permission_profile() + .unwrap() + .id, + ":workspace" + ); + assert_eq!( + app.runtime_permission_profile_override, + Some(RuntimePermissionProfileOverride::from_config( + app.chat_widget.config_ref() + )) + ); + app_server.shutdown().await?; Ok(()) } @@ -3112,23 +3173,6 @@ async fn server_only_profile_selection_keeps_turns_on_the_selected_profile() -> @"• Permission selection requested: server-only" ); let mut tui = crate::tui::test_support::make_test_tui()?; - app.select_permission_profile(&mut server, selection.clone()) - .await; - insta::assert_snapshot!( - next_history_message(&mut events), - @"■ Wait for permissions to update before changing permissions." - ); - app.handle_event( - &mut tui, - &mut server, - AppEvent::ForkCurrentSession { name: None }, - ) - .await?; - let _ = next_history_message(&mut events); - insta::assert_snapshot!( - next_history_message(&mut events), - @"■ Wait for permissions to update before forking." - ); app.chat_widget .restore_user_message_to_composer("use the selected profile".into()); app.chat_widget @@ -3171,6 +3215,7 @@ async fn server_only_profile_selection_keeps_turns_on_the_selected_profile() -> ); let endpoint = crate::resolve_remote_addr("ws://127.0.0.1:8765")?; app.app_server_target = crate::AppServerTarget::Remote { endpoint }; + assert!(app.selected_server_profile(thread_id).is_none()); while events.try_recv().is_ok() {} app.change_working_directory(&mut tui, &mut server, home.path().abs()) .await; @@ -3206,20 +3251,24 @@ async fn server_only_profile_selection_keeps_turns_on_the_selected_profile() -> .restore_user_message_to_composer("queued follow-up".into()); app.chat_widget .handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); - let selection = app - .confirmed_server_profile(thread_id) - .expect("server profile"); + let selection = PermissionProfileSelection { + profile_id: "server-only".into(), + approval_policy: None, + approvals_reviewer: None, + display_label: "server-only".into(), + }; app.select_permission_profile(&mut server, selection).await; insta::assert_snapshot!( next_history_message(&mut events), - @"■ Wait for the current turn to finish before changing permissions." + @"• Permission selection requested: server-only" ); app.handle_event(&mut tui, &mut server, AppEvent::SettingsSelectionSettled) .await?; app.handle_thread_event_now(ThreadBufferedEvent::Notification(Box::new( turn_completed_notification(thread_id, "second", TurnStatus::Completed), ))); - assert!(app.chat_widget.has_queued_follow_up_messages()); + assert!(!app.chat_widget.has_queued_follow_up_messages()); + let _ = next_user_turn_op(&mut ops); server.shutdown().await?; Ok(()) } @@ -9232,7 +9281,10 @@ async fn selecting_cyber_model_defaults_active_thread_to_auto_review() { .await .expect("pending permission selection should block the cyber model"); assert_eq!(app.chat_widget.current_model(), previous_model); - app.pending_server_profiles.remove(&thread_id); + let previous_selection = app.pending_server_profiles.remove(&thread_id).unwrap(); + app.agents_overview + .requested_permission_profiles + .insert(thread_id, previous_selection); app.handle_event( &mut tui, &mut app_server, @@ -9242,6 +9294,11 @@ async fn selecting_cyber_model_defaults_active_thread_to_auto_review() { .expect("model selection should succeed"); assert!(app.pending_server_profiles.contains_key(&thread_id)); + assert!( + !app.agents_overview + .requested_permission_profiles + .contains_key(&thread_id) + ); let notification = next_thread_settings_updated(&mut app_server, thread_id).await; assert_eq!( notification.thread_settings.approval_policy, diff --git a/codex-rs/tui/src/app/tests/active_reconnect_tests.rs b/codex-rs/tui/src/app/tests/active_reconnect_tests.rs index 8f9c547618..de130ba8d3 100644 --- a/codex-rs/tui/src/app/tests/active_reconnect_tests.rs +++ b/codex-rs/tui/src/app/tests/active_reconnect_tests.rs @@ -206,7 +206,7 @@ async fn reconnect_restores_history_permissions_and_resumes_unsent_input() -> Re let mut tui = crate::tui::test_support::make_test_tui()?; if pending_profile { app.runtime_approvals_reviewer_override = Some(ApprovalsReviewer::User); - app.pending_server_profiles.insert( + app.agents_overview.requested_permission_profiles.insert( id, PermissionProfileSelection { profile_id: "server-only".into(), @@ -303,6 +303,7 @@ async fn reconnect_restores_history_permissions_and_resumes_unsent_input() -> Re ); } assert!(app.pending_server_profiles.is_empty()); + assert!(app.agents_overview.requested_permission_profiles.is_empty()); if pending_profile { assert_eq!( app.resume_permission_overrides(&app.config), diff --git a/codex-rs/tui/src/app/tests/background_task_defaults_tests.rs b/codex-rs/tui/src/app/tests/background_task_defaults_tests.rs index 0162d1e1a6..16862db0fa 100644 --- a/codex-rs/tui/src/app/tests/background_task_defaults_tests.rs +++ b/codex-rs/tui/src/app/tests/background_task_defaults_tests.rs @@ -23,7 +23,12 @@ async fn confirm_permission_selection( ServerNotification::ThreadSettingsUpdated(settings), ) .await?; - if !app.pending_server_profiles.contains_key(&thread_id) { + if !app.pending_server_profiles.contains_key(&thread_id) + && !app + .agents_overview + .requested_permission_profiles + .contains_key(&thread_id) + { return Ok(()); } } @@ -659,7 +664,7 @@ async fn command_center_new_preserves_only_selected_server_profiles() -> Result< let home = tempdir()?; std::fs::write( home.path().join("config.toml"), - "default_permissions = \":workspace\"\n[permissions.server-only]\nextends = \":read-only\"\n", + "approval_policy = \"never\"\napprovals_reviewer = \"auto_review\"\ndefault_permissions = \":workspace\"\n[permissions.server-only]\nextends = \":read-only\"\n", )?; let server_config = ConfigBuilder::default() .codex_home(home.path().into()) @@ -729,16 +734,53 @@ async fn command_center_new_preserves_only_selected_server_profiles() -> Result< "type": "message", "role": "assistant", "content": [{"type": "output_text", "text": "saved history"}] }))?]).await?; } - app.select_permission_profile(&mut server, selection.clone()) - .await; + // Omitted custom-profile fields must preserve the preceding accepted choice, + // even before either settings notification reaches the TUI. + app.select_permission_profile( + &mut server, + PermissionProfileSelection { + profile_id: ":read-only".into(), + ..selection.clone() + }, + ) + .await; + // Drain without applying it: confirmation is still pending in the TUI, but + // the server cannot coalesce both updates into one latest snapshot. + let _ = next_thread_settings_updated(&mut server, thread_id).await; + app.select_permission_profile( + &mut server, + PermissionProfileSelection { + approval_policy: None, + approvals_reviewer: None, + ..selection.clone() + }, + ) + .await; let view = app.agents_overview_view(Vec::new(), /*selected_thread_id*/ None); app.chat_widget.show_bottom_pane_view(Box::new(view)); app.new_agents_overview_session(&mut tui, &mut server, /*cwd*/ None) .await?; - assert_eq!(app.chat_widget.thread_id(), Some(thread_id)); - insta::assert_snapshot!( - "command_center_pending_server_permissions", - crate::chatwidget::tests::helpers::render_bottom_popup(&app.chat_widget, /*width*/ 80) + assert_ne!(app.chat_widget.thread_id(), Some(thread_id)); + assert_eq!( + app.chat_widget + .config_ref() + .permissions + .active_permission_profile(), + Some(ActivePermissionProfile { + id: "server-only".into(), + extends: Some(":read-only".into()) + }), + ); + assert_eq!( + ( + app.chat_widget + .config_ref() + .permissions + .approval_policy + .value(), + app.chat_widget.config_ref().approvals_reviewer, + ), + (AskForApproval::OnRequest.to_core(), ApprovalsReviewer::User) ); let settings = next_thread_settings_updated(&mut server, thread_id).await; app.enqueue_thread_notification( @@ -786,6 +828,7 @@ async fn command_center_new_preserves_only_selected_server_profiles() -> Result< .remove(&app.chat_widget.thread_id().unwrap()); app.select_permission_profile(&mut server, selection.clone()) .await; + assert!(!app.reject_pending_permission_change()); } } server.shutdown().await?; @@ -1133,6 +1176,17 @@ async fn command_center_new_checkout_and_worktree_preserve_source_and_default_br app.chat_widget .set_feature_enabled(Feature::GuardianApproval, /*enabled*/ true); Box::pin(app.handle_event(&mut tui, &mut server, AppEvent::OpenPermissionsPopup)).await?; + tokio::time::timeout(Duration::from_secs(3), async { + while let Some(event) = events.recv().await { + let loaded = matches!(&event, AppEvent::PermissionProfilesLoaded { .. }); + Box::pin(app.handle_event(&mut tui, &mut server, event)).await?; + if loaded { + return Ok::<_, color_eyre::Report>(()); + } + } + color_eyre::eyre::bail!("permissions menu event stream closed") + }) + .await??; app.chat_widget.handle_key_event(KeyCode::Up.into()); app.chat_widget.handle_key_event(KeyCode::Enter.into()); while let Ok(event) = events.try_recv() { diff --git a/codex-rs/tui/src/app/tests/permission_selection_tests.rs b/codex-rs/tui/src/app/tests/permission_selection_tests.rs index 2c66fdfe44..d24946fa30 100644 --- a/codex-rs/tui/src/app/tests/permission_selection_tests.rs +++ b/codex-rs/tui/src/app/tests/permission_selection_tests.rs @@ -11,7 +11,6 @@ async fn builtin_permission_selection_adopts_server_settings() -> Result<()> { let thread_id = started.session.thread_id; app.enqueue_primary_thread_session(started.session, started.turns) .await?; - let other = server.start_thread(&app.config).await?.session.thread_id; // A previously selected custom profile may have left a profile-specific proxy cached. let home = tempdir()?; std::fs::write( @@ -65,17 +64,12 @@ proxy_url = "http://127.0.0.1:43128" RuntimePermissionProfileOverride::from_config(app.chat_widget.config_ref()), before, ); - assert!(app.pending_server_profiles.contains_key(&thread_id)); + assert!( + app.agents_overview + .requested_permission_profiles + .contains_key(&thread_id) + ); insta::assert_snapshot!(next_history_message(&mut events).replace(profile_id, ""), @"• Permission selection requested: "); - while events.try_recv().is_ok() {} - let mut tui = crate::tui::test_support::make_test_tui()?; - app.handle_event(&mut tui, &mut server, AppEvent::NewSession { name: None }) - .await?; - assert_eq!(app.chat_widget.thread_id(), Some(thread_id)); - insta::assert_snapshot!(next_history_message(&mut events), @"■ Wait for permissions to update before switching tasks."); - app.select_agents_overview_thread(&mut tui, &mut server, other) - .await?; - assert_eq!(app.chat_widget.thread_id(), Some(thread_id)); let settings = next_thread_settings_updated(&mut server, thread_id).await; app.enqueue_thread_notification( thread_id, @@ -139,3 +133,251 @@ async fn matching_builtin_permission_selection_submits_pending_prompt() -> Resul server.shutdown().await?; Ok(()) } + +#[tokio::test] +async fn custom_permission_selection_uses_server_definition_and_preserves_state_on_rejection() +-> Result<()> { + let (mut app, mut events, _ops) = make_test_app_with_channels().await; + app.config.ephemeral = false; + let home = tempdir()?; + std::fs::write( + home.path().join("config.toml"), + "default_permissions = \":workspace\"\n[permissions.shared]\nextends = \":workspace\"\n", + )?; + let server_config = ConfigBuilder::default() + .codex_home(home.path().into()) + .build() + .await?; + let mut server = crate::start_embedded_app_server_for_picker(&server_config).await?; + let started = server.start_thread(&app.config).await?; + let thread_id = started.session.thread_id; + app.enqueue_primary_thread_session(started.session, started.turns) + .await?; + let client_home = tempdir()?; + std::fs::write( + client_home.path().join("config.toml"), + "default_permissions = \":workspace\"\n[permissions.shared]\nextends = \":danger-full-access\"\n", + )?; + let client_config = ConfigBuilder::default() + .codex_home(client_home.path().into()) + .build() + .await?; + app.config.codex_home = client_config.codex_home; + app.config.custom_permission_profiles = client_config.custom_permission_profiles; + let selection = PermissionProfileSelection { + profile_id: "shared".into(), + approval_policy: Some(AskForApproval::Never), + approvals_reviewer: None, + display_label: "shared".into(), + }; + app.chat_widget.handle_server_notification( + turn_started_notification(thread_id, "turn-1"), + /*replay_kind*/ None, + ); + assert!(app.chat_widget.is_user_turn_pending_or_running()); + let before = RuntimePermissionProfileOverride::from_config(app.chat_widget.config_ref()); + app.select_permission_profile(&mut server, selection.clone()) + .await; + assert_eq!( + RuntimePermissionProfileOverride::from_config(app.chat_widget.config_ref()), + before + ); + let settings = next_thread_settings_updated(&mut server, thread_id).await; + let expected = PermissionProfile::from_legacy_sandbox_policy_for_cwd( + &settings.thread_settings.sandbox_policy.to_core(), + settings.thread_settings.cwd.as_path(), + ); + assert!(matches!( + settings.thread_settings.sandbox_policy, + codex_app_server_protocol::SandboxPolicy::WorkspaceWrite { .. } + )); + app.enqueue_thread_notification( + thread_id, + ServerNotification::ThreadSettingsUpdated(settings), + ) + .await?; + assert_eq!( + ( + app.chat_widget + .config_ref() + .permissions + .permission_profile(), + app.chat_widget + .config_ref() + .permissions + .approval_policy + .value() + ), + (&expected, AskForApproval::Never.to_core()) + ); + std::fs::write( + home.path().join("config.toml"), + "default_permissions = \":workspace\"\n[permissions.shared]\nextends = \":read-only\"\n", + )?; + app.select_permission_profile( + &mut server, + PermissionProfileSelection { + approval_policy: None, + ..selection.clone() + }, + ) + .await; + let settings = next_thread_settings_updated(&mut server, thread_id).await; + assert!(matches!( + settings.thread_settings.sandbox_policy, + codex_app_server_protocol::SandboxPolicy::ReadOnly { .. } + )); + app.enqueue_thread_notification( + thread_id, + ServerNotification::ThreadSettingsUpdated(settings), + ) + .await?; + app.chat_widget.handle_server_notification( + turn_completed_notification(thread_id, "turn-1", TurnStatus::Completed), + /*replay_kind*/ None, + ); + // Unchanged re-selection is accepted without requiring a notification. + app.select_permission_profile(&mut server, selection.clone()) + .await; + assert!(!app.reject_pending_permission_change()); + let confirmed = RuntimePermissionProfileOverride::from_config(app.chat_widget.config_ref()); + let confirmed_config = app.config.clone(); + app.select_permission_profile( + &mut server, + PermissionProfileSelection { + profile_id: "missing-on-server".into(), + ..selection + }, + ) + .await; + assert_eq!( + RuntimePermissionProfileOverride::from_config(app.chat_widget.config_ref()), + confirmed + ); + assert!(!app.pending_server_profiles.contains_key(&thread_id)); + assert_eq!(app.config, confirmed_config); + assert_eq!( + app.agents_overview + .selected_permission_profiles + .get(&thread_id) + .map(String::as_str), + Some("shared") + ); + // Materialize the task so /fork exercises persisted history with the confirmed profile. + server.thread_inject_items(thread_id, vec![serde_json::from_value(serde_json::json!({ + "type": "message", "role": "assistant", "content": [{"type": "output_text", "text": "saved history"}] + }))?]).await?; + let mut tui = crate::tui::test_support::make_test_tui()?; + while events.try_recv().is_ok() {} + app.change_working_directory(&mut tui, &mut server, client_home.path().abs()) + .await; + assert_eq!(app.chat_widget.thread_id(), Some(thread_id)); + insta::assert_snapshot!(next_history_message(&mut events), @"■ Changing directories with an unconfirmed named profile is not supported."); + let side = server + .fork_side_thread( + &app.local_settings, + app.config.clone(), + thread_id, + app.agents_overview + .requested_permission_profiles + .get(&thread_id), + ) + .await?; + assert_eq!( + ( + side.session.permission_profile, + side.session.approval_policy + ), + (PermissionProfile::read_only(), AskForApproval::Never) + ); + app.cli_kv_overrides.push(( + "permissions.shared.extends".into(), + TomlValue::String(":danger-full-access".into()), + )); + // Both the accepted request and its confirmed descendant remain server-owned. + for _ in 0..2 { + let previous_thread_id = app.chat_widget.thread_id(); + app.handle_event( + &mut tui, + &mut server, + AppEvent::ForkCurrentSession { name: None }, + ) + .await?; + assert_ne!(app.chat_widget.thread_id(), previous_thread_id); + assert_eq!( + RuntimePermissionProfileOverride::from_config(app.chat_widget.config_ref()), + confirmed + ); + } + server.shutdown().await?; + Ok(()) +} + +#[tokio::test] +async fn accepted_permissions_survive_directory_change_before_notification() -> Result<()> { + for preserve_history in [false, true] { + let (mut app, _events, _ops) = make_test_app_with_channels().await; + app.config.ephemeral = false; + app.config + .permissions + .set_permission_profile(PermissionProfile::Disabled)?; + let destination = app.config.codex_home.join("destination"); + std::fs::create_dir(&destination)?; + crate::legacy_core::config::set_project_trust_level( + app.config.codex_home.as_path(), + &destination, + codex_protocol::config_types::TrustLevel::Trusted, + ) + .map_err(|err| color_eyre::eyre::eyre!(err.to_string()))?; + let mut server = crate::start_embedded_app_server_for_picker(&app.config).await?; + let started = server.start_thread(&app.config).await?; + let source = started.session.thread_id; + app.enqueue_primary_thread_session(started.session, started.turns) + .await?; + app.runtime_permission_profile_override = + Some(RuntimePermissionProfileOverride::from_config(&app.config)); + if preserve_history { + server.thread_inject_items(source, vec![serde_json::from_value(serde_json::json!({ + "type": "message", "role": "assistant", "content": [{"type": "output_text", "text": "saved history"}] + }))?]).await?; + } + app.select_permission_profile( + &mut server, + PermissionProfileSelection { + profile_id: ":read-only".into(), + approval_policy: Some(AskForApproval::OnRequest), + approvals_reviewer: Some(ApprovalsReviewer::User), + display_label: "Read Only".into(), + }, + ) + .await; + assert!( + app.agents_overview + .requested_permission_profiles + .contains_key(&source), + "selection must be unconfirmed" + ); + // Destination constraints belong to the client; the server's result is authoritative. + let requirements = app.config.codex_home.join("client-requirements.toml"); + std::fs::write(&requirements, "allowed_approval_policies = [\"never\"]")?; + app.loader_overrides.system_requirements_path = Some(requirements.to_path_buf()); + let mut tui = crate::tui::test_support::make_test_tui()?; + app.change_working_directory(&mut tui, &mut server, destination.clone()) + .await; + assert_ne!(app.chat_widget.thread_id(), Some(source)); + assert_eq!( + (&app.config.cwd, app.config.permissions.permission_profile()), + (&destination, &PermissionProfile::read_only()) + ); + assert_eq!( + app.config.permissions.approval_policy.value(), + AskForApproval::OnRequest.to_core() + ); + assert_eq!( + app.runtime_permission_profile_override, + Some(RuntimePermissionProfileOverride::from_config(&app.config)) + ); + server.shutdown().await?; + } + Ok(()) +} diff --git a/codex-rs/tui/src/app/tests/permission_shortcuts_tests.rs b/codex-rs/tui/src/app/tests/permission_shortcuts_tests.rs index 111ff663b6..7076d8997b 100644 --- a/codex-rs/tui/src/app/tests/permission_shortcuts_tests.rs +++ b/codex-rs/tui/src/app/tests/permission_shortcuts_tests.rs @@ -105,7 +105,11 @@ async fn permission_shortcut_confirms_without_persisting() -> Result<()> { RuntimePermissionProfileOverride::from_config(app.chat_widget.config_ref()), before ); - assert!(app.pending_server_profiles.contains_key(&thread_id)); + assert!( + app.agents_overview + .requested_permission_profiles + .contains_key(&thread_id) + ); insta::assert_snapshot!( next_history_message(&mut events), @"• Permission selection requested: Read Only" diff --git a/codex-rs/tui/src/app/tests/safety_buffering.rs b/codex-rs/tui/src/app/tests/safety_buffering.rs index ca516eda0f..f366341c79 100644 --- a/codex-rs/tui/src/app/tests/safety_buffering.rs +++ b/codex-rs/tui/src/app/tests/safety_buffering.rs @@ -40,6 +40,7 @@ const SAFETY_RETRY_THREAD_NAME: &str = "Safety retry source"; enum SafetyRetryScenario { Once, RetryTwice, + PendingPermissions, InterruptedPrevious, UnsupportedPermissions, } @@ -569,6 +570,16 @@ goals = true .enable(Feature::Goals) .expect("test config should allow goals"); + if scenario == SafetyRetryScenario::PendingPermissions { + app.config + .permissions + .set_permission_profile_from_session_snapshot( + PermissionProfileSnapshot::from_session_snapshot( + PermissionProfile::Disabled, + Some(ActivePermissionProfile::new(":danger-full-access")), + ), + )?; + } let mut tui = crate::tui::test_support::make_test_tui()?; let mut app_server = Box::pin(crate::start_embedded_app_server_for_picker(&app.config)).await?; let mut started = app_server.start_thread(&app.config).await?; @@ -783,6 +794,22 @@ goals = true } } + if scenario == SafetyRetryScenario::PendingPermissions { + app.runtime_permission_profile_override = Some( + RuntimePermissionProfileOverride::from_config(app.chat_widget.config_ref()), + ); + app.select_permission_profile( + &mut app_server, + PermissionProfileSelection { + profile_id: ":read-only".into(), + approval_policy: Some(AskForApproval::OnRequest), + approvals_reviewer: Some(ApprovalsReviewer::User), + display_label: "Read Only".into(), + }, + ) + .await; + } + Box::pin(app.retry_safety_buffered_turn( &mut tui, &mut app_server, @@ -923,6 +950,15 @@ goals = true let retry_thread_id = app.chat_widget.thread_id().expect("retry thread id"); // Capture the completed retry before processing its automatic goal continuation. wait_for_turn_completed(&mut app, &mut app_server, retry_thread_id).await; + if scenario == SafetyRetryScenario::PendingPermissions { + assert_eq!( + app.chat_widget + .config_ref() + .permissions + .permission_profile(), + &PermissionProfile::read_only() + ); + } let mut replayed_history = String::new(); while let Ok(event) = app_event_rx.try_recv() { if let AppEvent::InsertHistoryCell(cell) = event { @@ -1165,3 +1201,14 @@ async fn safety_retry_rejects_unsupported_permissions_before_interrupting() -> R ) .await } + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn safety_retry_preserves_unconfirmed_permissions_on_its_first_turn() -> Result<()> { + run_safety_retry( + /*previous_prompt*/ None, + /*failing_draft*/ None, + /*committed_steer*/ None, + SafetyRetryScenario::PendingPermissions, + ) + .await +} diff --git a/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs b/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs index bb74f6c17c..4c788dc0f7 100644 --- a/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs +++ b/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs @@ -725,7 +725,12 @@ async fn delete_current_thread_navigates_only_after_success() -> Result<()> { let mut side_config = app.config.clone(); side_config.ephemeral = true; let side = server - .fork_side_thread(&app.local_settings, side_config.clone(), thread_id) + .fork_side_thread( + &app.local_settings, + side_config.clone(), + thread_id, + /*selected_profile*/ None, + ) .await?; let side_id = side.session.thread_id; app.side_threads @@ -735,7 +740,12 @@ async fn delete_current_thread_navigates_only_after_success() -> Result<()> { side_config.cwd = side_config.cwd.join("failure"); assert!( server - .fork_side_thread(&app.local_settings, side_config, thread_id) + .fork_side_thread( + &app.local_settings, + side_config, + thread_id, + /*selected_profile*/ None + ) .await .is_err() ); @@ -1013,7 +1023,12 @@ async fn archive_current_thread_returns_shared_servers_to_agents() -> Result<()> let mut side_config = app.config.clone(); side_config.ephemeral = true; let side = server - .fork_side_thread(&app.local_settings, side_config, thread_id) + .fork_side_thread( + &app.local_settings, + side_config, + thread_id, + /*selected_profile*/ None, + ) .await?; let side_id = side.session.thread_id; app.side_threads @@ -2822,6 +2837,7 @@ async fn paginated_workflows_never_request_full_thread_history() -> Result<()> { &crate::local_settings::LocalSettings::from(&side_config), side_config, paginated_thread_id, + /*selected_profile*/ None, )) .await?; diff --git a/codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__background_task_defaults_tests__command_center_pending_server_permissions.snap b/codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__background_task_defaults_tests__command_center_pending_server_permissions.snap deleted file mode 100644 index bd7838ae7c..0000000000 --- a/codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__background_task_defaults_tests__command_center_pending_server_permissions.snap +++ /dev/null @@ -1,11 +0,0 @@ ---- -source: tui/src/app/tests/background_task_defaults_tests.rs -expression: "crate::chatwidget::tests::helpers::render_bottom_popup(&app.chat_widget, 80)" ---- - Unable to complete action - Wait for permissions to update before starting a session. - - -› 1. Return to command center - - enter select · esc back diff --git a/codex-rs/tui/src/app/thread_routing.rs b/codex-rs/tui/src/app/thread_routing.rs index 341eaa057c..188d971ed5 100644 --- a/codex-rs/tui/src/app/thread_routing.rs +++ b/codex-rs/tui/src/app/thread_routing.rs @@ -827,7 +827,12 @@ impl App { } if should_start_turn { let config = self.chat_widget.config_ref(); - let selected_profile = self.pending_server_profiles.get(&thread_id); + let selected_profile = + self.pending_server_profiles.get(&thread_id).or_else(|| { + self.agents_overview + .requested_permission_profiles + .get(&thread_id) + }); let selected_active = selected_profile .map(|profile| ActivePermissionProfile::new(profile.profile_id.clone())); let (turn_approval_policy, turn_approvals_reviewer) = @@ -1196,6 +1201,11 @@ impl App { if self .pending_server_profiles .get(&thread_id) + .or_else(|| { + self.agents_overview + .requested_permission_profiles + .get(&thread_id) + }) .is_some_and(|selected| { notification .thread_settings @@ -1210,7 +1220,11 @@ impl App { }) }) { - confirmed_profile = self.pending_server_profiles.remove(&thread_id); + confirmed_profile = self.pending_server_profiles.remove(&thread_id).or_else(|| { + self.agents_overview + .requested_permission_profiles + .remove(&thread_id) + }); } } let inferred_session = if let ServerNotification::ThreadStarted(started) = ¬ification @@ -1298,41 +1312,9 @@ impl App { .on_thread_settings_updated(settings.clone()); notification = None; } - if let Some(selected) = confirmed_profile { + if confirmed_profile.is_some() { if self.chat_widget.thread_id() == Some(thread_id) { - if selected.approvals_reviewer.is_some() { - self.runtime_approvals_reviewer_override = None; - } - self.runtime_permission_profile_override = None; - } - if self.chat_widget.thread_id() == Some(thread_id) - && let Some(profile) = self - .chat_widget - .config_ref() - .permissions - .active_permission_profile() - && profile.id.starts_with(':') - { - let config = self.chat_widget.config_ref(); - let network = config - .network_proxy_spec_for_active_permission_profile( - &profile, - config.permissions.permission_profile(), - ) - .unwrap_or_else(|err| { - tracing::warn!(%err, "failed to refresh local permission network settings"); - None - }); - self.chat_widget.set_permission_network(network); - self.config.permissions = self.chat_widget.config_ref().permissions.clone(); - self.config.approvals_reviewer = self.chat_widget.config_ref().approvals_reviewer; - self.runtime_approval_policy_override = - Some(RuntimeApprovalPolicyOverride::Explicit( - self.config.permissions.approval_policy.value().into(), - )); - self.runtime_approvals_reviewer_override = Some(self.config.approvals_reviewer); - self.runtime_permission_profile_override = - Some(RuntimePermissionProfileOverride::from_config(&self.config)); + self.adopt_server_permissions(); } self.app_event_tx.send(AppEvent::SettingsSelectionSettled); } @@ -1574,6 +1556,9 @@ impl App { let thread_id = session.thread_id; self.pending_server_profiles.remove(&thread_id); + self.agents_overview + .requested_permission_profiles + .remove(&thread_id); if self.primary_thread_id != Some(thread_id) { self.recap.reset_for_new_thread(Instant::now()); } diff --git a/codex-rs/tui/src/app/thread_settings.rs b/codex-rs/tui/src/app/thread_settings.rs index 2a3eae013f..66c7b4e64e 100644 --- a/codex-rs/tui/src/app/thread_settings.rs +++ b/codex-rs/tui/src/app/thread_settings.rs @@ -218,19 +218,29 @@ impl App { if !thread_settings_update_has_changes(¶ms) { return false; } - if ThreadId::from_string(¶ms.thread_id) - .ok() - .is_some_and(|thread_id| self.pending_server_profiles.contains_key(&thread_id)) - && (params.cwd.is_some() - || params.approval_policy.is_some() - || params.approvals_reviewer.is_some() - || params.sandbox_policy.is_some() - || params.permissions.is_some()) + let thread_id = ThreadId::from_string(¶ms.thread_id).ok(); + let changes_permissions = params.cwd.is_some() + || params.approval_policy.is_some() + || params.approvals_reviewer.is_some() + || params.sandbox_policy.is_some() + || params.permissions.is_some(); + if changes_permissions + && thread_id.is_some_and(|id| self.pending_server_profiles.contains_key(&id)) { return false; } match app_server.thread_settings_update(params).await { - Ok(settings_updated) => settings_updated, + Ok(settings_updated) => { + if settings_updated + && changes_permissions + && let Some(thread_id) = thread_id + { + self.agents_overview + .requested_permission_profiles + .remove(&thread_id); + } + settings_updated + } Err(err) => { tracing::warn!("failed to update app-server thread settings from TUI: {err}"); self.chat_widget diff --git a/codex-rs/tui/src/app/working_directory.rs b/codex-rs/tui/src/app/working_directory.rs index 76187b34a3..db28e72c92 100644 --- a/codex-rs/tui/src/app/working_directory.rs +++ b/codex-rs/tui/src/app/working_directory.rs @@ -22,6 +22,7 @@ pub(super) struct ManagedWorktreeAttach { keymap: RuntimeKeymap, cwd: AbsolutePathBuf, name_error: Option, + selected_profile: Option, } /// A /cd request awaiting a fresh event-loop iteration. @@ -177,6 +178,19 @@ impl App { "Changing directories with a named profile is not supported.", ); } + let selected_profile = self + .agents_overview + .requested_permission_profiles + .get(&thread_id) + .cloned(); + if selected_profile + .as_ref() + .is_some_and(|profile| !profile.profile_id.starts_with(':')) + { + return self.working_directory_error( + "Changing directories with an unconfirmed named profile is not supported.", + ); + } let cells = &self.transcript_cells; if cells.iter().any(|cell| cell.as_any().is::()) { return self.working_directory_error("MCP inventory is still loading."); @@ -330,6 +344,7 @@ impl App { let profile = self .runtime_permission_profile_override .as_ref() + .filter(|_| selected_profile.is_none()) .filter(|profile| { scope == RuntimePolicyOverrideScope::All || profile.turn_override == RuntimePermissionProfileTurnOverride::LegacySandbox @@ -356,12 +371,15 @@ impl App { { return self.working_directory_error("Permission profile cannot be preserved by /cd."); } - if let Err(error) = self.apply_runtime_policy_overrides(&mut config, scope) { + if selected_profile.is_none() + && let Err(error) = self.apply_runtime_policy_overrides(&mut config, scope) + { return self.working_directory_error(format!("{error:#}")); } let actual = config.permissions.approval_policy.value(); let approval = self .runtime_approval_policy_override + .filter(|_| selected_profile.is_none()) .filter(|value| { scope == RuntimePolicyOverrideScope::All || matches!(value, RuntimeApprovalPolicyOverride::Explicit(_)) @@ -436,7 +454,7 @@ impl App { /*last_turn_id*/ None, /*before_turn_id*/ None, DeferUntilNextTurn, - /*selected_profile*/ None, + selected_profile.as_ref(), ) .await } else { @@ -446,7 +464,7 @@ impl App { &config, /*session_start_source*/ None, /*remote_cwd_override*/ None, - /*selected_profile*/ None, + selected_profile.as_ref(), ) .await }; @@ -455,12 +473,27 @@ impl App { Err(e) => return self.working_directory_error(format!("Failed to change: {e}")), }; let session = &transitioned.session; + let permissions_match = if let Some(selected) = &selected_profile { + session + .active_permission_profile + .as_ref() + .is_some_and(|active| active.id == selected.profile_id) + && selected + .approval_policy + .is_none_or(|policy| session.approval_policy == policy) + && selected + .approvals_reviewer + .is_none_or(|reviewer| session.approvals_reviewer == reviewer) + } else { + session.approval_policy.to_core() == config.permissions.approval_policy.value() + && session.approvals_reviewer == config.approvals_reviewer + && session.active_permission_profile + == config.permissions.active_permission_profile() + }; if session.thread_id == thread_id || crate::session_resume::cwds_differ(session.cwd.as_path(), cwd.as_path()) || session.runtime_workspace_roots != config.workspace_roots - || session.approval_policy.to_core() != config.permissions.approval_policy.value() - || session.approvals_reviewer != config.approvals_reviewer - || session.active_permission_profile != config.permissions.active_permission_profile() + || !permissions_match { if session.thread_id != thread_id { let _ = app_server.thread_unsubscribe(session.thread_id).await; @@ -520,6 +553,7 @@ impl App { keymap, cwd, name_error, + selected_profile, }; if managed_worktree.is_some() { // Let the large synchronous ChatWidget constructor run on a fresh event-loop stack. @@ -544,6 +578,7 @@ impl App { keymap, cwd, name_error, + selected_profile, } = attach; self.local_settings = local_settings; self.refresh_server_version_overview_notice(CODEX_CLI_VERSION); @@ -560,6 +595,9 @@ impl App { if let Err(error) = attach_widget(self, tui, started, lineage, message).await { return self.working_directory_error(format!("Could not restore session: {error}")); } + if selected_profile.is_some() { + self.adopt_inherited_server_selection(); + } if matches!( self.runtime_approval_policy_override, Some(RuntimeApprovalPolicyOverride::Restored(_)) diff --git a/codex-rs/tui/src/app_server_session.rs b/codex-rs/tui/src/app_server_session.rs index f45e2b1264..f9c6f5fd77 100644 --- a/codex-rs/tui/src/app_server_session.rs +++ b/codex-rs/tui/src/app_server_session.rs @@ -793,6 +793,13 @@ impl AppServerSession { if history_support == ThreadHistorySupport::LegacyOnly { self.history_support = ThreadHistorySupport::LegacyOnly; } + // An explicit server selection must not reuse the client's local display policy. + let selected_permissions = selected_profile.map(|_| { + PermissionProfile::from_legacy_sandbox_policy_for_cwd( + &response.sandbox.to_core(), + response.cwd.as_path(), + ) + }); let mut started = started_thread_from_start_response( response, local_settings, @@ -800,6 +807,9 @@ impl AppServerSession { self.thread_params_mode(), ) .await?; + if let Some(permissions) = selected_permissions { + started.session.permission_profile = permissions; + } started.task_tools_available = task_tools_available; if task_tools_available { self.remember_task_tool_thread(started.session.thread_id); @@ -879,6 +889,7 @@ impl AppServerSession { local_settings: &LocalSettings, config: Config, thread_id: ThreadId, + selected_profile: Option<&PermissionProfileSelection>, ) -> Result { self.fork_thread_at_with_presentation( local_settings, @@ -888,7 +899,7 @@ impl AppServerSession { /*before_turn_id*/ None, ForkGoalContinuation::StartIfIdle, ForkPresentation::SideConversation, - /*selected_profile*/ None, + selected_profile, ForkPermissionMode::InheritSaved, ForkConfigSource::Session, ) @@ -957,6 +968,7 @@ impl AppServerSession { } if self.thread_params_mode() == ThreadParamsMode::Remote && permission_mode == ForkPermissionMode::InheritSaved + && selected_profile.is_none() { params.approval_policy = None; params.approvals_reviewer = None; @@ -1022,6 +1034,13 @@ impl AppServerSession { "preserving the created fork after bounded history hydration failed" ); } + // Explicit selections use the server's effective policy, including on local daemons. + let selected_permissions = selected_profile.map(|_| { + PermissionProfile::from_legacy_sandbox_policy_for_cwd( + &response.sandbox.to_core(), + response.cwd.as_path(), + ) + }); let mut started = started_thread_from_fork_response( response, local_settings, @@ -1029,6 +1048,9 @@ impl AppServerSession { self.thread_params_mode(), ) .await?; + if let Some(permissions) = selected_permissions { + started.session.permission_profile = permissions; + } started.session.fork_parent_title = fork_parent.and_then(|thread| thread.name); if self.task_tools_available(thread_id) { started.task_tools_available = true; @@ -3660,6 +3682,7 @@ mod tests { &LocalSettings::from(&ephemeral_config), ephemeral_config, source_thread_id, + /*selected_profile*/ None, ) .await?; @@ -3786,6 +3809,7 @@ mod tests { &LocalSettings::from(&side_config), side_config, source_thread_id, + /*selected_profile*/ None, ) .await?; @@ -3917,7 +3941,12 @@ mod tests { .fork_thread(&LocalSettings::from(&config), config.clone(), thread_id) .await?; let side = app_server - .fork_side_thread(&LocalSettings::from(&config), config, thread_id) + .fork_side_thread( + &LocalSettings::from(&config), + config, + thread_id, + /*selected_profile*/ None, + ) .await?; assert_eq!(regular.turns.len(), 1); diff --git a/codex-rs/tui/src/app_server_session/rollout_history_tests.rs b/codex-rs/tui/src/app_server_session/rollout_history_tests.rs index 0a6cd18c92..ecce710de8 100644 --- a/codex-rs/tui/src/app_server_session/rollout_history_tests.rs +++ b/codex-rs/tui/src/app_server_session/rollout_history_tests.rs @@ -130,13 +130,7 @@ async fn remote_resume_restores_saved_server_profile_without_permission_override ), ); } - // A locally remembered profile may have been removed since selection. - let stale_selection = crate::app_event::PermissionProfileSelection { - profile_id: "removed-profile".into(), - approval_policy: None, - approvals_reviewer: None, - display_label: "removed-profile".into(), - }; + // With no new selection, remote forks retain the saved profile. let forked = server .fork_thread_at( &local_settings, @@ -145,7 +139,7 @@ async fn remote_resume_restores_saved_server_profile_without_permission_override /*last_turn_id*/ None, /*before_turn_id*/ None, ForkGoalContinuation::StartIfIdle, - Some(&stale_selection), + /*selected_profile*/ None, ) .await?; assert_eq!( @@ -165,7 +159,12 @@ async fn remote_resume_restores_saved_server_profile_without_permission_override codex_protocol::config_types::ApprovalsReviewer::AutoReview ); let side = server - .fork_side_thread(&local_settings, client_config, thread_id) + .fork_side_thread( + &local_settings, + client_config, + thread_id, + /*selected_profile*/ None, + ) .await?; assert_eq!( side.session.active_permission_profile.unwrap().id, diff --git a/codex-rs/tui/src/app_server_session/workspace_roots_tests.rs b/codex-rs/tui/src/app_server_session/workspace_roots_tests.rs index ee2dcef069..644a0a3f57 100644 --- a/codex-rs/tui/src/app_server_session/workspace_roots_tests.rs +++ b/codex-rs/tui/src/app_server_session/workspace_roots_tests.rs @@ -150,7 +150,12 @@ stream_max_retries = 0 .await?; assert_eq!(forked.session.runtime_workspace_roots, expected_roots); let side = app_server - .fork_side_thread(&local_settings, session_config, thread_id) + .fork_side_thread( + &local_settings, + session_config, + thread_id, + /*selected_profile*/ None, + ) .await?; assert_eq!(side.session.runtime_workspace_roots, expected_roots); diff --git a/codex-rs/tui/src/chatwidget/permission_discovery.rs b/codex-rs/tui/src/chatwidget/permission_discovery.rs index 09f7f418d5..42ee1b5fa1 100644 --- a/codex-rs/tui/src/chatwidget/permission_discovery.rs +++ b/codex-rs/tui/src/chatwidget/permission_discovery.rs @@ -56,9 +56,6 @@ impl ChatWidget { 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); diff --git a/codex-rs/tui/src/chatwidget/permission_popups.rs b/codex-rs/tui/src/chatwidget/permission_popups.rs index fc98703f7e..6b4ee93329 100644 --- a/codex-rs/tui/src/chatwidget/permission_popups.rs +++ b/codex-rs/tui/src/chatwidget/permission_popups.rs @@ -17,7 +17,8 @@ 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 + if self.thread_id.is_some() + || self.config.explicit_permission_profile_mode || self.permission_profiles_menu_opened || self .config diff --git a/codex-rs/tui/src/chatwidget/permissions_menu.rs b/codex-rs/tui/src/chatwidget/permissions_menu.rs index ed653846f0..5ebd1dbf7c 100644 --- a/codex-rs/tui/src/chatwidget/permissions_menu.rs +++ b/codex-rs/tui/src/chatwidget/permissions_menu.rs @@ -215,8 +215,17 @@ impl ChatWidget { SelectionItem { name: label.to_string(), description: Some(description), - is_current: active_profile_id.as_deref() == Some(id) - && current_approval == approval_policy + is_current: active_profile_id.as_deref().map_or_else( + || { + Self::preset_matches_current( + current_approval, + self.config.permissions.permission_profile(), + self.config.cwd.as_path(), + preset, + ) + }, + |active| active == id, + ) && current_approval == approval_policy && current_reviewer == approvals_reviewer, actions: self.permission_mode_actions( preset, @@ -226,9 +235,11 @@ impl ChatWidget { /*return_to_permissions*/ true, ), dismiss_on_select: true, - disabled_reason: discovery - .disabled_reason(id, Some(approval_policy), Some(approvals_reviewer.into())) - .or_else(|| self.permission_mode_disabled_reason(preset, approval_policy)), + disabled_reason: discovery.disabled_reason( + id, + Some(approval_policy), + Some(approvals_reviewer.into()), + ), ..Default::default() } } diff --git a/codex-rs/tui/src/chatwidget/settings.rs b/codex-rs/tui/src/chatwidget/settings.rs index a9c2d1932d..5e2e21f328 100644 --- a/codex-rs/tui/src/chatwidget/settings.rs +++ b/codex-rs/tui/src/chatwidget/settings.rs @@ -31,6 +31,7 @@ impl ChatWidget { Ok(()) } + #[cfg(test)] pub(crate) fn set_permission_profile_with_active_profile( &mut self, profile: PermissionProfile, @@ -449,7 +450,16 @@ impl ChatWidget { self.apply_thread_settings_cwd(settings.cwd.clone()); self.config.model_provider_id = settings.model_provider.clone(); self.set_service_tier(settings.service_tier.clone()); - self.set_approval_policy(settings.approval_policy); + if let Err(err) = self + .config + .permissions + .approval_policy + .set(settings.approval_policy.to_core()) + { + tracing::warn!(%err, "failed to sync approval_policy from ThreadSettingsUpdated"); + self.config.permissions.approval_policy = + Constrained::allow_only(settings.approval_policy.to_core()); + } self.set_approvals_reviewer(settings.approvals_reviewer.to_core()); self.config.personality = settings.personality; diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__permissions_unnamed_server_profile.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__permissions_unnamed_server_profile.snap new file mode 100644 index 0000000000..3d46671836 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__permissions_unnamed_server_profile.snap @@ -0,0 +1,15 @@ +--- +source: tui/src/chatwidget/tests/permissions.rs +expression: popup +--- + Update Model Permissions + + + 1. Ask for approval Read and edit workspace files and run commands, with approval required for internet + access or edits outside the workspace +› 2. Approve for me (current) Only ask for actions detected as potentially unsafe + 3. Full Access Use with caution: Codex can edit files outside this workspace and access the internet + without approval + 4. Read Only Read workspace files, with approval required for edits or internet access + + enter select · esc back diff --git a/codex-rs/tui/src/chatwidget/tests/app_server.rs b/codex-rs/tui/src/chatwidget/tests/app_server.rs index e03e997d85..b19aa7b8c5 100644 --- a/codex-rs/tui/src/chatwidget/tests/app_server.rs +++ b/codex-rs/tui/src/chatwidget/tests/app_server.rs @@ -625,6 +625,8 @@ async fn thread_settings_updated_updates_visible_state_without_transcript() { let mut session = configured_thread_session(thread_id); session.cwd = test_path_buf("/tmp/original-workspace").abs(); chat.handle_thread_session(session); + chat.config.permissions.approval_policy = + Constrained::allow_only(AskForApproval::Never.to_core()); let previous_generation = chat.connector_scope_generation(); let old_app = serde_json::from_str(r#"{"id":"old","name":"Old","isAccessible":true}"#) .expect("valid app"); diff --git a/codex-rs/tui/src/chatwidget/tests/permissions.rs b/codex-rs/tui/src/chatwidget/tests/permissions.rs index b20599a43d..bc60719673 100644 --- a/codex-rs/tui/src/chatwidget/tests/permissions.rs +++ b/codex-rs/tui/src/chatwidget/tests/permissions.rs @@ -60,7 +60,13 @@ async fn permission_discovery_uses_server_catalog_for_remote_custom_selection() })) .unwrap(), ); - chat.request_permission_profiles(); + // Conflicting local availability must not disable server-provided choices. + chat.config.config_layer_stack = requirements_stack(codex_config::ConfigRequirementsToml { + allowed_sandbox_modes: Some(vec![codex_config::SandboxModeRequirement::ReadOnly]), + ..Default::default() + }); + chat.thread_id = Some(ThreadId::new()); + chat.open_permissions_popup(); let request_id = chat.permission_popup_request_id.unwrap(); rx.try_recv().unwrap(); assert_chatwidget_snapshot!( @@ -79,16 +85,6 @@ async fn permission_discovery_uses_server_catalog_for_remote_custom_selection() AppEvent::SelectPermissionProfile(PermissionProfileSelection { profile_id, .. }) if profile_id == "server-only" )); - 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] @@ -265,7 +261,14 @@ async fn profile_permissions_selection_popup_with_disallowed_full_access_snapsho ..Default::default() }); - chat.open_permission_profiles_popup(Discovery::local(&chat.config)); + let mut discovery = Discovery::local(&chat.config); + discovery + .profiles + .iter_mut() + .find(|profile| profile.id == ":danger-full-access") + .unwrap() + .allowed = false; + chat.open_permission_profiles_popup(discovery); assert_chatwidget_snapshot!( "profile_permissions_selection_popup_with_disallowed_full_access", @@ -1234,8 +1237,13 @@ async fn permissions_selection_marks_auto_review_current_after_session_configure }); chat.open_permissions_popup(); + chat.on_permission_profiles_loaded( + chat.permission_popup_request_id.unwrap(), + Ok(Discovery::local(&chat.config)), + ); let popup = render_bottom_popup(&chat, /*width*/ 120); + assert_chatwidget_snapshot!("permissions_unnamed_server_profile", popup); assert!( popup.contains("Approve for me (current)"), "expected Approve for me to be current after SessionConfigured sync: {popup}" @@ -1283,6 +1291,10 @@ async fn permissions_selection_marks_auto_review_current_with_custom_workspace_w }); chat.open_permissions_popup(); + chat.on_permission_profiles_loaded( + chat.permission_popup_request_id.unwrap(), + Ok(Discovery::local(&chat.config)), + ); let popup = render_bottom_popup(&chat, /*width*/ 120); assert!( diff --git a/codex-rs/tui/src/permission_discovery.rs b/codex-rs/tui/src/permission_discovery.rs index c40ada989c..2579fc6e9b 100644 --- a/codex-rs/tui/src/permission_discovery.rs +++ b/codex-rs/tui/src/permission_discovery.rs @@ -1,5 +1,5 @@ //! Bounded discovery for permission pickers; fixed request IDs bound abandoned RPCs. -//! This never applies a profile; the app chooses local or server-owned selection. +//! Discovery and availability come from the connected server; selection uses its settings API. use crate::app_event::AppEvent; use crate::app_event_sender::AppEventSender; @@ -10,14 +10,13 @@ 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; +#[cfg(test)] use codex_utils_approval_presets::builtin_approval_presets; use std::collections::HashSet; use std::time::Duration; @@ -27,10 +26,10 @@ use uuid::Uuid; pub(crate) struct PermissionDiscovery { pub(crate) profiles: Vec, pub(crate) requirements: Option, - pub(crate) explicit_profile_mode: bool, } impl PermissionDiscovery { + #[cfg(test)] pub(crate) fn local(config: &Config) -> Self { let mut profiles = builtin_approval_presets() .into_iter() @@ -50,7 +49,6 @@ impl PermissionDiscovery { Self { profiles, requirements: None, - explicit_profile_mode: true, } } @@ -96,47 +94,8 @@ pub(crate) fn fetch( 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, - explicit_profile_mode: false, - }); - } - } let requirements: ConfigRequirementsReadResponse = handle .request_typed(ClientRequest::ConfigRequirementsRead { request_id: RequestId::String("tui-permission-requirements".to_string()), @@ -174,7 +133,6 @@ pub(crate) fn fetch( return Ok(PermissionDiscovery { profiles, requirements: requirements.requirements, - explicit_profile_mode: true, }); }; if !cursors.insert(next.clone()) { diff --git a/codex-rs/tui/src/permission_discovery_tests.rs b/codex-rs/tui/src/permission_discovery_tests.rs index 27f29bf0df..f40cbb2390 100644 --- a/codex-rs/tui/src/permission_discovery_tests.rs +++ b/codex-rs/tui/src/permission_discovery_tests.rs @@ -76,14 +76,7 @@ async fn discovery_preserves_config_scope_and_bounds_server_requests() { 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" { + if case == "timeout" { replies = vec![Value::Null]; } let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); @@ -151,15 +144,7 @@ async fn discovery_preserves_config_scope_and_bounds_server_requests() { 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") { + if case == "empty" { assert!(discovery.profiles.is_empty()); } else { assert_eq!(serde_json::to_value(&discovery.profiles).unwrap(), data); @@ -172,9 +157,8 @@ async fn discovery_preserves_config_scope_and_bounds_server_requests() { 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" => { + "timeout" => assert_eq!(requests.len(), 1), + "local" | "session-only" | "remote" | "remote-default" | "thread" | "legacy" => { let pages: Vec<_> = requests .iter() .filter(|request| request["method"] == "permissionProfile/list") @@ -187,9 +171,6 @@ async fn discovery_preserves_config_scope_and_bounds_server_requests() { json!({"cwd": cwd, "limit": 100, "cursor": "second"}) ] ); - if mode == ThreadParamsMode::Remote { - assert_eq!(requests[0]["params"]["cwd"], json!(cwd)); - } } _ => {} }