Detailed changes
@@ -502,13 +502,15 @@ pub enum SelectedPermissionParams {
#[derive(Debug)]
pub struct SelectedPermissionOutcome {
pub option_id: acp::PermissionOptionId,
+ pub option_kind: acp::PermissionOptionKind,
pub params: Option<SelectedPermissionParams>,
}
impl SelectedPermissionOutcome {
- pub fn new(option_id: acp::PermissionOptionId) -> Self {
+ pub fn new(option_id: acp::PermissionOptionId, option_kind: acp::PermissionOptionKind) -> Self {
Self {
option_id,
+ option_kind,
params: None,
}
}
@@ -519,12 +521,6 @@ impl SelectedPermissionOutcome {
}
}
-impl From<acp::PermissionOptionId> for SelectedPermissionOutcome {
- fn from(option_id: acp::PermissionOptionId) -> Self {
- Self::new(option_id)
- }
-}
-
impl From<SelectedPermissionOutcome> for acp::SelectedPermissionOutcome {
fn from(value: SelectedPermissionOutcome) -> Self {
Self::new(value.option_id)
@@ -2013,14 +2009,13 @@ impl AcpThread {
&mut self,
id: acp::ToolCallId,
outcome: SelectedPermissionOutcome,
- option_kind: acp::PermissionOptionKind,
cx: &mut Context<Self>,
) {
let Some((ix, call)) = self.tool_call_mut(&id) else {
return;
};
- let new_status = match option_kind {
+ let new_status = match outcome.option_kind {
acp::PermissionOptionKind::RejectOnce | acp::PermissionOptionKind::RejectAlways => {
ToolCallStatus::Rejected
}
@@ -477,6 +477,24 @@ impl PermissionOptionChoice {
pub fn label(&self) -> SharedString {
self.allow.name.clone().into()
}
+
+ /// Build a `SelectedPermissionOutcome` for this choice.
+ ///
+ /// If the choice carries `sub_patterns`, they are attached as
+ /// `SelectedPermissionParams::Terminal`.
+ pub fn build_outcome(&self, is_allow: bool) -> crate::SelectedPermissionOutcome {
+ let option = if is_allow { &self.allow } else { &self.deny };
+
+ let params = if !self.sub_patterns.is_empty() {
+ Some(crate::SelectedPermissionParams::Terminal {
+ patterns: self.sub_patterns.clone(),
+ })
+ } else {
+ None
+ };
+
+ crate::SelectedPermissionOutcome::new(option.option_id.clone(), option.kind).params(params)
+ }
}
/// Pairs a tool's permission pattern with its display name
@@ -548,6 +566,57 @@ impl PermissionOptions {
self.first_option_of_kind(acp::PermissionOptionKind::RejectOnce)
.map(|option| option.option_id.clone())
}
+
+ /// Build a `SelectedPermissionOutcome` for the `DropdownWithPatterns`
+ /// variant when the user has checked specific pattern indices.
+ ///
+ /// Returns `Some` with the always-allow/deny outcome when at least one
+ /// pattern is checked. Returns `None` when zero patterns are checked,
+ /// signaling that the caller should degrade to allow-once / deny-once.
+ ///
+ /// Panics (debug) or returns `None` (release) if called on a non-
+ /// `DropdownWithPatterns` variant.
+ pub fn build_outcome_for_checked_patterns(
+ &self,
+ checked_indices: &[usize],
+ is_allow: bool,
+ ) -> Option<crate::SelectedPermissionOutcome> {
+ let PermissionOptions::DropdownWithPatterns {
+ choices, patterns, ..
+ } = self
+ else {
+ debug_assert!(
+ false,
+ "build_outcome_for_checked_patterns called on non-DropdownWithPatterns"
+ );
+ return None;
+ };
+
+ let checked_patterns: Vec<String> = patterns
+ .iter()
+ .enumerate()
+ .filter(|(index, _)| checked_indices.contains(index))
+ .map(|(_, cp)| cp.pattern.clone())
+ .collect();
+
+ if checked_patterns.is_empty() {
+ return None;
+ }
+
+ // Use the first choice (the "Always" choice) as the base for the outcome.
+ let always_choice = choices.first()?;
+ let option = if is_allow {
+ &always_choice.allow
+ } else {
+ &always_choice.deny
+ };
+
+ let outcome = crate::SelectedPermissionOutcome::new(option.option_id.clone(), option.kind)
+ .params(Some(crate::SelectedPermissionParams::Terminal {
+ patterns: checked_patterns,
+ }));
+ Some(outcome)
+ }
}
#[cfg(feature = "test-support")]
@@ -841,14 +841,20 @@ async fn test_tool_authorization(cx: &mut TestAppContext) {
// Approve the first - send "allow" option_id (UI transforms "once" to "allow")
tool_call_auth_1
.response
- .send(acp::PermissionOptionId::new("allow").into())
+ .send(acp_thread::SelectedPermissionOutcome::new(
+ acp::PermissionOptionId::new("allow"),
+ acp::PermissionOptionKind::AllowOnce,
+ ))
.unwrap();
cx.run_until_parked();
// Reject the second - send "deny" option_id directly since Deny is now a button
tool_call_auth_2
.response
- .send(acp::PermissionOptionId::new("deny").into())
+ .send(acp_thread::SelectedPermissionOutcome::new(
+ acp::PermissionOptionId::new("deny"),
+ acp::PermissionOptionKind::RejectOnce,
+ ))
.unwrap();
cx.run_until_parked();
@@ -892,7 +898,10 @@ async fn test_tool_authorization(cx: &mut TestAppContext) {
let tool_call_auth_3 = next_tool_call_authorization(&mut events).await;
tool_call_auth_3
.response
- .send(acp::PermissionOptionId::new("always_allow:tool_requiring_permission").into())
+ .send(acp_thread::SelectedPermissionOutcome::new(
+ acp::PermissionOptionId::new("always_allow:tool_requiring_permission"),
+ acp::PermissionOptionKind::AllowAlways,
+ ))
.unwrap();
cx.run_until_parked();
let completion = fake_model.pending_completions().pop().unwrap();
@@ -266,7 +266,10 @@ mod tests {
);
auth.response
- .send(acp::PermissionOptionId::new("allow").into())
+ .send(acp_thread::SelectedPermissionOutcome::new(
+ acp::PermissionOptionId::new("allow"),
+ acp::PermissionOptionKind::AllowOnce,
+ ))
.unwrap();
let result = task.await;
@@ -372,7 +375,10 @@ mod tests {
);
auth.response
- .send(acp::PermissionOptionId::new("allow").into())
+ .send(acp_thread::SelectedPermissionOutcome::new(
+ acp::PermissionOptionId::new("allow"),
+ acp::PermissionOptionKind::AllowOnce,
+ ))
.unwrap();
assert!(
@@ -241,7 +241,10 @@ mod tests {
);
auth.response
- .send(acp::PermissionOptionId::new("allow").into())
+ .send(acp_thread::SelectedPermissionOutcome::new(
+ acp::PermissionOptionId::new("allow"),
+ acp::PermissionOptionKind::AllowOnce,
+ ))
.unwrap();
let result = task.await;
@@ -359,7 +362,10 @@ mod tests {
);
auth.response
- .send(acp::PermissionOptionId::new("allow").into())
+ .send(acp_thread::SelectedPermissionOutcome::new(
+ acp::PermissionOptionId::new("allow"),
+ acp::PermissionOptionKind::AllowOnce,
+ ))
.unwrap();
assert!(
@@ -301,7 +301,10 @@ mod tests {
);
auth.response
- .send(acp::PermissionOptionId::new("allow").into())
+ .send(acp_thread::SelectedPermissionOutcome::new(
+ acp::PermissionOptionId::new("allow"),
+ acp::PermissionOptionKind::AllowOnce,
+ ))
.unwrap();
let result = task.await;
@@ -428,7 +431,10 @@ mod tests {
);
auth.response
- .send(acp::PermissionOptionId::new("allow").into())
+ .send(acp_thread::SelectedPermissionOutcome::new(
+ acp::PermissionOptionId::new("allow"),
+ acp::PermissionOptionKind::AllowOnce,
+ ))
.unwrap();
assert!(
@@ -1374,7 +1374,10 @@ mod tests {
event
.response
- .send(acp::PermissionOptionId::new("allow").into())
+ .send(acp_thread::SelectedPermissionOutcome::new(
+ acp::PermissionOptionId::new("allow"),
+ acp::PermissionOptionKind::AllowOnce,
+ ))
.unwrap();
authorize_task.await.unwrap();
}
@@ -848,7 +848,10 @@ mod tests {
);
auth.response
- .send(acp::PermissionOptionId::new("allow").into())
+ .send(acp_thread::SelectedPermissionOutcome::new(
+ acp::PermissionOptionId::new("allow"),
+ acp::PermissionOptionKind::AllowOnce,
+ ))
.unwrap();
let result = task.await;
@@ -273,7 +273,10 @@ mod tests {
);
auth.response
- .send(acp::PermissionOptionId::new("allow").into())
+ .send(acp_thread::SelectedPermissionOutcome::new(
+ acp::PermissionOptionId::new("allow"),
+ acp::PermissionOptionKind::AllowOnce,
+ ))
.unwrap();
let result = task.await;
@@ -379,7 +382,10 @@ mod tests {
);
auth.response
- .send(acp::PermissionOptionId::new("allow").into())
+ .send(acp_thread::SelectedPermissionOutcome::new(
+ acp::PermissionOptionId::new("allow"),
+ acp::PermissionOptionKind::AllowOnce,
+ ))
.unwrap();
assert!(
@@ -896,7 +896,10 @@ mod test {
);
authorization
.response
- .send(acp::PermissionOptionId::new("allow").into())
+ .send(acp_thread::SelectedPermissionOutcome::new(
+ acp::PermissionOptionId::new("allow"),
+ acp::PermissionOptionKind::AllowOnce,
+ ))
.unwrap();
let result = read_task.await;
@@ -1185,7 +1188,10 @@ mod test {
);
auth.response
- .send(acp::PermissionOptionId::new("allow").into())
+ .send(acp_thread::SelectedPermissionOutcome::new(
+ acp::PermissionOptionId::new("allow"),
+ acp::PermissionOptionKind::AllowOnce,
+ ))
.unwrap();
let result = task.await;
@@ -523,7 +523,10 @@ mod tests {
);
auth.response
- .send(acp::PermissionOptionId::new("allow").into())
+ .send(acp_thread::SelectedPermissionOutcome::new(
+ acp::PermissionOptionId::new("allow"),
+ acp::PermissionOptionKind::AllowOnce,
+ ))
.unwrap();
let _result = task.await;
@@ -651,7 +654,10 @@ mod tests {
);
auth.response
- .send(acp::PermissionOptionId::new("allow").into())
+ .send(acp_thread::SelectedPermissionOutcome::new(
+ acp::PermissionOptionId::new("allow"),
+ acp::PermissionOptionKind::AllowOnce,
+ ))
.unwrap();
assert!(
@@ -518,7 +518,10 @@ mod tests {
);
auth.response
- .send(acp::PermissionOptionId::new("allow").into())
+ .send(acp_thread::SelectedPermissionOutcome::new(
+ acp::PermissionOptionId::new("allow"),
+ acp::PermissionOptionKind::AllowOnce,
+ ))
.unwrap();
let _result = task.await;
@@ -646,7 +649,10 @@ mod tests {
);
auth.response
- .send(acp::PermissionOptionId::new("allow").into())
+ .send(acp_thread::SelectedPermissionOutcome::new(
+ acp::PermissionOptionId::new("allow"),
+ acp::PermissionOptionKind::AllowOnce,
+ ))
.unwrap();
assert!(
@@ -727,7 +733,10 @@ mod tests {
let auth = event_rx.expect_authorization().await;
auth.response
- .send(acp::PermissionOptionId::new("deny").into())
+ .send(acp_thread::SelectedPermissionOutcome::new(
+ acp::PermissionOptionId::new("deny"),
+ acp::PermissionOptionKind::RejectOnce,
+ ))
.unwrap();
let output = task.await.unwrap();
@@ -2581,7 +2581,10 @@ mod tests {
event
.response
- .send(acp::PermissionOptionId::new("allow").into())
+ .send(acp_thread::SelectedPermissionOutcome::new(
+ acp::PermissionOptionId::new("allow"),
+ acp::PermissionOptionKind::AllowOnce,
+ ))
.unwrap();
authorize_task.await.unwrap();
}
@@ -208,8 +208,10 @@ pub async fn test_tool_call_with_permission<T, F>(
thread.update(cx, |thread, cx| {
thread.authorize_tool_call(
tool_call_id,
- allow_option_id.into(),
- acp::PermissionOptionKind::AllowOnce,
+ acp_thread::SelectedPermissionOutcome::new(
+ allow_option_id,
+ acp::PermissionOptionKind::AllowOnce,
+ ),
cx,
);
@@ -1,9 +1,8 @@
use acp_thread::{
AcpThread, AcpThreadEvent, AgentSessionInfo, AgentThreadEntry, AssistantMessage,
AssistantMessageChunk, AuthRequired, LoadError, MentionUri, PermissionOptionChoice,
- PermissionOptions, PermissionPattern, RetryStatus, SelectedPermissionOutcome,
- SelectedPermissionParams, ThreadStatus, ToolCall, ToolCallContent, ToolCallStatus,
- UserMessageId,
+ PermissionOptions, PermissionPattern, RetryStatus, SelectedPermissionOutcome, ThreadStatus,
+ ToolCall, ToolCallContent, ToolCallStatus, UserMessageId,
};
use acp_thread::{AgentConnection, Plan};
use action_log::{ActionLog, ActionLogTelemetry, DiffStats};
@@ -249,8 +248,7 @@ impl Conversation {
self.authorize_tool_call(
session_id.clone(),
tool_call_id,
- option.option_id.clone().into(),
- option.kind,
+ SelectedPermissionOutcome::new(option.option_id.clone(), option.kind),
cx,
);
Some(())
@@ -261,7 +259,6 @@ impl Conversation {
session_id: acp::SessionId,
tool_call_id: acp::ToolCallId,
outcome: SelectedPermissionOutcome,
- option_kind: acp::PermissionOptionKind,
cx: &mut Context<Self>,
) {
let Some(thread) = self.threads.get(&session_id) else {
@@ -273,11 +270,11 @@ impl Conversation {
"Agent Tool Call Authorized",
agent = agent_telemetry_id,
session = session_id,
- option = option_kind
+ option = outcome.option_kind
);
thread.update(cx, |thread, cx| {
- thread.authorize_tool_call(tool_call_id, outcome, option_kind, cx);
+ thread.authorize_tool_call(tool_call_id, outcome, cx);
});
cx.notify();
}
@@ -6276,8 +6273,10 @@ pub(crate) mod tests {
conversation.authorize_tool_call(
acp::SessionId::new("session-1"),
acp::ToolCallId::new("tc-1"),
- acp::PermissionOptionId::new("allow-1").into(),
- acp::PermissionOptionKind::AllowOnce,
+ SelectedPermissionOutcome::new(
+ acp::PermissionOptionId::new("allow-1"),
+ acp::PermissionOptionKind::AllowOnce,
+ ),
cx,
);
});
@@ -6299,8 +6298,10 @@ pub(crate) mod tests {
conversation.authorize_tool_call(
acp::SessionId::new("session-1"),
acp::ToolCallId::new("tc-2"),
- acp::PermissionOptionId::new("allow-2").into(),
- acp::PermissionOptionKind::AllowOnce,
+ SelectedPermissionOutcome::new(
+ acp::PermissionOptionId::new("allow-2"),
+ acp::PermissionOptionKind::AllowOnce,
+ ),
cx,
);
});
@@ -6438,8 +6439,10 @@ pub(crate) mod tests {
conversation.authorize_tool_call(
acp::SessionId::new("thread-a"),
acp::ToolCallId::new("tc-a"),
- acp::PermissionOptionId::new("allow-a").into(),
- acp::PermissionOptionKind::AllowOnce,
+ SelectedPermissionOutcome::new(
+ acp::PermissionOptionId::new("allow-a"),
+ acp::PermissionOptionKind::AllowOnce,
+ ),
cx,
);
});
@@ -1580,12 +1580,11 @@ impl ThreadView {
session_id: acp::SessionId,
tool_call_id: acp::ToolCallId,
outcome: SelectedPermissionOutcome,
- option_kind: acp::PermissionOptionKind,
window: &mut Window,
cx: &mut Context<Self>,
) {
self.conversation.update(cx, |conversation, cx| {
- conversation.authorize_tool_call(session_id, tool_call_id, outcome, option_kind, cx);
+ conversation.authorize_tool_call(session_id, tool_call_id, outcome, cx);
});
if self.should_be_following {
self.workspace
@@ -1648,8 +1647,7 @@ impl ThreadView {
self.authorize_tool_call(
self.id.clone(),
tool_call_id,
- option_id.into(),
- option_kind,
+ SelectedPermissionOutcome::new(option_id, option_kind),
window,
cx,
);
@@ -1740,16 +1738,9 @@ impl ThreadView {
window: &mut Window,
cx: &mut Context<Self>,
) -> Option<()> {
- let (choices, dropdown_with_patterns) = match options {
- PermissionOptions::Dropdown(choices) => (choices.as_slice(), None),
- PermissionOptions::DropdownWithPatterns {
- choices,
- patterns,
- tool_name,
- } => (
- choices.as_slice(),
- Some((patterns.as_slice(), tool_name.as_str())),
- ),
+ let choices = match options {
+ PermissionOptions::Dropdown(choices) => choices.as_slice(),
+ PermissionOptions::DropdownWithPatterns { choices, .. } => choices.as_slice(),
_ => {
let kind = if is_allow {
acp::PermissionOptionKind::AllowOnce
@@ -1763,34 +1754,9 @@ impl ThreadView {
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
- && let Some((patterns, tool_name)) = dropdown_with_patterns
- {
- let checked_patterns: Vec<_> = patterns
- .iter()
- .enumerate()
- .filter(|(index, _)| checked.contains(index))
- .map(|(_, cp)| cp.pattern.clone())
- .collect();
-
- if !checked_patterns.is_empty() {
- let (option_id_str, kind) = if is_allow {
- (
- format!("always_allow:{}", tool_name),
- acp::PermissionOptionKind::AllowAlways,
- )
- } else {
- (
- format!("always_deny:{}", tool_name),
- acp::PermissionOptionKind::RejectAlways,
- )
- };
- let outcome =
- SelectedPermissionOutcome::new(acp::PermissionOptionId::new(option_id_str))
- .params(Some(SelectedPermissionParams::Terminal {
- patterns: checked_patterns,
- }));
- self.authorize_tool_call(session_id, tool_call_id, outcome, kind, window, cx);
+ 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(());
}
}
@@ -1801,32 +1767,9 @@ impl ThreadView {
.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);
- let selected_option = if is_allow {
- &selected_choice.allow
- } else {
- &selected_choice.deny
- };
-
- let params = if !selected_choice.sub_patterns.is_empty() {
- Some(SelectedPermissionParams::Terminal {
- patterns: selected_choice.sub_patterns.clone(),
- })
- } else {
- None
- };
-
- let outcome =
- SelectedPermissionOutcome::new(selected_option.option_id.clone()).params(params);
-
- self.authorize_tool_call(
- session_id,
- tool_call_id,
- outcome,
- selected_option.kind,
- window,
- cx,
- );
+ self.authorize_tool_call(session_id, tool_call_id, outcome, window, cx);
Some(())
}
@@ -6442,8 +6385,7 @@ impl ThreadView {
this.authorize_tool_call(
session_id.clone(),
tool_call_id.clone(),
- option_id.clone().into(),
- option_kind,
+ SelectedPermissionOutcome::new(option_id.clone(), option_kind),
window,
cx,
);