mirror of
https://github.com/zed-industries/zed.git
synced 2026-06-01 05:51:14 +00:00
agent: Make sure ACP still gets classic tool permission UI (#47142)
This makes sure all of the new granular permission logic and ui only applies to the zed agent and doesn't affect the UI of external agents. Release Notes: - N/A
This commit is contained in:
parent
6c712d88e4
commit
36873662cf
6 changed files with 415 additions and 248 deletions
|
|
@ -477,7 +477,7 @@ pub enum ToolCallStatus {
|
|||
Pending,
|
||||
/// The tool call is waiting for confirmation from the user.
|
||||
WaitingForConfirmation {
|
||||
options: Vec<acp::PermissionOption>,
|
||||
options: PermissionOptions,
|
||||
respond_tx: oneshot::Sender<acp::PermissionOptionId>,
|
||||
},
|
||||
/// The tool call is currently running.
|
||||
|
|
@ -1717,7 +1717,7 @@ impl AcpThread {
|
|||
pub fn request_tool_call_authorization(
|
||||
&mut self,
|
||||
tool_call: acp::ToolCallUpdate,
|
||||
options: Vec<acp::PermissionOption>,
|
||||
options: PermissionOptions,
|
||||
respect_always_allow_setting: bool,
|
||||
cx: &mut Context<Self>,
|
||||
) -> Result<BoxFuture<'static, acp::RequestPermissionOutcome>> {
|
||||
|
|
@ -1726,13 +1726,7 @@ impl AcpThread {
|
|||
if respect_always_allow_setting && AgentSettings::get_global(cx).always_allow_tool_actions {
|
||||
// Don't use AllowAlways, because then if you were to turn off always_allow_tool_actions,
|
||||
// some tools would (incorrectly) continue to auto-accept.
|
||||
if let Some(allow_once_option) = options.iter().find_map(|option| {
|
||||
if matches!(option.kind, acp::PermissionOptionKind::AllowOnce) {
|
||||
Some(option.option_id.clone())
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}) {
|
||||
if let Some(allow_once_option) = options.allow_once_option_id() {
|
||||
self.upsert_tool_call_inner(tool_call, ToolCallStatus::Pending, cx)?;
|
||||
return Ok(async {
|
||||
acp::RequestPermissionOutcome::Selected(acp::SelectedPermissionOutcome::new(
|
||||
|
|
|
|||
|
|
@ -387,6 +387,56 @@ impl AgentModelList {
|
|||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct PermissionOptionChoice {
|
||||
pub allow: acp::PermissionOption,
|
||||
pub deny: acp::PermissionOption,
|
||||
}
|
||||
|
||||
impl PermissionOptionChoice {
|
||||
pub fn label(&self) -> SharedString {
|
||||
self.allow.name.clone().into()
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
pub enum PermissionOptions {
|
||||
Flat(Vec<acp::PermissionOption>),
|
||||
Dropdown(Vec<PermissionOptionChoice>),
|
||||
}
|
||||
|
||||
impl PermissionOptions {
|
||||
pub fn is_empty(&self) -> bool {
|
||||
match self {
|
||||
PermissionOptions::Flat(options) => options.is_empty(),
|
||||
PermissionOptions::Dropdown(options) => options.is_empty(),
|
||||
}
|
||||
}
|
||||
|
||||
pub fn first_option_of_kind(
|
||||
&self,
|
||||
kind: acp::PermissionOptionKind,
|
||||
) -> Option<&acp::PermissionOption> {
|
||||
match self {
|
||||
PermissionOptions::Flat(options) => options.iter().find(|option| option.kind == kind),
|
||||
PermissionOptions::Dropdown(options) => options.iter().find_map(|choice| {
|
||||
if choice.allow.kind == kind {
|
||||
Some(&choice.allow)
|
||||
} else if choice.deny.kind == kind {
|
||||
Some(&choice.deny)
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}),
|
||||
}
|
||||
}
|
||||
|
||||
pub fn allow_once_option_id(&self) -> Option<acp::PermissionOptionId> {
|
||||
self.first_option_of_kind(acp::PermissionOptionKind::AllowOnce)
|
||||
.map(|option| option.option_id.clone())
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(feature = "test-support")]
|
||||
mod test_support {
|
||||
//! Test-only stubs and helpers for acp_thread.
|
||||
|
|
@ -435,7 +485,7 @@ mod test_support {
|
|||
#[derive(Clone, Default)]
|
||||
pub struct StubAgentConnection {
|
||||
sessions: Arc<Mutex<HashMap<acp::SessionId, Session>>>,
|
||||
permission_requests: HashMap<acp::ToolCallId, Vec<acp::PermissionOption>>,
|
||||
permission_requests: HashMap<acp::ToolCallId, PermissionOptions>,
|
||||
next_prompt_updates: Arc<Mutex<Vec<acp::SessionUpdate>>>,
|
||||
}
|
||||
|
||||
|
|
@ -459,7 +509,7 @@ mod test_support {
|
|||
|
||||
pub fn with_permission_requests(
|
||||
mut self,
|
||||
permission_requests: HashMap<acp::ToolCallId, Vec<acp::PermissionOption>>,
|
||||
permission_requests: HashMap<acp::ToolCallId, PermissionOptions>,
|
||||
) -> Self {
|
||||
self.permission_requests = permission_requests;
|
||||
self
|
||||
|
|
|
|||
|
|
@ -1,5 +1,7 @@
|
|||
use super::*;
|
||||
use acp_thread::{AgentConnection, AgentModelGroupName, AgentModelList, UserMessageId};
|
||||
use acp_thread::{
|
||||
AgentConnection, AgentModelGroupName, AgentModelList, PermissionOptions, UserMessageId,
|
||||
};
|
||||
use agent_client_protocol::{self as acp};
|
||||
use agent_settings::AgentProfileId;
|
||||
use anyhow::Result;
|
||||
|
|
@ -947,23 +949,132 @@ async fn next_tool_call_authorization(
|
|||
if let ThreadEvent::ToolCallAuthorization(tool_call_authorization) = event {
|
||||
let permission_kinds = tool_call_authorization
|
||||
.options
|
||||
.iter()
|
||||
.map(|o| o.kind)
|
||||
.collect::<Vec<_>>();
|
||||
// Only 2 options now: AllowAlways (for tool) and AllowOnce (granularity only)
|
||||
// Deny is handled by the UI buttons, not as a separate option
|
||||
.first_option_of_kind(acp::PermissionOptionKind::AllowAlways)
|
||||
.map(|option| option.kind);
|
||||
let allow_once = tool_call_authorization
|
||||
.options
|
||||
.first_option_of_kind(acp::PermissionOptionKind::AllowOnce)
|
||||
.map(|option| option.kind);
|
||||
|
||||
assert_eq!(
|
||||
permission_kinds,
|
||||
vec![
|
||||
acp::PermissionOptionKind::AllowAlways,
|
||||
acp::PermissionOptionKind::AllowOnce,
|
||||
]
|
||||
Some(acp::PermissionOptionKind::AllowAlways)
|
||||
);
|
||||
assert_eq!(allow_once, Some(acp::PermissionOptionKind::AllowOnce));
|
||||
return tool_call_authorization;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_permission_options_terminal_with_pattern() {
|
||||
let permission_options =
|
||||
ToolPermissionContext::new("terminal", "cargo build --release").build_permission_options();
|
||||
|
||||
let PermissionOptions::Dropdown(choices) = permission_options else {
|
||||
panic!("Expected dropdown permission options");
|
||||
};
|
||||
|
||||
assert_eq!(choices.len(), 3);
|
||||
let labels: Vec<&str> = choices
|
||||
.iter()
|
||||
.map(|choice| choice.allow.name.as_ref())
|
||||
.collect();
|
||||
assert!(labels.contains(&"Always for terminal"));
|
||||
assert!(labels.contains(&"Always for `cargo` commands"));
|
||||
assert!(labels.contains(&"Only this time"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_permission_options_edit_file_with_path_pattern() {
|
||||
let permission_options =
|
||||
ToolPermissionContext::new("edit_file", "src/main.rs").build_permission_options();
|
||||
|
||||
let PermissionOptions::Dropdown(choices) = permission_options else {
|
||||
panic!("Expected dropdown permission options");
|
||||
};
|
||||
|
||||
let labels: Vec<&str> = choices
|
||||
.iter()
|
||||
.map(|choice| choice.allow.name.as_ref())
|
||||
.collect();
|
||||
assert!(labels.contains(&"Always for edit file"));
|
||||
assert!(labels.contains(&"Always for `src/`"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_permission_options_fetch_with_domain_pattern() {
|
||||
let permission_options =
|
||||
ToolPermissionContext::new("fetch", "https://docs.rs/gpui").build_permission_options();
|
||||
|
||||
let PermissionOptions::Dropdown(choices) = permission_options else {
|
||||
panic!("Expected dropdown permission options");
|
||||
};
|
||||
|
||||
let labels: Vec<&str> = choices
|
||||
.iter()
|
||||
.map(|choice| choice.allow.name.as_ref())
|
||||
.collect();
|
||||
assert!(labels.contains(&"Always for fetch"));
|
||||
assert!(labels.contains(&"Always for `docs.rs`"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_permission_options_without_pattern() {
|
||||
let permission_options = ToolPermissionContext::new("terminal", "./deploy.sh --production")
|
||||
.build_permission_options();
|
||||
|
||||
let PermissionOptions::Dropdown(choices) = permission_options else {
|
||||
panic!("Expected dropdown permission options");
|
||||
};
|
||||
|
||||
assert_eq!(choices.len(), 2);
|
||||
let labels: Vec<&str> = choices
|
||||
.iter()
|
||||
.map(|choice| choice.allow.name.as_ref())
|
||||
.collect();
|
||||
assert!(labels.contains(&"Always for terminal"));
|
||||
assert!(labels.contains(&"Only this time"));
|
||||
assert!(!labels.iter().any(|label| label.contains("commands")));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_permission_option_ids_for_terminal() {
|
||||
let permission_options =
|
||||
ToolPermissionContext::new("terminal", "cargo build --release").build_permission_options();
|
||||
|
||||
let PermissionOptions::Dropdown(choices) = permission_options else {
|
||||
panic!("Expected dropdown permission options");
|
||||
};
|
||||
|
||||
let allow_ids: Vec<String> = choices
|
||||
.iter()
|
||||
.map(|choice| choice.allow.option_id.0.to_string())
|
||||
.collect();
|
||||
let deny_ids: Vec<String> = choices
|
||||
.iter()
|
||||
.map(|choice| choice.deny.option_id.0.to_string())
|
||||
.collect();
|
||||
|
||||
assert!(allow_ids.contains(&"always_allow:terminal".to_string()));
|
||||
assert!(allow_ids.contains(&"allow".to_string()));
|
||||
assert!(
|
||||
allow_ids
|
||||
.iter()
|
||||
.any(|id| id.starts_with("always_allow_pattern:terminal:")),
|
||||
"Missing allow pattern option"
|
||||
);
|
||||
|
||||
assert!(deny_ids.contains(&"always_deny:terminal".to_string()));
|
||||
assert!(deny_ids.contains(&"deny".to_string()));
|
||||
assert!(
|
||||
deny_ids
|
||||
.iter()
|
||||
.any(|id| id.starts_with("always_deny_pattern:terminal:")),
|
||||
"Missing deny pattern option"
|
||||
);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
#[cfg_attr(not(feature = "e2e"), ignore)]
|
||||
async fn test_concurrent_tool_calls(cx: &mut TestAppContext) {
|
||||
|
|
|
|||
|
|
@ -612,7 +612,7 @@ impl ToolPermissionContext {
|
|||
///
|
||||
/// This is the canonical source for permission option generation.
|
||||
/// Tests should use this function rather than manually constructing options.
|
||||
pub fn build_permission_options(&self) -> Vec<acp::PermissionOption> {
|
||||
pub fn build_permission_options(&self) -> acp_thread::PermissionOptions {
|
||||
use crate::pattern_extraction::*;
|
||||
|
||||
let tool_name = &self.tool_name;
|
||||
|
|
@ -634,11 +634,30 @@ impl ToolPermissionContext {
|
|||
_ => (None, None),
|
||||
};
|
||||
|
||||
let mut options = vec![acp::PermissionOption::new(
|
||||
acp::PermissionOptionId::new(format!("always:{}", tool_name)),
|
||||
let mut choices = Vec::new();
|
||||
|
||||
let mut push_choice = |label: String, allow_id, deny_id, allow_kind, deny_kind| {
|
||||
choices.push(acp_thread::PermissionOptionChoice {
|
||||
allow: acp::PermissionOption::new(
|
||||
acp::PermissionOptionId::new(allow_id),
|
||||
label.clone(),
|
||||
allow_kind,
|
||||
),
|
||||
deny: acp::PermissionOption::new(
|
||||
acp::PermissionOptionId::new(deny_id),
|
||||
label,
|
||||
deny_kind,
|
||||
),
|
||||
});
|
||||
};
|
||||
|
||||
push_choice(
|
||||
format!("Always for {}", tool_name.replace('_', " ")),
|
||||
format!("always_allow:{}", tool_name),
|
||||
format!("always_deny:{}", tool_name),
|
||||
acp::PermissionOptionKind::AllowAlways,
|
||||
)];
|
||||
acp::PermissionOptionKind::RejectAlways,
|
||||
);
|
||||
|
||||
if let (Some(pattern), Some(display)) = (pattern, pattern_display) {
|
||||
let button_text = match tool_name.as_str() {
|
||||
|
|
@ -646,27 +665,31 @@ impl ToolPermissionContext {
|
|||
"fetch" => format!("Always for `{}`", display),
|
||||
_ => format!("Always for `{}`", display),
|
||||
};
|
||||
options.push(acp::PermissionOption::new(
|
||||
acp::PermissionOptionId::new(format!("always_pattern:{}:{}", tool_name, pattern)),
|
||||
push_choice(
|
||||
button_text,
|
||||
format!("always_allow_pattern:{}:{}", tool_name, pattern),
|
||||
format!("always_deny_pattern:{}:{}", tool_name, pattern),
|
||||
acp::PermissionOptionKind::AllowAlways,
|
||||
));
|
||||
acp::PermissionOptionKind::RejectAlways,
|
||||
);
|
||||
}
|
||||
|
||||
options.push(acp::PermissionOption::new(
|
||||
acp::PermissionOptionId::new("once"),
|
||||
"Only this time",
|
||||
push_choice(
|
||||
"Only this time".to_string(),
|
||||
"allow".to_string(),
|
||||
"deny".to_string(),
|
||||
acp::PermissionOptionKind::AllowOnce,
|
||||
));
|
||||
acp::PermissionOptionKind::RejectOnce,
|
||||
);
|
||||
|
||||
options
|
||||
acp_thread::PermissionOptions::Dropdown(choices)
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
pub struct ToolCallAuthorization {
|
||||
pub tool_call: acp::ToolCallUpdate,
|
||||
pub options: Vec<acp::PermissionOption>,
|
||||
pub options: acp_thread::PermissionOptions,
|
||||
pub response: oneshot::Sender<acp::PermissionOptionId>,
|
||||
pub context: Option<ToolPermissionContext>,
|
||||
}
|
||||
|
|
@ -2937,10 +2960,9 @@ impl ToolCallEventStream {
|
|||
/// Unlike built-in tools, third-party tools don't support pattern-based permissions.
|
||||
/// They only support `default_mode` (allow/deny/confirm) per tool.
|
||||
///
|
||||
/// Shows 3 buttons:
|
||||
/// - "Always allow <display_name> MCP tool" → sets `tools.<tool_id>.default_mode = "allow"`
|
||||
/// - "Allow" → approve once
|
||||
/// - "Deny" → reject once
|
||||
/// Uses the dropdown authorization flow with two granularities:
|
||||
/// - "Always for <display_name> MCP tool" → sets `tools.<tool_id>.default_mode = "allow"` or "deny"
|
||||
/// - "Only this time" → allow/deny once
|
||||
pub fn authorize_third_party_tool(
|
||||
&self,
|
||||
title: impl Into<String>,
|
||||
|
|
@ -2967,23 +2989,38 @@ impl ToolCallEventStream {
|
|||
self.tool_use_id.to_string(),
|
||||
acp::ToolCallUpdateFields::new().title(title.into()),
|
||||
),
|
||||
options: vec![
|
||||
acp::PermissionOption::new(
|
||||
acp::PermissionOptionId::new(format!("always_allow_mcp:{}", tool_id)),
|
||||
format!("Always allow {} MCP tool", display_name),
|
||||
acp::PermissionOptionKind::AllowAlways,
|
||||
),
|
||||
acp::PermissionOption::new(
|
||||
acp::PermissionOptionId::new("allow"),
|
||||
"Allow once",
|
||||
acp::PermissionOptionKind::AllowOnce,
|
||||
),
|
||||
acp::PermissionOption::new(
|
||||
acp::PermissionOptionId::new("deny"),
|
||||
"Deny",
|
||||
acp::PermissionOptionKind::RejectOnce,
|
||||
),
|
||||
],
|
||||
options: acp_thread::PermissionOptions::Dropdown(vec![
|
||||
acp_thread::PermissionOptionChoice {
|
||||
allow: acp::PermissionOption::new(
|
||||
acp::PermissionOptionId::new(format!(
|
||||
"always_allow_mcp:{}",
|
||||
tool_id
|
||||
)),
|
||||
format!("Always for {} MCP tool", display_name),
|
||||
acp::PermissionOptionKind::AllowAlways,
|
||||
),
|
||||
deny: acp::PermissionOption::new(
|
||||
acp::PermissionOptionId::new(format!(
|
||||
"always_deny_mcp:{}",
|
||||
tool_id
|
||||
)),
|
||||
format!("Always for {} MCP tool", display_name),
|
||||
acp::PermissionOptionKind::RejectAlways,
|
||||
),
|
||||
},
|
||||
acp_thread::PermissionOptionChoice {
|
||||
allow: acp::PermissionOption::new(
|
||||
acp::PermissionOptionId::new("allow"),
|
||||
"Only this time",
|
||||
acp::PermissionOptionKind::AllowOnce,
|
||||
),
|
||||
deny: acp::PermissionOption::new(
|
||||
acp::PermissionOptionId::new("deny"),
|
||||
"Only this time",
|
||||
acp::PermissionOptionKind::RejectOnce,
|
||||
),
|
||||
},
|
||||
]),
|
||||
response: response_tx,
|
||||
context: None,
|
||||
},
|
||||
|
|
@ -3007,6 +3044,19 @@ impl ToolCallEventStream {
|
|||
}
|
||||
return Ok(());
|
||||
}
|
||||
if response_str == format!("always_deny_mcp:{}", tool_id) {
|
||||
if let Some(fs) = fs.clone() {
|
||||
cx.update(|cx| {
|
||||
update_settings_file(fs, cx, move |settings, _| {
|
||||
settings
|
||||
.agent
|
||||
.get_or_insert_default()
|
||||
.set_tool_default_mode(&tool_id, ToolPermissionMode::Deny);
|
||||
});
|
||||
});
|
||||
}
|
||||
return Err(anyhow!("Permission to run tool denied by user"));
|
||||
}
|
||||
|
||||
if response_str == "allow" {
|
||||
return Ok(());
|
||||
|
|
|
|||
|
|
@ -1070,7 +1070,7 @@ impl acp::Client for ClientDelegate {
|
|||
let task = thread.update(cx, |thread, cx| {
|
||||
thread.request_tool_call_authorization(
|
||||
arguments.tool_call,
|
||||
arguments.options,
|
||||
acp_thread::PermissionOptions::Flat(arguments.options),
|
||||
respect_always_allow_setting,
|
||||
cx,
|
||||
)
|
||||
|
|
|
|||
|
|
@ -1,7 +1,8 @@
|
|||
use acp_thread::{
|
||||
AcpThread, AcpThreadEvent, AgentSessionInfo, AgentThreadEntry, AssistantMessage,
|
||||
AssistantMessageChunk, AuthRequired, LoadError, MentionUri, RetryStatus, ThreadStatus,
|
||||
ToolCall, ToolCallContent, ToolCallStatus, UserMessageId,
|
||||
AssistantMessageChunk, AuthRequired, LoadError, MentionUri, PermissionOptionChoice,
|
||||
PermissionOptions, RetryStatus, ThreadStatus, ToolCall, ToolCallContent, ToolCallStatus,
|
||||
UserMessageId,
|
||||
};
|
||||
use acp_thread::{AgentConnection, Plan};
|
||||
use action_log::{ActionLog, ActionLogTelemetry};
|
||||
|
|
@ -3121,7 +3122,6 @@ impl AcpThreadView {
|
|||
)
|
||||
})
|
||||
.child(self.render_permission_buttons(
|
||||
tool_call.kind,
|
||||
options,
|
||||
entry_ix,
|
||||
tool_call.id.clone(),
|
||||
|
|
@ -3907,8 +3907,24 @@ impl AcpThreadView {
|
|||
|
||||
fn render_permission_buttons(
|
||||
&self,
|
||||
kind: acp::ToolKind,
|
||||
options: &[acp::PermissionOption],
|
||||
options: &PermissionOptions,
|
||||
entry_ix: usize,
|
||||
tool_call_id: acp::ToolCallId,
|
||||
cx: &Context<Self>,
|
||||
) -> Div {
|
||||
match options {
|
||||
PermissionOptions::Flat(options) => {
|
||||
self.render_permission_buttons_flat(options, entry_ix, tool_call_id, cx)
|
||||
}
|
||||
PermissionOptions::Dropdown(options) => {
|
||||
self.render_permission_buttons_dropdown(options, entry_ix, tool_call_id, cx)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn render_permission_buttons_dropdown(
|
||||
&self,
|
||||
choices: &[PermissionOptionChoice],
|
||||
entry_ix: usize,
|
||||
tool_call_id: acp::ToolCallId,
|
||||
cx: &Context<Self>,
|
||||
|
|
@ -3920,77 +3936,26 @@ impl AcpThreadView {
|
|||
.is_some_and(|call| call.id == tool_call_id)
|
||||
});
|
||||
|
||||
// For SwitchMode, use the old layout with all buttons
|
||||
if kind == acp::ToolKind::SwitchMode {
|
||||
return self.render_permission_buttons_legacy(options, entry_ix, tool_call_id, cx);
|
||||
}
|
||||
|
||||
let granularity_options: Vec<_> = options
|
||||
.iter()
|
||||
.filter(|o| {
|
||||
matches!(
|
||||
o.kind,
|
||||
acp::PermissionOptionKind::AllowOnce | acp::PermissionOptionKind::AllowAlways
|
||||
)
|
||||
})
|
||||
.collect();
|
||||
|
||||
// Get the selected granularity index, defaulting to the last option ("Only this time")
|
||||
let selected_index = self
|
||||
.selected_permission_granularity
|
||||
.get(&tool_call_id)
|
||||
.copied()
|
||||
.unwrap_or_else(|| granularity_options.len().saturating_sub(1));
|
||||
.unwrap_or_else(|| choices.len().saturating_sub(1));
|
||||
|
||||
let selected_option = granularity_options
|
||||
.get(selected_index)
|
||||
.or(granularity_options.last())
|
||||
.copied();
|
||||
let selected_choice = choices.get(selected_index).or(choices.last());
|
||||
|
||||
let dropdown_label: SharedString = selected_option
|
||||
.map(|o| o.name.clone().into())
|
||||
let dropdown_label: SharedString = selected_choice
|
||||
.map(|choice| choice.label())
|
||||
.unwrap_or_else(|| "Only this time".into());
|
||||
|
||||
let (allow_option_id, allow_option_kind, deny_option_id, deny_option_kind) =
|
||||
if let Some(option) = selected_option {
|
||||
let option_id_str = option.option_id.0.to_string();
|
||||
|
||||
// Transform option_id for allow: "always:tool" -> "always_allow:tool", "once" -> "allow"
|
||||
let allow_id = if option_id_str == "once" {
|
||||
"allow".to_string()
|
||||
} else if let Some(rest) = option_id_str.strip_prefix("always:") {
|
||||
format!("always_allow:{}", rest)
|
||||
} else if let Some(rest) = option_id_str.strip_prefix("always_pattern:") {
|
||||
format!("always_allow_pattern:{}", rest)
|
||||
} else {
|
||||
option_id_str.clone()
|
||||
};
|
||||
|
||||
// Transform option_id for deny: "always:tool" -> "always_deny:tool", "once" -> "deny"
|
||||
let deny_id = if option_id_str == "once" {
|
||||
"deny".to_string()
|
||||
} else if let Some(rest) = option_id_str.strip_prefix("always:") {
|
||||
format!("always_deny:{}", rest)
|
||||
} else if let Some(rest) = option_id_str.strip_prefix("always_pattern:") {
|
||||
format!("always_deny_pattern:{}", rest)
|
||||
} else {
|
||||
option_id_str.replace("allow", "deny")
|
||||
};
|
||||
|
||||
let allow_kind = option.kind;
|
||||
let deny_kind = match option.kind {
|
||||
acp::PermissionOptionKind::AllowOnce => acp::PermissionOptionKind::RejectOnce,
|
||||
acp::PermissionOptionKind::AllowAlways => {
|
||||
acp::PermissionOptionKind::RejectAlways
|
||||
}
|
||||
other => other,
|
||||
};
|
||||
|
||||
if let Some(choice) = selected_choice {
|
||||
(
|
||||
acp::PermissionOptionId::new(allow_id),
|
||||
allow_kind,
|
||||
acp::PermissionOptionId::new(deny_id),
|
||||
deny_kind,
|
||||
choice.allow.option_id.clone(),
|
||||
choice.allow.kind,
|
||||
choice.deny.option_id.clone(),
|
||||
choice.deny.kind,
|
||||
)
|
||||
} else {
|
||||
(
|
||||
|
|
@ -4077,7 +4042,7 @@ impl AcpThreadView {
|
|||
),
|
||||
)
|
||||
.child(self.render_permission_granularity_dropdown(
|
||||
&granularity_options,
|
||||
choices,
|
||||
dropdown_label,
|
||||
entry_ix,
|
||||
tool_call_id,
|
||||
|
|
@ -4089,7 +4054,7 @@ impl AcpThreadView {
|
|||
|
||||
fn render_permission_granularity_dropdown(
|
||||
&self,
|
||||
granularity_options: &[&acp::PermissionOption],
|
||||
choices: &[PermissionOptionChoice],
|
||||
current_label: SharedString,
|
||||
entry_ix: usize,
|
||||
tool_call_id: acp::ToolCallId,
|
||||
|
|
@ -4097,10 +4062,10 @@ impl AcpThreadView {
|
|||
is_first: bool,
|
||||
cx: &Context<Self>,
|
||||
) -> impl IntoElement {
|
||||
let menu_options: Vec<(usize, SharedString)> = granularity_options
|
||||
let menu_options: Vec<(usize, SharedString)> = choices
|
||||
.iter()
|
||||
.enumerate()
|
||||
.map(|(i, o)| (i, o.name.clone().into()))
|
||||
.map(|(i, choice)| (i, choice.label()))
|
||||
.collect();
|
||||
|
||||
PopoverMenu::new(("permission-granularity", entry_ix))
|
||||
|
|
@ -4156,7 +4121,7 @@ impl AcpThreadView {
|
|||
})
|
||||
}
|
||||
|
||||
fn render_permission_buttons_legacy(
|
||||
fn render_permission_buttons_flat(
|
||||
&self,
|
||||
options: &[acp::PermissionOption],
|
||||
entry_ix: usize,
|
||||
|
|
@ -6286,62 +6251,37 @@ impl AcpThreadView {
|
|||
};
|
||||
let tool_call_id = tool_call.id.clone();
|
||||
|
||||
// Get granularity options (all options except old deny option)
|
||||
let granularity_options: Vec<_> = options
|
||||
.iter()
|
||||
.filter(|o| {
|
||||
matches!(
|
||||
o.kind,
|
||||
acp::PermissionOptionKind::AllowOnce | acp::PermissionOptionKind::AllowAlways
|
||||
)
|
||||
})
|
||||
.collect();
|
||||
let PermissionOptions::Dropdown(choices) = options else {
|
||||
let kind = if is_allow {
|
||||
acp::PermissionOptionKind::AllowOnce
|
||||
} else {
|
||||
acp::PermissionOptionKind::RejectOnce
|
||||
};
|
||||
return self.authorize_pending_tool_call(kind, window, cx);
|
||||
};
|
||||
|
||||
// Get selected index, defaulting to last option ("Only this time")
|
||||
let selected_index = self
|
||||
.selected_permission_granularity
|
||||
.get(&tool_call_id)
|
||||
.copied()
|
||||
.unwrap_or_else(|| granularity_options.len().saturating_sub(1));
|
||||
.unwrap_or_else(|| choices.len().saturating_sub(1));
|
||||
|
||||
let selected_option = granularity_options
|
||||
.get(selected_index)
|
||||
.or(granularity_options.last())
|
||||
.copied()?;
|
||||
let selected_choice = choices.get(selected_index).or(choices.last())?;
|
||||
|
||||
let option_id_str = selected_option.option_id.0.to_string();
|
||||
|
||||
// Transform option_id based on allow/deny
|
||||
let (final_option_id, final_option_kind) = if is_allow {
|
||||
let allow_id = if option_id_str == "once" {
|
||||
"allow".to_string()
|
||||
} else if let Some(rest) = option_id_str.strip_prefix("always:") {
|
||||
format!("always_allow:{}", rest)
|
||||
} else if let Some(rest) = option_id_str.strip_prefix("always_pattern:") {
|
||||
format!("always_allow_pattern:{}", rest)
|
||||
} else {
|
||||
option_id_str
|
||||
};
|
||||
(acp::PermissionOptionId::new(allow_id), selected_option.kind)
|
||||
let selected_option = if is_allow {
|
||||
&selected_choice.allow
|
||||
} else {
|
||||
let deny_id = if option_id_str == "once" {
|
||||
"deny".to_string()
|
||||
} else if let Some(rest) = option_id_str.strip_prefix("always:") {
|
||||
format!("always_deny:{}", rest)
|
||||
} else if let Some(rest) = option_id_str.strip_prefix("always_pattern:") {
|
||||
format!("always_deny_pattern:{}", rest)
|
||||
} else {
|
||||
option_id_str.replace("allow", "deny")
|
||||
};
|
||||
let deny_kind = match selected_option.kind {
|
||||
acp::PermissionOptionKind::AllowOnce => acp::PermissionOptionKind::RejectOnce,
|
||||
acp::PermissionOptionKind::AllowAlways => acp::PermissionOptionKind::RejectAlways,
|
||||
other => other,
|
||||
};
|
||||
(acp::PermissionOptionId::new(deny_id), deny_kind)
|
||||
&selected_choice.deny
|
||||
};
|
||||
|
||||
self.authorize_tool_call(tool_call_id, final_option_id, final_option_kind, window, cx);
|
||||
self.authorize_tool_call(
|
||||
tool_call_id,
|
||||
selected_option.option_id.clone(),
|
||||
selected_option.kind,
|
||||
window,
|
||||
cx,
|
||||
);
|
||||
|
||||
Some(())
|
||||
}
|
||||
|
|
@ -6397,7 +6337,7 @@ impl AcpThreadView {
|
|||
let ToolCallStatus::WaitingForConfirmation { options, .. } = &tool_call.status else {
|
||||
return None;
|
||||
};
|
||||
let option = options.iter().find(|o| o.kind == kind)?;
|
||||
let option = options.first_option_of_kind(kind)?;
|
||||
|
||||
self.authorize_tool_call(
|
||||
tool_call.id.clone(),
|
||||
|
|
@ -8463,11 +8403,11 @@ pub(crate) mod tests {
|
|||
let connection =
|
||||
StubAgentConnection::new().with_permission_requests(HashMap::from_iter([(
|
||||
tool_call_id,
|
||||
vec![acp::PermissionOption::new(
|
||||
PermissionOptions::Flat(vec![acp::PermissionOption::new(
|
||||
"1",
|
||||
"Allow",
|
||||
acp::PermissionOptionKind::AllowOnce,
|
||||
)],
|
||||
)]),
|
||||
)]));
|
||||
|
||||
connection.set_next_prompt_updates(vec![acp::SessionUpdate::ToolCall(tool_call)]);
|
||||
|
|
@ -9866,14 +9806,21 @@ pub(crate) mod tests {
|
|||
if let acp_thread::ToolCallStatus::WaitingForConfirmation { options, .. } =
|
||||
&tool_call.status
|
||||
{
|
||||
let PermissionOptions::Dropdown(choices) = options else {
|
||||
panic!("Expected dropdown permission options");
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
options.len(),
|
||||
choices.len(),
|
||||
3,
|
||||
"Expected 3 permission options (granularity only)"
|
||||
);
|
||||
|
||||
// Verify specific button labels (now using neutral names)
|
||||
let labels: Vec<&str> = options.iter().map(|o| o.name.as_ref()).collect();
|
||||
let labels: Vec<&str> = choices
|
||||
.iter()
|
||||
.map(|choice| choice.allow.name.as_ref())
|
||||
.collect();
|
||||
assert!(
|
||||
labels.contains(&"Always for terminal"),
|
||||
"Missing 'Always for terminal' option"
|
||||
|
|
@ -9952,7 +9899,14 @@ pub(crate) mod tests {
|
|||
if let acp_thread::ToolCallStatus::WaitingForConfirmation { options, .. } =
|
||||
&tool_call.status
|
||||
{
|
||||
let labels: Vec<&str> = options.iter().map(|o| o.name.as_ref()).collect();
|
||||
let PermissionOptions::Dropdown(choices) = options else {
|
||||
panic!("Expected dropdown permission options");
|
||||
};
|
||||
|
||||
let labels: Vec<&str> = choices
|
||||
.iter()
|
||||
.map(|choice| choice.allow.name.as_ref())
|
||||
.collect();
|
||||
assert!(
|
||||
labels.contains(&"Always for edit file"),
|
||||
"Missing 'Always for edit file' option"
|
||||
|
|
@ -10029,7 +9983,14 @@ pub(crate) mod tests {
|
|||
if let acp_thread::ToolCallStatus::WaitingForConfirmation { options, .. } =
|
||||
&tool_call.status
|
||||
{
|
||||
let labels: Vec<&str> = options.iter().map(|o| o.name.as_ref()).collect();
|
||||
let PermissionOptions::Dropdown(choices) = options else {
|
||||
panic!("Expected dropdown permission options");
|
||||
};
|
||||
|
||||
let labels: Vec<&str> = choices
|
||||
.iter()
|
||||
.map(|choice| choice.allow.name.as_ref())
|
||||
.collect();
|
||||
assert!(
|
||||
labels.contains(&"Always for fetch"),
|
||||
"Missing 'Always for fetch' option"
|
||||
|
|
@ -10107,13 +10068,20 @@ pub(crate) mod tests {
|
|||
if let acp_thread::ToolCallStatus::WaitingForConfirmation { options, .. } =
|
||||
&tool_call.status
|
||||
{
|
||||
let PermissionOptions::Dropdown(choices) = options else {
|
||||
panic!("Expected dropdown permission options");
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
options.len(),
|
||||
choices.len(),
|
||||
2,
|
||||
"Expected 2 permission options (no pattern option)"
|
||||
);
|
||||
|
||||
let labels: Vec<&str> = options.iter().map(|o| o.name.as_ref()).collect();
|
||||
let labels: Vec<&str> = choices
|
||||
.iter()
|
||||
.map(|choice| choice.allow.name.as_ref())
|
||||
.collect();
|
||||
assert!(
|
||||
labels.contains(&"Always for terminal"),
|
||||
"Missing 'Always for terminal' option"
|
||||
|
|
@ -10258,10 +10226,20 @@ pub(crate) mod tests {
|
|||
cx.run_until_parked();
|
||||
|
||||
// Find the pattern option ID
|
||||
let pattern_option = permission_options
|
||||
.iter()
|
||||
.find(|o| o.option_id.0.starts_with("always_pattern:"))
|
||||
.expect("Should have a pattern option for npm command");
|
||||
let pattern_option = match &permission_options {
|
||||
PermissionOptions::Dropdown(choices) => choices
|
||||
.iter()
|
||||
.find(|choice| {
|
||||
choice
|
||||
.allow
|
||||
.option_id
|
||||
.0
|
||||
.starts_with("always_allow_pattern:")
|
||||
})
|
||||
.map(|choice| &choice.allow)
|
||||
.expect("Should have a pattern option for npm command"),
|
||||
_ => panic!("Expected dropdown permission options"),
|
||||
};
|
||||
|
||||
// Dispatch action with the pattern option (simulating "Always allow `npm` commands")
|
||||
thread_view.update_in(cx, |_, window, cx| {
|
||||
|
|
@ -10379,20 +10357,26 @@ pub(crate) mod tests {
|
|||
ToolPermissionContext::new("terminal", "npm install").build_permission_options();
|
||||
|
||||
// Verify we have the expected options
|
||||
assert_eq!(permission_options.len(), 3);
|
||||
let PermissionOptions::Dropdown(choices) = &permission_options else {
|
||||
panic!("Expected dropdown permission options");
|
||||
};
|
||||
|
||||
assert_eq!(choices.len(), 3);
|
||||
assert!(
|
||||
permission_options[0]
|
||||
choices[0]
|
||||
.allow
|
||||
.option_id
|
||||
.0
|
||||
.contains("always:terminal")
|
||||
.contains("always_allow:terminal")
|
||||
);
|
||||
assert!(
|
||||
permission_options[1]
|
||||
choices[1]
|
||||
.allow
|
||||
.option_id
|
||||
.0
|
||||
.contains("always_pattern:terminal")
|
||||
.contains("always_allow_pattern:terminal")
|
||||
);
|
||||
assert_eq!(permission_options[2].option_id.0.as_ref(), "once");
|
||||
assert_eq!(choices[2].allow.option_id.0.as_ref(), "allow");
|
||||
|
||||
let connection =
|
||||
StubAgentConnection::new().with_permission_requests(HashMap::from_iter([(
|
||||
|
|
@ -10525,71 +10509,49 @@ pub(crate) mod tests {
|
|||
|
||||
#[gpui::test]
|
||||
async fn test_option_id_transformation_for_allow() {
|
||||
// Test the option_id transformation logic directly
|
||||
// "once" -> "allow"
|
||||
// "always:terminal" -> "always_allow:terminal"
|
||||
// "always_pattern:terminal:^cargo\s" -> "always_allow_pattern:terminal:^cargo\s"
|
||||
let permission_options = ToolPermissionContext::new("terminal", "cargo build --release")
|
||||
.build_permission_options();
|
||||
|
||||
let test_cases = vec![
|
||||
("once", "allow"),
|
||||
("always:terminal", "always_allow:terminal"),
|
||||
(
|
||||
"always_pattern:terminal:^cargo\\s",
|
||||
"always_allow_pattern:terminal:^cargo\\s",
|
||||
),
|
||||
("always:fetch", "always_allow:fetch"),
|
||||
(
|
||||
"always_pattern:fetch:^https?://docs\\.rs",
|
||||
"always_allow_pattern:fetch:^https?://docs\\.rs",
|
||||
),
|
||||
];
|
||||
let PermissionOptions::Dropdown(choices) = permission_options else {
|
||||
panic!("Expected dropdown permission options");
|
||||
};
|
||||
|
||||
for (input, expected) in test_cases {
|
||||
let result = if input == "once" {
|
||||
"allow".to_string()
|
||||
} else if let Some(rest) = input.strip_prefix("always:") {
|
||||
format!("always_allow:{}", rest)
|
||||
} else if let Some(rest) = input.strip_prefix("always_pattern:") {
|
||||
format!("always_allow_pattern:{}", rest)
|
||||
} else {
|
||||
input.to_string()
|
||||
};
|
||||
assert_eq!(result, expected, "Failed for input: {}", input);
|
||||
}
|
||||
let allow_ids: Vec<String> = choices
|
||||
.iter()
|
||||
.map(|choice| choice.allow.option_id.0.to_string())
|
||||
.collect();
|
||||
|
||||
assert!(allow_ids.contains(&"always_allow:terminal".to_string()));
|
||||
assert!(allow_ids.contains(&"allow".to_string()));
|
||||
assert!(
|
||||
allow_ids
|
||||
.iter()
|
||||
.any(|id| id.starts_with("always_allow_pattern:terminal:")),
|
||||
"Missing allow pattern option"
|
||||
);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_option_id_transformation_for_deny() {
|
||||
// Test the option_id transformation logic for deny
|
||||
// "once" -> "deny"
|
||||
// "always:terminal" -> "always_deny:terminal"
|
||||
// "always_pattern:terminal:^cargo\s" -> "always_deny_pattern:terminal:^cargo\s"
|
||||
let permission_options = ToolPermissionContext::new("terminal", "cargo build --release")
|
||||
.build_permission_options();
|
||||
|
||||
let test_cases = vec![
|
||||
("once", "deny"),
|
||||
("always:terminal", "always_deny:terminal"),
|
||||
(
|
||||
"always_pattern:terminal:^cargo\\s",
|
||||
"always_deny_pattern:terminal:^cargo\\s",
|
||||
),
|
||||
("always:fetch", "always_deny:fetch"),
|
||||
(
|
||||
"always_pattern:fetch:^https?://docs\\.rs",
|
||||
"always_deny_pattern:fetch:^https?://docs\\.rs",
|
||||
),
|
||||
];
|
||||
let PermissionOptions::Dropdown(choices) = permission_options else {
|
||||
panic!("Expected dropdown permission options");
|
||||
};
|
||||
|
||||
for (input, expected) in test_cases {
|
||||
let result = if input == "once" {
|
||||
"deny".to_string()
|
||||
} else if let Some(rest) = input.strip_prefix("always:") {
|
||||
format!("always_deny:{}", rest)
|
||||
} else if let Some(rest) = input.strip_prefix("always_pattern:") {
|
||||
format!("always_deny_pattern:{}", rest)
|
||||
} else {
|
||||
input.replace("allow", "deny")
|
||||
};
|
||||
assert_eq!(result, expected, "Failed for input: {}", input);
|
||||
}
|
||||
let deny_ids: Vec<String> = choices
|
||||
.iter()
|
||||
.map(|choice| choice.deny.option_id.0.to_string())
|
||||
.collect();
|
||||
|
||||
assert!(deny_ids.contains(&"always_deny:terminal".to_string()));
|
||||
assert!(deny_ids.contains(&"deny".to_string()));
|
||||
assert!(
|
||||
deny_ids
|
||||
.iter()
|
||||
.any(|id| id.starts_with("always_deny_pattern:terminal:")),
|
||||
"Missing deny pattern option"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue