diff --git a/codex-rs/config/src/loader/mod.rs b/codex-rs/config/src/loader/mod.rs index 31cd64e75b..7e82fc9e34 100644 --- a/codex-rs/config/src/loader/mod.rs +++ b/codex-rs/config/src/loader/mod.rs @@ -1127,6 +1127,19 @@ fn sanitize_project_config(config: &mut TomlValue) -> Vec { { ignored_keys.push("features.respect_system_proxy".to_string()); } + // Repository contents must not turn an ordinary key into a permission increase. + if let Some(chat) = table + .get_mut("tui") + .and_then(|tui| tui.get_mut("keymap")) + .and_then(|keymap| keymap.get_mut("chat")) + .and_then(TomlValue::as_table_mut) + { + for key in ["previous_permission_mode", "next_permission_mode"] { + if chat.remove(key).is_some() { + ignored_keys.push(format!("tui.keymap.chat.{key}")); + } + } + } ignored_keys } diff --git a/codex-rs/config/src/loader/tests.rs b/codex-rs/config/src/loader/tests.rs index 30cdd4be22..92c3102a77 100644 --- a/codex-rs/config/src/loader/tests.rs +++ b/codex-rs/config/src/loader/tests.rs @@ -19,6 +19,19 @@ use tempfile::tempdir; pub(super) struct TestFileSystem; +#[test] +fn project_config_cannot_bind_permission_shortcuts() { + let safe = "[tui.keymap.chat]\nincrease_reasoning_effort = 'f9'\n"; + for key in ["previous_permission_mode", "next_permission_mode"] { + let mut config = toml::from_str(&format!("{safe}{key} = 'page-down'")).unwrap(); + assert_eq!( + sanitize_project_config(&mut config), + [format!("tui.keymap.chat.{key}")] + ); + assert_eq!(config, toml::from_str::(safe).unwrap()); + } +} + #[tokio::test] async fn managed_browser_import_denial_survives_user_and_session_config() { let tmp = tempdir().expect("tempdir"); diff --git a/codex-rs/config/src/tui_keymap.rs b/codex-rs/config/src/tui_keymap.rs index 2180596a36..942bbaf4c8 100644 --- a/codex-rs/config/src/tui_keymap.rs +++ b/codex-rs/config/src/tui_keymap.rs @@ -129,6 +129,10 @@ pub struct TuiChatKeymap { pub decrease_reasoning_effort: Option, /// Increase the active reasoning effort. pub increase_reasoning_effort: Option, + /// Switch to the previous available permission mode. + pub previous_permission_mode: Option, + /// Switch to the next available permission mode. + pub next_permission_mode: Option, /// Edit the most recently queued message. pub edit_queued_message: Option, } diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 723cb827cf..5128c49fca 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -3571,7 +3571,9 @@ "decrease_reasoning_effort": null, "edit_queued_message": null, "increase_reasoning_effort": null, - "interrupt_turn": null + "interrupt_turn": null, + "next_permission_mode": null, + "previous_permission_mode": null }, "composer": { "history_search_next": null, @@ -3967,6 +3969,22 @@ } ], "description": "Interrupt the active turn." + }, + "next_permission_mode": { + "allOf": [ + { + "$ref": "#/definitions/KeybindingsSpec" + } + ], + "description": "Switch to the next available permission mode." + }, + "previous_permission_mode": { + "allOf": [ + { + "$ref": "#/definitions/KeybindingsSpec" + } + ], + "description": "Switch to the previous available permission mode." } }, "type": "object" @@ -4309,7 +4327,9 @@ "decrease_reasoning_effort": null, "edit_queued_message": null, "increase_reasoning_effort": null, - "interrupt_turn": null + "interrupt_turn": null, + "next_permission_mode": null, + "previous_permission_mode": null } }, "composer": { diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 70d147de53..30e4ece2f6 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -223,6 +223,7 @@ mod history_ui; mod input; mod loaded_threads; mod pending_interactive_replay; +mod permission_shortcuts; mod pets; mod platform_actions; mod plugin_mentions; diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index e967543b8e..7aaea2109f 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -1519,6 +1519,9 @@ impl App { profile_selection, ); } + AppEvent::ApplyPermissionShortcut { thread_id, selection } => { + self.apply_permission_shortcut(app_server, tui, thread_id, selection).await; + } AppEvent::OpenWorldWritableWarningConfirmation { preset, profile_selection, diff --git a/codex-rs/tui/src/app/permission_shortcuts.rs b/codex-rs/tui/src/app/permission_shortcuts.rs new file mode 100644 index 0000000000..8922636d3d --- /dev/null +++ b/codex-rs/tui/src/app/permission_shortcuts.rs @@ -0,0 +1,71 @@ +//! Confirm session-only permission changes before updating local state. + +use super::*; +use codex_app_server_protocol::ThreadSettingsUpdateParams; + +impl App { + pub(super) async fn apply_permission_shortcut( + &mut self, + app_server: &mut AppServerSession, + tui: &mut tui::Tui, + thread_id: ThreadId, + selection: PermissionProfileSelection, + ) { + if self.current_displayed_thread_id() != Some(thread_id) + || self.chat_widget.thread_id() != Some(thread_id) + { + 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) + .ok_or_else(|| color_eyre::eyre::eyre!("unknown built-in permission profile"))?; + let mut config = self.chat_widget.config_ref().clone(); + if let Some(policy) = selection.approval_policy { + config.permissions.approval_policy.set(policy.to_core())?; + } + if let Some(reviewer) = selection.approvals_reviewer { + config.config_layer_stack.requirements().approvals_reviewer.can_set(&reviewer)?; + config.approvals_reviewer = reviewer; + } + config.permissions.set_permission_profile_from_session_snapshot( + PermissionProfileSnapshot::active(profile, active_profile), + )?; + + if !app_server.thread_settings_update(ThreadSettingsUpdateParams { + thread_id: thread_id.to_string(), + permissions: Some(selection.profile_id), + approval_policy: selection.approval_policy, + approvals_reviewer: selection.approvals_reviewer.map(Into::into), + ..Default::default() + }).await? { + color_eyre::eyre::bail!("this app server does not support confirmed permission changes; use /permissions"); + } + + // This exact widget configuration was validated before the request. + self.chat_widget.set_permission_profile_with_active_profile( + config.permissions.permission_profile().clone(), + config.permissions.active_permission_profile(), + )?; + self.chat_widget.set_approval_policy(AskForApproval::from(config.permissions.approval_policy.value())); + self.config.permissions = config.permissions.clone(); + self.set_approvals_reviewer_in_app_and_widget(config.approvals_reviewer); + self.runtime_approval_policy_override = selection.approval_policy; + self.runtime_permission_profile_override = Some(RuntimePermissionProfileOverride::from_config(&config)); + self.sync_active_thread_permission_settings_to_cached_session().await; + self.insert_history_cell(tui, Box::new(history_cell::new_info_event(format!("Permissions updated to {}", selection.display_label), /*hint*/ None))); + Ok(()) + }.await; + self.chat_widget.complete_permission_shortcut(thread_id); + if let Err(err) = result { + let error = crate::config_update::format_config_error(&err); + self.insert_history_cell( + tui, + Box::new(history_cell::new_error_event(format!( + "Failed to update permissions: {error}" + ))), + ); + } + } +} diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index 7b043346f0..71b2c47337 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -11,6 +11,8 @@ mod key_chords; #[path = "tests/mcp_startup.rs"] mod mcp_startup; mod model_catalog; +#[path = "tests/permission_shortcuts_tests.rs"] +mod permission_shortcuts_tests; mod plugin_catalog; mod rate_limits; mod safety_buffering; diff --git a/codex-rs/tui/src/app/tests/permission_shortcuts_tests.rs b/codex-rs/tui/src/app/tests/permission_shortcuts_tests.rs new file mode 100644 index 0000000000..a15b2a0966 --- /dev/null +++ b/codex-rs/tui/src/app/tests/permission_shortcuts_tests.rs @@ -0,0 +1,138 @@ +use super::*; +use codex_arg0::Arg0DispatchPaths; +use pretty_assertions::assert_eq; + +fn read_only_selection() -> PermissionProfileSelection { + PermissionProfileSelection { + profile_id: ":read-only".to_string(), + approval_policy: Some(AskForApproval::OnRequest), + approvals_reviewer: Some(ApprovalsReviewer::User), + display_label: "Read Only".to_string(), + } +} + +#[tokio::test] +async fn permission_shortcut_rejections_leave_state_unchanged() -> Result<()> { + for experimental_api in [false, true] { + let (mut app, mut events, _op_rx) = make_test_app_with_channels().await; + let mut tui = crate::tui::test_support::make_test_tui()?; + let thread_id = ThreadId::new(); + app.active_thread_id = Some(thread_id); + app.chat_widget + .handle_thread_session_quiet(test_thread_session( + thread_id, + app.config.cwd.to_path_buf(), + )); + let original = RuntimePermissionProfileOverride::from_config(app.chat_widget.config_ref()); + let original_reviewer = app.config.approvals_reviewer; + let client = crate::start_embedded_app_server_with( + Arg0DispatchPaths::default(), + app.config.clone(), + Vec::new(), + LoaderOverrides::without_managed_config_for_tests(), + /*strict_config*/ false, + CloudConfigBundleLoader::default(), + codex_feedback::CodexFeedback::new(), + /*log_db*/ None, + /*state_db*/ None, + Arc::clone(&app.environment_manager), + |mut args| { + args.experimental_api = experimental_api; + codex_app_server_client::InProcessAppServerClient::start(args) + }, + ) + .await?; + let mut app_server = AppServerSession::new( + codex_app_server_client::AppServerClient::InProcess(client), + crate::app_server_session::ThreadParamsMode::Embedded, + ); + while events.try_recv().is_ok() {} + let transcript_len = app.transcript_cells.len(); + app.apply_permission_shortcut( + &mut app_server, + &mut tui, + ThreadId::new(), + read_only_selection(), + ) + .await; + assert_eq!(app.transcript_cells.len(), transcript_len); + assert!(events.try_recv().is_err()); + app.apply_permission_shortcut(&mut app_server, &mut tui, thread_id, read_only_selection()) + .await; + assert_eq!( + RuntimePermissionProfileOverride::from_config(app.chat_widget.config_ref()), + original + ); + assert_eq!(app.config.approvals_reviewer, original_reviewer); + let cell = app.transcript_cells.last().expect("rejection notice"); + insta::assert_snapshot!( + if experimental_api { + "permission_shortcut_server_error" + } else { + "permission_shortcut_unsupported" + }, + lines_to_single_string(&cell.display_lines(/*width*/ 120)) + .replace(&thread_id.to_string(), "") + ); + assert!(events.try_recv().is_err()); + app_server.shutdown().await?; + } + Ok(()) +} + +#[tokio::test] +async fn permission_shortcut_confirms_without_persisting() -> Result<()> { + let (mut app, mut events, _op_rx) = make_test_app_with_channels().await; + let mut tui = crate::tui::test_support::make_test_tui()?; + let codex_home = tempdir()?; + app.config.codex_home = codex_home.path().to_path_buf().abs(); + app.config + .permissions + .set_permission_profile_from_session_snapshot(PermissionProfileSnapshot::active( + PermissionProfile::workspace_write(), + ActivePermissionProfile::new(":workspace"), + ))?; + let config_path = codex_home.path().join("config.toml"); + let contents = "approvals_reviewer = \"auto_review\"\n"; + std::fs::write(&config_path, contents)?; + let mut app_server = start_config_write_test_app_server(&app).await?; + let started = app_server.start_thread(&app.config).await?; + let thread_id = started.session.thread_id; + app.chat_widget + .handle_thread_session_quiet(started.session.clone()); + app.enqueue_primary_thread_session(started.session, started.turns) + .await?; + let contents = std::fs::read_to_string(&config_path)?; + while events.try_recv().is_ok() {} + + app.apply_permission_shortcut(&mut app_server, &mut tui, thread_id, read_only_selection()) + .await; + + let cell = app.transcript_cells.last().expect("confirmed notice"); + insta::assert_snapshot!( + lines_to_single_string(&cell.display_lines(/*width*/ 80)), + @"• Permissions updated to Read Only" + ); + + let settings = next_thread_settings_updated(&mut app_server, thread_id) + .await + .thread_settings; + let profile = app + .chat_widget + .config_ref() + .permissions + .active_permission_profile(); + assert_eq!( + settings.active_permission_profile, + profile.clone().map(Into::into) + ); + assert_eq!(profile, Some(ActivePermissionProfile::new(":read-only"))); + assert_eq!(app.config.approvals_reviewer, ApprovalsReviewer::User); + assert_eq!(std::fs::read_to_string(config_path)?, contents); + assert!( + events.try_recv().is_err(), + "must not queue another update or config write" + ); + app_server.shutdown().await?; + Ok(()) +} diff --git a/codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__permission_shortcuts_tests__permission_shortcut_server_error.snap b/codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__permission_shortcuts_tests__permission_shortcut_server_error.snap new file mode 100644 index 0000000000..9b439c6614 --- /dev/null +++ b/codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__permission_shortcuts_tests__permission_shortcut_server_error.snap @@ -0,0 +1,6 @@ +--- +source: tui/src/app/tests/permission_shortcuts_tests.rs +assertion_line: 63 +expression: "lines_to_single_string(&cell.display_lines(120)).replace(&missing_thread.to_string(),\n\"\")" +--- +■ Failed to update permissions: thread/settings/update failed in TUI: thread/settings/update failed: thread not found: (code -32600) diff --git a/codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__permission_shortcuts_tests__permission_shortcut_unsupported.snap b/codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__permission_shortcuts_tests__permission_shortcut_unsupported.snap new file mode 100644 index 0000000000..310cad4785 --- /dev/null +++ b/codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__permission_shortcuts_tests__permission_shortcut_unsupported.snap @@ -0,0 +1,6 @@ +--- +source: tui/src/app/tests/permission_shortcuts_tests.rs +assertion_line: 33 +expression: lines_to_single_string(&cell.display_lines(120)) +--- +■ Failed to update permissions: this app server does not support confirmed permission changes; use /permissions diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index a22dd9db8a..46b18ba8f3 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -948,6 +948,12 @@ pub(crate) enum AppEvent { profile_selection: Option, }, + /// Apply a permission shortcut only while its originating thread is displayed. + ApplyPermissionShortcut { + thread_id: ThreadId, + selection: PermissionProfileSelection, + }, + /// Open the Windows world-writable directories warning. /// If `preset` is `Some`, the confirmation will apply the provided /// approval/sandbox configuration on Continue; if `None`, it performs no diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 331b347bf3..b9b5e0f0ef 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -389,6 +389,7 @@ mod model_popups; mod notifications; use self::notifications::Notification; mod permission_popups; +mod permission_shortcuts; mod permissions_menu; pub(crate) use self::permissions_menu::auto_review_available; pub(crate) use self::permissions_menu::cyber_model_approval_reviewer; @@ -696,6 +697,7 @@ pub(crate) struct ChatWidget { safety_buffering_prompt: Option, /// Main chat-surface bindings resolved from `tui.keymap.chat`. chat_keymap: ChatKeymap, + permission_shortcut_pending: bool, /// Keybinding to show for popping the most-recently queued message back /// into the composer. This may differ from the first configured binding /// when the default set includes a terminal-specific fallback. diff --git a/codex-rs/tui/src/chatwidget/constructor.rs b/codex-rs/tui/src/chatwidget/constructor.rs index e772e5f481..6cbac52b19 100644 --- a/codex-rs/tui/src/chatwidget/constructor.rs +++ b/codex-rs/tui/src/chatwidget/constructor.rs @@ -211,6 +211,7 @@ impl ChatWidget { input_queue: InputQueueState::default(), safety_buffering_prompt: None, chat_keymap, + permission_shortcut_pending: false, queued_message_edit_hint_binding, show_welcome_banner: is_first_run, startup_tooltip_override, diff --git a/codex-rs/tui/src/chatwidget/interaction.rs b/codex-rs/tui/src/chatwidget/interaction.rs index dffb1e37df..255d519455 100644 --- a/codex-rs/tui/src/chatwidget/interaction.rs +++ b/codex-rs/tui/src/chatwidget/interaction.rs @@ -36,7 +36,7 @@ impl ChatWidget { return; } - if self.handle_reasoning_shortcut(key_event) { + if self.handle_reasoning_shortcut(key_event) || self.handle_permission_shortcut(key_event) { self.bottom_pane.clear_quit_shortcut_hint(); self.quit_shortcut_expires_at = None; self.quit_shortcut_key = None; diff --git a/codex-rs/tui/src/chatwidget/permission_shortcuts.rs b/codex-rs/tui/src/chatwidget/permission_shortcuts.rs new file mode 100644 index 0000000000..499dbe4b90 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/permission_shortcuts.rs @@ -0,0 +1,130 @@ +//! Session-scoped shortcuts for the ordinary built-in permission modes. + +use super::*; + +impl ChatWidget { + pub(super) fn handle_permission_shortcut(&mut self, key_event: KeyEvent) -> bool { + let forward = if self.chat_keymap.next_permission_mode.is_pressed(key_event) { + true + } else if self + .chat_keymap + .previous_permission_mode + .is_pressed(key_event) + { + false + } else { + return false; + }; + if !self.bottom_pane.no_modal_or_popup_active() { + return false; + } + if self.permission_shortcut_pending { + return true; + } + if self.blocks_direct_input { + self.add_error_message(PARENT_OWNED_INPUT_MESSAGE.to_string()); + return true; + } + let Some(thread_id) = self.thread_id else { + return true; + }; + + let current_approval = + AskForApproval::from(self.config.permissions.approval_policy.value()); + let active_profile = self.config.permissions.active_permission_profile(); + let mut choices = Vec::new(); + for preset in builtin_approval_presets() { + if !matches!(preset.id, "read-only" | "auto") { + continue; + } + for reviewer in [ApprovalsReviewer::User, ApprovalsReviewer::AutoReview] { + if reviewer == ApprovalsReviewer::AutoReview + && (preset.id != "auto" + || !self.config.features.enabled(Feature::GuardianApproval)) + { + continue; + } + let approval = AskForApproval::from(preset.approval); + let requirements = self.config.config_layer_stack.requirements(); + if self + .permission_mode_disabled_reason(&preset, approval) + .is_some() + || requirements.approvals_reviewer.can_set(&reviewer).is_err() + || (requirements.auto_review_required_for_model(self.current_model()) + && reviewer != ApprovalsReviewer::AutoReview) + { + continue; + } + // These modes still need the explicit Windows setup/warning flow. + #[cfg(target_os = "windows")] + if preset.id == "auto" + && reviewer == ApprovalsReviewer::User + && (crate::windows_sandbox::level_from_config(&self.config) + == WindowsSandboxLevel::Disabled + || self.world_writable_warning_details().is_some()) + { + continue; + } + let is_current = current_approval == approval + && self.config.approvals_reviewer == reviewer + && active_profile.as_ref().map_or_else( + || { + Self::preset_matches_current( + current_approval, + self.config.permissions.permission_profile(), + self.config.cwd.as_path(), + &preset, + ) + }, + |active| active.id == preset.active_permission_profile.id, + ); + let label = match (preset.id, reviewer) { + ("auto", ApprovalsReviewer::User) => ASK_FOR_APPROVAL_LABEL, + ("auto", ApprovalsReviewer::AutoReview) => APPROVE_FOR_ME_LABEL, + _ => preset.label, + }; + choices.push(( + is_current, + PermissionProfileSelection { + profile_id: preset.active_permission_profile.id.clone(), + approval_policy: Some(approval), + approvals_reviewer: Some(reviewer), + display_label: label.to_string(), + }, + )); + } + } + if !forward { + choices.reverse(); + } + let start = choices + .iter() + .position(|(current, _)| *current) + .map_or(0, |index| index + 1); + if let Some((_, selection)) = choices + .iter() + .cycle() + .skip(start) + .take(choices.len()) + .find(|(current, _)| !current) + { + self.permission_shortcut_pending = true; + self.app_event_tx.send(AppEvent::ApplyPermissionShortcut { + thread_id, + selection: selection.clone(), + }); + } else { + self.add_info_message( + "No other permission modes are available.".to_string(), + /*hint*/ None, + ); + } + true + } + + pub(crate) fn complete_permission_shortcut(&mut self, thread_id: ThreadId) { + if self.thread_id == Some(thread_id) { + self.permission_shortcut_pending = false; + } + } +} diff --git a/codex-rs/tui/src/chatwidget/permissions_menu.rs b/codex-rs/tui/src/chatwidget/permissions_menu.rs index 2bf0bf733e..49251f8736 100644 --- a/codex-rs/tui/src/chatwidget/permissions_menu.rs +++ b/codex-rs/tui/src/chatwidget/permissions_menu.rs @@ -33,6 +33,31 @@ pub(crate) fn cyber_model_approval_reviewer(config: &Config) -> Option Option { + self.config + .permissions + .approval_policy + .can_set(&approval_policy.to_core()) + .and_then(|()| { + self.config + .permissions + .can_set_permission_profile(&preset.permission_profile) + }) + .err() + .map(|err| err.to_string()) + .or_else(|| { + (!self.config.is_permission_profile_allowed( + &preset.active_permission_profile.id, + &preset.permission_profile, + )) + .then(|| "Disabled by requirements.".to_string()) + }) + } + pub(super) fn open_permission_profiles_popup(&mut self) { let active_profile_id = self .config @@ -161,26 +186,7 @@ impl ChatWidget { /*return_to_permissions*/ true, ), dismiss_on_select: true, - disabled_reason: self - .config - .permissions - .approval_policy - .can_set(&approval_policy.to_core()) - .err() - .map(|err| err.to_string()) - .or_else(|| { - self.config - .permissions - .can_set_permission_profile(&preset.permission_profile) - .err() - .map(|err| err.to_string()) - }) - .or_else(|| { - (!self - .config - .is_permission_profile_allowed(id, &preset.permission_profile)) - .then(|| "Disabled by requirements.".to_string()) - }), + disabled_reason: self.permission_mode_disabled_reason(preset, approval_policy), ..Default::default() } } diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 2eb02f3a78..2de6b97774 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -245,6 +245,8 @@ mod history_replay; mod mcp_startup; #[path = "tests/misalignment_policy_tests.rs"] mod misalignment_policy; +#[path = "tests/permission_shortcuts_tests.rs"] +mod permission_shortcuts_tests; mod permissions; mod plan_mode; #[path = "tests/plugin_catalog_tests.rs"] diff --git a/codex-rs/tui/src/chatwidget/tests/permission_shortcuts_tests.rs b/codex-rs/tui/src/chatwidget/tests/permission_shortcuts_tests.rs new file mode 100644 index 0000000000..76b7eb5472 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/tests/permission_shortcuts_tests.rs @@ -0,0 +1,133 @@ +use super::permissions::requirements_stack; +use super::*; +use ApprovalsReviewer::AutoReview; +use ApprovalsReviewer::User; +use pretty_assertions::assert_eq; + +#[tokio::test] +async fn permission_shortcuts_cycle_builtin_modes() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + let thread_id = ThreadId::new(); + chat.thread_id = Some(thread_id); + chat.set_feature_enabled(Feature::GuardianApproval, /*enabled*/ true); + chat.chat_keymap.next_permission_mode = vec![crate::key_hint::plain(KeyCode::F(8))]; + chat.chat_keymap.previous_permission_mode = vec![crate::key_hint::plain(KeyCode::F(7))]; + #[cfg(target_os = "windows")] + { + chat.config.notices.hide_world_writable_warning = Some(true); + chat.set_windows_sandbox_mode(Some(WindowsSandboxModeToml::Unelevated)); + } + for (current, reviewer, key, expected, next_reviewer) in [ + (":workspace", User, KeyCode::F(8), ":workspace", AutoReview), + (":workspace", AutoReview, KeyCode::F(8), ":read-only", User), + (":read-only", User, KeyCode::F(8), ":workspace", User), + (":read-only", User, KeyCode::F(7), ":workspace", AutoReview), + ] { + let profile = if current == ":read-only" { + PermissionProfile::read_only() + } else { + PermissionProfile::workspace_write() + }; + chat.config + .permissions + .set_permission_profile_from_session_snapshot(PermissionProfileSnapshot::active( + profile, + ActivePermissionProfile::new(current), + )) + .expect("set current profile"); + chat.config.approvals_reviewer = reviewer; + chat.handle_key_event(KeyEvent::from(key)); + chat.handle_key_event(KeyEvent::from(key)); + let AppEvent::ApplyPermissionShortcut { + thread_id: target, + selection, + } = rx.try_recv().expect("permission selection") + else { + panic!("expected one typed permission selection"); + }; + assert_eq!( + ( + target, + selection.profile_id.as_str(), + selection.approval_policy, + selection.approvals_reviewer + ), + ( + thread_id, + expected, + Some(AskForApproval::OnRequest), + Some(next_reviewer) + ) + ); + assert!( + rx.try_recv().is_err(), + "pending shortcut must not be duplicated" + ); + chat.complete_permission_shortcut(thread_id); + } + #[cfg(target_os = "windows")] + { + chat.set_windows_sandbox_mode(/*mode*/ None); + chat.set_feature_enabled(Feature::WindowsSandbox, /*enabled*/ false); + chat.set_feature_enabled(Feature::WindowsSandboxElevated, /*enabled*/ false); + chat.config + .permissions + .set_permission_profile(PermissionProfile::read_only()) + .unwrap(); + chat.config.approvals_reviewer = User; + chat.handle_key_event(KeyEvent::from(KeyCode::F(8))); + assert!(matches!( + rx.try_recv(), + Ok(AppEvent::ApplyPermissionShortcut { + selection: PermissionProfileSelection { + approvals_reviewer: Some(AutoReview), + .. + }, + .. + }) + )); + assert!(rx.try_recv().is_err()); + } +} + +#[tokio::test] +async fn permission_shortcuts_respect_managed_mode_requirements() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + chat.thread_id = Some(ThreadId::new()); + chat.set_feature_enabled(Feature::GuardianApproval, /*enabled*/ true); + chat.config.approvals_reviewer = AutoReview; + chat.chat_keymap.next_permission_mode = vec![crate::key_hint::plain(KeyCode::F(8))]; + chat.config + .permissions + .set_permission_profile_from_session_snapshot(PermissionProfileSnapshot::active( + PermissionProfile::workspace_write(), + ActivePermissionProfile::new(":workspace"), + )) + .expect("set active profile"); + + for requirements in [ + codex_config::ConfigRequirementsToml { + allowed_approvals_reviewers: Some(vec![AutoReview]), + ..Default::default() + }, + codex_config::ConfigRequirementsToml { + auto_review: Some(codex_config::AutoReviewRequirementsToml { + required_on_models: Some(vec![chat.current_model().to_string()]), + ..Default::default() + }), + ..Default::default() + }, + ] { + chat.config.config_layer_stack = requirements_stack(requirements); + chat.handle_key_event(KeyEvent::from(KeyCode::F(8))); + let AppEvent::InsertHistoryCell(cell) = rx.try_recv().expect("unavailable-mode notice") + else { + panic!("must not submit a forbidden mode"); + }; + insta::assert_snapshot!( + "permission_shortcut_no_alternative", + lines_to_single_string(&cell.display_lines(/*width*/ 80)) + ); + assert!(rx.try_recv().is_err(), "must not submit a forbidden mode"); + } +} diff --git a/codex-rs/tui/src/chatwidget/tests/permissions.rs b/codex-rs/tui/src/chatwidget/tests/permissions.rs index da90e2dbd5..1f8fe08bc0 100644 --- a/codex-rs/tui/src/chatwidget/tests/permissions.rs +++ b/codex-rs/tui/src/chatwidget/tests/permissions.rs @@ -67,7 +67,9 @@ fn windows_sandbox_requirements_stack( requirements_stack(requirements_toml) } -fn requirements_stack(requirements_toml: codex_config::ConfigRequirementsToml) -> ConfigLayerStack { +pub(super) fn requirements_stack( + requirements_toml: codex_config::ConfigRequirementsToml, +) -> ConfigLayerStack { let mut requirements_with_sources = codex_config::ConfigRequirementsWithSources::default(); requirements_with_sources .merge_unset_fields(RequirementSource::Unknown, requirements_toml.clone()); diff --git a/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__permission_shortcuts_tests__permission_shortcut_no_alternative.snap b/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__permission_shortcuts_tests__permission_shortcut_no_alternative.snap new file mode 100644 index 0000000000..dd85ec62d7 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__permission_shortcuts_tests__permission_shortcut_no_alternative.snap @@ -0,0 +1,5 @@ +--- +source: tui/src/chatwidget/tests/permission_shortcuts_tests.rs +expression: lines_to_single_string(&cell.display_lines(80)) +--- +• No other permission modes are available. diff --git a/codex-rs/tui/src/keymap.rs b/codex-rs/tui/src/keymap.rs index 7ba6d09fe6..193d048e75 100644 --- a/codex-rs/tui/src/keymap.rs +++ b/codex-rs/tui/src/keymap.rs @@ -109,6 +109,10 @@ pub(crate) struct ChatKeymap { pub(crate) decrease_reasoning_effort: Vec, /// Increase the active reasoning effort. pub(crate) increase_reasoning_effort: Vec, + /// Switch to the previous available permission mode. + pub(crate) previous_permission_mode: Vec, + /// Switch to the next available permission mode. + pub(crate) next_permission_mode: Vec, /// Edit the most recently queued message. pub(crate) edit_queued_message: Vec, } @@ -648,6 +652,13 @@ impl RuntimeKeymap { &defaults.chat.increase_reasoning_effort, "tui.keymap.chat.increase_reasoning_effort", )?, + previous_permission_mode: resolve_local!( + keymap, + defaults, + chat, + previous_permission_mode + ), + next_permission_mode: resolve_local!(keymap, defaults, chat, next_permission_mode), edit_queued_message: resolve_bindings( keymap.chat.edit_queued_message.as_ref(), &defaults.chat.edit_queued_message, @@ -1211,6 +1222,8 @@ impl RuntimeKeymap { alt(KeyCode::Char('.')), shift(KeyCode::Up) ], + previous_permission_mode: default_bindings![], + next_permission_mode: default_bindings![], edit_queued_message: default_bindings![alt(KeyCode::Up), shift(KeyCode::Left)], }, composer: ComposerKeymap { @@ -1458,6 +1471,23 @@ impl RuntimeKeymap { /// 2. Contexts with hard-coded sequence behavior, such as edit-previous /// backtracking, intentionally stay outside this configurable keymap. fn validate_conflicts(&self) -> Result<(), String> { + for (action, bindings) in [ + ( + "previous_permission_mode", + &self.chat.previous_permission_mode, + ), + ("next_permission_mode", &self.chat.next_permission_mode), + ] { + if bindings.iter().any(|binding| { + let (code, modifiers) = binding.parts(); + crate::key_hint::is_plain_text_key_event(KeyEvent::new(code, modifiers)) + || matches!(code, KeyCode::Char(_)) && crate::key_hint::is_altgr(modifiers) + }) { + return Err(format!( + "tui.keymap.chat.{action}: printable keys are reserved for text input" + )); + } + } #[cfg(unix)] if self .app @@ -1510,6 +1540,14 @@ impl RuntimeKeymap { "chat.increase_reasoning_effort", self.chat.increase_reasoning_effort.as_slice(), ), + ( + "chat.previous_permission_mode", + self.chat.previous_permission_mode.as_slice(), + ), + ( + "chat.next_permission_mode", + self.chat.next_permission_mode.as_slice(), + ), ( "chat.edit_queued_message", self.chat.edit_queued_message.as_slice(), @@ -1555,6 +1593,14 @@ impl RuntimeKeymap { "chat.increase_reasoning_effort", self.chat.increase_reasoning_effort.as_slice(), ), + ( + "chat.previous_permission_mode", + self.chat.previous_permission_mode.as_slice(), + ), + ( + "chat.next_permission_mode", + self.chat.next_permission_mode.as_slice(), + ), ( "chat.edit_queued_message", self.chat.edit_queued_message.as_slice(), @@ -1668,6 +1714,14 @@ impl RuntimeKeymap { "chat.increase_reasoning_effort", self.chat.increase_reasoning_effort.as_slice(), ), + ( + "chat.previous_permission_mode", + self.chat.previous_permission_mode.as_slice(), + ), + ( + "chat.next_permission_mode", + self.chat.next_permission_mode.as_slice(), + ), ("composer.submit", self.composer.submit.as_slice()), ("toggle_vim_mode", self.app.toggle_vim_mode.as_slice()), ("toggle_fast_mode", self.app.toggle_fast_mode.as_slice()), @@ -2561,6 +2615,32 @@ mod tests { assert_eq!(runtime.app.copy, vec![key_hint::ctrl(KeyCode::Char('o'))]); } + #[test] + fn permission_shortcuts_reserve_plain_text() { + for binding in ["a", "shift-a", "2", "space"] { + let mut keymap = TuiKeymap::default(); + keymap.chat.previous_permission_mode = Some(one(binding)); + assert!( + RuntimeKeymap::from_config(&keymap) + .expect_err("permission shortcuts must not intercept typing") + .contains("printable keys") + ); + keymap.chat.previous_permission_mode = None; + keymap.chat.next_permission_mode = Some(one(binding)); + assert!(RuntimeKeymap::from_config(&keymap).is_err()); + } + #[cfg(windows)] + { + let mut keymap = TuiKeymap::default(); + keymap.chat.next_permission_mode = Some(one("ctrl-alt-q")); + assert!(RuntimeKeymap::from_config(&keymap).is_err()); + } + let mut keymap = TuiKeymap::default(); + keymap.chat.previous_permission_mode = Some(one("f7")); + keymap.chat.next_permission_mode = Some(one("ctrl-x enter")); + assert!(RuntimeKeymap::from_config(&keymap).is_ok()); + } + #[test] fn defaults_include_reassignable_main_surface_actions() { let runtime = RuntimeKeymap::defaults(); diff --git a/codex-rs/tui/src/keymap/bindings.rs b/codex-rs/tui/src/keymap/bindings.rs index 8b2d2d18f2..003af4dfb1 100644 --- a/codex-rs/tui/src/keymap/bindings.rs +++ b/codex-rs/tui/src/keymap/bindings.rs @@ -239,6 +239,8 @@ define_runtime_action_bindings! { interrupt_turn, decrease_reasoning_effort, increase_reasoning_effort, + previous_permission_mode, + next_permission_mode, edit_queued_message, ], "composer" => Composer, composer, composer [ diff --git a/codex-rs/tui/src/keymap_setup.rs b/codex-rs/tui/src/keymap_setup.rs index 0b7da69d77..bd4615c783 100644 --- a/codex-rs/tui/src/keymap_setup.rs +++ b/codex-rs/tui/src/keymap_setup.rs @@ -1013,7 +1013,7 @@ mod tests { let selectable = tab.items.iter().filter(|item| !item.is_disabled).count(); format!("tab: {} ({selectable} selectable)", tab.label) }) - .chain(all_tab.items.iter().take(12).map(|item| { + .chain(all_tab.items.iter().take(13).map(|item| { format!( "{} | {} | {}", item.name, @@ -1059,13 +1059,23 @@ mod tests { let params = build_keymap_picker_params(&runtime, &TuiKeymap::default()); let unbound_tab = selection_tab(¶ms, KEYMAP_UNBOUND_TAB_ID); - assert_eq!(unbound_tab.items.len(), 2); - assert_eq!(unbound_tab.items[0].name, "Toggle Vim Mode"); - assert_eq!(unbound_tab.items[0].description.as_deref(), Some("unbound")); - assert!(!unbound_tab.items[0].is_disabled); - assert_eq!(unbound_tab.items[1].name, "Kill Whole Line"); - assert_eq!(unbound_tab.items[1].description.as_deref(), Some("unbound")); - assert!(!unbound_tab.items[1].is_disabled); + assert_eq!( + unbound_tab + .items + .iter() + .map(|item| ( + item.name.as_str(), + item.description.as_deref(), + item.is_disabled + )) + .collect::>(), + vec![ + ("Toggle Vim Mode", Some("unbound"), false), + ("Previous Permission Mode", Some("unbound"), false), + ("Next Permission Mode", Some("unbound"), false), + ("Kill Whole Line", Some("unbound"), false), + ] + ); } #[test] diff --git a/codex-rs/tui/src/keymap_setup/actions.rs b/codex-rs/tui/src/keymap_setup/actions.rs index f8726d2ef0..707777e7bf 100644 --- a/codex-rs/tui/src/keymap_setup/actions.rs +++ b/codex-rs/tui/src/keymap_setup/actions.rs @@ -96,6 +96,8 @@ pub(super) const KEYMAP_ACTIONS: &[KeymapActionDescriptor] = &[ action("chat", "Chat", "interrupt_turn", "Interrupt the active turn."), action("chat", "Chat", "decrease_reasoning_effort", "Decrease reasoning effort."), action("chat", "Chat", "increase_reasoning_effort", "Increase reasoning effort."), + action("chat", "Chat", "previous_permission_mode", "Switch to the previous available permission mode."), + action("chat", "Chat", "next_permission_mode", "Switch to the next available permission mode."), action("chat", "Chat", "edit_queued_message", "Edit the most recently queued message."), action("composer", "Composer", "submit", "Submit the current composer draft."), action("composer", "Composer", "queue", "Queue the draft while a task is running."), @@ -247,6 +249,8 @@ pub(super) fn binding_slot<'a>( ("chat", "interrupt_turn") => Some(&mut keymap.chat.interrupt_turn), ("chat", "decrease_reasoning_effort") => Some(&mut keymap.chat.decrease_reasoning_effort), ("chat", "increase_reasoning_effort") => Some(&mut keymap.chat.increase_reasoning_effort), + ("chat", "previous_permission_mode") => Some(&mut keymap.chat.previous_permission_mode), + ("chat", "next_permission_mode") => Some(&mut keymap.chat.next_permission_mode), ("chat", "edit_queued_message") => Some(&mut keymap.chat.edit_queued_message), ("composer", "submit") => Some(&mut keymap.composer.submit), ("composer", "queue") => Some(&mut keymap.composer.queue), diff --git a/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_all_tab_search.snap b/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_all_tab_search.snap index ccdb70a0a2..6b2c6717f2 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_all_tab_search.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_all_tab_search.snap @@ -13,4 +13,4 @@ Toggle Side Conversation | ctrl-/ | Global toggle_side_conversation Toggle Side Interrupt Turn | esc | Chat interrupt_turn Interrupt Turn Interrupt the active turn. esc Default Decrease Reasoning Effort | alt-,, shift-down | Chat decrease_reasoning_effort Decrease Reasoning Effort Decrease reasoning effort. alt-,, shift-down Default Increase Reasoning Effort | alt-., shift-up | Chat increase_reasoning_effort Increase Reasoning Effort Increase reasoning effort. alt-., shift-up Default -Edit Queued Message | alt-up, shift-left | Chat edit_queued_message Edit Queued Message Edit the most recently queued message. alt-up, shift-left Default +Previous Permission Mode | unbound | Chat previous_permission_mode Previous Permission Mode Switch to the previous available permission mode. unbound Default diff --git a/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_custom.snap b/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_custom.snap index be89127c36..6719125553 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_custom.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_custom.snap @@ -5,9 +5,9 @@ expression: "render_picker(params, 120)" Keymap All configurable shortcuts. - 116 actions, 1 customized, 2 unbound. + 118 actions, 1 customized, 4 unbound. - [All] Common Customized (1) Unbound (2) App Composer Editor Vim Navigation Agents Approval Debug + [All] Common Customized (1) Unbound (4) App Composer Editor Vim Navigation Agents Approval Debug Type to search shortcuts › Global Open Agents alt-a diff --git a/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_fast_mode_enabled.snap b/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_fast_mode_enabled.snap index dbe9302278..6f4db88d71 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_fast_mode_enabled.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_fast_mode_enabled.snap @@ -5,9 +5,9 @@ expression: "render_picker(params, 120)" Keymap All configurable shortcuts. - 117 actions, 0 customized, 3 unbound. + 119 actions, 0 customized, 5 unbound. - [All] Common Customized (0) Unbound (3) App Composer Editor Vim Navigation Agents Approval Debug + [All] Common Customized (0) Unbound (5) App Composer Editor Vim Navigation Agents Approval Debug Type to search shortcuts › Global Open Agents alt-a diff --git a/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_first_actions.snap b/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_first_actions.snap index 1e7bc25c9f..04247dff62 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_first_actions.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_first_actions.snap @@ -2,11 +2,11 @@ source: tui/src/keymap_setup.rs expression: snapshot --- -tab: All (116 selectable) +tab: All (118 selectable) tab: Common (20 selectable) tab: Customized (0) (0 selectable) -tab: Unbound (2) (2 selectable) -tab: App (12 selectable) +tab: Unbound (4) (4 selectable) +tab: App (14 selectable) tab: Composer (5 selectable) tab: Editor (17 selectable) tab: Vim (49 selectable) @@ -25,4 +25,5 @@ Toggle Side Conversation | ctrl-/ | Global toggle_side_conversation Toggle Side Interrupt Turn | esc | Chat interrupt_turn Interrupt Turn Interrupt the active turn. esc Default Decrease Reasoning Effort | alt-,, shift-down | Chat decrease_reasoning_effort Decrease Reasoning Effort Decrease reasoning effort. alt-,, shift-down Default Increase Reasoning Effort | alt-., shift-up | Chat increase_reasoning_effort Increase Reasoning Effort Increase reasoning effort. alt-., shift-up Default -Edit Queued Message | alt-up, shift-left | Chat edit_queued_message Edit Queued Message Edit the most recently queued message. alt-up, shift-left Default +Previous Permission Mode | unbound | Chat previous_permission_mode Previous Permission Mode Switch to the previous available permission mode. unbound Default +Next Permission Mode | unbound | Chat next_permission_mode Next Permission Mode Switch to the next available permission mode. unbound Default diff --git a/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_global_chord.snap b/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_global_chord.snap index 5a43aec382..cc7cdd49cd 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_global_chord.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_global_chord.snap @@ -5,9 +5,9 @@ expression: "render_picker(params, 120)" Keymap All configurable shortcuts. - 116 actions, 1 customized, 2 unbound. + 118 actions, 1 customized, 4 unbound. - [All] Common Customized (1) Unbound (2) App Composer Editor Vim Navigation Agents Approval Debug + [All] Common Customized (1) Unbound (4) App Composer Editor Vim Navigation Agents Approval Debug Type to search shortcuts › Global Open Agents alt-a diff --git a/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_narrow.snap b/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_narrow.snap index d5b2c4b09e..20655b315d 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_narrow.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_narrow.snap @@ -5,9 +5,9 @@ expression: "render_picker(params, 78)" Keymap All configurable shortcuts. - 116 actions, 0 customized, 2 unbound. + 118 actions, 0 customized, 4 unbound. - [All] Common Customized (0) Unbound (2) App Composer Editor Vim + [All] Common Customized (0) Unbound (4) App Composer Editor Vim Navigation Agents Approval Debug Type to search shortcuts diff --git a/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_wide.snap b/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_wide.snap index 86b85dbc9c..f1f656b82c 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_wide.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_wide.snap @@ -5,9 +5,9 @@ expression: "render_picker(params, 120)" Keymap All configurable shortcuts. - 116 actions, 0 customized, 2 unbound. + 118 actions, 0 customized, 4 unbound. - [All] Common Customized (0) Unbound (2) App Composer Editor Vim Navigation Agents Approval Debug + [All] Common Customized (0) Unbound (4) App Composer Editor Vim Navigation Agents Approval Debug Type to search shortcuts › Global Open Agents alt-a