diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 1462597048..e310a45f8b 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -131,6 +131,7 @@ use codex_app_server_protocol::SkillsListResponse; use codex_app_server_protocol::ThreadItem; use codex_app_server_protocol::ThreadLoadedListParams; use codex_app_server_protocol::ThreadMemoryMode; +use codex_app_server_protocol::ThreadSettingsUpdateParams; use codex_app_server_protocol::ThreadStartSource; use codex_app_server_protocol::Turn; use codex_app_server_protocol::TurnError as AppServerTurnError; @@ -559,6 +560,8 @@ pub(crate) struct App { cloud_config_bundle: CloudConfigBundleLoader, runtime_approval_policy_override: Option, runtime_permission_profile_override: Option, + /// In-flight remote selections; confirmed settings live in each task's server snapshot. + pending_server_profiles: HashMap, pub(crate) file_search: FileSearchManager, diff --git a/codex-rs/tui/src/app/agents_overview.rs b/codex-rs/tui/src/app/agents_overview.rs index 6c423b978a..9962185b9c 100644 --- a/codex-rs/tui/src/app/agents_overview.rs +++ b/codex-rs/tui/src/app/agents_overview.rs @@ -303,7 +303,6 @@ impl App { { return Ok(AppRunControl::Continue); } - if self.primary_thread_id != Some(root_thread_id) { let previous_displayed_thread_id = self.current_displayed_thread_id(); let mut previous_thread_ids = @@ -725,6 +724,7 @@ impl App { &config, /*session_start_source*/ None, remote_cwd.as_deref(), + /*selected_profile*/ None, ) .await { diff --git a/codex-rs/tui/src/app/config_persistence.rs b/codex-rs/tui/src/app/config_persistence.rs index 10f9209687..878fb739d9 100644 --- a/codex-rs/tui/src/app/config_persistence.rs +++ b/codex-rs/tui/src/app/config_persistence.rs @@ -152,6 +152,9 @@ impl App { &mut self, selection: PermissionProfileSelection, ) -> bool { + if self.reject_pending_permission_change() { + return false; + } let PermissionProfileSelection { profile_id, approval_policy, @@ -266,6 +269,135 @@ impl App { true } + pub(super) async fn select_permission_profile( + &mut self, + app_server: &mut AppServerSession, + selection: PermissionProfileSelection, + ) { + if self.reject_pending_permission_change() { + return; + } + if 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(); + self.chat_widget.add_error_message( + "Wait for the task to connect before selecting permissions.".into(), + ); + return; + }; + if 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) + && selection + .approval_policy + .is_none_or(|policy| config.permissions.approval_policy.value() == policy.to_core()) + && selection + .approvals_reviewer + .is_none_or(|reviewer| config.approvals_reviewer == reviewer) + { + return; + } + let params = ThreadSettingsUpdateParams { + thread_id: thread_id.to_string(), + permissions: Some(selection.profile_id.clone()), + approval_policy: selection.approval_policy, + approvals_reviewer: selection.approvals_reviewer.map(Into::into), + ..Default::default() + }; + match app_server.thread_settings_update(params).await { + Ok(true) => { + self.pending_server_profiles + .insert(thread_id, selection.clone()); + self.chat_widget.add_info_message( + format!( + "Permission selection requested: {}", + selection.display_label + ), + /*hint*/ None, + ); + self.chat_widget.submit_initial_user_message_if_pending(); + } + Ok(false) => { + self.chat_widget + .retain_input_after_failed_permission_selection(); + self.chat_widget + .add_error_message("Named profiles require a newer app server.".into()); + } + Err(error) => { + self.chat_widget + .retain_input_after_failed_permission_selection(); + self.chat_widget + .add_error_message(format!("Failed to select permissions: {error}")); + } + } + } + + pub(super) fn reject_pending_permission_change(&mut self) -> bool { + if self + .chat_widget + .thread_id() + .is_some_and(|thread_id| self.pending_server_profiles.contains_key(&thread_id)) + { + self.chat_widget.add_error_message( + "Wait for permissions to update before changing permissions.".into(), + ); + return true; + } + false + } + + pub(super) fn confirmed_server_profile( + &self, + thread_id: ThreadId, + ) -> Option { + if self.chat_widget.thread_id() != Some(thread_id) { + 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 + .config + .custom_permission_profiles + .iter() + .any(|profile| profile.id == active.id)) + { + return None; + } + Some(PermissionProfileSelection { + profile_id: active.id.clone(), + approval_policy: Some(config.permissions.approval_policy.value().into()), + approvals_reviewer: Some(config.approvals_reviewer), + display_label: active.id, + }) + } + 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()) @@ -463,6 +595,13 @@ impl App { if updates.is_empty() { return; } + if updates + .iter() + .any(|(feature, _)| *feature == Feature::GuardianApproval) + && self.reject_pending_permission_change() + { + return; + } let auto_review_preset = auto_review_mode(); let mut next_config = self.config.clone(); diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index ce5e56c931..7c0b18e96e 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -357,13 +357,28 @@ impl App { self.chat_widget .add_plain_history_lines(vec!["/fork".magenta().into()]); if let Some(thread_id) = self.chat_widget.thread_id() { + if self.pending_server_profiles.contains_key(&thread_id) { + self.chat_widget.add_error_message( + "Wait for permissions to update before forking.".into(), + ); + return Ok(AppRunControl::Continue); + } self.refresh_in_memory_config_from_disk_best_effort("forking the thread") .await; let mut fork_config = self.config.clone(); fork_config.model = Some(self.chat_widget.current_model().to_string()); fork_config.model_reasoning_effort = self.chat_widget.current_reasoning_effort(); - match app_server.fork_thread(&self.local_settings, fork_config, thread_id).await { + let selected_profile = self.confirmed_server_profile(thread_id); + match app_server.fork_thread_at( + &self.local_settings, + fork_config, + thread_id, + /*last_turn_id*/ None, + /*before_turn_id*/ None, + ForkGoalContinuation::StartIfIdle, + selected_profile.as_ref(), + ).await { Ok(mut forked) => { let name_error = if let Some(name) = name { match app_server @@ -441,6 +456,14 @@ impl App { if self.chat_widget.thread_id() != Some(thread_id) { return Ok(AppRunControl::Continue); } + if self.pending_server_profiles.contains_key(&thread_id) { + self.chat_widget.restore_user_message_to_composer(prompt); + self.chat_widget.add_error_message( + "Wait for permissions to update before editing this prompt.".into(), + ); + tui.frame_requester().schedule_frame(); + return Ok(AppRunControl::Continue); + } self.session_telemetry.counter( "codex.thread.fork", /*inc*/ 1, @@ -449,6 +472,7 @@ impl App { self.refresh_in_memory_config_from_disk_best_effort("forking the thread") .await; let config = self.fresh_session_config(); + let selected_profile = self.confirmed_server_profile(thread_id); let turns = match self.thread_event_channels.get(&thread_id) { Some(channel) => { let store = channel.store.lock().await; @@ -521,6 +545,7 @@ impl App { /*last_turn_id*/ None, before_turn_id, ForkGoalContinuation::StartIfIdle, + selected_profile.as_ref(), ) .await } @@ -530,6 +555,7 @@ impl App { &self.local_settings, &config, /*session_start_source*/ None, /*remote_cwd_override*/ None, + selected_profile.as_ref(), ) .await } @@ -724,6 +750,25 @@ impl App { return Ok(AppRunControl::Exit(ExitReason::Fatal(message))); } AppEvent::CodexOp(mut op) => { + if let AppCommand::OverrideTurnContext { + cwd, + approval_policy, + approvals_reviewer, + permission_profile, + active_permission_profile, + windows_sandbox_level, + .. + } = &op + && (cwd.is_some() + || approval_policy.is_some() + || approvals_reviewer.is_some() + || permission_profile.is_some() + || active_permission_profile.is_some() + || windows_sandbox_level.is_some()) + && self.reject_pending_permission_change() + { + return Ok(AppRunControl::Continue); + } if self.active_thread_id == self.chat_widget.thread_id() { if matches!(&op, AppCommand::UserTurn { .. }) && self.chat_widget.defer_pending_turn_for_luna_reserve() @@ -1583,6 +1628,13 @@ impl App { .await; } AppEvent::UpdateModel(model) => { + if self + .active_thread_model_setting_update_params(model.clone()) + .is_some_and(|params| params.permissions.is_some()) + && self.reject_pending_permission_change() + { + return Ok(AppRunControl::Continue); + } let model_changed = self.chat_widget.current_model() != model || self.chat_widget.current_collaboration_mode().model() != model; if model_changed { @@ -1602,7 +1654,12 @@ impl App { self.app_event_tx.send(AppEvent::SettingsSelectionSettled); } AppEvent::SettingsSelectionSettled => { - if self.chat_widget.no_modal_or_popup_active() { + if self.chat_widget.no_modal_or_popup_active() + && !self + .chat_widget + .thread_id() + .is_some_and(|thread_id| self.pending_server_profiles.contains_key(&thread_id)) + { let config = self.chat_widget.config_ref(); let permissions_override = Self::turn_permissions_override_from_config( config, @@ -1653,6 +1710,13 @@ impl App { self.chat_widget.open_advanced_reasoning_popup(model); } AppEvent::ApplyAdvancedReasoning { model, effort } => { + if self + .active_thread_model_setting_update_params(model.clone()) + .is_some_and(|params| params.permissions.is_some()) + && self.reject_pending_permission_change() + { + return Ok(AppRunControl::Continue); + } let model_changed = self.chat_widget.current_model() != model || self.chat_widget.current_collaboration_mode().model() != model; let default_effort = @@ -2327,6 +2391,9 @@ impl App { } } AppEvent::UpdateAskForApprovalPolicy(policy) => { + if self.reject_pending_permission_change() { + return Ok(AppRunControl::Continue); + } let mut config = self.config.clone(); if !self.try_set_approval_policy_on_config( &mut config, @@ -2346,6 +2413,9 @@ impl App { .await; } AppEvent::UpdateActivePermissionProfile(active_permission_profile) => { + if self.reject_pending_permission_change() { + return Ok(AppRunControl::Continue); + } let mut config = self.config.clone(); let Some(permission_profile) = self .try_set_builtin_active_permission_profile_on_config( @@ -2420,11 +2490,12 @@ impl App { } } AppEvent::SelectPermissionProfile(selection) => { - if self.apply_permission_profile_selection(selection).await { - self.chat_widget.submit_initial_user_message_if_pending(); - } + self.select_permission_profile(app_server, selection).await; } AppEvent::UpdateApprovalsReviewer(policy) => { + if self.reject_pending_permission_change() { + return Ok(AppRunControl::Continue); + } self.config.approvals_reviewer = policy; self.chat_widget.set_approvals_reviewer(policy); self.sync_active_thread_permission_settings_to_cached_session() @@ -2804,6 +2875,9 @@ impl App { } } AppEvent::OpenPermissionsPopup | AppEvent::OpenApprovalsPopup => { + if self.reject_pending_permission_change() { + return Ok(AppRunControl::Continue); + } if app_server.uses_remote_workspace() { self.chat_widget.request_permission_profiles(); } else { diff --git a/codex-rs/tui/src/app/permission_shortcuts.rs b/codex-rs/tui/src/app/permission_shortcuts.rs index 4b1526df51..a5f61da523 100644 --- a/codex-rs/tui/src/app/permission_shortcuts.rs +++ b/codex-rs/tui/src/app/permission_shortcuts.rs @@ -17,6 +17,10 @@ impl App { self.chat_widget.complete_permission_shortcut(thread_id); return; } + if self.reject_pending_permission_change() { + self.chat_widget.complete_permission_shortcut(thread_id); + return; + } let result: Result<()> = async { let active_profile = ActivePermissionProfile::new(selection.profile_id.clone()); let profile = builtin_permission_profile_for_active_permission_profile(&active_profile) diff --git a/codex-rs/tui/src/app/reconnect.rs b/codex-rs/tui/src/app/reconnect.rs index f457dbb01b..07ed253600 100644 --- a/codex-rs/tui/src/app/reconnect.rs +++ b/codex-rs/tui/src/app/reconnect.rs @@ -267,6 +267,17 @@ impl App { .with_collaboration_modes(bootstrap.collaboration_modes), ); self.pending_app_server_requests.clear(); + let pending_displayed_profile = + displayed.is_some_and(|id| self.pending_server_profiles.contains_key(&id)); + if pending_displayed_profile { + self.runtime_approval_policy_override = None; + self.runtime_permission_profile_override = None; + } + // 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. + if let Some(id) = displayed { + self.pending_server_profiles.remove(&id); + } self.pending_primary_events.clear(); self.pending_plugin_enabled_writes.clear(); self.pending_hook_enabled_writes.clear(); @@ -313,7 +324,8 @@ impl App { } if let Some(mut started) = thread { let id = started.session.thread_id; - if let Some(channel) = self.thread_event_channels.get(&id) + if !pending_displayed_profile + && let Some(channel) = self.thread_event_channels.get(&id) && let Some(cached) = channel.store.lock().await.session.as_ref() { self.restore_runtime_permissions(&mut started.session, cached); diff --git a/codex-rs/tui/src/app/safety_buffering.rs b/codex-rs/tui/src/app/safety_buffering.rs index dee5108352..6cdaa69ffd 100644 --- a/codex-rs/tui/src/app/safety_buffering.rs +++ b/codex-rs/tui/src/app/safety_buffering.rs @@ -46,6 +46,14 @@ impl App { let retry_config = self.chat_widget.config_ref().clone(); let input_state = self.chat_widget.capture_thread_input_state(); + if self.pending_server_profiles.contains_key(&thread_id) { + self.fail_safety_buffered_branch( + input_state, + prompt, + color_eyre::eyre::eyre!("Wait for permissions to update before forking."), + ); + return; + } let AppCommand::UserTurn { items, @@ -171,6 +179,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 started = app_server .fork_thread_at( &self.local_settings, @@ -179,6 +188,7 @@ impl App { /*last_turn_id*/ None, /*before_turn_id*/ Some(turn_id), ForkGoalContinuation::DeferUntilNextTurn, + selected_profile.as_ref(), ) .await; let started = match started { diff --git a/codex-rs/tui/src/app/session_lifecycle.rs b/codex-rs/tui/src/app/session_lifecycle.rs index 70a4b7b28f..8f2a0d804f 100644 --- a/codex-rs/tui/src/app/session_lifecycle.rs +++ b/codex-rs/tui/src/app/session_lifecycle.rs @@ -518,6 +518,8 @@ impl App { app_server: &mut AppServerSession, thread_id: ThreadId, ) -> Result<()> { + let pending_profile_on_unavailable_thread = self.thread_unavailable(thread_id) + && self.pending_server_profiles.contains_key(&thread_id); let cached_session = if self.thread_unavailable(thread_id) { self.thread_event_channels[&thread_id] .store @@ -591,13 +593,30 @@ impl App { return Ok(()); }; - self.refresh_snapshot_session_if_needed( - app_server, - thread_id, - is_replay_only, - &mut snapshot, - ) - .await; + let session_verified = self + .refresh_snapshot_session_if_needed( + app_server, + thread_id, + is_replay_only, + &mut snapshot, + ) + .await; + if !is_replay_only + && !self.side_threads.contains_key(&thread_id) + && snapshot.session.as_ref().is_some_and(|session| { + session + .active_permission_profile + .as_ref() + .is_some_and(|profile| !profile.id.starts_with(':')) + }) + && !session_verified + { + is_replay_only = true; + attached_replay_only = true; + } + if pending_profile_on_unavailable_thread && !is_replay_only && session_verified { + self.pending_server_profiles.remove(&thread_id); + } // Refreshing can merge restored turns into the store, so recap progress must be read only // after the refresh while the activated thread channel is still retained. let Some(channel) = self.thread_event_channels.get(&thread_id) else { @@ -610,7 +629,9 @@ impl App { if let (Some(cached), Some(session)) = (cached_session.as_ref(), snapshot.session.as_mut()) { - self.restore_runtime_permissions(session, cached); + if !pending_profile_on_unavailable_thread { + self.restore_runtime_permissions(session, cached); + } store.session = Some(session.clone()); if !is_replay_only && self.primary_thread_id == Some(thread_id) { self.primary_session_configured = Some(session.clone()); @@ -728,6 +749,7 @@ impl App { pub(super) fn reset_thread_event_state(&mut self) { self.abort_all_thread_event_listeners(); self.thread_event_channels.clear(); + self.pending_server_profiles.clear(); self.agents_overview.activity.clear(); self.agent_navigation.clear(); self.side_threads.clear(); @@ -886,6 +908,7 @@ impl App { &config, session_start_source, /*remote_cwd_override*/ None, + /*selected_profile*/ None, ) .await { diff --git a/codex-rs/tui/src/app/side.rs b/codex-rs/tui/src/app/side.rs index 9193b4da70..9dca8bb7f7 100644 --- a/codex-rs/tui/src/app/side.rs +++ b/codex-rs/tui/src/app/side.rs @@ -515,6 +515,7 @@ impl App { }); self.abort_thread_event_listener(thread_id); self.thread_event_channels.remove(&thread_id); + self.pending_server_profiles.remove(&thread_id); self.agents_overview.activity.remove(&thread_id); self.side_threads.remove(&thread_id); self.agent_navigation.remove(thread_id); @@ -683,6 +684,13 @@ impl App { self.chat_widget.add_error_message(message.to_string()); return Ok(AppRunControl::Continue); } + if self.pending_server_profiles.contains_key(&parent_thread_id) { + self.restore_side_user_message(user_message.take()); + self.sync_side_thread_ui(); + self.chat_widget + .add_error_message("Wait for permissions to update before forking.".into()); + return Ok(AppRunControl::Continue); + } if let Some((&side_thread_id, state)) = self.side_threads.iter().next() && (parent_thread_id != state.parent_thread_id diff --git a/codex-rs/tui/src/app/startup.rs b/codex-rs/tui/src/app/startup.rs index c579ed7c70..4a81bbe819 100644 --- a/codex-rs/tui/src/app/startup.rs +++ b/codex-rs/tui/src/app/startup.rs @@ -682,6 +682,7 @@ See the Codex keymap documentation for supported actions and examples." cloud_config_bundle, runtime_approval_policy_override: None, runtime_permission_profile_override: None, + pending_server_profiles: HashMap::new(), file_search, enhanced_keys_supported, keymap: runtime_keymap, diff --git a/codex-rs/tui/src/app/test_support.rs b/codex-rs/tui/src/app/test_support.rs index 04bbc76104..94fef77e76 100644 --- a/codex-rs/tui/src/app/test_support.rs +++ b/codex-rs/tui/src/app/test_support.rs @@ -34,6 +34,7 @@ pub(super) async fn make_test_app() -> App { cloud_config_bundle: CloudConfigBundleLoader::default(), runtime_approval_policy_override: None, runtime_permission_profile_override: None, + pending_server_profiles: HashMap::new(), file_search, transcript_cells: Vec::new(), last_rendered_history_tail: None, diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index 2758813e55..4b70ff51a8 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -787,6 +787,15 @@ async fn reset_thread_event_state_aborts_listener_tasks() { std::future::pending::<()>().await; }); app.thread_event_listener_tasks.insert(thread_id, handle); + app.pending_server_profiles.insert( + thread_id, + PermissionProfileSelection { + profile_id: "server-only".into(), + approval_policy: None, + approvals_reviewer: None, + display_label: "server-only".into(), + }, + ); started_rx .await .expect("listener task should report it started"); @@ -794,6 +803,7 @@ async fn reset_thread_event_state_aborts_listener_tasks() { app.reset_thread_event_state(); assert_eq!(app.thread_event_listener_tasks.is_empty(), true); + assert!(app.pending_server_profiles.is_empty()); time::timeout(Duration::from_millis(50), dropped_rx) .await .expect("timed out waiting for listener task abort") @@ -2896,6 +2906,129 @@ async fn reset_memories_clears_local_memory_directories() -> Result<()> { .await } +#[tokio::test] +async fn server_only_profile_selection_keeps_turns_on_the_selected_profile() -> Result<()> { + let (mut app, mut events, mut 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.server-only]\nextends = \":read-only\"\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 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.clone()) + .await; + insta::assert_snapshot!( + next_history_message(&mut events), + @"■ Wait for the task to connect before selecting permissions." + ); + 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?; + while events.try_recv().is_ok() {} + app.select_permission_profile(&mut server, selection.clone()) + .await; + insta::assert_snapshot!( + next_history_message(&mut events), + @"• Permission selection requested: server-only" + ); + let mut tui = crate::tui::test_support::make_test_tui()?; + app.select_permission_profile(&mut server, selection).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 + .handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + let turn = next_user_turn_op(&mut ops); + app.submit_thread_op(&mut server, thread_id, turn).await?; + server.startup_interrupt(thread_id).await?; + let settings = next_thread_settings_updated(&mut server, thread_id).await; + app.enqueue_thread_notification( + thread_id, + ServerNotification::ThreadSettingsUpdated(settings), + ) + .await?; + assert!(!app.pending_server_profiles.contains_key(&thread_id)); + let endpoint = crate::resolve_remote_addr("ws://127.0.0.1:8765")?; + app.app_server_target = crate::AppServerTarget::Remote { endpoint }; + while events.try_recv().is_ok() {} + app.change_working_directory(&mut tui, &mut server, home.path().abs()) + .await; + insta::assert_snapshot!( + next_history_message(&mut events), + @"■ Changing directories with a named profile is not supported." + ); + app.handle_thread_event_now(ThreadBufferedEvent::Notification(Box::new( + turn_completed_notification(thread_id, "first", TurnStatus::Interrupted), + ))); + while events.try_recv().is_ok() {} + app.chat_widget + .restore_user_message_to_composer("busy".into()); + app.chat_widget + .handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + let mut turn = next_user_turn_op(&mut ops); + if let Op::UserTurn { + active_permission_profile, + .. + } = &mut turn + { + *active_permission_profile = Some(ActivePermissionProfile::new("stale-profile")); + } + app.submit_thread_op(&mut server, thread_id, turn).await?; + let thread = server + .thread_read(thread_id, /*include_turns*/ true) + .await?; + assert_eq!(thread.turns.len(), 2); + while events.try_recv().is_ok() {} + app.chat_widget + .set_queue_autosend_suppressed(/*suppressed*/ true); + app.chat_widget + .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"); + 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." + ); + 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()); + server.shutdown().await?; + Ok(()) +} + #[tokio::test] async fn resume_rejects_selected_config_profile_permission_definitions() -> Result<()> { let home = tempdir()?; @@ -5708,6 +5841,7 @@ async fn make_test_app() -> App { pending_thread_titles: HashSet::new(), thread_event_listener_tasks: HashMap::new(), agent_navigation: AgentNavigationState::default(), + pending_server_profiles: HashMap::new(), agents_overview: Default::default(), side_threads: HashMap::new(), abandoned_side_threads: HashSet::new(), @@ -5799,6 +5933,7 @@ pub(super) async fn make_test_app_with_channels() -> ( pending_thread_titles: HashSet::new(), thread_event_listener_tasks: HashMap::new(), agent_navigation: AgentNavigationState::default(), + pending_server_profiles: HashMap::new(), agents_overview: Default::default(), side_threads: HashMap::new(), abandoned_side_threads: HashSet::new(), @@ -6766,6 +6901,16 @@ async fn feedback_submission_for_inactive_thread_replays_into_origin_thread() { })); } +fn next_history_message(events: &mut tokio::sync::mpsc::UnboundedReceiver) -> String { + let cell = std::iter::from_fn(|| events.try_recv().ok()) + .find_map(|event| match event { + AppEvent::InsertHistoryCell(cell) => Some(cell), + _ => None, + }) + .expect("history message"); + lines_to_single_string(&cell.display_lines(/*width*/ 80)) +} + fn next_user_turn_op(op_rx: &mut tokio::sync::mpsc::UnboundedReceiver) -> Op { let mut seen = Vec::new(); while let Ok(op) = op_rx.try_recv() { @@ -7767,7 +7912,16 @@ async fn prompt_edit_before_first_prompt_starts_fresh_thread() -> Result<()> { ) .expect("materialized rollout should be created"); let source_thread_id = ThreadId::from_string(&source_thread_id)?; - let mut app_server = Box::pin(crate::start_embedded_app_server_for_picker(&config)).await?; + std::fs::write( + config.codex_home.join("config.toml"), + "default_permissions = \":workspace\"\n[permissions.server-only]\nextends = \":read-only\"\n", + )?; + let server_config = ConfigBuilder::default() + .codex_home(config.codex_home.to_path_buf()) + .build() + .await?; + let mut app_server = + Box::pin(crate::start_embedded_app_server_for_picker(&server_config)).await?; let started = app_server .resume_thread( &crate::local_settings::LocalSettings::from(&config), @@ -7778,8 +7932,50 @@ async fn prompt_edit_before_first_prompt_starts_fresh_thread() -> Result<()> { .await?; app.enqueue_primary_thread_session(started.session, started.turns) .await?; + app.select_permission_profile( + &mut app_server, + PermissionProfileSelection { + profile_id: "server-only".into(), + approval_policy: None, + approvals_reviewer: None, + display_label: "server-only".into(), + }, + ) + .await; while app_event_rx.try_recv().is_ok() {} let mut tui = crate::tui::test_support::make_test_tui()?; + Box::pin(app.handle_event( + &mut tui, + &mut app_server, + AppEvent::ForkSessionForPromptEdit { + thread_id: source_thread_id, + nth_user_message: 0, + prompt: crate::chatwidget::UserMessage::from("first prompt"), + }, + )) + .await?; + assert_eq!(app.chat_widget.thread_id(), Some(source_thread_id)); + assert_eq!(app.chat_widget.composer_text_with_pending(), "first prompt"); + insta::assert_snapshot!( + next_history_message(&mut app_event_rx), + @"■ Wait for permissions to update before editing this prompt." + ); + let settings = next_thread_settings_updated(&mut app_server, source_thread_id).await; + app.enqueue_thread_notification( + source_thread_id, + ServerNotification::ThreadSettingsUpdated(settings), + ) + .await?; + while app_event_rx.try_recv().is_ok() {} + let endpoint = crate::resolve_remote_addr("ws://127.0.0.1:8765")?; + app.app_server_target = crate::AppServerTarget::Remote { endpoint }; + let local_only_root = tempdir()?; + let local_only_root_path = local_only_root.path().abs(); + app.harness_overrides + .additional_writable_roots + .push(local_only_root.path().to_path_buf()); + app.refresh_in_memory_config_from_disk().await?; + assert!(app.config.workspace_roots.contains(&local_only_root_path)); let control = Box::pin(app.handle_event( &mut tui, @@ -7798,6 +7994,19 @@ async fn prompt_edit_before_first_prompt_starts_fresh_thread() -> Result<()> { .thread_id() .expect("first prompt edit should start a fresh thread"); assert_ne!(fresh_thread_id, source_thread_id); + let active = app + .chat_widget + .config_ref() + .permissions + .active_permission_profile() + .unwrap(); + assert_eq!(active.id, "server-only"); + let session = app + .primary_session_configured + .as_ref() + .expect("new session"); + let roots = &session.runtime_workspace_roots; + assert!(!roots.contains(&local_only_root_path)); assert_eq!(app.chat_widget.composer_text_with_pending(), "first prompt"); let history = std::iter::from_fn(|| app_event_rx.try_recv().ok()) .filter_map(|event| match event { @@ -8400,6 +8609,28 @@ async fn selecting_cyber_model_defaults_active_thread_to_auto_review() { .expect("primary thread should be registered"); let mut tui = crate::tui::test_support::make_test_tui().expect("test tui"); + let previous_model = app.chat_widget.current_model().to_string(); + app.pending_server_profiles.insert( + thread_id, + PermissionProfileSelection { + profile_id: "server-only".into(), + approval_policy: None, + approvals_reviewer: None, + display_label: "server-only".into(), + }, + ); + app.handle_event( + &mut tui, + &mut app_server, + AppEvent::ApplyAdvancedReasoning { + model: "gpt-5.4".to_string(), + effort: ReasoningEffortConfig::High, + }, + ) + .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); app.handle_event( &mut tui, &mut app_server, @@ -8408,6 +8639,7 @@ async fn selecting_cyber_model_defaults_active_thread_to_auto_review() { .await .expect("model selection should succeed"); + assert!(app.pending_server_profiles.contains_key(&thread_id)); let notification = next_thread_settings_updated(&mut app_server, thread_id).await; assert_eq!( notification.thread_settings.approval_policy, @@ -8421,10 +8653,18 @@ async fn selecting_cyber_model_defaults_active_thread_to_auto_review() { notification .thread_settings .active_permission_profile + .as_ref() .expect("active permission profile") .id, codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_WORKSPACE ); + app.enqueue_thread_notification( + thread_id, + ServerNotification::ThreadSettingsUpdated(notification), + ) + .await + .expect("settings notification should apply"); + assert!(!app.pending_server_profiles.contains_key(&thread_id)); }) .await; } 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 786efd734d..c5c63e361c 100644 --- a/codex-rs/tui/src/app/tests/active_reconnect_tests.rs +++ b/codex-rs/tui/src/app/tests/active_reconnect_tests.rs @@ -19,6 +19,7 @@ async fn reconnect_restores_history_permissions_and_keeps_old_input_paused() -> (false, false, -32600), (true, true, -32600), ] { + let pending_profile = !recovered_queue && resume_error_code == -32600; let (mut app, mut events, mut ops) = make_test_app_with_channels().await; let id = ThreadId::new(); let cwd = app.config.cwd.clone(); @@ -108,8 +109,8 @@ async fn reconnect_restores_history_permissions_and_keeps_old_input_paused() -> assert!(!recovered_queue); let params = request.params.as_ref().unwrap(); assert_eq!(params["input"][0]["text"], "fresh follow-up"); - assert_eq!(params["approvalPolicy"], "on-request"); - assert_eq!(params["sandboxPolicy"]["type"], "readOnly"); + assert_eq!(params["approvalPolicy"], if pending_profile { "never" } else { "on-request" }); + assert_eq!(params["sandboxPolicy"]["type"], if pending_profile { json!(null) } else { json!("readOnly") }); assert_eq!(params["collaborationMode"], json!(expected_submitted_mode)); Some(json!({"result": {"turn": {"id": "fresh", "items": [], "status": "inProgress"}}})) } @@ -166,6 +167,17 @@ async fn reconnect_restores_history_permissions_and_keeps_old_input_paused() -> Some("1.0.0"), ); let mut tui = crate::tui::test_support::make_test_tui()?; + if pending_profile { + app.pending_server_profiles.insert( + id, + PermissionProfileSelection { + profile_id: "server-only".into(), + approval_policy: None, + approvals_reviewer: None, + display_label: "server-only".into(), + }, + ); + } app.begin_reconnect(); if edit_offline { app.handle_tui_event( @@ -215,6 +227,7 @@ async fn reconnect_restores_history_permissions_and_keeps_old_input_paused() -> ); app.finish_reconnect(&mut tui, &mut session, &mut events, connected) .await?; + assert!(app.pending_server_profiles.is_empty()); assert!(!app.reconnect.offline); assert!(!app.thread_unavailable(id)); assert_eq!(app.last_subagent_backfill_attempt, None); @@ -330,6 +343,147 @@ async fn reconnect_restores_history_permissions_and_keeps_old_input_paused() -> Ok(()) } +#[tokio::test] +async fn reconnect_reconciles_offscreen_pending_profile_before_restoring_permissions() -> Result<()> +{ + let (mut app, mut events, _) = make_test_app_with_channels().await; + let primary = ThreadId::new(); + let displayed = ThreadId::new(); + let cwd = app.config.cwd.to_path_buf(); + app.config + .permissions + .set_permission_profile(PermissionProfile::Disabled)?; + app.runtime_permission_profile_override = + Some(RuntimePermissionProfileOverride::from_config(&app.config)); + app.primary_thread_id = Some(primary); + app.active_thread_id = Some(displayed); + for id in [primary, displayed] { + let mut session = test_thread_session(id, cwd.clone()); + session.permission_profile = PermissionProfile::Disabled; + app.ensure_thread_channel(id) + .store + .lock() + .await + .set_session(session.clone(), Vec::new()); + if id == primary { + app.primary_session_configured = Some(session); + app.upsert_agent_picker_thread( + id, /*agent_nickname*/ None, /*agent_role*/ None, + /*is_closed*/ false, + ); + } else { + app.chat_widget.handle_thread_session(session); + } + } + app.pending_server_profiles.insert( + primary, + PermissionProfileSelection { + profile_id: "server-only".into(), + approval_policy: None, + approvals_reviewer: None, + display_label: "server-only".into(), + }, + ); + + let listener = TcpListener::bind("127.0.0.1:0").await?; + app.app_server_target = AppServerTarget::Remote { + endpoint: crate::resolve_remote_addr(&format!("ws://{}", listener.local_addr()?))?, + }; + let server = tokio::spawn(async move { + let (stream, _) = listener.accept().await?; + serve_reconnect_requests(tokio_tungstenite::accept_async(stream).await?, |request| { + let id = request.params.as_ref().and_then(|params| params["threadId"].as_str()); + let thread = |id: ThreadId| json!({ + "id": id, "sessionId": id, "preview": "task", "ephemeral": false, + "modelProvider": "test-provider", "createdAt": 1, "updatedAt": 2, + "status": {"type": "idle"}, "cwd": cwd, "cliVersion": "0.0.0", + "source": "cli", "turns": [] + }); + std::future::ready(Some(match request.method.as_str() { + "thread/resume" => { + let pending = id == Some(primary.to_string().as_str()); + json!({"result": {"thread": thread(if pending { primary } else { displayed }), + "model": "gpt-test", "modelProvider": "test-provider", "cwd": cwd, + "approvalPolicy": if pending { "on-request" } else { "never" }, + "approvalsReviewer": "user", + "sandbox": {"type": if pending { "readOnly" } else { "dangerFullAccess" }}, + "activePermissionProfile": if pending { Some(json!({"id": "server-only"})) } else { None }, + "reasoningEffort": null}}) + } + "thread/read" => json!({"result": {"thread": thread(primary)}}), + "turn/start" => { + let params = request.params.as_ref().unwrap(); + assert_eq!(params["permissions"], "server-only"); + assert_eq!(params["approvalPolicy"], "on-request"); + assert_eq!(params["sandboxPolicy"], json!(null)); + json!({"result": {"turn": {"id": "fresh", "items": [], "status": "inProgress"}}}) + } + "thread/list" | "thread/loaded/list" => + json!({"result": {"data": [], "nextCursor": null}}), + "thread/goal/get" => json!({"result": {"goal": null}}), + method => panic!("unexpected reconnect request: {method}"), + })) + }) + .await + }); + let mut session = crate::start_embedded_app_server_for_picker(&app.config).await?; + let mut tui = crate::tui::test_support::make_test_tui()?; + app.begin_reconnect(); + let connected = reconnect( + app.app_server_target.clone(), + app.config.clone(), + app.local_settings.clone(), + Some(displayed), + /*remote_cwd*/ None, + session.thread_tool_transport(), + ReconnectPresentation::Conversation, + ) + .await?; + app.finish_reconnect(&mut tui, &mut session, &mut events, connected) + .await?; + assert!(app.pending_server_profiles.contains_key(&primary)); + + app.select_agent_thread(&mut tui, &mut session, primary) + .await?; + assert!(!app.thread_unavailable(primary)); + assert!(!app.pending_server_profiles.contains_key(&primary)); + assert_eq!( + app.chat_widget + .config_ref() + .permissions + .permission_profile(), + &PermissionProfile::read_only() + ); + assert_eq!( + app.chat_widget + .config_ref() + .permissions + .active_permission_profile() + .as_ref() + .map(|profile| profile.id.as_str()), + Some("server-only") + ); + app.chat_widget + .restore_user_message_to_composer("safe follow-up".into()); + app.chat_widget + .handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + while let Ok(event) = events.try_recv() { + if matches!(event, AppEvent::CodexOp(AppCommand::UserTurn { .. })) { + app.handle_event(&mut tui, &mut session, event).await?; + } + } + session.shutdown().await?; + let methods = server.await??; + assert_eq!( + methods + .iter() + .filter(|method| *method == "turn/start") + .count(), + 1 + ); + Ok(()) +} + #[tokio::test] async fn reconnect_exhaustion_and_unknown_initial_thread_stay_offline() -> Result<()> { let (mut app, _, _) = make_test_app_with_channels().await; 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 a15b2a0966..dacdff5058 100644 --- a/codex-rs/tui/src/app/tests/permission_shortcuts_tests.rs +++ b/codex-rs/tui/src/app/tests/permission_shortcuts_tests.rs @@ -75,6 +75,22 @@ async fn permission_shortcut_rejections_leave_state_unchanged() -> Result<()> { .replace(&thread_id.to_string(), "") ); assert!(events.try_recv().is_err()); + if !experimental_api { + app.select_permission_profile( + &mut app_server, + PermissionProfileSelection { + profile_id: "server-only".into(), + approval_policy: None, + approvals_reviewer: None, + display_label: "server-only".into(), + }, + ) + .await; + insta::assert_snapshot!( + next_history_message(&mut events), + @"■ Named profiles require a newer app server." + ); + } app_server.shutdown().await?; } Ok(()) diff --git a/codex-rs/tui/src/app/thread_routing.rs b/codex-rs/tui/src/app/thread_routing.rs index b65b4d5997..768322320f 100644 --- a/codex-rs/tui/src/app/thread_routing.rs +++ b/codex-rs/tui/src/app/thread_routing.rs @@ -22,6 +22,7 @@ impl App { tracing::warn!("failed to unsubscribe thread {thread_id}: {err}"); } self.abort_thread_event_listener(thread_id); + self.pending_server_profiles.remove(&thread_id); } } @@ -754,11 +755,42 @@ impl App { } if should_start_turn { let config = self.chat_widget.config_ref(); - let approvals_reviewer = - approvals_reviewer.unwrap_or(config.approvals_reviewer); + let selected_profile = self.pending_server_profiles.get(&thread_id); + let selected_active = selected_profile + .map(|profile| ActivePermissionProfile::new(profile.profile_id.clone())); + let confirmed_active = (self.app_server_target.thread_params_mode() + == crate::app_server_session::ThreadParamsMode::Remote) + .then(|| config.permissions.active_permission_profile()) + .flatten(); + let (turn_approval_policy, turn_approvals_reviewer) = + if let Some(profile) = selected_profile { + ( + profile.approval_policy, + profile.approvals_reviewer.map(Into::into), + ) + } else if self.app_server_target.thread_params_mode() + == crate::app_server_session::ThreadParamsMode::Remote + { + ( + Some(config.permissions.approval_policy.value().into()), + Some(config.approvals_reviewer.into()), + ) + } else { + ( + Some(*approval_policy), + Some( + approvals_reviewer + .unwrap_or(config.approvals_reviewer) + .into(), + ), + ) + }; let permissions_override = Self::turn_permissions_override_from_config( config, - active_permission_profile.as_ref(), + selected_active + .as_ref() + .or(confirmed_active.as_ref()) + .or(active_permission_profile.as_ref()), self.runtime_permission_profile_override .as_ref() .and_then(RuntimePermissionProfileOverride::turn_permission_profile), @@ -769,8 +801,8 @@ impl App { client_user_message_id.clone(), items.to_vec(), cwd.clone(), - *approval_policy, - approvals_reviewer, + turn_approval_policy, + turn_approvals_reviewer, permissions_override, config.permissions.user_visible_workspace_roots(), model.to_string(), @@ -1021,9 +1053,30 @@ impl App { { return Ok(()); } + let mut permission_change_confirmed = false; if let ServerNotification::ThreadSettingsUpdated(notification) = ¬ification { self.apply_thread_settings_to_cached_session(thread_id, ¬ification.thread_settings) .await; + if self + .pending_server_profiles + .get(&thread_id) + .is_some_and(|selected| { + notification + .thread_settings + .active_permission_profile + .as_ref() + .is_some_and(|active| active.id == selected.profile_id) + && selected.approval_policy.is_none_or(|policy| { + notification.thread_settings.approval_policy == policy + }) + && selected.approvals_reviewer.is_none_or(|reviewer| { + notification.thread_settings.approvals_reviewer.to_core() == reviewer + }) + }) + { + self.pending_server_profiles.remove(&thread_id); + permission_change_confirmed = true; + } } let inferred_session = if let ServerNotification::ThreadStarted(started) = ¬ification && self.primary_session_configured.is_some() @@ -1058,7 +1111,7 @@ impl App { let channel = self.ensure_thread_channel(thread_id); (channel.sender.clone(), Arc::clone(&channel.store)) }; - let (notification, previous_pending_status, pending_status, turn_stopped) = { + let (mut notification, previous_pending_status, pending_status, turn_stopped) = { let mut guard = store.lock().await; if guard.session.is_none() && let Some(session) = inferred_session @@ -1094,6 +1147,19 @@ impl App { self.agent_navigation.mark_stopped(thread_id); } + // Settings snapshots do not belong in the transcript queue: apply them in receive order. + if let Some(ServerNotification::ThreadSettingsUpdated(settings)) = notification.as_ref() + && self.active_thread_id == Some(thread_id) + && self.chat_widget.thread_id() == Some(thread_id) + { + self.chat_widget + .on_thread_settings_updated(settings.clone()); + notification = None; + } + if permission_change_confirmed { + self.app_event_tx.send(AppEvent::SettingsSelectionSettled); + } + if let Some(notification) = notification { match sender.try_send(ThreadBufferedEvent::Notification(Box::new(notification))) { Ok(()) => {} @@ -1321,6 +1387,7 @@ impl App { self.config.approvals_reviewer = session.approvals_reviewer; let thread_id = session.thread_id; + self.pending_server_profiles.remove(&thread_id); if self.primary_thread_id != Some(thread_id) { self.recap.reset_for_new_thread(Instant::now()); } @@ -1426,9 +1493,9 @@ impl App { thread_id: ThreadId, is_replay_only: bool, snapshot: &mut ThreadEventSnapshot, - ) { + ) -> bool { if !self.should_refresh_snapshot_session(thread_id, is_replay_only, snapshot) { - return; + return true; } match app_server @@ -1442,7 +1509,8 @@ impl App { { Ok(started) => { self.apply_refreshed_snapshot_thread(thread_id, started, snapshot) - .await + .await; + true } Err(err) => { tracing::warn!( @@ -1450,6 +1518,7 @@ impl App { error = %err, "failed to refresh inferred thread session before replay" ); + false } } } @@ -1463,7 +1532,13 @@ impl App { !is_replay_only && !self.side_threads.contains_key(&thread_id) && snapshot.session.as_ref().is_none_or(|session| { - session.model.trim().is_empty() || session.rollout_path.is_none() + session.model.trim().is_empty() + || session.rollout_path.is_none() + || (self.primary_thread_id != Some(thread_id) + && session + .active_permission_profile + .as_ref() + .is_some_and(|profile| !profile.id.starts_with(':'))) }) } diff --git a/codex-rs/tui/src/app/thread_settings.rs b/codex-rs/tui/src/app/thread_settings.rs index 58383388b7..66abceebef 100644 --- a/codex-rs/tui/src/app/thread_settings.rs +++ b/codex-rs/tui/src/app/thread_settings.rs @@ -3,6 +3,7 @@ use super::App; use crate::app_command::AppCommand; use crate::app_event::AppEvent; +use crate::app_event::PermissionProfileSelection; use crate::app_server_session::AppServerSession; use crate::chatwidget::cyber_model_approval_reviewer; use crate::session_state::ThreadSessionState; @@ -38,7 +39,25 @@ impl App { .approval_policy .value(), ) != AppServerAskForApproval::OnRequest); + let pending_cyber_profile = + params + .permissions + .as_ref() + .map(|profile_id| PermissionProfileSelection { + profile_id: profile_id.clone(), + approval_policy: params.approval_policy, + approvals_reviewer: params + .approvals_reviewer + .map(codex_app_server_protocol::ApprovalsReviewer::to_core), + display_label: profile_id.clone(), + }); let settings_updated = self.send_thread_settings_update(app_server, params).await; + if settings_updated + && let (Some(thread_id), Some(selection)) = + (self.active_thread_id, pending_cyber_profile) + { + self.pending_server_profiles.insert(thread_id, selection); + } if defaulted_to_auto_review && settings_updated { self.app_event_tx.send(AppEvent::CyberModelAutoReviewNotice); } @@ -213,6 +232,17 @@ 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()) + { + return false; + } match app_server.thread_settings_update(params).await { Ok(settings_updated) => settings_updated, Err(err) => { diff --git a/codex-rs/tui/src/app/working_directory.rs b/codex-rs/tui/src/app/working_directory.rs index 443b0b459f..e8903c5a9b 100644 --- a/codex-rs/tui/src/app/working_directory.rs +++ b/codex-rs/tui/src/app/working_directory.rs @@ -125,6 +125,24 @@ impl App { let Some(thread_id) = self.chat_widget.thread_id() else { return; }; + if self.pending_server_profiles.contains_key(&thread_id) { + return self.working_directory_error( + "Wait for permissions to update before changing directories.", + ); + } + if self.app_server_target.thread_params_mode() + == crate::app_server_session::ThreadParamsMode::Remote + && self + .chat_widget + .config_ref() + .permissions + .active_permission_profile() + .is_some_and(|profile| !profile.id.starts_with(':')) + { + return self.working_directory_error( + "Changing directories with a 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."); @@ -286,6 +304,7 @@ impl App { /*last_turn_id*/ None, /*before_turn_id*/ None, DeferUntilNextTurn, + /*selected_profile*/ None, ) .await } else { @@ -295,6 +314,7 @@ impl App { &config, /*session_start_source*/ None, /*remote_cwd_override*/ None, + /*selected_profile*/ None, ) .await }; diff --git a/codex-rs/tui/src/app_server_session.rs b/codex-rs/tui/src/app_server_session.rs index d1a5641587..0ca5407cec 100644 --- a/codex-rs/tui/src/app_server_session.rs +++ b/codex-rs/tui/src/app_server_session.rs @@ -18,6 +18,7 @@ pub(crate) use history::HISTORY_ITEM_SCAN_LIMIT; pub(crate) use history::HistoryHydrationScope; pub(crate) use history::thread_items_page_params; +use crate::app_event::PermissionProfileSelection; use crate::app_event_sender::AppEventSender; use crate::bottom_pane::FeedbackAudience; use crate::dynamic_tools_mcp::DynamicToolMcpServer; @@ -783,6 +784,7 @@ impl AppServerSession { config, /*session_start_source*/ None, /*remote_cwd_override*/ None, + /*selected_profile*/ None, ) .await } @@ -793,6 +795,7 @@ impl AppServerSession { config: &Config, session_start_source: Option, remote_cwd_override: Option<&std::path::Path>, + selected_profile: Option<&PermissionProfileSelection>, ) -> Result { let request_id = self.next_request_id(); let session_config = self.session_config_with_effective_service_tier(config); @@ -802,6 +805,14 @@ impl AppServerSession { remote_cwd_override.or(self.remote_cwd_override.as_deref()), session_start_source, ); + if let Some(selected_profile) = selected_profile { + params.runtime_workspace_roots = None; + params.permissions = Some(selected_profile.profile_id.clone()); + params.sandbox = None; + params.approval_policy = selected_profile.approval_policy; + params.approvals_reviewer = selected_profile.approvals_reviewer.map(Into::into); + remove_permission_config_overrides(&mut params.config); + } if self.history_support == ThreadHistorySupport::LegacyOnly { params.history_mode = None; } @@ -830,6 +841,7 @@ impl AppServerSession { Ok(started) } + #[cfg(test)] pub(crate) async fn fork_thread( &mut self, local_settings: &LocalSettings, @@ -860,11 +872,16 @@ impl AppServerSession { /*before_turn_id*/ None, ForkGoalContinuation::StartIfIdle, ForkPresentation::Regular, + /*selected_profile*/ None, permission_mode, ) .await } + #[expect( + clippy::too_many_arguments, + reason = "fork position and named permissions are independent" + )] pub(crate) async fn fork_thread_at( &mut self, local_settings: &LocalSettings, @@ -873,6 +890,7 @@ impl AppServerSession { last_turn_id: Option, before_turn_id: Option, goal_continuation: ForkGoalContinuation, + selected_profile: Option<&PermissionProfileSelection>, ) -> Result { self.fork_thread_at_with_presentation( local_settings, @@ -882,6 +900,7 @@ impl AppServerSession { before_turn_id, goal_continuation, ForkPresentation::Regular, + selected_profile, ForkPermissionMode::InheritSaved, ) .await @@ -901,6 +920,7 @@ impl AppServerSession { /*before_turn_id*/ None, ForkGoalContinuation::StartIfIdle, ForkPresentation::SideConversation, + /*selected_profile*/ None, ForkPermissionMode::InheritSaved, ) .await @@ -919,6 +939,7 @@ impl AppServerSession { before_turn_id: Option, goal_continuation: ForkGoalContinuation, presentation: ForkPresentation, + selected_profile: Option<&PermissionProfileSelection>, permission_mode: ForkPermissionMode, ) -> Result { let fork_parent = match presentation { @@ -955,6 +976,13 @@ impl AppServerSession { params.sandbox = None; params.permissions = None; remove_permission_config_overrides(&mut params.config); + } else if let Some(selected_profile) = selected_profile { + params.runtime_workspace_roots = None; + params.permissions = Some(selected_profile.profile_id.clone()); + params.sandbox = None; + params.approval_policy = selected_profile.approval_policy; + params.approvals_reviewer = selected_profile.approvals_reviewer.map(Into::into); + remove_permission_config_overrides(&mut params.config); } self.thread_tool_transport() .configure_mcp(&mut params.config); @@ -1276,8 +1304,8 @@ impl AppServerSession { client_user_message_id: String, items: Vec, cwd: PathBuf, - approval_policy: AskForApproval, - approvals_reviewer: codex_protocol::config_types::ApprovalsReviewer, + approval_policy: Option, + approvals_reviewer: Option, permissions_override: TurnPermissionsOverride, workspace_roots: &[AbsolutePathBuf], model: String, @@ -1305,8 +1333,8 @@ impl AppServerSession { environments: None, cwd: Some(cwd), runtime_workspace_roots: Some(workspace_roots.to_vec()), - approval_policy: Some(approval_policy), - approvals_reviewer: Some(approvals_reviewer.into()), + approval_policy, + approvals_reviewer, sandbox_policy, permissions, model: Some(model), diff --git a/codex-rs/tui/src/app_server_session/prompt_history_tests.rs b/codex-rs/tui/src/app_server_session/prompt_history_tests.rs index e5bc164c72..3969bc2ab8 100644 --- a/codex-rs/tui/src/app_server_session/prompt_history_tests.rs +++ b/codex-rs/tui/src/app_server_session/prompt_history_tests.rs @@ -53,6 +53,7 @@ async fn lifecycle_metadata_uses_local_prompt_history() -> Result<()> { &config, /*session_start_source*/ None, /*remote_cwd_override*/ None, + /*selected_profile*/ None, ) .await?; assert_eq!(started.session.message_history, expected); 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 adc0154dca..e3b5333412 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 @@ -1,3 +1,4 @@ +use super::super::ForkGoalContinuation; use super::super::ForkPermissionMode; use super::super::ResumeModelSettings; use super::super::ThreadParamsMode; @@ -126,8 +127,23 @@ async fn remote_resume_restores_saved_server_profile_without_permission_override resumed.session.approvals_reviewer, codex_protocol::config_types::ApprovalsReviewer::AutoReview ); + // 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(), + }; let forked = server - .fork_thread(&local_settings, client_config, thread_id) + .fork_thread_at( + &local_settings, + client_config.clone(), + thread_id, + /*last_turn_id*/ None, + /*before_turn_id*/ None, + ForkGoalContinuation::StartIfIdle, + Some(&stale_selection), + ) .await?; assert_eq!( forked @@ -145,6 +161,13 @@ async fn remote_resume_restores_saved_server_profile_without_permission_override forked.session.approvals_reviewer, codex_protocol::config_types::ApprovalsReviewer::AutoReview ); + let side = server + .fork_side_thread(&local_settings, client_config, thread_id) + .await?; + assert_eq!( + side.session.active_permission_profile.unwrap().id, + "server-only" + ); let explicit_config = ConfigBuilder::default() .codex_home(home.path().to_path_buf()) .harness_overrides(crate::legacy_core::config::ConfigOverrides { diff --git a/codex-rs/tui/src/chatwidget/permission_discovery.rs b/codex-rs/tui/src/chatwidget/permission_discovery.rs index 120d2c0595..44d5df49e1 100644 --- a/codex-rs/tui/src/chatwidget/permission_discovery.rs +++ b/codex-rs/tui/src/chatwidget/permission_discovery.rs @@ -6,6 +6,13 @@ use crate::permission_discovery::PermissionDiscovery; pub(super) const VIEW_ID: &str = "permission-profiles"; impl ChatWidget { + pub(crate) fn retain_input_after_failed_permission_selection(&mut self) { + self.input_queue.recovered_queue |= self.input_queue.has_queued_follow_up_messages() + || !self.input_queue.pending_steers.is_empty(); + if let Some(message) = self.initial_user_message.take() { + self.restore_user_message_to_composer(message); + } + } pub(crate) fn request_permission_profiles(&mut self) { self.invalidate_permission_discovery(); let request_id = uuid::Uuid::new_v4(); 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 index 151d454af2..5b4e7562fe 100644 --- 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 @@ -1,6 +1,6 @@ --- source: tui/src/chatwidget/tests/permissions.rs -assertion_line: 50 +assertion_line: 46 expression: "render_bottom_popup(&chat, 110)" --- Update Model Permissions @@ -14,7 +14,6 @@ expression: "render_bottom_popup(&chat, 110)" 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.) + 2. server-only Description from the connected server. Press enter to confirm or esc to go back diff --git a/codex-rs/tui/src/chatwidget/tests/permissions.rs b/codex-rs/tui/src/chatwidget/tests/permissions.rs index e1a66ed204..680a94ed14 100644 --- a/codex-rs/tui/src/chatwidget/tests/permissions.rs +++ b/codex-rs/tui/src/chatwidget/tests/permissions.rs @@ -12,7 +12,7 @@ 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() { +async fn permission_discovery_uses_server_catalog_for_remote_custom_selection() { 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( @@ -21,7 +21,6 @@ async fn permission_discovery_uses_server_catalog_and_keeps_remote_custom_select )) .unwrap(); let mut discovery = Discovery::local(&chat.config); - discovery.remote = true; discovery .profiles .push(codex_app_server_protocol::PermissionProfileSummary { @@ -48,9 +47,13 @@ async fn permission_discovery_uses_server_catalog_and_keeps_remote_custom_select "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()); + // Server-only alternatives are routed through App rather than resolved locally. + chat.handle_key_event(KeyEvent::from(KeyCode::Char('2'))); + assert!(matches!( + rx.try_recv().expect("server profile 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(); diff --git a/codex-rs/tui/src/permission_discovery.rs b/codex-rs/tui/src/permission_discovery.rs index f25ec548ec..6fce51f7b1 100644 --- a/codex-rs/tui/src/permission_discovery.rs +++ b/codex-rs/tui/src/permission_discovery.rs @@ -1,6 +1,5 @@ //! 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. +//! This never applies a profile; the app chooses local or server-owned selection. use crate::app_event::AppEvent; use crate::app_event_sender::AppEventSender; @@ -28,7 +27,6 @@ use uuid::Uuid; pub(crate) struct PermissionDiscovery { pub(crate) profiles: Vec, pub(crate) requirements: Option, - pub(crate) remote: bool, pub(crate) explicit_profile_mode: bool, } @@ -52,7 +50,6 @@ impl PermissionDiscovery { Self { profiles, requirements: None, - remote: false, explicit_profile_mode: true, } } @@ -80,9 +77,7 @@ impl PermissionDiscovery { { 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() - }) + None } } @@ -138,7 +133,6 @@ pub(crate) fn fetch( return Ok(PermissionDiscovery { profiles: Vec::new(), requirements: None, - remote: true, explicit_profile_mode: false, }); } @@ -180,7 +174,6 @@ pub(crate) fn fetch( return Ok(PermissionDiscovery { profiles, requirements: requirements.requirements, - remote: mode == ThreadParamsMode::Remote, explicit_profile_mode: true, }); };