diff --git a/crates/agent_ui/src/conversation_view.rs b/crates/agent_ui/src/conversation_view.rs index 992d5ac937..96b963de19 100644 --- a/crates/agent_ui/src/conversation_view.rs +++ b/crates/agent_ui/src/conversation_view.rs @@ -296,6 +296,20 @@ impl Conversation { self.threads.insert(session_id, thread); } + pub fn permission_options_for_tool_call<'a>( + &'a self, + session_id: &acp::SessionId, + tool_call_id: acp::ToolCallId, + cx: &'a App, + ) -> Option<&'a PermissionOptions> { + let thread = self.threads.get(session_id)?; + let (_, tool_call) = thread.read(cx).tool_call(&tool_call_id)?; + let ToolCallStatus::WaitingForConfirmation { options, .. } = &tool_call.status else { + return None; + }; + Some(options) + } + pub fn pending_tool_call<'a>( &'a self, session_id: &acp::SessionId, @@ -352,6 +366,21 @@ impl Conversation { Some(()) } + pub fn authorize_with_granularity( + &mut self, + session_id: acp::SessionId, + tool_call_id: acp::ToolCallId, + selection: Option<&thread_view::PermissionSelection>, + is_allow: bool, + cx: &mut Context, + ) -> Option<()> { + let options = + self.permission_options_for_tool_call(&session_id, tool_call_id.clone(), cx)?; + let outcome = resolve_outcome_from_selection(options, selection, is_allow)?; + self.authorize_tool_call(session_id, tool_call_id, outcome, cx); + Some(()) + } + pub fn authorize_tool_call( &mut self, session_id: acp::SessionId, @@ -391,6 +420,43 @@ pub(crate) struct RootThreadUpdated; impl EventEmitter for ConversationView {} +fn resolve_outcome_from_selection( + options: &PermissionOptions, + selection: Option<&thread_view::PermissionSelection>, + is_allow: bool, +) -> Option { + let choices = match options { + PermissionOptions::Dropdown(choices) => choices.as_slice(), + PermissionOptions::DropdownWithPatterns { choices, .. } => choices.as_slice(), + PermissionOptions::Flat(_) => { + let kind = if is_allow { + acp::PermissionOptionKind::AllowOnce + } else { + acp::PermissionOptionKind::RejectOnce + }; + let option = options.first_option_of_kind(kind)?; + return Some(SelectedPermissionOutcome::new( + option.option_id.clone(), + option.kind, + )); + } + }; + + // When in per-command pattern mode, use the checked patterns. + if let Some(thread_view::PermissionSelection::SelectedPatterns(checked)) = selection { + if let Some(outcome) = options.build_outcome_for_checked_patterns(checked, is_allow) { + return Some(outcome); + } + } + + // Use the selected granularity choice ("Always for terminal" or "Only this time"). + let selected_index = selection + .and_then(|s| s.choice_index()) + .unwrap_or_else(|| choices.len().saturating_sub(1)); + let selected_choice = choices.get(selected_index).or(choices.last())?; + Some(selected_choice.build_outcome(is_allow)) +} + fn affects_thread_metadata(event: &AcpThreadEvent) -> bool { match event { AcpThreadEvent::NewEntry @@ -6629,6 +6695,143 @@ pub(crate) mod tests { ); } + fn flat_allow_deny_options() -> PermissionOptions { + PermissionOptions::Flat(vec![ + acp::PermissionOption::new( + acp::PermissionOptionId::new("allow"), + "Yes", + acp::PermissionOptionKind::AllowOnce, + ), + acp::PermissionOption::new( + acp::PermissionOptionId::new("deny"), + "No", + acp::PermissionOptionKind::RejectOnce, + ), + ]) + } + + #[test] + fn resolve_outcome_from_selection_flat_allow_picks_allow_once() { + let options = flat_allow_deny_options(); + + let outcome = super::resolve_outcome_from_selection(&options, None, true).unwrap(); + + assert_eq!(outcome.option_id.0.as_ref(), "allow"); + assert_eq!(outcome.option_kind, acp::PermissionOptionKind::AllowOnce); + } + + #[test] + fn resolve_outcome_from_selection_flat_deny_picks_reject_once() { + let options = flat_allow_deny_options(); + + let outcome = super::resolve_outcome_from_selection(&options, None, false).unwrap(); + + assert_eq!(outcome.option_id.0.as_ref(), "deny"); + assert_eq!(outcome.option_kind, acp::PermissionOptionKind::RejectOnce); + } + + #[test] + fn resolve_outcome_from_selection_flat_ignores_selection() { + let options = flat_allow_deny_options(); + // Flat options never consult the granularity choice, even if one is set. + let selection = thread_view::PermissionSelection::Choice(42); + + let outcome = + super::resolve_outcome_from_selection(&options, Some(&selection), true).unwrap(); + + assert_eq!(outcome.option_id.0.as_ref(), "allow"); + } + + #[test] + fn resolve_outcome_from_selection_dropdown_defaults_to_last_choice_when_no_selection() { + let options = + ToolPermissionContext::new(TerminalTool::NAME, vec!["cargo build".to_string()]) + .build_permission_options(); + + let outcome = super::resolve_outcome_from_selection(&options, None, true).unwrap(); + + // Last choice is "Only this time" → option_id "allow". + assert_eq!(outcome.option_id.0.as_ref(), "allow"); + assert_eq!(outcome.option_kind, acp::PermissionOptionKind::AllowOnce); + } + + #[test] + fn resolve_outcome_from_selection_dropdown_uses_selected_choice() { + let options = + ToolPermissionContext::new(TerminalTool::NAME, vec!["cargo build".to_string()]) + .build_permission_options(); + let selection = thread_view::PermissionSelection::Choice(0); + + let outcome = + super::resolve_outcome_from_selection(&options, Some(&selection), true).unwrap(); + + // Choice 0 = "Always for terminal". + assert!(outcome.option_id.0.contains("always_allow:terminal")); + assert_eq!(outcome.option_kind, acp::PermissionOptionKind::AllowAlways); + } + + #[test] + fn resolve_outcome_from_selection_dropdown_out_of_range_falls_back_to_last() { + let options = + ToolPermissionContext::new(TerminalTool::NAME, vec!["cargo build".to_string()]) + .build_permission_options(); + let selection = thread_view::PermissionSelection::Choice(999); + + let outcome = + super::resolve_outcome_from_selection(&options, Some(&selection), true).unwrap(); + + // choices.get(999) is None, falls back to choices.last() → "Only this time". + assert_eq!(outcome.option_id.0.as_ref(), "allow"); + } + + #[test] + fn resolve_outcome_from_selection_pattern_mode_with_empty_checked_falls_back_to_last_choice() { + // Pipeline commands produce `DropdownWithPatterns`, which is required for + // `SelectedPatterns` to be meaningful. + let options = ToolPermissionContext::new( + TerminalTool::NAME, + vec!["cargo test 2>&1 | tail".to_string()], + ) + .build_permission_options(); + assert!(matches!( + options, + PermissionOptions::DropdownWithPatterns { .. } + )); + // Pattern mode with zero checked patterns: `build_outcome_for_checked_patterns` + // returns None, so we fall through to `choice_index()` (which is None for + // `SelectedPatterns`) and default to `choices.last()`. + let selection = thread_view::PermissionSelection::SelectedPatterns(vec![]); + + let outcome = + super::resolve_outcome_from_selection(&options, Some(&selection), true).unwrap(); + + assert_eq!(outcome.option_id.0.as_ref(), "allow"); + assert_eq!(outcome.option_kind, acp::PermissionOptionKind::AllowOnce); + } + + #[test] + fn resolve_outcome_from_selection_pattern_mode_with_checked_uses_always_with_params() { + let options = ToolPermissionContext::new( + TerminalTool::NAME, + vec!["cargo test 2>&1 | tail".to_string()], + ) + .build_permission_options(); + assert!(matches!( + options, + PermissionOptions::DropdownWithPatterns { .. } + )); + let selection = thread_view::PermissionSelection::SelectedPatterns(vec![0]); + + let outcome = + super::resolve_outcome_from_selection(&options, Some(&selection), true).unwrap(); + + assert_eq!(outcome.option_kind, acp::PermissionOptionKind::AllowAlways); + assert!( + outcome.params.is_some(), + "checked patterns should attach terminal params" + ); + } + #[gpui::test] async fn test_manually_editing_title_updates_acp_thread_title(cx: &mut TestAppContext) { init_test(cx); diff --git a/crates/agent_ui/src/conversation_view/thread_view.rs b/crates/agent_ui/src/conversation_view/thread_view.rs index 6f134850c1..439d5f1929 100644 --- a/crates/agent_ui/src/conversation_view/thread_view.rs +++ b/crates/agent_ui/src/conversation_view/thread_view.rs @@ -1857,64 +1857,40 @@ impl ThreadView { cx: &mut Context, ) -> Option<()> { let session_id = self.thread.read(cx).session_id().clone(); - let (returned_session_id, tool_call_id, options) = self + let (returned_session_id, tool_call_id, _) = self .conversation .read(cx) .pending_tool_call(&session_id, cx)?; - let options = options.clone(); - self.authorize_with_granularity( - returned_session_id, - tool_call_id, - &options, - is_allow, - window, - cx, - ) + self.authorize_with_granularity(returned_session_id, tool_call_id, is_allow, window, cx) } fn authorize_with_granularity( &mut self, session_id: acp::SessionId, tool_call_id: acp::ToolCallId, - options: &PermissionOptions, is_allow: bool, window: &mut Window, cx: &mut Context, ) -> Option<()> { - let choices = match options { - PermissionOptions::Dropdown(choices) => choices.as_slice(), - PermissionOptions::DropdownWithPatterns { choices, .. } => choices.as_slice(), - _ => { - let kind = if is_allow { - acp::PermissionOptionKind::AllowOnce - } else { - acp::PermissionOptionKind::RejectOnce - }; - return self.authorize_pending_tool_call(kind, window, cx); - } - }; - - let selection = self.permission_selections.get(&tool_call_id); - - // When in per-command pattern mode, use the checked patterns. - if let Some(PermissionSelection::SelectedPatterns(checked)) = selection { - if let Some(outcome) = options.build_outcome_for_checked_patterns(checked, is_allow) { - self.authorize_tool_call(session_id, tool_call_id, outcome, window, cx); - return Some(()); - } + let selection = self.permission_selections.get(&tool_call_id).cloned(); + let result = self.conversation.update(cx, |conversation, cx| { + conversation.authorize_with_granularity( + session_id, + tool_call_id, + selection.as_ref(), + is_allow, + cx, + ) + }); + if self.should_be_following { + self.workspace + .update(cx, |workspace, cx| { + workspace.follow(CollaboratorId::Agent, window, cx); + }) + .ok(); } - - // Use the selected granularity choice ("Always for terminal" or "Only this time") - let selected_index = selection - .and_then(|s| s.choice_index()) - .unwrap_or_else(|| choices.len().saturating_sub(1)); - - let selected_choice = choices.get(selected_index).or(choices.last())?; - let outcome = selected_choice.build_outcome(is_allow); - - self.authorize_tool_call(session_id, tool_call_id, outcome, window, cx); - - Some(()) + cx.notify(); + result } // edits @@ -6786,6 +6762,7 @@ impl ThreadView { choices, None, entry_ix, + session_id, tool_call_id, focus_handle, cx, @@ -6799,6 +6776,7 @@ impl ThreadView { choices, Some((patterns, tool_name)), entry_ix, + session_id, tool_call_id, focus_handle, cx, @@ -6812,6 +6790,7 @@ impl ThreadView { choices: &[PermissionOptionChoice], patterns: Option<(&[PermissionPattern], &str)>, entry_ix: usize, + session_id: acp::SessionId, tool_call_id: acp::ToolCallId, focus_handle: &FocusHandle, cx: &Context, @@ -6885,8 +6864,16 @@ impl ThreadView { ) }) .on_click(cx.listener({ + let session_id = session_id.clone(); + let tool_call_id = tool_call_id.clone(); move |this, _, window, cx| { - this.authorize_pending_with_granularity(true, window, cx); + this.authorize_with_granularity( + session_id.clone(), + tool_call_id.clone(), + true, + window, + cx, + ); } })), ) @@ -6910,7 +6897,13 @@ impl ThreadView { }) .on_click(cx.listener({ move |this, _, window, cx| { - this.authorize_pending_with_granularity(false, window, cx); + this.authorize_with_granularity( + session_id.clone(), + tool_call_id.clone(), + false, + window, + cx, + ); } })), ),