From 4e9666e92d21edaf9711914f11837a3e644eb794 Mon Sep 17 00:00:00 2001 From: "zed-zippy[bot]" <234243425+zed-zippy[bot]@users.noreply.github.com> Date: Fri, 13 Mar 2026 14:27:47 +0100 Subject: [PATCH] agent: Fix session close capability check (#51479) (cherry-pick to preview) (#51486) Cherry-pick of #51479 to preview ---- Release Notes: - agent: Fixed an issue where external agents would return an error because unsupported ACP method was called --------- Co-authored-by: Bennet Bo Fenner Co-authored-by: Bennet Bo Fenner --- crates/agent_servers/src/acp.rs | 2 +- crates/agent_ui/src/connection_view.rs | 237 ++++++++++++++++++++++++- 2 files changed, 234 insertions(+), 5 deletions(-) diff --git a/crates/agent_servers/src/acp.rs b/crates/agent_servers/src/acp.rs index a661289f6221818c6f63c799b0593907bb665eb9..ba0851565e4ee84e1eb4360a6391a1ad442602cf 100644 --- a/crates/agent_servers/src/acp.rs +++ b/crates/agent_servers/src/acp.rs @@ -753,7 +753,7 @@ impl AgentConnection for AcpConnection { session_id: &acp::SessionId, cx: &mut App, ) -> Task> { - if !self.agent_capabilities.session_capabilities.close.is_none() { + if !self.supports_close_session() { return Task::ready(Err(anyhow!(LoadError::Other( "Closing sessions is not supported by this agent.".into() )))); diff --git a/crates/agent_ui/src/connection_view.rs b/crates/agent_ui/src/connection_view.rs index 2fd86f9c9d91abb7d5b08bd7a779b93592f2011c..1a1ecf4ec1dac068d03bc5969e911c988d0bbc05 100644 --- a/crates/agent_ui/src/connection_view.rs +++ b/crates/agent_ui/src/connection_view.rs @@ -460,10 +460,13 @@ impl ConnectedServerState { } pub fn close_all_sessions(&self, cx: &mut App) -> Task<()> { - let tasks = self - .threads - .keys() - .map(|id| self.connection.clone().close_session(id, cx)); + let tasks = self.threads.keys().filter_map(|id| { + if self.connection.supports_close_session() { + Some(self.connection.clone().close_session(id, cx)) + } else { + None + } + }); let task = futures::future::join_all(tasks); cx.background_spawn(async move { task.await; @@ -6522,4 +6525,230 @@ pub(crate) mod tests { "Main editor should have existing content and queued message separated by two newlines" ); } + + #[gpui::test] + async fn test_close_all_sessions_skips_when_unsupported(cx: &mut TestAppContext) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + let project = Project::test(fs, [], cx).await; + let (multi_workspace, cx) = + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); + let workspace = multi_workspace.read_with(cx, |mw, _| mw.workspace().clone()); + + let thread_store = cx.update(|_window, cx| cx.new(|cx| ThreadStore::new(cx))); + let connection_store = + cx.update(|_window, cx| cx.new(|cx| AgentConnectionStore::new(project.clone(), cx))); + let history = cx.update(|window, cx| cx.new(|cx| ThreadHistory::new(None, window, cx))); + + // StubAgentConnection defaults to supports_close_session() -> false + let thread_view = cx.update(|window, cx| { + cx.new(|cx| { + ConnectionView::new( + Rc::new(StubAgentServer::default_response()), + connection_store, + ExternalAgent::Custom { + name: "Test".into(), + }, + None, + None, + None, + None, + workspace.downgrade(), + project, + Some(thread_store), + None, + history, + window, + cx, + ) + }) + }); + + cx.run_until_parked(); + + thread_view.read_with(cx, |view, _cx| { + let connected = view.as_connected().expect("Should be connected"); + assert!( + !connected.threads.is_empty(), + "There should be at least one thread" + ); + assert!( + !connected.connection.supports_close_session(), + "StubAgentConnection should not support close" + ); + }); + + thread_view + .update(cx, |view, cx| { + view.as_connected() + .expect("Should be connected") + .close_all_sessions(cx) + }) + .await; + } + + #[gpui::test] + async fn test_close_all_sessions_calls_close_when_supported(cx: &mut TestAppContext) { + init_test(cx); + + let (thread_view, cx) = + setup_thread_view(StubAgentServer::new(CloseCapableConnection::new()), cx).await; + + cx.run_until_parked(); + + let close_capable = thread_view.read_with(cx, |view, _cx| { + let connected = view.as_connected().expect("Should be connected"); + assert!( + !connected.threads.is_empty(), + "There should be at least one thread" + ); + assert!( + connected.connection.supports_close_session(), + "CloseCapableConnection should support close" + ); + connected + .connection + .clone() + .into_any() + .downcast::() + .expect("Should be CloseCapableConnection") + }); + + thread_view + .update(cx, |view, cx| { + view.as_connected() + .expect("Should be connected") + .close_all_sessions(cx) + }) + .await; + + let closed_count = close_capable.closed_sessions.lock().len(); + assert!( + closed_count > 0, + "close_session should have been called for each thread" + ); + } + + #[gpui::test] + async fn test_close_session_returns_error_when_unsupported(cx: &mut TestAppContext) { + init_test(cx); + + let (thread_view, cx) = setup_thread_view(StubAgentServer::default_response(), cx).await; + + cx.run_until_parked(); + + let result = thread_view + .update(cx, |view, cx| { + let connected = view.as_connected().expect("Should be connected"); + assert!( + !connected.connection.supports_close_session(), + "StubAgentConnection should not support close" + ); + let session_id = connected + .threads + .keys() + .next() + .expect("Should have at least one thread") + .clone(); + connected.connection.clone().close_session(&session_id, cx) + }) + .await; + + assert!( + result.is_err(), + "close_session should return an error when close is not supported" + ); + assert!( + result.unwrap_err().to_string().contains("not supported"), + "Error message should indicate that closing is not supported" + ); + } + + #[derive(Clone)] + struct CloseCapableConnection { + closed_sessions: Arc>>, + } + + impl CloseCapableConnection { + fn new() -> Self { + Self { + closed_sessions: Arc::new(Mutex::new(Vec::new())), + } + } + } + + impl AgentConnection for CloseCapableConnection { + fn telemetry_id(&self) -> SharedString { + "close-capable".into() + } + + fn new_session( + self: Rc, + project: Entity, + cwd: &Path, + cx: &mut gpui::App, + ) -> Task>> { + let action_log = cx.new(|_| ActionLog::new(project.clone())); + let thread = cx.new(|cx| { + AcpThread::new( + None, + "CloseCapableConnection", + Some(cwd.to_path_buf()), + self, + project, + action_log, + SessionId::new("close-capable-session"), + watch::Receiver::constant( + acp::PromptCapabilities::new() + .image(true) + .audio(true) + .embedded_context(true), + ), + cx, + ) + }); + Task::ready(Ok(thread)) + } + + fn supports_close_session(&self) -> bool { + true + } + + fn close_session( + self: Rc, + session_id: &acp::SessionId, + _cx: &mut App, + ) -> Task> { + self.closed_sessions.lock().push(session_id.clone()); + Task::ready(Ok(())) + } + + fn auth_methods(&self) -> &[acp::AuthMethod] { + &[] + } + + fn authenticate( + &self, + _method_id: acp::AuthMethodId, + _cx: &mut App, + ) -> Task> { + Task::ready(Ok(())) + } + + fn prompt( + &self, + _id: Option, + _params: acp::PromptRequest, + _cx: &mut App, + ) -> Task> { + Task::ready(Ok(acp::PromptResponse::new(acp::StopReason::EndTurn))) + } + + fn cancel(&self, _session_id: &acp::SessionId, _cx: &mut App) {} + + fn into_any(self: Rc) -> Rc { + self + } + } }