From 73ac553316c6e31358fdf37b69185f9d4691f48e Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Wed, 2 Jul 2025 16:09:51 -0600 Subject: [PATCH 1/3] Show errors from ACP when requests error Co-authored-by: Mikayla Maki --- Cargo.lock | 1 + crates/acp/Cargo.toml | 1 + crates/acp/src/server.rs | 22 ++++++++++-- crates/acp/src/thread_view.rs | 66 +++++++++++++++++++++++++++++++++-- 4 files changed, 84 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d5bf270e5d194e9af3c27453d9961653ce8db3c3..b35a7ca94c4c4af9a08ba9aab7931a5234479693 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -22,6 +22,7 @@ dependencies = [ "markdown", "parking_lot", "project", + "proto", "serde_json", "settings", "smol", diff --git a/crates/acp/Cargo.toml b/crates/acp/Cargo.toml index 3992a06cdb757378a49eb90d310f6e9b5ca06f59..9482bcbcffd40826bb8b490a568da77ee3d26336 100644 --- a/crates/acp/Cargo.toml +++ b/crates/acp/Cargo.toml @@ -31,6 +31,7 @@ log.workspace = true markdown.workspace = true parking_lot.workspace = true project.workspace = true +proto.workspace = true settings.workspace = true smol.workspace = true theme.workspace = true diff --git a/crates/acp/src/server.rs b/crates/acp/src/server.rs index 5d7115219a0219bdecc35a026162b00d4e0e199b..983b329041b45586a7a83524de054d4217b21dbb 100644 --- a/crates/acp/src/server.rs +++ b/crates/acp/src/server.rs @@ -270,13 +270,18 @@ impl AcpServer { } pub async fn create_thread(self: Arc, cx: &mut AsyncApp) -> Result> { - let response = self.connection.request(acp::CreateThreadParams).await?; + let response = self + .connection + .request(acp::CreateThreadParams) + .await + .map_err(to_anyhow)?; + let thread_id: ThreadId = response.thread_id.into(); let server = self.clone(); let thread = cx.new(|_| AcpThread { // todo! title: "ACP Thread".into(), - id: thread_id.clone(), + id: thread_id.clone(), // Either next_entry_id: ThreadEntryId(0), entries: Vec::default(), project: self.project.clone(), @@ -297,7 +302,8 @@ impl AcpServer { thread_id: thread_id.clone().into(), message, }) - .await?; + .await + .map_err(to_anyhow)?; Ok(()) } @@ -306,6 +312,16 @@ impl AcpServer { } } +#[track_caller] +fn to_anyhow(e: acp::Error) -> anyhow::Error { + log::error!( + "failed to send message: {code}: {message}", + code = e.code, + message = e.message + ); + anyhow::anyhow!(e.message) +} + impl From for ThreadId { fn from(thread_id: acp::ThreadId) -> Self { Self(thread_id.0.into()) diff --git a/crates/acp/src/thread_view.rs b/crates/acp/src/thread_view.rs index 209e15aed290d995df95339d8967b0244aec58cd..ed4bafb46c39b4ccb1f8d61e628993f82e28f948 100644 --- a/crates/acp/src/thread_view.rs +++ b/crates/acp/src/thread_view.rs @@ -12,8 +12,14 @@ use gpui::{ }; use gpui::{FocusHandle, Task}; use language::Buffer; +<<<<<<< HEAD use language::language_settings::SoftWrap; use markdown::{HeadingLevelStyles, MarkdownElement, MarkdownStyle}; +||||||| parent of 47b80cc740 (Show errors from ACP when requests error) +use markdown::{HeadingLevelStyles, MarkdownElement, MarkdownStyle}; +======= +use markdown::{HeadingLevelStyles, Markdown, MarkdownElement, MarkdownStyle}; +>>>>>>> 47b80cc740 (Show errors from ACP when requests error) use project::Project; use settings::Settings as _; use theme::ThemeSettings; @@ -32,6 +38,7 @@ pub struct AcpThreadView { // todo! reconsider structure. currently pretty sparse, but easy to clean up if we need to delete entries. thread_entry_views: Vec>, message_editor: Entity, + last_error: Option>, list_state: ListState, send_task: Option>>, } @@ -96,6 +103,7 @@ impl AcpThreadView { message_editor, send_task: None, list_state: list_state, + last_error: None, } } @@ -137,8 +145,37 @@ impl AcpThreadView { this.update_in(cx, |this, window, cx| { match result { Ok(thread) => { +<<<<<<< HEAD let subscription = cx.subscribe_in(&thread, window, Self::handle_thread_event); +||||||| parent of 47b80cc740 (Show errors from ACP when requests error) + let subscription = cx.subscribe(&thread, |this, _, event, cx| { + let count = this.list_state.item_count(); + match event { + AcpThreadEvent::NewEntry => { + this.list_state.splice(count..count, 1); + } + AcpThreadEvent::EntryUpdated(index) => { + this.list_state.splice(*index..*index + 1, 1); + } + } + cx.notify(); + }); +======= + dbg!(&thread); + let subscription = cx.subscribe(&thread, |this, _, event, cx| { + let count = this.list_state.item_count(); + match event { + AcpThreadEvent::NewEntry => { + this.list_state.splice(count..count, 1); + } + AcpThreadEvent::EntryUpdated(index) => { + this.list_state.splice(*index..*index + 1, 1); + } + } + cx.notify(); + }); +>>>>>>> 47b80cc740 (Show errors from ACP when requests error) this.list_state .splice(0..0, thread.read(cx).entries().len()); @@ -148,6 +185,7 @@ impl AcpThreadView { }; } Err(e) => { + dbg!(&e); if let Some(exit_status) = agent.exit_status() { this.thread_state = ThreadState::LoadError( format!( @@ -189,6 +227,7 @@ impl AcpThreadView { } fn chat(&mut self, _: &Chat, window: &mut Window, cx: &mut Context) { + self.last_error.take(); let text = self.message_editor.read(cx).text(cx); if text.is_empty() { return; @@ -198,9 +237,15 @@ impl AcpThreadView { let task = thread.update(cx, |thread, cx| thread.send(&text, cx)); self.send_task = Some(cx.spawn(async move |this, cx| { - task.await?; + let result = task.await; - this.update(cx, |this, _cx| { + this.update(cx, |this, cx| { + if let Err(err) = result { + this.last_error = + Some(cx.new(|cx| { + Markdown::new(format!("Error: {err}").into(), None, None, cx) + })) + } this.send_task.take(); }) })); @@ -865,7 +910,7 @@ impl Focusable for AcpThreadView { } impl Render for AcpThreadView { - fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { + fn render(&mut self, window: &mut Window, cx: &mut Context) -> impl IntoElement { let text = self.message_editor.read(cx).text(cx); let is_editor_empty = text.is_empty(); let focus_handle = self.message_editor.focus_handle(cx); @@ -907,6 +952,21 @@ impl Render for AcpThreadView { .into() })), }) + .when_some(self.last_error.clone(), |el, error| { + el.child( + div() + .text_xs() + .p_2() + .gap_2() + .border_t_1() + .border_color(cx.theme().status().error_border) + .bg(cx.theme().status().error_background) + .child(MarkdownElement::new( + error, + default_markdown_style(window, cx), + )), + ) + }) .child( v_flex() .bg(cx.theme().colors().editor_background) From 405f7cf64fa4a0edbfef314cb8f617f930fde70d Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Wed, 2 Jul 2025 16:49:37 -0600 Subject: [PATCH 2/3] Hack in authentication Co-authored-by: Mikayla Maki --- crates/acp/src/server.rs | 20 +++++- crates/acp/src/thread_view.rs | 122 ++++++++++++++++++++-------------- 2 files changed, 89 insertions(+), 53 deletions(-) diff --git a/crates/acp/src/server.rs b/crates/acp/src/server.rs index 983b329041b45586a7a83524de054d4217b21dbb..6881e347cfc8624d5d469fa396487eca1ca898b2 100644 --- a/crates/acp/src/server.rs +++ b/crates/acp/src/server.rs @@ -238,13 +238,13 @@ impl acp::Client for AcpClientDelegate { } impl AcpServer { - pub fn stdio(mut process: Child, project: Entity, cx: &mut AsyncApp) -> Arc { + pub fn stdio(mut process: Child, project: Entity, cx: &mut App) -> Arc { let stdin = process.stdin.take().expect("process didn't have stdin"); let stdout = process.stdout.take().expect("process didn't have stdout"); let threads: Arc>>> = Default::default(); let (connection, handler_fut, io_fut) = acp::AgentConnection::connect_to_agent( - AcpClientDelegate::new(project.clone(), threads.clone(), cx.clone()), + AcpClientDelegate::new(project.clone(), threads.clone(), cx.to_async()), stdin, stdout, ); @@ -269,6 +269,22 @@ impl AcpServer { }) } + pub async fn initialize(&self) -> Result { + self.connection + .request(acp::InitializeParams) + .await + .map_err(to_anyhow) + } + + pub async fn authenticate(&self) -> Result<()> { + self.connection + .request(acp::AuthenticateParams) + .await + .map_err(to_anyhow)?; + + Ok(()) + } + pub async fn create_thread(self: Arc, cx: &mut AsyncApp) -> Result> { let response = self .connection diff --git a/crates/acp/src/thread_view.rs b/crates/acp/src/thread_view.rs index ed4bafb46c39b4ccb1f8d61e628993f82e28f948..96bae43c2e56cfc3a049f055811b982cb778d9bb 100644 --- a/crates/acp/src/thread_view.rs +++ b/crates/acp/src/thread_view.rs @@ -1,5 +1,6 @@ use std::path::Path; use std::rc::Rc; +use std::sync::Arc; use std::time::Duration; use agentic_coding_protocol::{self as acp, ToolCallConfirmation}; @@ -12,20 +13,14 @@ use gpui::{ }; use gpui::{FocusHandle, Task}; use language::Buffer; -<<<<<<< HEAD use language::language_settings::SoftWrap; -use markdown::{HeadingLevelStyles, MarkdownElement, MarkdownStyle}; -||||||| parent of 47b80cc740 (Show errors from ACP when requests error) -use markdown::{HeadingLevelStyles, MarkdownElement, MarkdownStyle}; -======= use markdown::{HeadingLevelStyles, Markdown, MarkdownElement, MarkdownStyle}; ->>>>>>> 47b80cc740 (Show errors from ACP when requests error) use project::Project; use settings::Settings as _; use theme::ThemeSettings; use ui::prelude::*; use ui::{Button, Tooltip}; -use util::ResultExt; +use util::{ResultExt, paths}; use zed_actions::agent::Chat; use crate::{ @@ -34,6 +29,7 @@ use crate::{ }; pub struct AcpThreadView { + agent: Arc, thread_state: ThreadState, // todo! reconsider structure. currently pretty sparse, but easy to clean up if we need to delete entries. thread_entry_views: Vec>, @@ -41,6 +37,7 @@ pub struct AcpThreadView { last_error: Option>, list_state: ListState, send_task: Option>>, + auth_task: Option>, } #[derive(Debug)] @@ -57,6 +54,7 @@ enum ThreadState { _subscription: Subscription, }, LoadError(SharedString), + Unauthenticated, } impl AcpThreadView { @@ -107,21 +105,12 @@ impl AcpThreadView { } } - fn initial_state( - project: Entity, - window: &mut Window, - cx: &mut Context, - ) -> ThreadState { - let Some(root_dir) = project + let root_dir = project .read(cx) .visible_worktrees(cx) .next() .map(|worktree| worktree.read(cx).abs_path()) - else { - return ThreadState::LoadError( - "Gemini threads must be created within a project".into(), - ); - }; + .unwrap_or_else(|| paths::home_dir().as_path().into()); let cli_path = Path::new(env!("CARGO_MANIFEST_DIR")).join("../../../gemini-cli/packages/cli"); @@ -137,45 +126,44 @@ impl AcpThreadView { .spawn() .unwrap(); - let project = project.clone(); + let agent = AcpServer::stdio(child, project, cx); + + Self { + thread_state: Self::initial_state(agent.clone(), window, cx), + agent, + message_editor, + send_task: None, + list_state: list_state, + last_error: None, + auth_task: None, + } + } + + fn initial_state( + agent: Arc, + window: &mut Window, + cx: &mut Context, + ) -> ThreadState { let load_task = cx.spawn_in(window, async move |this, cx| { - let agent = AcpServer::stdio(child, project, cx); - let result = agent.clone().create_thread(cx).await; + let result = match agent.initialize().await { + Err(e) => Err(e), + Ok(response) => { + if !response.is_authenticated { + this.update(cx, |this, _| { + this.thread_state = ThreadState::Unauthenticated; + }) + .ok(); + return; + } + agent.clone().create_thread(cx).await + } + }; this.update_in(cx, |this, window, cx| { match result { Ok(thread) => { -<<<<<<< HEAD let subscription = cx.subscribe_in(&thread, window, Self::handle_thread_event); -||||||| parent of 47b80cc740 (Show errors from ACP when requests error) - let subscription = cx.subscribe(&thread, |this, _, event, cx| { - let count = this.list_state.item_count(); - match event { - AcpThreadEvent::NewEntry => { - this.list_state.splice(count..count, 1); - } - AcpThreadEvent::EntryUpdated(index) => { - this.list_state.splice(*index..*index + 1, 1); - } - } - cx.notify(); - }); -======= - dbg!(&thread); - let subscription = cx.subscribe(&thread, |this, _, event, cx| { - let count = this.list_state.item_count(); - match event { - AcpThreadEvent::NewEntry => { - this.list_state.splice(count..count, 1); - } - AcpThreadEvent::EntryUpdated(index) => { - this.list_state.splice(*index..*index + 1, 1); - } - } - cx.notify(); - }); ->>>>>>> 47b80cc740 (Show errors from ACP when requests error) this.list_state .splice(0..0, thread.read(cx).entries().len()); @@ -210,7 +198,9 @@ impl AcpThreadView { fn thread(&self) -> Option<&Entity> { match &self.thread_state { ThreadState::Ready { thread, .. } => Some(thread), - ThreadState::Loading { .. } | ThreadState::LoadError(..) => None, + ThreadState::Loading { .. } + | ThreadState::LoadError(..) + | ThreadState::Unauthenticated => None, } } @@ -219,6 +209,7 @@ impl AcpThreadView { ThreadState::Ready { thread, .. } => thread.read(cx).title(), ThreadState::Loading { .. } => "Loading...".into(), ThreadState::LoadError(_) => "Failed to load".into(), + ThreadState::Unauthenticated => "Not authenticated".into(), } } @@ -357,6 +348,27 @@ impl AcpThreadView { } } + fn authenticate(&mut self, window: &mut Window, cx: &mut Context) { + let agent = self.agent.clone(); + + self.auth_task = Some(cx.spawn_in(window, async move |this, cx| { + let result = agent.authenticate().await; + + this.update_in(cx, |this, window, cx| { + if let Err(err) = result { + this.last_error = + Some(cx.new(|cx| { + Markdown::new(format!("Error: {err}").into(), None, None, cx) + })) + } else { + this.thread_state = Self::initial_state(agent, window, cx) + } + this.auth_task.take() + }) + .ok(); + })); + } + fn authorize_tool_call( &mut self, id: ToolCallId, @@ -920,6 +932,14 @@ impl Render for AcpThreadView { .on_action(cx.listener(Self::chat)) .h_full() .child(match &self.thread_state { + ThreadState::Unauthenticated => v_flex() + .p_2() + .flex_1() + .justify_end() + .child(Label::new("Not authenticated")) + .child(Button::new("sign-in", "Sign in via Gemini CLI").on_click( + cx.listener(|this, _, window, cx| this.authenticate(window, cx)), + )), ThreadState::Loading { .. } => v_flex() .p_2() .flex_1() From d6c76d8d33fec68165ee05c1a562ec4acef1a5e9 Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Wed, 2 Jul 2025 16:52:41 -0700 Subject: [PATCH 3/3] Resolve merge conflicts --- crates/acp/src/acp.rs | 4 ++-- crates/acp/src/thread_view.rs | 11 +---------- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/crates/acp/src/acp.rs b/crates/acp/src/acp.rs index e5a1461e6d3a0bee3f43b46687aee37931508e2d..8ee1b2892fdc12fd248ec8a00b0f99b38060c193 100644 --- a/crates/acp/src/acp.rs +++ b/crates/acp/src/acp.rs @@ -724,7 +724,7 @@ mod tests { } } - pub fn gemini_acp_server(project: Entity, mut cx: AsyncApp) -> Result> { + pub fn gemini_acp_server(project: Entity, cx: AsyncApp) -> Result> { let cli_path = Path::new(env!("CARGO_MANIFEST_DIR")).join("../../../gemini-cli/packages/cli"); let mut command = util::command::new_smol_command("node"); @@ -743,6 +743,6 @@ mod tests { let child = command.spawn().unwrap(); - Ok(AcpServer::stdio(child, project, &mut cx)) + cx.update(|cx| AcpServer::stdio(child, project, cx)) } } diff --git a/crates/acp/src/thread_view.rs b/crates/acp/src/thread_view.rs index 96bae43c2e56cfc3a049f055811b982cb778d9bb..9c3213dd3d6bc58522773f3fe063e8cfa0c3c596 100644 --- a/crates/acp/src/thread_view.rs +++ b/crates/acp/src/thread_view.rs @@ -95,16 +95,6 @@ impl AcpThreadView { }), ); - Self { - thread_state: Self::initial_state(project, window, cx), - thread_entry_views: Vec::new(), - message_editor, - send_task: None, - list_state: list_state, - last_error: None, - } - } - let root_dir = project .read(cx) .visible_worktrees(cx) @@ -132,6 +122,7 @@ impl AcpThreadView { thread_state: Self::initial_state(agent.clone(), window, cx), agent, message_editor, + thread_entry_views: Vec::new(), send_task: None, list_state: list_state, last_error: None,