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 <img src="https://github.com/user-attachments/assets/00f93c39-990f-4b79-84ec-0427b997167f" width="500"/> ### Settings View <img src="https://github.com/user-attachments/assets/52d32435-7c8d-4ab4-a319-6cabc007267b" width="500"/> Release Notes: - N/A --------- Co-authored-by: Thomas Mickley-Doyle <tmickleydoyle@gmail.com> Co-authored-by: Bennet Bo Fenner <bennetbo@gmx.de> Co-authored-by: Nathan Sobo <nathan@zed.dev> Co-authored-by: Antonio Scandurra <me@as-cii.com>
This commit is contained in:
Danilo Leal
GitHub
parent
3e2ac3e7bc
commit
0be8bf1b12
8 changed files with 207 additions and 120 deletions
1
assets/icons/check_double.svg
Normal file
1
assets/icons/check_double.svg
Normal file
|
|
@ -0,0 +1 @@
|
|||
<svg xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" class="lucide lucide-check-check-icon lucide-check-check"><path d="M18 6 7 17l-5-5"/><path d="m22 10-7.5 7.5L13 16"/></svg>
|
||||
|
After Width: | Height: | Size: 305 B |
|
|
@ -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": {
|
||||
|
|
|
|||
|
|
@ -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::<AssistantSettings>(
|
||||
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<str> = 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<Self>,
|
||||
) -> impl Iterator<Item = AnyElement> + '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<ActiveThread>) {
|
||||
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))
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<dyn Fs>,
|
||||
focus_handle: FocusHandle,
|
||||
configuration_views_by_provider: HashMap<LanguageModelProviderId, AnyView>,
|
||||
context_server_manager: Entity<ContextServerManager>,
|
||||
|
|
@ -29,6 +33,7 @@ pub struct AssistantConfiguration {
|
|||
|
||||
impl AssistantConfiguration {
|
||||
pub fn new(
|
||||
fs: Arc<dyn Fs>,
|
||||
context_server_manager: Entity<ContextServerManager>,
|
||||
tools: Arc<ToolWorkingSet>,
|
||||
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<Self>) -> 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::<AssistantSettings>(
|
||||
fs.clone(),
|
||||
cx,
|
||||
move |settings, _| {
|
||||
settings.set_always_allow_tool_actions(allow);
|
||||
},
|
||||
);
|
||||
}
|
||||
}),
|
||||
),
|
||||
)
|
||||
}
|
||||
|
||||
fn render_context_servers_section(&mut self, cx: &mut Context<Self>) -> 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(
|
||||
|
|
|
|||
|
|
@ -482,11 +482,13 @@ impl AssistantPanel {
|
|||
pub(crate) fn open_configuration(&mut self, window: &mut Window, cx: &mut Context<Self>) {
|
||||
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(
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -46,10 +46,21 @@ impl Tool for BashTool {
|
|||
fn ui_text(&self, input: &serde_json::Value) -> String {
|
||||
match serde_json::from_value::<BashToolInput>(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(),
|
||||
|
|
|
|||
|
|
@ -46,6 +46,7 @@ pub enum IconName {
|
|||
Brain,
|
||||
CaseSensitive,
|
||||
Check,
|
||||
CheckDouble,
|
||||
ChevronDown,
|
||||
/// This chevron indicates a popover menu.
|
||||
ChevronDownSmall,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue