diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index b5a5acf0db..81b7718dfa 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -3236,7 +3236,7 @@ impl App { return Ok(AppRunControl::Continue); } self.chat_widget - .select_permission_profile_by_id(&profile_id); + .select_permission_profile(&profile_id); } AppEvent::OpenPermissionsPopup | AppEvent::OpenApprovalsPopup => { if self.reject_pending_permission_change() { diff --git a/codex-rs/tui/src/app/tests/permission_selection_tests.rs b/codex-rs/tui/src/app/tests/permission_selection_tests.rs index 79bf0cf638..077624d923 100644 --- a/codex-rs/tui/src/app/tests/permission_selection_tests.rs +++ b/codex-rs/tui/src/app/tests/permission_selection_tests.rs @@ -154,8 +154,7 @@ async fn inline_server_permission_selection_holds_queued_prompt_until_confirmati while events.try_recv().is_ok() {} while ops.try_recv().is_ok() {} - app.chat_widget - .select_permission_profile_by_id("server-only"); + app.chat_widget.select_permission_profile("server-only"); assert!(app.chat_widget.queue_user_message_with_options( crate::chatwidget::UserMessage::from("use the selected profile"), crate::bottom_pane::QueuedInputAction::Plain, diff --git a/codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__model_defaults__session_only_model_confirmation.snap b/codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__model_defaults__session_only_model_confirmation.snap index e1960ed79f..bf2ac9d591 100644 --- a/codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__model_defaults__session_only_model_confirmation.snap +++ b/codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__model_defaults__session_only_model_confirmation.snap @@ -4,9 +4,9 @@ expression: "messages.join(\"\\n\")" --- • Model changed to gpt-5.6-terra high standard for this session only ■ Unexpected argument 'fast'. Use '--reasoning=' or '--speed='. -■ Unknown model setting '--speeed'. Expected '--reasoning=' or '--speed='. +■ Unknown command setting `--speeed`. ■ Reasoning level 'invalid' is unavailable for gpt-5.6-terra. Available: low, medium, high, xhigh, max, ultra. -■ Model setting '--speed' requires a value. -■ Model setting '--reasoning' requires a value. -■ Model setting '--reasoning' requires a value. -■ Model setting '--speed' was provided more than once. +■ Command setting `--speed` requires a value. +■ Command setting `--reasoning` requires a value. +■ Command setting `--reasoning` requires a value. +■ Command setting `--speed` was provided more than once. diff --git a/codex-rs/tui/src/app/tests/startup.rs b/codex-rs/tui/src/app/tests/startup.rs index a4c5a256a1..2cc4776903 100644 --- a/codex-rs/tui/src/app/tests/startup.rs +++ b/codex-rs/tui/src/app/tests/startup.rs @@ -148,8 +148,7 @@ async fn windows_sandbox_setup_completion_requires_matching_pending_mode() -> Re .expect("auto preset"); app.windows_sandbox.pending_setup = Some((WindowsSandboxEnableMode::Elevated, preset, None)); app.windows_sandbox.setup_started_at = Some(Instant::now()); - app.chat_widget - .select_permission_profile_by_id(":workspace"); + app.chat_widget.select_permission_profile(":workspace"); let AppEvent::FetchPermissionProfiles { request_id, .. } = events.try_recv()? else { panic!("expected profile discovery"); }; diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index f6d4c16260..0652fbd4f2 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -309,6 +309,7 @@ mod hooks; mod interaction; pub(crate) use interaction::KeyEventAction; mod skills; +mod slash_command_args; mod slash_dispatch; mod worktree_picker; use self::skills::collect_tool_mentions; diff --git a/codex-rs/tui/src/chatwidget/model_command.rs b/codex-rs/tui/src/chatwidget/model_command.rs index 9fa29504c0..0c7992b229 100644 --- a/codex-rs/tui/src/chatwidget/model_command.rs +++ b/codex-rs/tui/src/chatwidget/model_command.rs @@ -2,15 +2,16 @@ use super::ChatWidget; use super::session_model_selection::AstraModelPickerAction; +use super::slash_command_args; use codex_protocol::config_types::SERVICE_TIER_DEFAULT_REQUEST_VALUE; use codex_protocol::openai_models::ModelPreset; use codex_protocol::openai_models::ReasoningEffort; #[derive(Default)] -struct ModelCommandArgs<'a> { - model: Option<&'a str>, - effort: Option<&'a str>, - speed: Option<&'a str>, +struct ModelCommandArgs { + model: Option, + effort: Option, + speed: Option, } struct ModelCommand { @@ -19,62 +20,26 @@ struct ModelCommand { service_tier: Option, } -fn parse_model_command_args(args: &str) -> Result, String> { - let mut parsed = ModelCommandArgs::default(); - let mut words = args.split_whitespace().peekable(); - while let Some(word) = words.next() { - let (setting, inline_value) = word - .split_once('=') - .map_or((word, None), |(setting, value)| (setting, Some(value))); - match setting { - "--reasoning" => { - set_model_setting(&mut parsed.effort, setting, inline_value, &mut words)?; - } - "--speed" => { - set_model_setting(&mut parsed.speed, setting, inline_value, &mut words)?; - } - _ if word.starts_with("--") => { - return Err(format!( - "Unknown model setting '{setting}'. Expected '--reasoning=' or '--speed='." - )); - } - _ if parsed.model.replace(word).is_some() => { - return Err(format!( - "Unexpected argument '{word}'. Use '--reasoning=' or '--speed='." - )); - } - _ => {} - } - } - Ok(parsed) -} - -fn set_model_setting<'a>( - target: &mut Option<&'a str>, - setting: &str, - inline_value: Option<&'a str>, - words: &mut std::iter::Peekable>, -) -> Result<(), String> { - let value = match inline_value { - Some("") => None, - Some(value) => Some(value), - None => words.next_if(|word| !word.starts_with("--")), - } - .ok_or_else(|| format!("Model setting '{setting}' requires a value."))?; - if target.replace(value).is_some() { +fn parse_model_command_args(args: &str) -> Result { + let parsed = slash_command_args::parse(args, &["--reasoning", "--speed"], &[])?; + if let Some(unexpected) = parsed.positionals.get(1) { return Err(format!( - "Model setting '{setting}' was provided more than once." + "Unexpected argument '{unexpected}'. Use '--reasoning=' or '--speed='." )); } - Ok(()) + Ok(ModelCommandArgs { + model: parsed.positionals.first().cloned(), + effort: parsed.value("--reasoning").map(str::to_owned), + speed: parsed.value("--speed").map(str::to_owned), + }) } fn resolve_reasoning_effort( chat: &ChatWidget, - args: &ModelCommandArgs<'_>, + args: &ModelCommandArgs, model: &ModelPreset, ) -> Result { - let Some(effort) = args.effort else { + let Some(effort) = args.effort.as_deref() else { return Ok(if args.model.is_none() { chat.current_reasoning_effort() .unwrap_or_else(|| model.default_reasoning_effort.clone()) @@ -161,7 +126,7 @@ fn resolve_model_command(chat: &ChatWidget, args: &str) -> Result Result, + reviewer: Option, +} + +fn parse_approval_policy(value: &str) -> Result { + match value { + "untrusted" => Ok(AskForApproval::UnlessTrusted), + "on-request" => Ok(AskForApproval::OnRequest), + "never" => Ok(AskForApproval::Never), + value => serde_json::from_str(value).map_err(|_| { + format!( + "Unknown approval policy `{value}`. Expected `untrusted`, `on-request`, `never`, or a granular policy." + ) + }), + } +} + +fn parse_reviewer(value: &str) -> Result { + match value { + "user" => Ok(ApprovalsReviewer::User), + "auto_review" => Ok(ApprovalsReviewer::AutoReview), + value => Err(format!( + "Unknown permission reviewer `{value}`. Expected `user` or `auto_review`." + )), + } +} + +fn parse_permission_profile_args(args: &str) -> Result { + let parsed = slash_command_args::parse(args, &["--approval-policy", "--reviewer"], &[])?; + let profile_id = parsed.positionals.first().cloned().ok_or_else(|| { + "Usage: /permissions [--approval-policy=] [--reviewer=user|auto_review]" + .to_string() + })?; + if let Some(argument) = parsed.positionals.get(1) { + return Err(format!("Unexpected permission argument `{argument}`.")); + } + Ok(PermissionProfileArgs { + profile_id, + approval_policy: parsed + .value("--approval-policy") + .map(parse_approval_policy) + .transpose()?, + reviewer: parsed.value("--reviewer").map(parse_reviewer).transpose()?, + }) +} + impl ChatWidget { - pub(crate) fn select_permission_profile_by_id(&mut self, profile_id: &str) { + pub(crate) fn select_permission_profile(&mut self, args: &str) { + if let Err(message) = parse_permission_profile_args(args) { + self.add_error_message(message); + self.retain_input_after_failed_permission_selection(); + self.app_event_tx.send(AppEvent::SettingsSelectionClosed); + return; + } self.set_queue_autosend_suppressed(/*suppressed*/ true); self.inline_permission_profile_selection = Some( - InlinePermissionProfileSelection::Discovering(profile_id.to_string()), + InlinePermissionProfileSelection::Discovering(args.to_string()), ); if !self.apply_pending_permission_profile() { self.request_permission_profiles(); @@ -33,11 +88,20 @@ impl ChatWidget { let Some(discovery) = self.permission_discovery.clone() else { return false; }; - let Some(profile_id) = self.pending_permission_profile_id().map(str::to_owned) else { + let Some(args) = self.pending_permission_profile_args().map(str::to_owned) else { return false; }; + let Ok(args) = parse_permission_profile_args(&args) else { + self.recover_inline_permission_profile_selection(); + return true; + }; self.inline_permission_profile_selection = Some(InlinePermissionProfileSelection::Applying); - if !self.try_select_permission_profile_by_id(&discovery, &profile_id) { + if !self.try_select_permission_profile( + &discovery, + &args.profile_id, + args.approval_policy, + args.reviewer, + ) { self.recover_inline_permission_profile_selection(); } self.app_event_tx.send(AppEvent::SettingsSelectionClosed); @@ -45,7 +109,7 @@ impl ChatWidget { } pub(super) fn cancel_pending_permission_profile_selection(&mut self) -> bool { - if self.pending_permission_profile_id().is_none() { + if self.pending_permission_profile_args().is_none() { return false; } self.recover_inline_permission_profile_selection(); @@ -69,7 +133,7 @@ impl ChatWidget { self.inline_permission_profile_selection.is_some() } - fn pending_permission_profile_id(&self) -> Option<&str> { + fn pending_permission_profile_args(&self) -> Option<&str> { match self.inline_permission_profile_selection.as_ref() { Some(InlinePermissionProfileSelection::Discovering(profile_id)) => Some(profile_id), Some(InlinePermissionProfileSelection::Applying) | None => None, @@ -134,7 +198,7 @@ impl ChatWidget { return; } if let Err(message) = result - && self.pending_permission_profile_id().is_some() + && self.pending_permission_profile_args().is_some() { self.add_error_message(message); } @@ -149,7 +213,7 @@ impl ChatWidget { } } Err(message) => { - let retry_profile_id = self.pending_permission_profile_id().map(str::to_owned); + let retry_args = self.pending_permission_profile_args().map(str::to_owned); self.bottom_pane.show_selection_view(SelectionViewParams { view_id: Some(VIEW_ID), title: Some("Update Model Permissions".to_string()), @@ -157,8 +221,8 @@ impl ChatWidget { items: vec![SelectionItem { name: "Retry".to_string(), actions: vec![Box::new(move |tx| { - if let Some(profile_id) = retry_profile_id.clone() { - tx.send(AppEvent::ResolvePermissionProfile { profile_id }); + if let Some(args) = retry_args.clone() { + tx.send(AppEvent::ResolvePermissionProfile { profile_id: args }); } else { tx.send(AppEvent::OpenPermissionsPopup); } diff --git a/codex-rs/tui/src/chatwidget/permission_popups.rs b/codex-rs/tui/src/chatwidget/permission_popups.rs index 62877cfdf8..a91e85ce9f 100644 --- a/codex-rs/tui/src/chatwidget/permission_popups.rs +++ b/codex-rs/tui/src/chatwidget/permission_popups.rs @@ -337,8 +337,7 @@ impl ChatWidget { Self::permission_profile_selection_actions, ) }; - let requires_confirmation = - approvals_reviewer == ApprovalsReviewer::User && preset.id == "full-access"; + let requires_confirmation = preset.id == "full-access"; #[cfg(target_os = "windows")] if preset.id == "auto" && matches!( diff --git a/codex-rs/tui/src/chatwidget/permissions_menu.rs b/codex-rs/tui/src/chatwidget/permissions_menu.rs index eac72ab9fa..b6614dc81e 100644 --- a/codex-rs/tui/src/chatwidget/permissions_menu.rs +++ b/codex-rs/tui/src/chatwidget/permissions_menu.rs @@ -50,10 +50,12 @@ pub(crate) fn cyber_model_approval_reviewer(config: &Config) -> Option, + requested_reviewer: Option, ) -> bool { let reviewer = match profile_id { ":approve-for-me" if !self.config.features.enabled(Feature::GuardianApproval) => { @@ -83,10 +85,21 @@ impl ChatWidget { let preset = builtin_approval_presets() .into_iter() .find(|preset| preset.active_permission_profile.id == profile_id); - let approval_policy = preset - .as_ref() - .map(|preset| AskForApproval::from(preset.approval)); - let approvals_reviewer = preset.as_ref().map(|_| reviewer); + let approval_policy = requested_approval_policy.or_else(|| { + preset + .as_ref() + .map(|preset| AskForApproval::from(preset.approval)) + }); + let approvals_reviewer = requested_reviewer.or_else(|| preset.as_ref().map(|_| reviewer)); + if approvals_reviewer == Some(ApprovalsReviewer::AutoReview) + && !self.config.features.enabled(Feature::GuardianApproval) + { + self.add_error_message( + "Approve for me is unavailable. Run `/permissions` to see available profiles." + .into(), + ); + return false; + } if let Some(reason) = discovery.disabled_reason( profile_id, approval_policy, @@ -97,14 +110,13 @@ impl ChatWidget { )); return false; } - let display_label = - preset - .as_ref() - .map_or(profile.id.as_str(), |preset| match (preset.id, reviewer) { - ("auto", ApprovalsReviewer::AutoReview) => APPROVE_FOR_ME_LABEL, - ("auto", ApprovalsReviewer::User) => ASK_FOR_APPROVAL_LABEL, - _ => preset.label, - }); + let display_label = preset.as_ref().map_or(profile.id.as_str(), |preset| { + match (preset.id, approvals_reviewer) { + ("auto", Some(ApprovalsReviewer::AutoReview)) => APPROVE_FOR_ME_LABEL, + ("auto", _) => ASK_FOR_APPROVAL_LABEL, + _ => preset.label, + } + }); let selection = PermissionProfileSelection { profile_id: profile_id.to_string(), approval_policy, @@ -116,7 +128,7 @@ impl ChatWidget { .permission_mode_actions( &preset, display_label.to_string(), - reviewer, + approvals_reviewer.unwrap_or(reviewer), /*profile_selection*/ Some(selection), /*return_to_permissions*/ true, ) diff --git a/codex-rs/tui/src/chatwidget/quick_dials.rs b/codex-rs/tui/src/chatwidget/quick_dials.rs index 68bf5e1b2d..0414299f7d 100644 --- a/codex-rs/tui/src/chatwidget/quick_dials.rs +++ b/codex-rs/tui/src/chatwidget/quick_dials.rs @@ -351,36 +351,73 @@ impl ChatWidget { self.defer_input_until_settings_applied(); } - fn open_quick_dial_prefills(&mut self) { + pub(super) fn open_quick_dial_prefills(&mut self) { if !self.quick_dials_available() { return; } - let model = self.current_model_quick_dial(); - let (description, is_disabled, actions) = match model { - Ok(dial) => { - let description = Some(dial.input.clone()); - let actions = vec![Box::new(move |tx: &AppEventSender| { - tx.send(AppEvent::OpenQuickDialEditor { - target: QuickDialEditorTarget::PickSlot, - dial: Some(dial.clone()), - }) - }) as SelectionAction]; - (description, false, actions) - } - Err(reason) => (Some(reason.into()), true, Vec::new()), - }; - self.bottom_pane.show_selection_view(SelectionViewParams { - view_id: Some("quick-dial-prefills"), - max_visible_rows: 1, - title: Some("Start preset from current settings".into()), - items: vec![SelectionItem { - name: "Current model".into(), + let permissions = self + .config + .permissions + .active_permission_profile() + .map(|profile| -> Result { + let approval_policy = + AskForApproval::from(self.config.permissions.approval_policy.value()); + let approval_policy = match approval_policy { + AskForApproval::UnlessTrusted => "untrusted".to_string(), + AskForApproval::OnRequest => "on-request".to_string(), + AskForApproval::Never => "never".to_string(), + granular @ AskForApproval::Granular { .. } => serde_json::to_string(&granular) + .map_err(|_| "The approval policy cannot be recalled.")?, + }; + let profile_id = shlex::try_quote(&profile.id) + .map_err(|_| "The permission profile name cannot be recalled.")?; + let approval_policy = shlex::try_quote(&approval_policy) + .map_err(|_| "The approval policy cannot be recalled.")?; + let input = format!( + "/permissions {profile_id} --approval-policy={approval_policy} --reviewer={}", + self.config.approvals_reviewer + ); + let dial = QuickDial { input, label: None }; + dial.validate()?; + Ok(dial) + }) + .unwrap_or(Err( + "Select a named permission profile with /permissions first.", + )); + let items = [ + ("Current model", self.current_model_quick_dial()), + ("Current permissions", permissions), + ] + .into_iter() + .map(|(name, dial)| { + let (description, is_disabled, actions) = match dial { + Ok(dial) => { + let description = Some(dial.input.clone()); + let actions = vec![Box::new(move |tx: &AppEventSender| { + tx.send(AppEvent::OpenQuickDialEditor { + target: QuickDialEditorTarget::PickSlot, + dial: Some(dial.clone()), + }) + }) as SelectionAction]; + (description, false, actions) + } + Err(reason) => (Some(reason.into()), true, Vec::new()), + }; + SelectionItem { + name: name.into(), description, is_disabled, actions, dismiss_parent_on_child_accept: true, ..Default::default() - }], + } + }) + .collect(); + self.bottom_pane.show_selection_view(SelectionViewParams { + view_id: Some("quick-dial-prefills"), + max_visible_rows: 2, + title: Some("Start preset from current settings".into()), + items, ..SelectionViewParams::picker() }); self.defer_input_until_settings_applied(); diff --git a/codex-rs/tui/src/chatwidget/slash_command_args.rs b/codex-rs/tui/src/chatwidget/slash_command_args.rs new file mode 100644 index 0000000000..fa4ac26df6 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/slash_command_args.rs @@ -0,0 +1,95 @@ +//! Shared tokenization for slash commands with positional arguments and named settings. +//! +//! Values may follow their setting or use `=`, while switches never consume a value. A bare `--` +//! leaves all remaining tokens positional. + +#[derive(Debug, Default, PartialEq, Eq)] +pub(super) struct ParsedCommandArgs { + pub(super) positionals: Vec, + named: Vec, +} + +#[derive(Debug, PartialEq, Eq)] +struct NamedArgument { + name: String, + value: Option, +} + +impl ParsedCommandArgs { + pub(super) fn value(&self, name: &str) -> Option<&str> { + self.named + .iter() + .find(|argument| argument.name == name) + .and_then(|argument| argument.value.as_deref()) + } +} + +pub(super) fn parse( + args: &str, + value_settings: &[&str], + switches: &[&str], +) -> Result { + let mut words = shlex::split(args) + .ok_or_else(|| "Invalid command arguments. Check the quoting.".to_string())? + .into_iter() + .peekable(); + let mut parsed = ParsedCommandArgs::default(); + let mut parse_settings = true; + while let Some(word) = words.next() { + if parse_settings && word == "--" { + parse_settings = false; + } else if parse_settings && word.starts_with("--") { + let argument = parse_named_argument(&word, &mut words, value_settings, switches)?; + if parsed + .named + .iter() + .any(|existing| existing.name == argument.name) + { + return Err(format!( + "Command setting `{}` was provided more than once.", + argument.name + )); + } + parsed.named.push(argument); + } else { + parsed.positionals.push(word); + } + } + Ok(parsed) +} + +fn parse_named_argument( + word: &str, + words: &mut std::iter::Peekable>, + value_settings: &[&str], + switches: &[&str], +) -> Result { + let (name, inline_value) = word + .split_once('=') + .map_or((word, None), |(name, value)| (name, Some(value))); + if value_settings.contains(&name) { + let value = inline_value + .filter(|value| !value.is_empty()) + .map(str::to_owned) + .or_else(|| words.next_if(|value| !value.starts_with("--"))) + .ok_or_else(|| format!("Command setting `{name}` requires a value."))?; + return Ok(NamedArgument { + name: name.to_string(), + value: Some(value), + }); + } + if switches.contains(&name) { + if inline_value.is_some() { + return Err(format!("Command setting `{name}` does not take a value.")); + } + return Ok(NamedArgument { + name: name.to_string(), + value: None, + }); + } + Err(format!("Unknown command setting `{name}`.")) +} + +#[cfg(test)] +#[path = "slash_command_args_tests.rs"] +mod tests; diff --git a/codex-rs/tui/src/chatwidget/slash_command_args_tests.rs b/codex-rs/tui/src/chatwidget/slash_command_args_tests.rs new file mode 100644 index 0000000000..f413bba636 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/slash_command_args_tests.rs @@ -0,0 +1,62 @@ +use super::*; +use pretty_assertions::assert_eq; + +#[test] +fn parses_positional_values_switches_and_quoted_arguments() { + let parsed = parse( + "'team review' item --reviewer user --speed=fast --clear -- --literal", + &["--reviewer", "--speed"], + &["--clear"], + ) + .expect("valid command arguments"); + + assert_eq!( + parsed, + ParsedCommandArgs { + positionals: vec!["team review".into(), "item".into(), "--literal".into()], + named: vec![ + NamedArgument { + name: "--reviewer".into(), + value: Some("user".into()), + }, + NamedArgument { + name: "--speed".into(), + value: Some("fast".into()), + }, + NamedArgument { + name: "--clear".into(), + value: None, + }, + ], + } + ); + assert_eq!(parsed.value("--reviewer"), Some("user")); +} + +#[test] +fn rejects_invalid_named_arguments() { + for (args, expected) in [ + ( + "--reviewer", + "Command setting `--reviewer` requires a value.", + ), + ( + "--clear=yes", + "Command setting `--clear` does not take a value.", + ), + ( + "--clear --clear", + "Command setting `--clear` was provided more than once.", + ), + ("--unknown", "Unknown command setting `--unknown`."), + ( + "'unfinished", + "Invalid command arguments. Check the quoting.", + ), + ] { + assert_eq!( + parse(args, &["--reviewer"], &["--clear"]), + Err(expected.to_string()) + ); + } +} diff --git a/codex-rs/tui/src/chatwidget/slash_dispatch.rs b/codex-rs/tui/src/chatwidget/slash_dispatch.rs index 1ec8e08fa4..8977c41642 100644 --- a/codex-rs/tui/src/chatwidget/slash_dispatch.rs +++ b/codex-rs/tui/src/chatwidget/slash_dispatch.rs @@ -825,7 +825,7 @@ impl ChatWidget { profile_id: trimmed.to_string(), }); } - SlashCommand::Permissions => self.select_permission_profile_by_id(trimmed), + SlashCommand::Permissions => self.select_permission_profile(trimmed), SlashCommand::Export => { self.set_queue_autosend_suppressed(/*suppressed*/ true); self.app_event_tx.send(AppEvent::ExportTranscript { diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__saved_quick_dials_source.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__saved_quick_dials_source.snap new file mode 100644 index 0000000000..c07269a2f4 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__saved_quick_dials_source.snap @@ -0,0 +1,12 @@ +--- +source: tui/src/chatwidget/tests/quick_dials_tests.rs +expression: "render_bottom_popup(&chat, 80)" +--- + Start preset from current settings + + +› 1. Current model /model gpt-5.5 --speed=standard + 2. Current permissions /permissions :workspace --approval-policy=on-request + --reviewer=user + + enter select · esc back diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__saved_quick_dials_source_unavailable_model.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__saved_quick_dials_source_unavailable_model.snap new file mode 100644 index 0000000000..5e0df09238 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__saved_quick_dials_source_unavailable_model.snap @@ -0,0 +1,13 @@ +--- +source: tui/src/chatwidget/tests/quick_dials_tests.rs +expression: "render_bottom_popup(&chat, 80)" +--- + Start preset from current settings + + + Current model Choose an available reasoning effort with /model + before saving. +› 1. Current permissions /permissions :workspace --approval-policy=on-request + --reviewer=auto_review + + enter select · esc back diff --git a/codex-rs/tui/src/chatwidget/tests/permissions.rs b/codex-rs/tui/src/chatwidget/tests/permissions.rs index 221c58e3cb..edc6565840 100644 --- a/codex-rs/tui/src/chatwidget/tests/permissions.rs +++ b/codex-rs/tui/src/chatwidget/tests/permissions.rs @@ -92,7 +92,7 @@ async fn permission_discovery_uses_server_catalog_for_remote_custom_selection() async fn permissions_command_waits_for_covering_modal_before_selecting_profile() { let (mut chat, mut events, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; chat.thread_id = Some(ThreadId::new()); - chat.select_permission_profile_by_id(BUILT_IN_PERMISSION_PROFILE_READ_ONLY); + chat.select_permission_profile(BUILT_IN_PERMISSION_PROFILE_READ_ONLY); let request_id = assert_matches!( events.try_recv(), Ok(AppEvent::FetchPermissionProfiles { request_id, .. }) => request_id @@ -125,7 +125,7 @@ async fn cancelled_permissions_command_does_not_apply_after_later_modal() { chat.input_queue .queued_user_messages .push_back(UserMessage::from("follow-up").into()); - chat.select_permission_profile_by_id(BUILT_IN_PERMISSION_PROFILE_READ_ONLY); + chat.select_permission_profile(BUILT_IN_PERMISSION_PROFILE_READ_ONLY); let request_id = assert_matches!( events.try_recv(), Ok(AppEvent::FetchPermissionProfiles { request_id, .. }) => request_id @@ -153,6 +153,7 @@ async fn cancelled_permissions_command_does_not_apply_after_later_modal() { async fn permissions_command_preserves_full_access_confirmation() { let (mut chat, mut events, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; chat.thread_id = Some(ThreadId::new()); + chat.set_feature_enabled(Feature::GuardianApproval, /*enabled*/ true); chat.permission_discovery = Some(Discovery::local(&chat.config)); chat.input_queue .queued_user_messages @@ -160,7 +161,7 @@ async fn permissions_command_preserves_full_access_confirmation() { chat.dispatch_command_with_args( SlashCommand::Permissions, - BUILT_IN_PERMISSION_PROFILE_DANGER_FULL_ACCESS.to_string(), + format!("{BUILT_IN_PERMISSION_PROFILE_DANGER_FULL_ACCESS} --reviewer=auto_review"), Vec::new(), ); @@ -172,6 +173,7 @@ async fn permissions_command_preserves_full_access_confirmation() { profile_selection: Some(profile_selection), }) if preset.id == "full-access" && profile_selection.profile_id == BUILT_IN_PERMISSION_PROFILE_DANGER_FULL_ACCESS + && profile_selection.approvals_reviewer == Some(ApprovalsReviewer::AutoReview) => (preset, return_to_permissions, Some(profile_selection)) ); assert_matches!(events.try_recv(), Ok(AppEvent::SettingsSelectionClosed)); @@ -249,7 +251,7 @@ async fn invalidating_inline_permission_discovery_recovers_queued_input() { chat.input_queue .queued_user_messages .push_back(UserMessage::from("follow-up").into()); - chat.select_permission_profile_by_id(BUILT_IN_PERMISSION_PROFILE_READ_ONLY); + chat.select_permission_profile(BUILT_IN_PERMISSION_PROFILE_READ_ONLY); let request_id = assert_matches!( events.try_recv(), Ok(AppEvent::FetchPermissionProfiles { request_id, .. }) => request_id @@ -267,7 +269,7 @@ async fn invalidating_inline_permission_discovery_recovers_queued_input() { async fn inline_permission_discovery_retry_preserves_profile_id() { let (mut chat, mut events, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; chat.thread_id = Some(ThreadId::new()); - chat.select_permission_profile_by_id("server-only"); + chat.select_permission_profile("server-only"); let request_id = assert_matches!( events.try_recv(), Ok(AppEvent::FetchPermissionProfiles { request_id, .. }) => request_id @@ -534,12 +536,12 @@ async fn profile_permissions_selection_emits_named_profile_event_only() { let events = std::iter::from_fn(|| rx.try_recv().ok()).collect::>(); assert_eq!(events.len(), 1); - assert!( - chat.try_select_permission_profile_by_id( - &Discovery::local(&chat.config), - ":ask-for-approval", - ) - ); + assert!(chat.try_select_permission_profile( + &Discovery::local(&chat.config), + ":ask-for-approval", + /*requested_approval_policy*/ None, + /*requested_reviewer*/ None, + )); for event in events .into_iter() .chain(std::iter::from_fn(|| rx.try_recv().ok())) @@ -611,12 +613,12 @@ async fn profile_permissions_selection_emits_auto_review_mode_event() { let events = std::iter::from_fn(|| rx.try_recv().ok()).collect::>(); assert_eq!(events.len(), 1); - assert!( - chat.try_select_permission_profile_by_id( - &Discovery::local(&chat.config), - ":approve-for-me", - ) - ); + assert!(chat.try_select_permission_profile( + &Discovery::local(&chat.config), + ":approve-for-me", + /*requested_approval_policy*/ None, + /*requested_reviewer*/ None, + )); for event in events .into_iter() .chain(std::iter::from_fn(|| rx.try_recv().ok())) @@ -633,6 +635,109 @@ async fn profile_permissions_selection_emits_auto_review_mode_event() { } } +#[tokio::test] +async fn permissions_command_selects_auto_review_mode() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + #[cfg(target_os = "windows")] + { + chat.set_windows_sandbox_mode(Some(WindowsSandboxSetupMode::Unelevated)); + } + chat.config.explicit_permission_profile_mode = true; + chat.set_feature_enabled(Feature::GuardianApproval, /*enabled*/ true); + chat.permission_discovery = Some(Discovery::local(&chat.config)); + + chat.select_permission_profile( + ":workspace --approval-policy on-request --reviewer auto_review", + ); + + assert_matches!( + rx.try_recv(), + Ok(AppEvent::SelectPermissionProfile(PermissionProfileSelection { + profile_id, + approval_policy: Some(AskForApproval::OnRequest), + approvals_reviewer: Some(ApprovalsReviewer::AutoReview), + display_label, + })) if profile_id == ":workspace" && display_label == "Approve for me" + ); +} + +#[tokio::test] +async fn permissions_command_restores_quoted_profile_and_complete_settings() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + let granular = AskForApproval::Granular { + sandbox_approval: false, + rules: true, + skill_approval: false, + request_permissions: true, + mcp_elicitations: false, + }; + chat.config.custom_permission_profiles = vec![PermissionProfileCatalogEntry { + id: "team review".to_string(), + description: None, + allowed: true, + }]; + chat.permission_discovery = Some(Discovery::local(&chat.config)); + let policy = serde_json::to_string(&granular).expect("granular policy serializes"); + + chat.select_permission_profile(&format!( + "'team review' --approval-policy='{policy}' --reviewer user" + )); + + assert_matches!( + rx.try_recv(), + Ok(AppEvent::SelectPermissionProfile(PermissionProfileSelection { + profile_id, + approval_policy: Some(actual_policy), + approvals_reviewer: Some(ApprovalsReviewer::User), + display_label, + })) if profile_id == "team review" + && display_label == "team review" + && actual_policy == granular + ); +} + +#[tokio::test] +async fn permissions_command_rejects_unavailable_auto_review_and_settles_input() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + chat.set_feature_enabled(Feature::GuardianApproval, /*enabled*/ false); + chat.permission_discovery = Some(Discovery::local(&chat.config)); + chat.input_queue + .queued_user_messages + .push_back(UserMessage::from("follow-up").into()); + + chat.select_permission_profile(":workspace --reviewer=auto_review"); + + let events = std::iter::from_fn(|| rx.try_recv().ok()).collect::>(); + assert!( + events + .iter() + .all(|event| !matches!(event, AppEvent::SelectPermissionProfile(_))) + ); + assert!( + events + .iter() + .any(|event| matches!(event, AppEvent::SettingsSelectionClosed)) + ); + assert!(chat.input_queue.recovered_queue); +} + +#[tokio::test] +async fn invalid_permissions_arguments_settle_suppressed_input() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + chat.input_queue + .queued_user_messages + .push_back(UserMessage::from("follow-up").into()); + chat.set_queue_autosend_suppressed(/*suppressed*/ true); + + chat.select_permission_profile(":workspace --reviewer=bogus"); + + assert!(chat.input_queue.recovered_queue); + assert!( + std::iter::from_fn(|| rx.try_recv().ok()) + .any(|event| matches!(event, AppEvent::SettingsSelectionClosed)) + ); +} + #[tokio::test] async fn profile_permissions_full_access_always_opens_confirmation() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; @@ -1437,12 +1542,12 @@ async fn permissions_selection_hides_auto_review_when_feature_disabled() { !popup.contains("Approve for me"), "expected Approve for me to stay hidden until the feature is enabled: {popup}" ); - assert!( - !chat.try_select_permission_profile_by_id( - &Discovery::local(&chat.config), - ":approve-for-me", - ) - ); + assert!(!chat.try_select_permission_profile( + &Discovery::local(&chat.config), + ":approve-for-me", + /*requested_approval_policy*/ None, + /*requested_reviewer*/ None, + )); let cell = assert_matches!(rx.try_recv(), Ok(AppEvent::InsertHistoryCell(cell)) => cell); insta::assert_snapshot!(lines_to_single_string(&cell.display_lines(/*width*/ 120)), @"■ Approve for me is unavailable. Run `/permissions` to see available profiles."); } diff --git a/codex-rs/tui/src/chatwidget/tests/quick_dials_tests.rs b/codex-rs/tui/src/chatwidget/tests/quick_dials_tests.rs index 014e2c0ee1..23e278edbf 100644 --- a/codex-rs/tui/src/chatwidget/tests/quick_dials_tests.rs +++ b/codex-rs/tui/src/chatwidget/tests/quick_dials_tests.rs @@ -4,6 +4,83 @@ use super::*; use crate::chatwidget::quick_dials::QuickDialEditorTarget; use codex_config::types::QuickDial; +#[tokio::test] +async fn current_settings_offer_model_and_permissions() { + let (mut chat, mut events, _ops) = + make_chatwidget_manual(/*model_override*/ Some("gpt-5.5")).await; + chat.thread_id = Some(ThreadId::new()); + chat.set_permission_profile_from_session_snapshot(PermissionProfileSnapshot::active( + PermissionProfile::workspace_write(), + ActivePermissionProfile::new(":workspace"), + )) + .unwrap(); + + chat.open_quick_dial_prefills(); + + assert_chatwidget_snapshot!( + "saved_quick_dials_source", + render_bottom_popup(&chat, /*width*/ 80) + ); + chat.handle_key_event(KeyCode::Down.into()); + chat.handle_key_event(KeyCode::Enter.into()); + assert_matches!( + events.try_recv(), + Ok(AppEvent::OpenQuickDialEditor { + target: QuickDialEditorTarget::PickSlot, + dial: Some(QuickDial { input, label: None }), + }) if input == "/permissions :workspace --approval-policy=on-request --reviewer=user" + ); + + chat.set_reasoning_effort(Some(ReasoningEffortConfig::Custom("removed-effort".into()))); + chat.config.approvals_reviewer = ApprovalsReviewer::AutoReview; + chat.open_quick_dial_prefills(); + assert_chatwidget_snapshot!( + "saved_quick_dials_source_unavailable_model", + render_bottom_popup(&chat, /*width*/ 80) + ); + chat.handle_key_event(KeyCode::Down.into()); + chat.handle_key_event(KeyCode::Enter.into()); + assert_matches!( + events.try_recv(), + Ok(AppEvent::OpenQuickDialEditor { + target: QuickDialEditorTarget::PickSlot, + dial: Some(QuickDial { input, label: None }), + }) if input == "/permissions :workspace --approval-policy=on-request --reviewer=auto_review" + ); + + let granular = AskForApproval::Granular { + sandbox_approval: false, + rules: true, + skill_approval: false, + request_permissions: true, + mcp_elicitations: false, + }; + chat.config + .permissions + .approval_policy + .set(granular.to_core()) + .expect("set granular approval policy"); + chat.config.approvals_reviewer = ApprovalsReviewer::User; + chat.set_permission_profile_from_session_snapshot(PermissionProfileSnapshot::active( + PermissionProfile::workspace_write(), + ActivePermissionProfile::new("team review"), + )) + .expect("set named permission profile"); + chat.open_quick_dial_prefills(); + chat.handle_key_event(KeyCode::Down.into()); + chat.handle_key_event(KeyCode::Enter.into()); + let policy = serde_json::to_string(&granular).expect("granular policy serializes"); + assert_matches!( + events.try_recv(), + Ok(AppEvent::OpenQuickDialEditor { + target: QuickDialEditorTarget::PickSlot, + dial: Some(QuickDial { input, label: None }), + }) if input == format!( + "/permissions 'team review' --approval-policy='{policy}' --reviewer=user" + ) + ); +} + #[tokio::test] async fn saved_quick_dials_save_and_clear_picker_snapshots() { const DRAFT: &str = "Review the current changes\nfor safety "; diff --git a/codex-rs/tui/src/tooltips/keybinding_tests.rs b/codex-rs/tui/src/tooltips/keybinding_tests.rs index 5b8a549f56..c77371bfed 100644 --- a/codex-rs/tui/src/tooltips/keybinding_tests.rs +++ b/codex-rs/tui/src/tooltips/keybinding_tests.rs @@ -40,6 +40,14 @@ fn configured_shortcut_tips_render_at_narrow_width() { lines.push("".into()); } } + // This snapshot owns the preset hint's content and layout; Markdown tests own the + // terminal-default foreground applied to the shared "Tip:" prefix. + for line in lines + .iter_mut() + .filter(|line| line.to_string().contains("then 0–9")) + { + line.spans[0].style.fg = None; + } insta::assert_snapshot!( lines .iter()