From 0be8bf1b12b954eb39cfa317b92b39506fd224e3 Mon Sep 17 00:00:00 2001
From: Danilo Leal <67129314+danilo-leal@users.noreply.github.com>
Date: Wed, 2 Apr 2025 14:40:49 -0300
Subject: [PATCH] agent: Improve action confirmation UX (#27932)
This PR makes the command permission prompt part of the tool card and
allow users to straight away change the `always_allow_tool_actions`
setting via the "Always Allow" button from that card. If that button is
clicked, that setting is turned on, and any command that requires
permission from that point on will auto-run.
Additionally, if a bash command spans multiple lines, we show the line
count at the end of the command string. (Note: this is not perfect yet
because it can likely be not visible by default, but we didn't think
this was a major blocker for now. We'll work on improving this next).
### Thread View
### Settings View
Release Notes:
- N/A
---------
Co-authored-by: Thomas Mickley-Doyle
Co-authored-by: Bennet Bo Fenner
Co-authored-by: Nathan Sobo
Co-authored-by: Antonio Scandurra
---
assets/icons/check_double.svg | 1 +
assets/settings/default.json | 2 +
crates/agent/src/active_thread.rs | 230 +++++++++---------
crates/agent/src/assistant_configuration.rs | 57 +++++
crates/agent/src/assistant_panel.rs | 8 +-
.../src/assistant_settings.rs | 9 +
crates/assistant_tools/src/bash_tool.rs | 19 +-
crates/icons/src/icons.rs | 1 +
8 files changed, 207 insertions(+), 120 deletions(-)
create mode 100644 assets/icons/check_double.svg
diff --git a/assets/icons/check_double.svg b/assets/icons/check_double.svg
new file mode 100644
index 0000000000000000000000000000000000000000..5c17d95a6bf23f00659508441595ae0f7bed0590
--- /dev/null
+++ b/assets/icons/check_double.svg
@@ -0,0 +1 @@
+
diff --git a/assets/settings/default.json b/assets/settings/default.json
index c5f3475be47a00e434bca26aee0146e3f8380a8f..c09c45f02540f014383455a797d7e4077dc52a3a 100644
--- a/assets/settings/default.json
+++ b/assets/settings/default.json
@@ -633,6 +633,8 @@
// The model to use.
"model": "claude-3-5-sonnet-latest"
},
+ // When enabled, the agent can run potentially destructive actions without asking for your confirmation.
+ "always_allow_tool_actions": false,
"default_profile": "write",
"profiles": {
"ask": {
diff --git a/crates/agent/src/active_thread.rs b/crates/agent/src/active_thread.rs
index 64b9f4cd799599baa33ba0b4a6fd4f5aa4059a37..ed224af9c656d048706908bc51bf16adaf8e691d 100644
--- a/crates/agent/src/active_thread.rs
+++ b/crates/agent/src/active_thread.rs
@@ -24,7 +24,7 @@ use language::{Buffer, LanguageRegistry};
use language_model::{LanguageModelRegistry, LanguageModelToolUseId, Role};
use markdown::{Markdown, MarkdownStyle};
use project::ProjectItem as _;
-use settings::Settings as _;
+use settings::{Settings as _, update_settings_file};
use std::rc::Rc;
use std::sync::Arc;
use std::time::Duration;
@@ -1686,6 +1686,12 @@ impl ActiveThread {
let is_status_finished = matches!(&tool_use.status, ToolUseStatus::Finished(_));
+ let fs = self
+ .workspace
+ .upgrade()
+ .map(|workspace| workspace.read(cx).app_state().fs.clone());
+ let needs_confirmation = matches!(&tool_use.status, ToolUseStatus::NeedsConfirmation);
+
let status_icons = div().child(match &tool_use.status {
ToolUseStatus::Pending | ToolUseStatus::NeedsConfirmation => {
let icon = Icon::new(IconName::Warning)
@@ -1810,7 +1816,7 @@ impl ActiveThread {
if is_status_finished {
element.right_7()
} else {
- element.right_12()
+ element.right(px(46.))
}
})
.bg(linear_gradient(
@@ -1904,7 +1910,6 @@ impl ActiveThread {
h_flex()
.group("disclosure-header")
.relative()
- .gap_1p5()
.justify_between()
.py_1()
.map(|element| {
@@ -1918,6 +1923,8 @@ impl ActiveThread {
.map(|element| {
if is_open {
element.border_b_1().rounded_t_md()
+ } else if needs_confirmation {
+ element.rounded_t_md()
} else {
element.rounded_md()
}
@@ -1975,9 +1982,115 @@ impl ActiveThread {
parent.child(
v_flex()
.bg(cx.theme().colors().editor_background)
- .rounded_b_lg()
+ .map(|element| {
+ if needs_confirmation {
+ element.rounded_none()
+ } else {
+ element.rounded_b_lg()
+ }
+ })
.child(results_content),
)
+ })
+ .when(needs_confirmation, |this| {
+ this.child(
+ h_flex()
+ .py_1()
+ .pl_2()
+ .pr_1()
+ .gap_1()
+ .justify_between()
+ .bg(cx.theme().colors().editor_background)
+ .border_t_1()
+ .border_color(self.tool_card_border_color(cx))
+ .rounded_b_lg()
+ .child(Label::new("Action Confirmation").color(Color::Muted).size(LabelSize::Small))
+ .child(
+ h_flex()
+ .gap_0p5()
+ .child({
+ let tool_id = tool_use.id.clone();
+ Button::new(
+ "always-allow-tool-action",
+ "Always Allow",
+ )
+ .label_size(LabelSize::Small)
+ .icon(IconName::CheckDouble)
+ .icon_position(IconPosition::Start)
+ .icon_size(IconSize::Small)
+ .icon_color(Color::Success)
+ .tooltip(move |window, cx| {
+ Tooltip::with_meta(
+ "Never ask for permission",
+ None,
+ "Restore the original behavior in your Agent Panel settings",
+ window,
+ cx,
+ )
+ })
+ .on_click(cx.listener(
+ move |this, event, window, cx| {
+ if let Some(fs) = fs.clone() {
+ update_settings_file::(
+ fs.clone(),
+ cx,
+ |settings, _| {
+ settings.set_always_allow_tool_actions(true);
+ },
+ );
+ }
+ this.handle_allow_tool(
+ tool_id.clone(),
+ event,
+ window,
+ cx,
+ )
+ },
+ ))
+ })
+ .child(ui::Divider::vertical())
+ .child({
+ let tool_id = tool_use.id.clone();
+ Button::new("allow-tool-action", "Allow")
+ .label_size(LabelSize::Small)
+ .icon(IconName::Check)
+ .icon_position(IconPosition::Start)
+ .icon_size(IconSize::Small)
+ .icon_color(Color::Success)
+ .on_click(cx.listener(
+ move |this, event, window, cx| {
+ this.handle_allow_tool(
+ tool_id.clone(),
+ event,
+ window,
+ cx,
+ )
+ },
+ ))
+ })
+ .child({
+ let tool_id = tool_use.id.clone();
+ let tool_name: Arc = tool_use.name.into();
+ Button::new("deny-tool", "Deny")
+ .label_size(LabelSize::Small)
+ .icon(IconName::Close)
+ .icon_position(IconPosition::Start)
+ .icon_size(IconSize::Small)
+ .icon_color(Color::Error)
+ .on_click(cx.listener(
+ move |this, event, window, cx| {
+ this.handle_deny_tool(
+ tool_id.clone(),
+ tool_name.clone(),
+ event,
+ window,
+ cx,
+ )
+ },
+ ))
+ }),
+ ),
+ )
}),
)
}
@@ -2102,114 +2215,6 @@ impl ActiveThread {
}
}
- fn render_confirmations<'a>(
- &'a mut self,
- cx: &'a mut Context,
- ) -> impl Iterator- + 'a {
- let thread = self.thread.read(cx);
-
- thread.tools_needing_confirmation().map(|tool| {
- // Note: This element should be removed once a more full-fledged permission UX is implemented.
- let beta_tag = h_flex()
- .id("beta-tag")
- .h(px(18.))
- .px_1()
- .gap_1()
- .border_1()
- .border_color(cx.theme().colors().text_accent.opacity(0.2))
- .border_dashed()
- .rounded_sm()
- .bg(cx.theme().colors().text_accent.opacity(0.1))
- .hover(|style| style.bg(cx.theme().colors().text_accent.opacity(0.2)))
- .child(Label::new("Beta").size(LabelSize::XSmall))
- .child(Icon::new(IconName::Info).color(Color::Accent).size(IconSize::Indicator))
- .tooltip(
- Tooltip::text(
- "A future release will introduce a way to remember your answers to these. In the meantime, you can avoid these prompts by adding \"assistant\": { \"always_allow_tool_actions\": true } to your settings.json."
- )
- );
-
- v_flex()
- .mt_2()
- .mx_4()
- .border_1()
- .border_color(self.tool_card_border_color(cx))
- .rounded_lg()
- .child(
- h_flex()
- .py_1()
- .pl_2()
- .pr_1()
- .justify_between()
- .rounded_t_lg()
- .border_b_1()
- .border_color(self.tool_card_border_color(cx))
- .bg(self.tool_card_header_bg(cx))
- .child(
- h_flex()
- .gap_1()
- .child(Label::new("Action Confirmation").size(LabelSize::Small))
- .child(beta_tag),
- )
- .child(
- h_flex()
- .gap_1()
- .child({
- let tool_id = tool.id.clone();
- Button::new("allow-tool-action", "Allow")
- .label_size(LabelSize::Small)
- .icon(IconName::Check)
- .icon_position(IconPosition::Start)
- .icon_size(IconSize::Small)
- .icon_color(Color::Success)
- .on_click(cx.listener(move |this, event, window, cx| {
- this.handle_allow_tool(
- tool_id.clone(),
- event,
- window,
- cx,
- )
- }))
- })
- .child({
- let tool_id = tool.id.clone();
- let tool_name = tool.name.clone();
- Button::new("deny-tool", "Deny")
- .label_size(LabelSize::Small)
- .icon(IconName::Close)
- .icon_position(IconPosition::Start)
- .icon_size(IconSize::Small)
- .icon_color(Color::Error)
- .on_click(cx.listener(move |this, event, window, cx| {
- this.handle_deny_tool(
- tool_id.clone(),
- tool_name.clone(),
- event,
- window,
- cx,
- )
- }))
- }),
- ),
- )
- .child(
- div()
- .id("action_container")
- .rounded_b_lg()
- .bg(cx.theme().colors().editor_background)
- .overflow_y_scroll()
- .max_h_40()
- .p_2p5()
- .child(
- Label::new(&tool.ui_text)
- .size(LabelSize::Small)
- .buffer_font(cx),
- ),
- )
- .into_any()
- })
- }
-
fn dismiss_notifications(&mut self, cx: &mut Context) {
for window in self.notifications.drain(..) {
window
@@ -2262,7 +2267,6 @@ impl Render for ActiveThread {
.size_full()
.relative()
.child(list(self.list_state.clone()).flex_grow())
- .children(self.render_confirmations(cx))
.child(self.render_vertical_scrollbar(cx))
}
}
diff --git a/crates/agent/src/assistant_configuration.rs b/crates/agent/src/assistant_configuration.rs
index 1b6b096ee47d31673c037f66c8646982a32efadb..150662051106508a62b57cf3fbbcdbdd9a41b037 100644
--- a/crates/agent/src/assistant_configuration.rs
+++ b/crates/agent/src/assistant_configuration.rs
@@ -4,11 +4,14 @@ mod tool_picker;
use std::sync::Arc;
+use assistant_settings::AssistantSettings;
use assistant_tool::{ToolSource, ToolWorkingSet};
use collections::HashMap;
use context_server::manager::ContextServerManager;
+use fs::Fs;
use gpui::{Action, AnyView, App, Entity, EventEmitter, FocusHandle, Focusable, Subscription};
use language_model::{LanguageModelProvider, LanguageModelProviderId, LanguageModelRegistry};
+use settings::{Settings, update_settings_file};
use ui::{Disclosure, Divider, DividerColor, ElevationIndex, Indicator, Switch, prelude::*};
use util::ResultExt as _;
use zed_actions::ExtensionCategoryFilter;
@@ -19,6 +22,7 @@ pub(crate) use manage_profiles_modal::ManageProfilesModal;
use crate::AddContextServer;
pub struct AssistantConfiguration {
+ fs: Arc,
focus_handle: FocusHandle,
configuration_views_by_provider: HashMap,
context_server_manager: Entity,
@@ -29,6 +33,7 @@ pub struct AssistantConfiguration {
impl AssistantConfiguration {
pub fn new(
+ fs: Arc,
context_server_manager: Entity,
tools: Arc,
window: &mut Window,
@@ -54,6 +59,7 @@ impl AssistantConfiguration {
);
let mut this = Self {
+ fs,
focus_handle,
configuration_views_by_provider: HashMap::default(),
context_server_manager,
@@ -167,6 +173,55 @@ impl AssistantConfiguration {
)
}
+ fn render_command_permission(&mut self, cx: &mut Context) -> impl IntoElement {
+ let always_allow_tool_actions = AssistantSettings::get_global(cx).always_allow_tool_actions;
+
+ const HEADING: &str = "Allow running tools without asking for confirmation";
+
+ v_flex()
+ .p(DynamicSpacing::Base16.rems(cx))
+ .gap_2()
+ .flex_1()
+ .child(Headline::new("General Settings").size(HeadlineSize::Small))
+ .child(
+ h_flex()
+ .p_2p5()
+ .rounded_sm()
+ .bg(cx.theme().colors().editor_background)
+ .border_1()
+ .border_color(cx.theme().colors().border)
+ .gap_4()
+ .justify_between()
+ .flex_wrap()
+ .child(
+ v_flex()
+ .gap_0p5()
+ .max_w_5_6()
+ .child(Label::new(HEADING))
+ .child(Label::new("When enabled, the agent can perform potentially destructive actions without asking for your confirmation.").color(Color::Muted)),
+ )
+ .child(
+ Switch::new(
+ "always-allow-tool-actions-switch",
+ always_allow_tool_actions.into(),
+ )
+ .on_click({
+ let fs = self.fs.clone();
+ move |state, _window, cx| {
+ let allow = state == &ToggleState::Selected;
+ update_settings_file::(
+ fs.clone(),
+ cx,
+ move |settings, _| {
+ settings.set_always_allow_tool_actions(allow);
+ },
+ );
+ }
+ }),
+ ),
+ )
+ }
+
fn render_context_servers_section(&mut self, cx: &mut Context) -> impl IntoElement {
let context_servers = self.context_server_manager.read(cx).all_servers().clone();
let tools_by_source = self.tools.tools_by_source(cx);
@@ -358,6 +413,8 @@ impl Render for AssistantConfiguration {
.bg(cx.theme().colors().panel_background)
.size_full()
.overflow_y_scroll()
+ .child(self.render_command_permission(cx))
+ .child(Divider::horizontal().color(DividerColor::Border))
.child(self.render_context_servers_section(cx))
.child(Divider::horizontal().color(DividerColor::Border))
.child(
diff --git a/crates/agent/src/assistant_panel.rs b/crates/agent/src/assistant_panel.rs
index cff3f61a2190137bc0c6c72ac722f73b14e59417..e64e5c73602df25aee008421b2d7cfec1c04e0ad 100644
--- a/crates/agent/src/assistant_panel.rs
+++ b/crates/agent/src/assistant_panel.rs
@@ -482,11 +482,13 @@ impl AssistantPanel {
pub(crate) fn open_configuration(&mut self, window: &mut Window, cx: &mut Context) {
let context_server_manager = self.thread_store.read(cx).context_server_manager();
let tools = self.thread_store.read(cx).tools();
+ let fs = self.fs.clone();
self.active_view = ActiveView::Configuration;
- self.configuration = Some(
- cx.new(|cx| AssistantConfiguration::new(context_server_manager, tools, window, cx)),
- );
+ self.configuration =
+ Some(cx.new(|cx| {
+ AssistantConfiguration::new(fs, context_server_manager, tools, window, cx)
+ }));
if let Some(configuration) = self.configuration.as_ref() {
self.configuration_subscription = Some(cx.subscribe_in(
diff --git a/crates/assistant_settings/src/assistant_settings.rs b/crates/assistant_settings/src/assistant_settings.rs
index c5b496e23db40d0d75fac97c619a2c865863e646..e3026fc118a7b870ea56351d422d97307e526ce5 100644
--- a/crates/assistant_settings/src/assistant_settings.rs
+++ b/crates/assistant_settings/src/assistant_settings.rs
@@ -325,6 +325,15 @@ impl AssistantSettingsContent {
}
}
+ pub fn set_always_allow_tool_actions(&mut self, allow: bool) {
+ let AssistantSettingsContent::Versioned(VersionedAssistantSettingsContent::V2(settings)) =
+ self
+ else {
+ return;
+ };
+ settings.always_allow_tool_actions = Some(allow);
+ }
+
pub fn set_profile(&mut self, profile_id: AgentProfileId) {
let AssistantSettingsContent::Versioned(VersionedAssistantSettingsContent::V2(settings)) =
self
diff --git a/crates/assistant_tools/src/bash_tool.rs b/crates/assistant_tools/src/bash_tool.rs
index 9423d3b81b64939830d3ea0189a1a98711e999b5..f504fb61c35a9d5072f5d2e001f344547a39abb7 100644
--- a/crates/assistant_tools/src/bash_tool.rs
+++ b/crates/assistant_tools/src/bash_tool.rs
@@ -46,10 +46,21 @@ impl Tool for BashTool {
fn ui_text(&self, input: &serde_json::Value) -> String {
match serde_json::from_value::(input.clone()) {
Ok(input) => {
- if input.command.contains('\n') {
- MarkdownString::code_block("bash", &input.command).0
- } else {
- MarkdownString::inline_code(&input.command).0
+ let mut lines = input.command.lines();
+ let first_line = lines.next().unwrap_or_default();
+ let remaining_line_count = lines.count();
+ match remaining_line_count {
+ 0 => MarkdownString::inline_code(&first_line).0,
+ 1 => {
+ MarkdownString::inline_code(&format!(
+ "{} - {} more line",
+ first_line, remaining_line_count
+ ))
+ .0
+ }
+ n => {
+ MarkdownString::inline_code(&format!("{} - {} more lines", first_line, n)).0
+ }
}
}
Err(_) => "Run bash command".to_string(),
diff --git a/crates/icons/src/icons.rs b/crates/icons/src/icons.rs
index 31224cb37b77e21f8b38751b5386a2198b4c8f6c..345a0d5ebfe5fb11f4f556b8438daef7bbfd5f22 100644
--- a/crates/icons/src/icons.rs
+++ b/crates/icons/src/icons.rs
@@ -46,6 +46,7 @@ pub enum IconName {
Brain,
CaseSensitive,
Check,
+ CheckDouble,
ChevronDown,
/// This chevron indicates a popover menu.
ChevronDownSmall,