Cargo.lock 🔗
@@ -355,6 +355,7 @@ dependencies = [
"agent_settings",
"ai_onboarding",
"anyhow",
+ "arrayvec",
"assistant_context",
"assistant_slash_command",
"assistant_slash_commands",
acp: Add keybindings for authorizing tool calls (#37876)
Bennet Bo Fenner and Agus Zubiaga created
TODO:
- [x] Double-check if we like the naming of the new actions
- [x] Only show keybinding hint once per option (e.g. if there are two
`allow_once` buttons only show it on the first one)
- [x] If there are multiple tool calls that need authorisation, only
show keybindings on the first tool call
- [x] Figure out which keybindings to use
- [x] Add linux keybindings
- [x] Add windows keybindings
- [x] Bug: long keybindings can make the buttons overflow
Release Notes:
- Add keybindings for authorizing tool calls (`agent: Allow once`,
`agent: Allow always`, `agent: Reject once`) in the agent panel
---------
Co-authored-by: Agus Zubiaga <agus@zed.dev>Change summary
Cargo.lock | 1
assets/keymaps/default-linux.json | 5
assets/keymaps/default-macos.json | 7
assets/keymaps/default-windows.json | 5
crates/acp_thread/src/acp_thread.rs | 36 ++---
crates/agent_ui/Cargo.toml | 1
crates/agent_ui/src/acp/thread_view.rs | 171 ++++++++++++++++++---------
crates/agent_ui/src/agent_ui.rs | 6
8 files changed, 151 insertions(+), 81 deletions(-)
Detailed changes
assets/keymaps/default-linux.json 🔗
@@ -247,7 +247,10 @@
"ctrl-shift-e": "project_panel::ToggleFocus",
"ctrl-shift-enter": "agent::ContinueThread",
"super-ctrl-b": "agent::ToggleBurnMode",
- "alt-enter": "agent::ContinueWithBurnMode"
+ "alt-enter": "agent::ContinueWithBurnMode",
+ "ctrl-y": "agent::AllowOnce",
+ "ctrl-alt-y": "agent::AllowAlways",
+ "ctrl-d": "agent::RejectOnce"
}
},
{
assets/keymaps/default-macos.json 🔗
@@ -218,7 +218,7 @@
}
},
{
- "context": "Editor && !agent_diff",
+ "context": "Editor && !agent_diff && !AgentPanel",
"use_key_equivalents": true,
"bindings": {
"cmd-alt-z": "git::Restore",
@@ -286,7 +286,10 @@
"cmd-shift-e": "project_panel::ToggleFocus",
"cmd-ctrl-b": "agent::ToggleBurnMode",
"cmd-shift-enter": "agent::ContinueThread",
- "alt-enter": "agent::ContinueWithBurnMode"
+ "alt-enter": "agent::ContinueWithBurnMode",
+ "cmd-y": "agent::AllowOnce",
+ "cmd-alt-y": "agent::AllowAlways",
+ "cmd-d": "agent::RejectOnce"
}
},
{
assets/keymaps/default-windows.json 🔗
@@ -249,7 +249,10 @@
"ctrl-shift-e": "project_panel::ToggleFocus",
"ctrl-shift-enter": "agent::ContinueThread",
"super-ctrl-b": "agent::ToggleBurnMode",
- "alt-enter": "agent::ContinueWithBurnMode"
+ "alt-enter": "agent::ContinueWithBurnMode",
+ "ctrl-y": "agent::AllowOnce",
+ "ctrl-alt-y": "agent::AllowAlways",
+ "ctrl-d": "agent::RejectOnce"
}
},
{
crates/acp_thread/src/acp_thread.rs 🔗
@@ -813,7 +813,6 @@ impl EventEmitter<AcpThreadEvent> for AcpThread {}
#[derive(PartialEq, Eq, Debug)]
pub enum ThreadStatus {
Idle,
- WaitingForToolConfirmation,
Generating,
}
@@ -936,11 +935,7 @@ impl AcpThread {
pub fn status(&self) -> ThreadStatus {
if self.send_task.is_some() {
- if self.waiting_for_tool_confirmation() {
- ThreadStatus::WaitingForToolConfirmation
- } else {
- ThreadStatus::Generating
- }
+ ThreadStatus::Generating
} else {
ThreadStatus::Idle
}
@@ -1382,26 +1377,27 @@ impl AcpThread {
cx.emit(AcpThreadEvent::EntryUpdated(ix));
}
- /// Returns true if the last turn is awaiting tool authorization
- pub fn waiting_for_tool_confirmation(&self) -> bool {
+ pub fn first_tool_awaiting_confirmation(&self) -> Option<&ToolCall> {
+ let mut first_tool_call = None;
+
for entry in self.entries.iter().rev() {
match &entry {
- AgentThreadEntry::ToolCall(call) => match call.status {
- ToolCallStatus::WaitingForConfirmation { .. } => return true,
- ToolCallStatus::Pending
- | ToolCallStatus::InProgress
- | ToolCallStatus::Completed
- | ToolCallStatus::Failed
- | ToolCallStatus::Rejected
- | ToolCallStatus::Canceled => continue,
- },
+ AgentThreadEntry::ToolCall(call) => {
+ if let ToolCallStatus::WaitingForConfirmation { .. } = call.status {
+ first_tool_call = Some(call);
+ } else {
+ continue;
+ }
+ }
AgentThreadEntry::UserMessage(_) | AgentThreadEntry::AssistantMessage(_) => {
- // Reached the beginning of the turn
- return false;
+ // Reached the beginning of the turn.
+ // If we had pending permission requests in the previous turn, they have been cancelled.
+ break;
}
}
}
- false
+
+ first_tool_call
}
pub fn plan(&self) -> &Plan {
crates/agent_ui/Cargo.toml 🔗
@@ -25,6 +25,7 @@ agent_servers.workspace = true
agent_settings.workspace = true
ai_onboarding.workspace = true
anyhow.workspace = true
+arrayvec.workspace = true
assistant_context.workspace = true
assistant_slash_command.workspace = true
assistant_slash_commands.workspace = true
crates/agent_ui/src/acp/thread_view.rs 🔗
@@ -10,6 +10,7 @@ use agent_servers::{AgentServer, AgentServerDelegate};
use agent_settings::{AgentProfileId, AgentSettings, CompletionMode, NotifyWhenAgentWaiting};
use agent2::{DbThreadMetadata, HistoryEntry, HistoryEntryId, HistoryStore, NativeAgentServer};
use anyhow::{Context as _, Result, anyhow, bail};
+use arrayvec::ArrayVec;
use audio::{Audio, Sound};
use buffer_diff::BufferDiff;
use client::zed_urls;
@@ -65,9 +66,9 @@ use crate::ui::{
AgentNotification, AgentNotificationEvent, BurnModeTooltip, UnavailableEditingTooltip,
};
use crate::{
- AgentDiffPane, AgentPanel, ContinueThread, ContinueWithBurnMode, CycleModeSelector,
- ExpandMessageEditor, Follow, KeepAll, OpenAgentDiff, OpenHistory, RejectAll, ToggleBurnMode,
- ToggleProfileSelector,
+ AgentDiffPane, AgentPanel, AllowAlways, AllowOnce, ContinueThread, ContinueWithBurnMode,
+ CycleModeSelector, ExpandMessageEditor, Follow, KeepAll, OpenAgentDiff, OpenHistory, RejectAll,
+ RejectOnce, ToggleBurnMode, ToggleProfileSelector,
};
pub const MIN_EDITOR_LINES: usize = 4;
@@ -2167,6 +2168,7 @@ impl AcpThreadView {
options,
entry_ix,
tool_call.id.clone(),
+ window,
cx,
))
.into_any(),
@@ -2469,69 +2471,85 @@ impl AcpThreadView {
options: &[acp::PermissionOption],
entry_ix: usize,
tool_call_id: acp::ToolCallId,
+ window: &Window,
cx: &Context<Self>,
) -> Div {
- h_flex()
- .py_1()
- .px_1()
- .gap_1()
- .justify_between()
- .flex_wrap()
+ let is_first = self.thread().is_some_and(|thread| {
+ thread
+ .read(cx)
+ .first_tool_awaiting_confirmation()
+ .is_some_and(|call| call.id == tool_call_id)
+ });
+ let mut seen_kinds: ArrayVec<acp::PermissionOptionKind, 3> = ArrayVec::new();
+
+ div()
+ .p_1()
.border_t_1()
.border_color(self.tool_card_border_color(cx))
- .when(kind != acp::ToolKind::SwitchMode, |this| {
- this.pl_2().child(
- div().min_w(rems_from_px(145.)).child(
- LoadingLabel::new("Waiting for Confirmation").size(LabelSize::Small),
- ),
- )
+ .w_full()
+ .map(|this| {
+ if kind == acp::ToolKind::SwitchMode {
+ this.v_flex()
+ } else {
+ this.h_flex().justify_end().flex_wrap()
+ }
})
- .child({
- div()
+ .gap_0p5()
+ .children(options.iter().map(move |option| {
+ let option_id = SharedString::from(option.id.0.clone());
+ Button::new((option_id, entry_ix), option.name.clone())
.map(|this| {
- if kind == acp::ToolKind::SwitchMode {
- this.w_full().v_flex()
- } else {
- this.h_flex()
+ let (this, action) = match option.kind {
+ acp::PermissionOptionKind::AllowOnce => (
+ this.icon(IconName::Check).icon_color(Color::Success),
+ Some(&AllowOnce as &dyn Action),
+ ),
+ acp::PermissionOptionKind::AllowAlways => (
+ this.icon(IconName::CheckDouble).icon_color(Color::Success),
+ Some(&AllowAlways as &dyn Action),
+ ),
+ acp::PermissionOptionKind::RejectOnce => (
+ this.icon(IconName::Close).icon_color(Color::Error),
+ Some(&RejectOnce as &dyn Action),
+ ),
+ acp::PermissionOptionKind::RejectAlways => {
+ (this.icon(IconName::Close).icon_color(Color::Error), None)
+ }
+ };
+
+ let Some(action) = action else {
+ return this;
+ };
+
+ if !is_first || seen_kinds.contains(&option.kind) {
+ return this;
}
+
+ seen_kinds.push(option.kind);
+
+ this.key_binding(
+ KeyBinding::for_action_in(action, &self.focus_handle, window, cx)
+ .map(|kb| kb.size(rems_from_px(10.))),
+ )
})
- .gap_0p5()
- .children(options.iter().map(|option| {
- let option_id = SharedString::from(option.id.0.clone());
- Button::new((option_id, entry_ix), option.name.clone())
- .map(|this| match option.kind {
- acp::PermissionOptionKind::AllowOnce => {
- this.icon(IconName::Check).icon_color(Color::Success)
- }
- acp::PermissionOptionKind::AllowAlways => {
- this.icon(IconName::CheckDouble).icon_color(Color::Success)
- }
- acp::PermissionOptionKind::RejectOnce => {
- this.icon(IconName::Close).icon_color(Color::Error)
- }
- acp::PermissionOptionKind::RejectAlways => {
- this.icon(IconName::Close).icon_color(Color::Error)
- }
- })
- .icon_position(IconPosition::Start)
- .icon_size(IconSize::XSmall)
- .label_size(LabelSize::Small)
- .on_click(cx.listener({
- let tool_call_id = tool_call_id.clone();
- let option_id = option.id.clone();
- let option_kind = option.kind;
- move |this, _, window, cx| {
- this.authorize_tool_call(
- tool_call_id.clone(),
- option_id.clone(),
- option_kind,
- window,
- cx,
- );
- }
- }))
+ .icon_position(IconPosition::Start)
+ .icon_size(IconSize::XSmall)
+ .label_size(LabelSize::Small)
+ .on_click(cx.listener({
+ let tool_call_id = tool_call_id.clone();
+ let option_id = option.id.clone();
+ let option_kind = option.kind;
+ move |this, _, window, cx| {
+ this.authorize_tool_call(
+ tool_call_id.clone(),
+ option_id.clone(),
+ option_kind,
+ window,
+ cx,
+ );
+ }
}))
- })
+ }))
}
fn render_diff_loading(&self, cx: &Context<Self>) -> AnyElement {
@@ -3978,6 +3996,42 @@ impl AcpThreadView {
.detach();
}
+ fn allow_always(&mut self, _: &AllowAlways, window: &mut Window, cx: &mut Context<Self>) {
+ self.authorize_pending_tool_call(acp::PermissionOptionKind::AllowAlways, window, cx);
+ }
+
+ fn allow_once(&mut self, _: &AllowOnce, window: &mut Window, cx: &mut Context<Self>) {
+ self.authorize_pending_tool_call(acp::PermissionOptionKind::AllowOnce, window, cx);
+ }
+
+ fn reject_once(&mut self, _: &RejectOnce, window: &mut Window, cx: &mut Context<Self>) {
+ self.authorize_pending_tool_call(acp::PermissionOptionKind::RejectOnce, window, cx);
+ }
+
+ fn authorize_pending_tool_call(
+ &mut self,
+ kind: acp::PermissionOptionKind,
+ window: &mut Window,
+ cx: &mut Context<Self>,
+ ) -> Option<()> {
+ let thread = self.thread()?.read(cx);
+ let tool_call = thread.first_tool_awaiting_confirmation()?;
+ let ToolCallStatus::WaitingForConfirmation { options, .. } = &tool_call.status else {
+ return None;
+ };
+ let option = options.iter().find(|o| o.kind == kind)?;
+
+ self.authorize_tool_call(
+ tool_call.id.clone(),
+ option.id.clone(),
+ option.kind,
+ window,
+ cx,
+ );
+
+ Some(())
+ }
+
fn render_burn_mode_toggle(&self, cx: &mut Context<Self>) -> Option<AnyElement> {
let thread = self.as_native_thread(cx)?.read(cx);
@@ -5299,6 +5353,9 @@ impl Render for AcpThreadView {
.on_action(cx.listener(Self::toggle_burn_mode))
.on_action(cx.listener(Self::keep_all))
.on_action(cx.listener(Self::reject_all))
+ .on_action(cx.listener(Self::allow_always))
+ .on_action(cx.listener(Self::allow_once))
+ .on_action(cx.listener(Self::reject_once))
.track_focus(&self.focus_handle)
.bg(cx.theme().colors().panel_background)
.child(match &self.thread_state {
crates/agent_ui/src/agent_ui.rs 🔗
@@ -116,6 +116,12 @@ actions!(
RejectAll,
/// Keeps all suggestions or changes.
KeepAll,
+ /// Allow this operation only this time.
+ AllowOnce,
+ /// Allow this operation and remember the choice.
+ AllowAlways,
+ /// Reject this operation only this time.
+ RejectOnce,
/// Follows the agent's suggestions.
Follow,
/// Resets the trial upsell notification.