diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 4b9cc7ab00..df90e892c0 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -7,10 +7,13 @@ use crate::AppServerTarget; use crate::app_backtrack::BacktrackState; use crate::app_command::AppCommand; use crate::app_event::AppEvent; +use crate::app_event::AutoReviewPresetSelection; use crate::app_event::ExitMode; use crate::app_event::FeedbackCategory; use crate::app_event::HistoryLookupResponse; +use crate::app_event::ManagedNetworkChoice; use crate::app_event::PaneSlot; +use crate::app_event::PermissionPresetProfileUpdate; use crate::app_event::PermissionProfileSelection; use crate::app_event::PluginLocation; use crate::app_event::PluginRemoteSectionError; @@ -90,6 +93,7 @@ use codex_app_server_protocol::AskForApproval; use codex_app_server_protocol::ClientRequest; use codex_app_server_protocol::CodexErrorInfo as AppServerCodexErrorInfo; use codex_app_server_protocol::ConfigBatchWriteParams; +use codex_app_server_protocol::ConfigLayerSource; use codex_app_server_protocol::ConfigReadResponse; use codex_app_server_protocol::ConfigValueWriteParams; use codex_app_server_protocol::ConfigWriteResponse; @@ -205,6 +209,7 @@ mod agent_status_feed; mod app_server_event_targets; mod app_server_events; pub(crate) mod app_server_requests; +mod auto_review_permissions; mod background_requests; mod config_persistence; mod conversation_events; diff --git a/codex-rs/tui/src/app/auto_review_permissions.rs b/codex-rs/tui/src/app/auto_review_permissions.rs new file mode 100644 index 0000000000..981432239d --- /dev/null +++ b/codex-rs/tui/src/app/auto_review_permissions.rs @@ -0,0 +1,254 @@ +//! Auto Review permission transitions and managed-network restoration. + +use super::*; + +const WORKSPACE_NETWORK_ACCESS_KEY: &str = "sandbox_workspace_write.network_access"; +const WORKSPACE_NETWORK_ACCESS_POINTER: &str = "/sandbox_workspace_write/network_access"; + +fn user_network_restriction_overrides_managed_enabled(response: &ConfigReadResponse) -> bool { + let user_override_is_effective = response + .origins + .get(WORKSPACE_NETWORK_ACCESS_KEY) + .is_some_and(|metadata| matches!(metadata.name, ConfigLayerSource::User { .. })); + user_override_is_effective + && response.layers.as_ref().is_some_and(|layers| { + layers.iter().any(|layer| { + matches!( + layer.name, + ConfigLayerSource::Mdm { .. } + | ConfigLayerSource::System { .. } + | ConfigLayerSource::EnterpriseManaged { .. } + | ConfigLayerSource::LegacyManagedConfigTomlFromFile { .. } + | ConfigLayerSource::LegacyManagedConfigTomlFromMdm + ) && layer + .config + .pointer(WORKSPACE_NETWORK_ACCESS_POINTER) + .and_then(serde_json::Value::as_bool) + == Some(true) + }) + }) +} + +impl App { + pub(super) async fn apply_auto_review_preset( + &mut self, + app_server: &mut AppServerSession, + selection: AutoReviewPresetSelection, + network_choice: ManagedNetworkChoice, + ) { + let terminal_browser_enabled = self.config.features.enabled(Feature::TerminalBrowser); + let network_is_restricted = !self + .config + .permissions + .network_sandbox_policy() + .is_enabled(); + if network_choice == ManagedNetworkChoice::Detect + && terminal_browser_enabled + && network_is_restricted + { + let cwd = self.chat_widget.config_ref().cwd.display().to_string(); + match crate::config_update::read_effective_config_with_layers( + app_server.request_handle(), + cwd, + ) + .await + { + Ok(response) if user_network_restriction_overrides_managed_enabled(&response) => { + self.chat_widget + .open_managed_network_restore_confirmation(selection); + return; + } + Ok(_) => {} + Err(err) => tracing::warn!( + error = %err, + "failed to inspect managed network config before enabling Auto Review" + ), + } + } + + let restore_managed_network = network_choice == ManagedNetworkChoice::RestoreManaged; + let mut next_config = self.config.clone(); + if !self.try_set_approval_policy_on_config( + &mut next_config, + selection.approval_policy, + "Failed to enable Approve for me", + "failed to set Auto Review approval policy", + ) { + return; + } + next_config.approvals_reviewer = ApprovalsReviewer::AutoReview; + + let mut edits = vec![crate::config_update::replace_config_value( + "approvals_reviewer", + serde_json::json!(ApprovalsReviewer::AutoReview.to_string()), + )]; + if restore_managed_network { + edits.push(crate::config_update::clear_config_value( + WORKSPACE_NETWORK_ACCESS_KEY, + )); + } + if let Err(err) = + crate::config_update::write_config_batch(app_server.request_handle(), edits).await + { + tracing::error!(error = %err, "failed to persist Auto Review selection"); + self.chat_widget.add_error_message(format!( + "Failed to update Auto Review permissions: {}", + crate::config_update::format_config_error(&err) + )); + return; + } + + if restore_managed_network { + next_config = match self + .rebuild_config_for_cwd(self.chat_widget.config_ref().cwd.to_path_buf()) + .await + { + Ok(config) => config, + Err(err) => { + tracing::error!( + error = %err, + "failed to rebuild config after restoring managed network access" + ); + self.chat_widget.add_error_message(format!( + "Managed network access was restored on disk, but the current session could not refresh it: {err}" + )); + return; + } + }; + if !self.try_set_approval_policy_on_config( + &mut next_config, + selection.approval_policy, + "Failed to enable Approve for me", + "failed to restore Auto Review approval policy", + ) { + return; + } + next_config.approvals_reviewer = ApprovalsReviewer::AutoReview; + } + + let (permission_profile, active_permission_profile) = if restore_managed_network { + if !next_config + .permissions + .network_sandbox_policy() + .is_enabled() + { + self.chat_widget.add_error_message( + "The user network override was removed, but managed network access is still disabled." + .to_string(), + ); + return; + } + ( + Some(next_config.permissions.permission_profile().clone()), + next_config.permissions.active_permission_profile(), + ) + } else { + match &selection.profile_update { + PermissionPresetProfileUpdate::Preserve => (None, None), + PermissionPresetProfileUpdate::Replace { + permission_profile, + active_permission_profile, + } => ( + Some(permission_profile.clone()), + Some(active_permission_profile.clone()), + ), + } + }; + + if let Some(permission_profile) = permission_profile.as_ref() + && let Err(err) = next_config + .permissions + .set_permission_profile_from_session_snapshot( + PermissionProfileSnapshot::from_session_snapshot( + permission_profile.clone(), + active_permission_profile.clone(), + ), + ) + { + tracing::warn!(error = %err, "failed to apply Auto Review permission profile"); + self.chat_widget + .add_error_message(format!("Failed to enable Approve for me: {err}")); + return; + } + + let permission_network = next_config.permissions.network.clone(); + self.config = next_config; + self.runtime_approval_policy_override = Some(selection.approval_policy); + self.chat_widget + .set_approval_policy(selection.approval_policy); + self.chat_widget + .set_approvals_reviewer(ApprovalsReviewer::AutoReview); + if let Some(permission_profile) = permission_profile.as_ref() + && let Err(err) = self + .chat_widget + .set_permission_profile_from_session_snapshot( + PermissionProfileSnapshot::from_session_snapshot( + permission_profile.clone(), + active_permission_profile.clone(), + ), + ) + { + tracing::warn!(error = %err, "failed to apply Auto Review profile to chat config"); + self.chat_widget + .add_error_message(format!("Failed to enable Approve for me: {err}")); + return; + } + self.chat_widget.set_permission_network(permission_network); + if permission_profile.is_some() { + self.runtime_permission_profile_override = + Some(RuntimePermissionProfileOverride::from_config(&self.config)); + } + self.sync_active_thread_permission_settings_to_cached_session() + .await; + + let op = AppCommand::override_turn_context( + /*cwd*/ None, + Some(selection.approval_policy), + Some(ApprovalsReviewer::AutoReview), + permission_profile, + active_permission_profile, + /*windows_sandbox_level*/ None, + /*model*/ None, + /*effort*/ None, + /*summary*/ None, + /*service_tier*/ None, + /*collaboration_mode*/ None, + /*personality*/ None, + ); + let replay_state_op = + ThreadEventStore::op_can_change_pending_replay_state(&op).then(|| op.clone()); + let submitted = self.chat_widget.submit_op(op); + if submitted && let Some(op) = replay_state_op.as_ref() { + self.note_active_thread_outbound_op(op).await; + self.refresh_pending_thread_approvals().await; + } + + let network_enabled = self + .config + .permissions + .network_sandbox_policy() + .is_enabled(); + let message = if restore_managed_network { + format!( + "Permissions updated to {}; managed network access enabled", + selection.display_label + ) + } else if matches!( + selection.profile_update, + PermissionPresetProfileUpdate::Replace { .. } + ) { + format!("Permissions updated to {}", selection.display_label) + } else { + format!("Review mode updated to {}", selection.display_label) + }; + let hint = (terminal_browser_enabled && !network_enabled).then(|| { + "Network access remains disabled; the terminal browser is unavailable.".to_string() + }); + self.chat_widget.add_info_message(message, hint); + self.chat_widget.submit_initial_user_message_if_pending(); + } +} + +#[cfg(test)] +#[path = "auto_review_permissions_tests.rs"] +mod tests; diff --git a/codex-rs/tui/src/app/auto_review_permissions_tests.rs b/codex-rs/tui/src/app/auto_review_permissions_tests.rs new file mode 100644 index 0000000000..c5760ca1de --- /dev/null +++ b/codex-rs/tui/src/app/auto_review_permissions_tests.rs @@ -0,0 +1,256 @@ +use super::*; +use crate::app::test_support::make_test_app_with_channels; +use crate::chatwidget::tests::helpers::render_bottom_popup; +use crate::test_support::PathBufExt; +use assert_matches::assert_matches; +use codex_protocol::permissions::NetworkSandboxPolicy; +use crossterm::event::KeyCode; +use crossterm::event::KeyEvent; +use pretty_assertions::assert_eq; +use tempfile::tempdir; + +fn auto_review_selection() -> AutoReviewPresetSelection { + AutoReviewPresetSelection { + approval_policy: AskForApproval::OnRequest, + profile_update: PermissionPresetProfileUpdate::Preserve, + display_label: "Approve for me".to_string(), + } +} + +fn next_history_message(rx: &mut tokio::sync::mpsc::UnboundedReceiver) -> String { + let cell = std::iter::from_fn(|| rx.try_recv().ok()) + .find_map(|event| match event { + AppEvent::InsertHistoryCell(cell) => Some(cell), + _ => None, + }) + .expect("expected permission transition history"); + cell.display_lines(/*width*/ 120) + .into_iter() + .map(|line| line.to_string()) + .collect::>() + .join("\n") +} + +#[test] +fn terminal_browser_detects_user_network_restriction_over_managed_enabled_default() +-> color_eyre::eyre::Result<()> { + let config_dir = tempdir()?; + let user_config = config_dir.path().join("user-config.toml"); + let system_config = config_dir.path().join("system-config.toml"); + let response: ConfigReadResponse = serde_json::from_value(serde_json::json!({ + "config": { + "sandbox_workspace_write": { "network_access": false }, + }, + "origins": { + WORKSPACE_NETWORK_ACCESS_KEY: { + "name": { + "type": "user", + "file": user_config, + "profile": null, + }, + "version": "user-v1", + }, + }, + "layers": [ + { + "name": { + "type": "user", + "file": user_config, + "profile": null, + }, + "version": "user-v1", + "config": { + "sandbox_workspace_write": { "network_access": false }, + }, + }, + { + "name": { + "type": "system", + "file": system_config, + }, + "version": "system-v1", + "config": { + "sandbox_workspace_write": { "network_access": true }, + }, + }, + ], + }))?; + + assert!(user_network_restriction_overrides_managed_enabled( + &response + )); + Ok(()) +} + +#[tokio::test] +async fn terminal_browser_auto_review_restores_managed_network_override_before_reporting_success() +-> color_eyre::eyre::Result<()> { + let (mut app, mut app_event_rx, mut op_rx) = make_test_app_with_channels().await; + let config_dir = tempdir()?; + let user_config_path = config_dir.path().join("config.toml").abs(); + let system_config_path = config_dir.path().join("system-config.toml"); + std::fs::write( + &system_config_path, + r#" +sandbox_mode = "workspace-write" + +[sandbox_workspace_write] +network_access = true + +[features] +guardian_approval = true +terminal_browser = true +"#, + )?; + std::fs::write( + user_config_path.as_path(), + r#" +approvals_reviewer = "auto_review" +approval_policy = "on-request" +sandbox_mode = "workspace-write" + +[sandbox_workspace_write] +network_access = false + +[features] +guardian_approval = true +terminal_browser = true +"#, + )?; + app.config.codex_home = config_dir.path().to_path_buf().abs(); + app.loader_overrides.user_config_path = Some(user_config_path.clone()); + app.loader_overrides.system_config_path = Some(system_config_path); + let rebuilt = app + .rebuild_config_for_cwd(app.config.cwd.to_path_buf()) + .await?; + assert_eq!( + rebuilt.permissions.network_sandbox_policy(), + NetworkSandboxPolicy::Restricted + ); + app.config = rebuilt.clone(); + app.chat_widget + .set_feature_enabled(Feature::GuardianApproval, /*enabled*/ true); + app.chat_widget + .set_feature_enabled(Feature::TerminalBrowser, /*enabled*/ true); + app.chat_widget + .set_permission_profile_from_session_snapshot( + PermissionProfileSnapshot::from_session_snapshot( + rebuilt.permissions.permission_profile().clone(), + rebuilt.permissions.active_permission_profile(), + ), + )?; + app.chat_widget + .set_permission_network(rebuilt.permissions.network.clone()); + let mut app_server = crate::start_embedded_app_server_for_picker(&app.config).await?; + + app.apply_auto_review_preset( + &mut app_server, + auto_review_selection(), + ManagedNetworkChoice::Detect, + ) + .await; + + let popup = render_bottom_popup(&app.chat_widget, /*width*/ 100); + assert!(popup.contains("Enable managed network access?"), "{popup}"); + app.chat_widget + .handle_key_event(KeyEvent::from(KeyCode::Enter)); + let (selection, network_choice) = std::iter::from_fn(|| app_event_rx.try_recv().ok()) + .find_map(|event| match event { + AppEvent::ApplyAutoReviewPreset { + selection, + network_choice, + } => Some((selection, network_choice)), + _ => None, + }) + .expect("restore confirmation should emit an Auto Review transition"); + assert_eq!(network_choice, ManagedNetworkChoice::RestoreManaged); + + app.apply_auto_review_preset(&mut app_server, selection, network_choice) + .await; + + assert!(app.config.permissions.network_sandbox_policy().is_enabled()); + assert!( + app.chat_widget + .config_ref() + .permissions + .network_sandbox_policy() + .is_enabled() + ); + assert_matches!( + op_rx.try_recv().expect("expected permission context update"), + AppCommand::OverrideTurnContext { + approvals_reviewer: Some(ApprovalsReviewer::AutoReview), + permission_profile: Some(profile), + .. + } if profile.network_sandbox_policy().is_enabled() + ); + let rendered = next_history_message(&mut app_event_rx); + assert!( + rendered.contains("Permissions updated to Approve for me; managed network access enabled"), + "{rendered}" + ); + let user_config = std::fs::read_to_string(user_config_path.as_path())?; + assert!(!user_config.contains("network_access = false")); + app_server.shutdown().await?; + Ok(()) +} + +#[tokio::test] +async fn terminal_browser_auto_review_keep_restricted_reports_review_only() +-> color_eyre::eyre::Result<()> { + let (mut app, mut app_event_rx, mut op_rx) = make_test_app_with_channels().await; + let codex_home = tempdir()?; + app.config.codex_home = codex_home.path().to_path_buf().abs(); + app.config + .features + .enable(Feature::TerminalBrowser) + .expect("terminal browser feature should be mutable"); + app.chat_widget + .set_feature_enabled(Feature::TerminalBrowser, /*enabled*/ true); + assert_eq!( + app.config.permissions.network_sandbox_policy(), + NetworkSandboxPolicy::Restricted + ); + let permission_profile = app.config.permissions.permission_profile().clone(); + let mut app_server = crate::start_embedded_app_server_for_picker(&app.config).await?; + + app.apply_auto_review_preset( + &mut app_server, + auto_review_selection(), + ManagedNetworkChoice::KeepRestricted, + ) + .await; + + assert_eq!( + app.config.permissions.permission_profile(), + &permission_profile + ); + assert_eq!( + op_rx.try_recv(), + Ok(AppCommand::OverrideTurnContext { + cwd: None, + approval_policy: Some(AskForApproval::OnRequest), + approvals_reviewer: Some(ApprovalsReviewer::AutoReview), + permission_profile: None, + active_permission_profile: None, + windows_sandbox_level: None, + model: None, + effort: None, + summary: None, + service_tier: None, + collaboration_mode: None, + personality: None, + }) + ); + let rendered = next_history_message(&mut app_event_rx); + assert!( + rendered.contains("Review mode updated to Approve for me"), + "{rendered}" + ); + assert!( + rendered.contains("Network access remains disabled"), + "{rendered}" + ); + app_server.shutdown().await?; + Ok(()) +} diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index 7a25fb242a..f28338ab2c 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -2014,6 +2014,13 @@ impl App { .add_error_message(format!("Failed to save approvals reviewer: {err}")); } } + AppEvent::ApplyAutoReviewPreset { + selection, + network_choice, + } => { + self.apply_auto_review_preset(app_server, selection, network_choice) + .await; + } AppEvent::UpdateFeatureFlags { updates } => { self.update_feature_flags(app_server, updates).await; } diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index 68e40ced16..5fb6fa0dd7 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -54,6 +54,7 @@ use codex_plugin::PluginCapabilitySummary; use codex_protocol::config_types::CollaborationModeMask; use codex_protocol::config_types::Personality; use codex_protocol::models::ActivePermissionProfile; +use codex_protocol::models::PermissionProfile; use codex_protocol::openai_models::ReasoningEffort; use crate::history_cell::HistoryCell; @@ -1010,6 +1011,12 @@ pub(crate) enum AppEvent { /// Update the current approvals reviewer in the running app and widget. UpdateApprovalsReviewer(ApprovalsReviewer), + /// Apply the Auto Review preset after resolving any managed-network override. + ApplyAutoReviewPreset { + selection: AutoReviewPresetSelection, + network_choice: ManagedNetworkChoice, + }, + /// Update feature flags and persist them to the top-level config. UpdateFeatureFlags { updates: Vec<(Feature, bool)>, @@ -1252,6 +1259,32 @@ pub(crate) struct PermissionProfileSelection { pub display_label: String, } +/// Permission-profile portion of an Auto Review preset transition. +#[derive(Debug, Clone)] +pub(crate) enum PermissionPresetProfileUpdate { + Preserve, + Replace { + permission_profile: PermissionProfile, + active_permission_profile: ActivePermissionProfile, + }, +} + +/// Complete Auto Review selection passed from the permissions popup to the app layer. +#[derive(Debug, Clone)] +pub(crate) struct AutoReviewPresetSelection { + pub(crate) approval_policy: AskForApproval, + pub(crate) profile_update: PermissionPresetProfileUpdate, + pub(crate) display_label: String, +} + +/// How an Auto Review transition should handle a user network override. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum ManagedNetworkChoice { + Detect, + RestoreManaged, + KeepRestricted, +} + /// The exit strategy requested by the UI layer. /// /// Most user-initiated exits should use `ShutdownFirst` so core cleanup runs and the UI exits only diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index a94b467f10..9c7fdfa439 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -258,9 +258,12 @@ fn normalize_thread_name(name: &str) -> Option { } use crate::app_event::AppEvent; +use crate::app_event::AutoReviewPresetSelection; use crate::app_event::ConversationOrigin; use crate::app_event::ExitMode; +use crate::app_event::ManagedNetworkChoice; use crate::app_event::PaneSlot; +use crate::app_event::PermissionPresetProfileUpdate; use crate::app_event::PermissionProfileSelection; use crate::app_event::RateLimitRefreshOrigin; #[cfg(target_os = "windows")] @@ -480,15 +483,6 @@ const DEFAULT_OPENAI_BASE_URL: &str = "https://api.openai.com/v1"; const DEFAULT_STATUS_LINE_ITEMS: [&str; 2] = ["model-with-reasoning", "current-dir"]; const MAX_AGENT_COPY_HISTORY: usize = 32; -#[derive(Clone)] -enum PermissionPresetProfileUpdate { - Preserve, - Replace { - permission_profile: PermissionProfile, - active_permission_profile: ActivePermissionProfile, - }, -} - /// Common initialization parameters shared by all `ChatWidget` constructors. pub(crate) struct ChatWidgetInit { pub(crate) config: Config, diff --git a/codex-rs/tui/src/chatwidget/permission_popups.rs b/codex-rs/tui/src/chatwidget/permission_popups.rs index b15144b0fb..4fa8d8b9bf 100644 --- a/codex-rs/tui/src/chatwidget/permission_popups.rs +++ b/codex-rs/tui/src/chatwidget/permission_popups.rs @@ -244,6 +244,19 @@ impl ChatWidget { label: String, approvals_reviewer: ApprovalsReviewer, ) -> Vec { + if approvals_reviewer == ApprovalsReviewer::AutoReview { + return vec![Box::new(move |tx| { + tx.send(AppEvent::ApplyAutoReviewPreset { + selection: AutoReviewPresetSelection { + approval_policy: approval, + profile_update: profile_update.clone(), + display_label: label.clone(), + }, + network_choice: ManagedNetworkChoice::Detect, + }); + })]; + } + vec![Box::new(move |tx| { let (permission_profile, active_permission_profile) = match &profile_update { PermissionPresetProfileUpdate::Preserve => (None, None), @@ -282,13 +295,68 @@ impl ChatWidget { tx.send(AppEvent::UpdateApprovalsReviewer(approvals_reviewer)); tx.send(AppEvent::InsertHistoryCell(Box::new( history_cell::new_info_event( - format!("Permissions updated to {label}"), + match profile_update { + PermissionPresetProfileUpdate::Preserve => { + format!("Review mode updated to {label}") + } + PermissionPresetProfileUpdate::Replace { .. } => { + format!("Permissions updated to {label}") + } + }, /*hint*/ None, ), ))); })] } + pub(crate) fn open_managed_network_restore_confirmation( + &mut self, + selection: AutoReviewPresetSelection, + ) { + let restore_selection = selection.clone(); + let keep_selection = selection; + self.bottom_pane.show_selection_view(SelectionViewParams { + title: Some("Enable managed network access?".to_string()), + subtitle: Some( + "Your user config disables the managed network required by the terminal browser." + .to_string(), + ), + footer_hint: Some(standard_popup_hint_line()), + items: vec![ + SelectionItem { + name: "Restore managed network access".to_string(), + description: Some( + "Remove the user override and inherit the managed network policy" + .to_string(), + ), + actions: vec![Box::new(move |tx| { + tx.send(AppEvent::ApplyAutoReviewPreset { + selection: restore_selection.clone(), + network_choice: ManagedNetworkChoice::RestoreManaged, + }); + })], + dismiss_on_select: true, + ..Default::default() + }, + SelectionItem { + name: "Keep network disabled".to_string(), + description: Some( + "Use Auto Review without enabling the terminal browser".to_string(), + ), + actions: vec![Box::new(move |tx| { + tx.send(AppEvent::ApplyAutoReviewPreset { + selection: keep_selection.clone(), + network_choice: ManagedNetworkChoice::KeepRestricted, + }); + })], + dismiss_on_select: true, + ..Default::default() + }, + ], + ..Default::default() + }); + } + pub(super) fn permission_profile_selection_actions( selection: PermissionProfileSelection, ) -> Vec { diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__terminal_browser_managed_network_restore_confirmation.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__terminal_browser_managed_network_restore_confirmation.snap new file mode 100644 index 0000000000..e8cc3ca156 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__terminal_browser_managed_network_restore_confirmation.snap @@ -0,0 +1,12 @@ +--- +source: tui/src/chatwidget/tests/permissions.rs +expression: "render_bottom_popup(&chat, 100)" +--- + Enable managed network access? + Your user config disables the managed network required by the terminal browser. + +› 1. Restore managed network access Remove the user override and inherit the managed network + policy + 2. Keep network disabled Use Auto Review without enabling the terminal browser + + 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 8e4231a900..3ef39d5373 100644 --- a/codex-rs/tui/src/chatwidget/tests/permissions.rs +++ b/codex-rs/tui/src/chatwidget/tests/permissions.rs @@ -89,6 +89,22 @@ async fn approvals_selection_popup_snapshot() { assert_chatwidget_snapshot!("approvals_selection_popup", popup); } +#[tokio::test] +async fn terminal_browser_managed_network_restore_confirmation_snapshot() { + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + + chat.open_managed_network_restore_confirmation(AutoReviewPresetSelection { + approval_policy: AskForApproval::OnRequest, + profile_update: PermissionPresetProfileUpdate::Preserve, + display_label: "Approve for me".to_string(), + }); + + assert_chatwidget_snapshot!( + "terminal_browser_managed_network_restore_confirmation", + render_bottom_popup(&chat, /*width*/ 100) + ); +} + #[tokio::test] async fn profile_permissions_selection_popup_snapshot() { let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; @@ -745,7 +761,7 @@ async fn approvals_popup_navigation_skips_disabled() { } #[tokio::test] -async fn permissions_selection_emits_history_cell_when_selection_changes() { +async fn permissions_auto_review_selection_defers_history_until_app_transition() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; #[cfg(target_os = "windows")] { @@ -758,16 +774,18 @@ async fn permissions_selection_emits_history_cell_when_selection_changes() { chat.handle_key_event(KeyEvent::from(KeyCode::Down)); chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); - let cells = drain_insert_history(&mut rx); - assert_eq!( - cells.len(), - 1, - "expected one permissions selection history cell" - ); - let rendered = lines_to_single_string(&cells[0]); + let events = std::iter::from_fn(|| rx.try_recv().ok()).collect::>(); assert!( - rendered.contains("Permissions updated to"), - "expected permissions selection history message, got: {rendered}" + events + .iter() + .any(|event| matches!(event, AppEvent::ApplyAutoReviewPreset { .. })), + "expected app-owned Auto Review transition: {events:?}" + ); + assert!( + events + .iter() + .all(|event| !matches!(event, AppEvent::InsertHistoryCell(_))), + "history must wait until the app applies and persists Auto Review: {events:?}" ); } @@ -868,8 +886,8 @@ async fn permissions_selection_emits_history_cell_when_current_is_selected() { ); let rendered = lines_to_single_string(&cells[0]); assert!( - rendered.contains("Permissions updated to"), - "expected permissions update history message, got: {rendered}" + rendered.contains("Review mode updated to Ask for approval"), + "expected reviewer-only history message, got: {rendered}" ); } @@ -1106,40 +1124,27 @@ async fn terminal_browser_auto_review_preserves_network_enabled_workspace_profil ); chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); - let op = std::iter::from_fn(|| rx.try_recv().ok()) + let (selection, network_choice) = std::iter::from_fn(|| rx.try_recv().ok()) .find_map(|event| match event { - AppEvent::CodexOp(op @ Op::OverrideTurnContext { .. }) => Some(op), + AppEvent::ApplyAutoReviewPreset { + selection, + network_choice, + } => Some((selection, network_choice)), _ => None, }) - .expect("expected OverrideTurnContext op"); - - assert_eq!( - op, - Op::OverrideTurnContext { - cwd: None, - approval_policy: Some(AskForApproval::OnRequest), - approvals_reviewer: Some(ApprovalsReviewer::AutoReview), - permission_profile: None, - active_permission_profile: None, - windows_sandbox_level: None, - model: None, - effort: None, - summary: None, - service_tier: None, - collaboration_mode: None, - personality: None, - } - ); + .expect("expected app-owned Auto Review transition"); + assert_eq!(selection.approval_policy, AskForApproval::OnRequest); + assert!(matches!( + selection.profile_update, + PermissionPresetProfileUpdate::Preserve + )); + assert_eq!(selection.display_label, "Approve for me"); + assert_eq!(network_choice, ManagedNetworkChoice::Detect); assert_eq!( chat.config.permissions.permission_profile(), &permission_profile ); - assert!( - std::iter::from_fn(|| rx.try_recv().ok()) - .all(|event| !matches!(event, AppEvent::UpdateActivePermissionProfile(_))), - "reviewer-only changes must not replace the effective workspace profile" - ); } #[tokio::test] diff --git a/codex-rs/tui/src/config_update.rs b/codex-rs/tui/src/config_update.rs index a71f34b52b..77b1c24f9c 100644 --- a/codex-rs/tui/src/config_update.rs +++ b/codex-rs/tui/src/config_update.rs @@ -173,13 +173,28 @@ pub(crate) async fn write_trusted_project( pub(crate) async fn read_effective_config( request_handle: AppServerRequestHandle, cwd: String, +) -> Result { + read_effective_config_with_layer_option(request_handle, cwd, /*include_layers*/ false).await +} + +pub(crate) async fn read_effective_config_with_layers( + request_handle: AppServerRequestHandle, + cwd: String, +) -> Result { + read_effective_config_with_layer_option(request_handle, cwd, /*include_layers*/ true).await +} + +async fn read_effective_config_with_layer_option( + request_handle: AppServerRequestHandle, + cwd: String, + include_layers: bool, ) -> Result { let request_id = RequestId::String(format!("tui-config-read-{}", Uuid::new_v4())); request_handle .request_typed(ClientRequest::ConfigRead { request_id, params: ConfigReadParams { - include_layers: false, + include_layers, cwd: Some(cwd), }, })