diff --git a/crates/acp/src/acp.rs b/crates/acp/src/acp.rs index 652016eade05ceb2562583604d9bdfb23f8e45ab..b755db232a8d30381629cd8b85f49124737d106d 100644 --- a/crates/acp/src/acp.rs +++ b/crates/acp/src/acp.rs @@ -130,8 +130,8 @@ pub struct ToolCall { #[derive(Debug)] pub enum ToolCallStatus { WaitingForConfirmation { - description: Entity, - respond_tx: oneshot::Sender, + confirmation: acp::ToolCallConfirmation, + respond_tx: oneshot::Sender, }, // todo! Running? Allowed { @@ -269,24 +269,40 @@ impl AcpThread { ); } - pub fn push_tool_call( + pub fn request_tool_call( &mut self, title: String, - description: String, - confirmation_tx: Option>, + confirmation: acp::ToolCallConfirmation, + cx: &mut Context, + ) -> ToolCallRequest { + let (tx, rx) = oneshot::channel(); + + let status = ToolCallStatus::WaitingForConfirmation { + confirmation, + respond_tx: tx, + }; + + let id = self.insert_tool_call(title, status, cx); + ToolCallRequest { id, outcome: rx } + } + + pub fn push_tool_call(&mut self, title: String, cx: &mut Context) -> ToolCallId { + let status = ToolCallStatus::Allowed { + status: acp::ToolCallStatus::Running, + content: None, + }; + + self.insert_tool_call(title, status, cx) + } + + fn insert_tool_call( + &mut self, + title: String, + status: ToolCallStatus, cx: &mut Context, ) -> ToolCallId { let language_registry = self.project.read(cx).languages().clone(); - let description = cx.new(|cx| { - Markdown::new( - description.into(), - Some(language_registry.clone()), - None, - cx, - ) - }); - let entry_id = self.push_entry( AgentThreadEntryContent::ToolCall(ToolCall { // todo! clean up id creation @@ -294,17 +310,7 @@ impl AcpThread { tool_name: cx.new(|cx| { Markdown::new(title.into(), Some(language_registry.clone()), None, cx) }), - status: if let Some(respond_tx) = confirmation_tx { - ToolCallStatus::WaitingForConfirmation { - description, - respond_tx, - } - } else { - ToolCallStatus::Allowed { - status: acp::ToolCallStatus::Running, - content: Some(description), - } - }, + status, }), cx, ); @@ -312,7 +318,12 @@ impl AcpThread { ToolCallId(entry_id) } - pub fn authorize_tool_call(&mut self, id: ToolCallId, allowed: bool, cx: &mut Context) { + pub fn authorize_tool_call( + &mut self, + id: ToolCallId, + outcome: acp::ToolCallConfirmationOutcome, + cx: &mut Context, + ) { let Some(entry) = self.entry_mut(id.0) else { return; }; @@ -322,19 +333,19 @@ impl AcpThread { return; }; - let new_status = if allowed { + let new_status = if outcome == acp::ToolCallConfirmationOutcome::Reject { + ToolCallStatus::Rejected + } else { ToolCallStatus::Allowed { status: acp::ToolCallStatus::Running, content: None, } - } else { - ToolCallStatus::Rejected }; let curr_status = mem::replace(&mut call.status, new_status); if let ToolCallStatus::WaitingForConfirmation { respond_tx, .. } = curr_status { - respond_tx.send(allowed).log_err(); + respond_tx.send(outcome).log_err(); } else { debug_panic!("tried to authorize an already authorized tool call"); } @@ -422,6 +433,11 @@ impl AcpThread { } } +pub struct ToolCallRequest { + pub id: ToolCallId, + pub outcome: oneshot::Receiver, +} + #[cfg(test)] mod tests { use super::*; @@ -506,7 +522,7 @@ mod tests { let AgentThreadEntryContent::ToolCall(ToolCall { id, tool_name, - status: ToolCallStatus::WaitingForConfirmation { description, .. }, + status: ToolCallStatus::Allowed { .. }, }) = &thread.entries().last().unwrap().content else { panic!(); @@ -516,18 +532,19 @@ mod tests { assert_eq!(md.source(), "read_file"); }); - description.read_with(cx, |md, _cx| { - assert!( - md.source().contains("foo"), - "Expected description to contain 'foo', but got {}", - md.source() - ); - }); + // todo! + // description.read_with(cx, |md, _cx| { + // assert!( + // md.source().contains("foo"), + // "Expected description to contain 'foo', but got {}", + // md.source() + // ); + // }); *id }); thread.update(cx, |thread, cx| { - thread.authorize_tool_call(tool_call_id, true, cx); + thread.authorize_tool_call(tool_call_id, acp::ToolCallConfirmationOutcome::Allow, cx); assert!(matches!( thread.entries().last().unwrap().content, AgentThreadEntryContent::ToolCall(ToolCall { diff --git a/crates/acp/src/server.rs b/crates/acp/src/server.rs index 213588fc5314cf87878a88c9732757bf5a49aced..ed4032777f97915fc47cc906dcc8681bbe76c6aa 100644 --- a/crates/acp/src/server.rs +++ b/crates/acp/src/server.rs @@ -1,9 +1,8 @@ -use crate::{AcpThread, ThreadEntryId, ThreadId, ToolCallId}; +use crate::{AcpThread, ThreadEntryId, ThreadId, ToolCallId, ToolCallRequest}; use agentic_coding_protocol as acp; use anyhow::{Context as _, Result}; use async_trait::async_trait; use collections::HashMap; -use futures::channel::oneshot; use gpui::{App, AppContext, AsyncApp, Context, Entity, Task, WeakEntity}; use parking_lot::Mutex; use project::Project; @@ -182,52 +181,18 @@ impl acp::Client for AcpClientDelegate { &self, request: acp::RequestToolCallConfirmationParams, ) -> Result { - let (tx, rx) = oneshot::channel(); - let cx = &mut self.cx.clone(); - let entry_id = cx + let ToolCallRequest { id, outcome } = cx .update(|cx| { self.update_thread(&request.thread_id.into(), cx, |thread, cx| { - // todo! Should we pass through richer data than a description? - let description = match request.confirmation { - acp::ToolCallConfirmation::Edit { - file_name, - file_diff, - } => { - // todo! Nicer syntax/presentation based on file extension? Better way to communicate diff? - format!("Edit file `{file_name}` with diff:\n```\n{file_diff}\n```") - } - acp::ToolCallConfirmation::Execute { - command, - root_command: _, - } => { - format!("Execute command `{command}`") - } - acp::ToolCallConfirmation::Mcp { - server_name, - tool_name: _, - tool_display_name, - } => { - format!("MCP: {server_name} - {tool_display_name}") - } - acp::ToolCallConfirmation::Info { prompt, urls } => { - format!("Info: {prompt}\n{urls:?}") - } - }; - thread.push_tool_call(request.title, description, Some(tx), cx) + thread.request_tool_call(request.title, request.confirmation, cx) }) })? .context("Failed to update thread")?; - let outcome = if rx.await? { - // todo! Handle other outcomes - acp::ToolCallConfirmationOutcome::Allow - } else { - acp::ToolCallConfirmationOutcome::Reject - }; Ok(acp::RequestToolCallConfirmationResponse { - id: entry_id.into(), - outcome, + id: id.into(), + outcome: outcome.await?, }) } @@ -239,7 +204,7 @@ impl acp::Client for AcpClientDelegate { let entry_id = cx .update(|cx| { self.update_thread(&request.thread_id.into(), cx, |thread, cx| { - thread.push_tool_call(request.title, request.description, None, cx) + thread.push_tool_call(request.title, cx) }) })? .context("Failed to update thread")?; diff --git a/crates/acp/src/thread_view.rs b/crates/acp/src/thread_view.rs index f32218a3fe6bbdf09a86da0e403776350fa4445f..1ca02dab325b6ad8b6ae8a9141a31439f3637c06 100644 --- a/crates/acp/src/thread_view.rs +++ b/crates/acp/src/thread_view.rs @@ -2,7 +2,7 @@ use std::path::Path; use std::rc::Rc; use std::time::Duration; -use agentic_coding_protocol::{self as acp}; +use agentic_coding_protocol::{self as acp, ToolCallConfirmation}; use anyhow::Result; use editor::{Editor, MultiBuffer}; use gpui::{ @@ -191,12 +191,17 @@ impl AcpThreadView { }); } - fn authorize_tool_call(&mut self, id: ToolCallId, allowed: bool, cx: &mut Context) { + fn authorize_tool_call( + &mut self, + id: ToolCallId, + outcome: acp::ToolCallConfirmationOutcome, + cx: &mut Context, + ) { let Some(thread) = self.thread() else { return; }; thread.update(cx, |thread, cx| { - thread.authorize_tool_call(id, allowed, cx); + thread.authorize_tool_call(id, outcome, cx); }); cx.notify(); } @@ -289,48 +294,9 @@ impl AcpThreadView { }; let content = match &tool_call.status { - ToolCallStatus::WaitingForConfirmation { description, .. } => v_flex() - .border_color(cx.theme().colors().border) - .border_t_1() - .px_2() - .py_1p5() - .child(MarkdownElement::new( - description.clone(), - default_markdown_style(window, cx), - )) - .child( - h_flex() - .justify_end() - .gap_1() - .child( - Button::new(("allow", tool_call.id.as_u64()), "Allow") - .icon(IconName::Check) - .icon_position(IconPosition::Start) - .icon_size(IconSize::Small) - .icon_color(Color::Success) - .on_click(cx.listener({ - let id = tool_call.id; - move |this, _, _, cx| { - this.authorize_tool_call(id, true, cx); - } - })), - ) - .child( - Button::new(("reject", tool_call.id.as_u64()), "Reject") - .icon(IconName::X) - .icon_position(IconPosition::Start) - .icon_size(IconSize::Small) - .icon_color(Color::Error) - .on_click(cx.listener({ - let id = tool_call.id; - move |this, _, _, cx| { - this.authorize_tool_call(id, false, cx); - } - })), - ), - ) - .into_any() - .into(), + ToolCallStatus::WaitingForConfirmation { confirmation, .. } => { + Some(self.render_tool_call_confirmation(tool_call.id, confirmation, cx)) + } ToolCallStatus::Allowed { content, .. } => content.clone().map(|content| { div() .border_color(cx.theme().colors().border) @@ -372,6 +338,309 @@ impl AcpThreadView { ) .children(content) } + + fn render_tool_call_confirmation( + &self, + tool_call_id: ToolCallId, + confirmation: &ToolCallConfirmation, + cx: &Context, + ) -> AnyElement { + match confirmation { + ToolCallConfirmation::Edit { + file_name, + file_diff, + } => v_flex() + .border_color(cx.theme().colors().border) + .border_t_1() + .px_2() + .py_1p5() + // todo! nicer rendering + .child(file_name.clone()) + .child(file_diff.clone()) + .child( + h_flex() + .justify_end() + .gap_1() + .child( + Button::new(("allow", tool_call_id.as_u64()), "Always Allow Edits") + .icon(IconName::CheckDouble) + .icon_position(IconPosition::Start) + .icon_size(IconSize::Small) + .icon_color(Color::Success) + .on_click(cx.listener({ + let id = tool_call_id; + move |this, _, _, cx| { + this.authorize_tool_call( + id, + acp::ToolCallConfirmationOutcome::AlwaysAllow, + cx, + ); + } + })), + ) + .child( + Button::new(("allow", tool_call_id.as_u64()), "Allow") + .icon(IconName::Check) + .icon_position(IconPosition::Start) + .icon_size(IconSize::Small) + .icon_color(Color::Success) + .on_click(cx.listener({ + let id = tool_call_id; + move |this, _, _, cx| { + this.authorize_tool_call( + id, + acp::ToolCallConfirmationOutcome::Allow, + cx, + ); + } + })), + ) + .child( + Button::new(("reject", tool_call_id.as_u64()), "Reject") + .icon(IconName::X) + .icon_position(IconPosition::Start) + .icon_size(IconSize::Small) + .icon_color(Color::Error) + .on_click(cx.listener({ + let id = tool_call_id; + move |this, _, _, cx| { + this.authorize_tool_call( + id, + acp::ToolCallConfirmationOutcome::Reject, + cx, + ); + } + })), + ), + ) + .into_any(), + ToolCallConfirmation::Execute { + command, + root_command, + } => v_flex() + .border_color(cx.theme().colors().border) + .border_t_1() + .px_2() + .py_1p5() + // todo! nicer rendering + .child(command.clone()) + .child( + h_flex() + .justify_end() + .gap_1() + .child( + Button::new( + ("allow", tool_call_id.as_u64()), + format!("Always Allow {root_command}"), + ) + .icon(IconName::CheckDouble) + .icon_position(IconPosition::Start) + .icon_size(IconSize::Small) + .icon_color(Color::Success) + .on_click(cx.listener({ + let id = tool_call_id; + move |this, _, _, cx| { + this.authorize_tool_call( + id, + acp::ToolCallConfirmationOutcome::AlwaysAllow, + cx, + ); + } + })), + ) + .child( + Button::new(("allow", tool_call_id.as_u64()), "Allow") + .icon(IconName::Check) + .icon_position(IconPosition::Start) + .icon_size(IconSize::Small) + .icon_color(Color::Success) + .on_click(cx.listener({ + let id = tool_call_id; + move |this, _, _, cx| { + this.authorize_tool_call( + id, + acp::ToolCallConfirmationOutcome::Allow, + cx, + ); + } + })), + ) + .child( + Button::new(("reject", tool_call_id.as_u64()), "Reject") + .icon(IconName::X) + .icon_position(IconPosition::Start) + .icon_size(IconSize::Small) + .icon_color(Color::Error) + .on_click(cx.listener({ + let id = tool_call_id; + move |this, _, _, cx| { + this.authorize_tool_call( + id, + acp::ToolCallConfirmationOutcome::Reject, + cx, + ); + } + })), + ), + ) + .into_any(), + ToolCallConfirmation::Mcp { + server_name, + tool_name: _, + tool_display_name, + } => v_flex() + .border_color(cx.theme().colors().border) + .border_t_1() + .px_2() + .py_1p5() + // todo! nicer rendering + .child(format!("{server_name} - {tool_display_name}")) + .child( + h_flex() + .justify_end() + .gap_1() + .child( + Button::new( + ("allow", tool_call_id.as_u64()), + format!("Always Allow {server_name}"), + ) + .icon(IconName::CheckDouble) + .icon_position(IconPosition::Start) + .icon_size(IconSize::Small) + .icon_color(Color::Success) + .on_click(cx.listener({ + let id = tool_call_id; + move |this, _, _, cx| { + this.authorize_tool_call( + id, + acp::ToolCallConfirmationOutcome::AlwaysAllowMcpServer, + cx, + ); + } + })), + ) + .child( + Button::new( + ("allow", tool_call_id.as_u64()), + format!("Always Allow {tool_display_name}"), + ) + .icon(IconName::CheckDouble) + .icon_position(IconPosition::Start) + .icon_size(IconSize::Small) + .icon_color(Color::Success) + .on_click(cx.listener({ + let id = tool_call_id; + move |this, _, _, cx| { + this.authorize_tool_call( + id, + acp::ToolCallConfirmationOutcome::AlwaysAllowTool, + cx, + ); + } + })), + ) + .child( + Button::new(("allow", tool_call_id.as_u64()), "Allow") + .icon(IconName::Check) + .icon_position(IconPosition::Start) + .icon_size(IconSize::Small) + .icon_color(Color::Success) + .on_click(cx.listener({ + let id = tool_call_id; + move |this, _, _, cx| { + this.authorize_tool_call( + id, + acp::ToolCallConfirmationOutcome::Allow, + cx, + ); + } + })), + ) + .child( + Button::new(("reject", tool_call_id.as_u64()), "Reject") + .icon(IconName::X) + .icon_position(IconPosition::Start) + .icon_size(IconSize::Small) + .icon_color(Color::Error) + .on_click(cx.listener({ + let id = tool_call_id; + move |this, _, _, cx| { + this.authorize_tool_call( + id, + acp::ToolCallConfirmationOutcome::Reject, + cx, + ); + } + })), + ), + ) + .into_any(), + ToolCallConfirmation::Info { prompt, urls: _ } => v_flex() + .border_color(cx.theme().colors().border) + .border_t_1() + .px_2() + .py_1p5() + // todo! nicer rendering + .child(prompt.clone()) + .child( + h_flex() + .justify_end() + .gap_1() + .child( + Button::new(("allow", tool_call_id.as_u64()), "Always Allow") + .icon(IconName::CheckDouble) + .icon_position(IconPosition::Start) + .icon_size(IconSize::Small) + .icon_color(Color::Success) + .on_click(cx.listener({ + let id = tool_call_id; + move |this, _, _, cx| { + this.authorize_tool_call( + id, + acp::ToolCallConfirmationOutcome::AlwaysAllow, + cx, + ); + } + })), + ) + .child( + Button::new(("allow", tool_call_id.as_u64()), "Allow") + .icon(IconName::Check) + .icon_position(IconPosition::Start) + .icon_size(IconSize::Small) + .icon_color(Color::Success) + .on_click(cx.listener({ + let id = tool_call_id; + move |this, _, _, cx| { + this.authorize_tool_call( + id, + acp::ToolCallConfirmationOutcome::Allow, + cx, + ); + } + })), + ) + .child( + Button::new(("reject", tool_call_id.as_u64()), "Reject") + .icon(IconName::X) + .icon_position(IconPosition::Start) + .icon_size(IconSize::Small) + .icon_color(Color::Error) + .on_click(cx.listener({ + let id = tool_call_id; + move |this, _, _, cx| { + this.authorize_tool_call( + id, + acp::ToolCallConfirmationOutcome::Reject, + cx, + ); + } + })), + ), + ) + .into_any(), + } + } } impl Focusable for AcpThreadView {