yenru0/zed
1
0 Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

agent_ui: Fix permissions getting granted to wrong tool call (#54683)

Browse Source 
Release Notes:

- Fixed an issue where the first tool call would always be accepted when
parallel tool calls were made, rather than the one that was actually
clicked

---------

Co-authored-by: Bennet Bo Fenner <bennetbo@gmx.de>
This commit is contained in:
Tom HouléandBennet Bo Fenner authored and GitHub committed 2026-04-24 09:52:28 +00:00
1 parent 749fcfdfd8
commit 89d2dcebba
2 files changed
+242 -46
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

crates/agent_ui/src/conversation_view.rs
+203
View File
@@ -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<Self>,
) -> 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<RootThreadUpdated> for ConversationView {}
fn resolve_outcome_from_selection(
options: &PermissionOptions,
selection: Option<&thread_view::PermissionSelection>,
is_allow: bool,
) -> Option<SelectedPermissionOutcome> {
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);
crates/agent_ui/src/conversation_view/thread_view.rs
+39 -46
View File
@@ -1857,64 +1857,40 @@ impl ThreadView {
cx: &mut Context<Self>,
) -> 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<Self>,
) -> 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<Self>,
@@ -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,
);
}
})),
),