From 91531fad6de30b3cacf1875f92e0384a0599a7e5 Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Fri, 22 May 2026 14:25:26 +0200 Subject: [PATCH] acp: Add logout support (#57492) Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Release Notes: - acp: Add Logout flow for agents that support it. --- crates/acp_thread/src/connection.rs | 2 +- crates/agent_servers/src/acp.rs | 60 ++++-------------------- crates/agent_ui/src/agent_panel.rs | 2 +- crates/agent_ui/src/conversation_view.rs | 18 +++---- 4 files changed, 21 insertions(+), 61 deletions(-) diff --git a/crates/acp_thread/src/connection.rs b/crates/acp_thread/src/connection.rs index f58d8a581b8..87ef7345150 100644 --- a/crates/acp_thread/src/connection.rs +++ b/crates/acp_thread/src/connection.rs @@ -132,7 +132,7 @@ pub trait AgentConnection { fn authenticate(&self, method: acp::AuthMethodId, cx: &mut App) -> Task>; - fn supports_logout(&self, _cx: &App) -> bool { + fn supports_logout(&self) -> bool { false } diff --git a/crates/agent_servers/src/acp.rs b/crates/agent_servers/src/acp.rs index 3a718c7a9e8..3dfea245e97 100644 --- a/crates/agent_servers/src/acp.rs +++ b/crates/agent_servers/src/acp.rs @@ -1853,12 +1853,12 @@ impl AgentConnection for AcpConnection { }) } - fn supports_logout(&self, cx: &App) -> bool { - cx.has_flag::() && self.agent_capabilities.auth.logout.is_some() + fn supports_logout(&self) -> bool { + self.agent_capabilities.auth.logout.is_some() } fn logout(&self, cx: &mut App) -> Task> { - if !self.supports_logout(cx) { + if !self.supports_logout() { return Task::ready(Err(anyhow!("Logout is not supported by this agent."))); } @@ -2234,8 +2234,8 @@ pub mod test_support { self.inner.authenticate(method, cx) } - fn supports_logout(&self, cx: &App) -> bool { - self.inner.supports_logout(cx) + fn supports_logout(&self) -> bool { + self.inner.supports_logout() } fn logout(&self, cx: &mut App) -> Task> { @@ -3117,28 +3117,16 @@ mod tests { } #[gpui::test] - async fn logout_is_gated_by_beta_flag_and_agent_capability(cx: &mut gpui::TestAppContext) { - cx.update(|cx| { - let store = settings::SettingsStore::test(cx); - cx.set_global(store); - settings::SettingsStore::update_global(cx, |store, _| { - store.register_setting::(); - }); - feature_flags::FeatureFlagStore::init(cx); - }); + async fn logout_support_requires_agent_capability(cx: &mut gpui::TestAppContext) { + cx.update(|cx| set_acp_beta_override(cx, "off")); + assert!(!cx.update(|cx| cx.has_flag::())); let fs = fs::FakeFs::new(cx.executor()); fs.insert_tree("/", serde_json::json!({ "a": {} })).await; let project = project::Project::test(fs, [std::path::Path::new("/a")], cx).await; let mut harness = test_support::connect_fake_acp_connection(project, cx).await; - cx.update(|cx| { - settings::SettingsStore::update_global(cx, |store, _| { - store.register_setting::(); - }); - feature_flags::FeatureFlagStore::init(cx); - }); - assert!(!cx.update(|cx| harness.connection.supports_logout(cx))); + assert!(!harness.connection.supports_logout()); let unsupported_logout = cx.update(|cx| harness.connection.logout(cx)); let error = unsupported_logout .await @@ -3151,35 +3139,7 @@ mod tests { .agent_capabilities .auth = acp::AgentAuthCapabilities::new().logout(acp::LogoutCapabilities::new()); - cx.update(|cx| { - settings::SettingsStore::update_global(cx, |store, cx| { - store.update_user_settings(cx, |content| { - content - .feature_flags - .get_or_insert_default() - .insert("acp-beta".to_string(), "off".to_string()); - }); - }); - }); - assert!(!cx.update(|cx| harness.connection.supports_logout(cx))); - let disabled_logout = cx.update(|cx| harness.connection.logout(cx)); - let error = disabled_logout - .await - .expect_err("logout should be rejected when acp-beta is disabled"); - assert_eq!(error.to_string(), "Logout is not supported by this agent."); - assert_eq!(harness.logout_count.load(Ordering::SeqCst), 0); - - cx.update(|cx| { - settings::SettingsStore::update_global(cx, |store, cx| { - store.update_user_settings(cx, |content| { - content - .feature_flags - .get_or_insert_default() - .insert("acp-beta".to_string(), "on".to_string()); - }); - }); - }); - assert!(cx.update(|cx| harness.connection.supports_logout(cx))); + assert!(harness.connection.supports_logout()); cx.update(|cx| harness.connection.logout(cx)) .await .expect("logout should be sent when the agent advertises support"); diff --git a/crates/agent_ui/src/agent_panel.rs b/crates/agent_ui/src/agent_panel.rs index fb7441d186a..9dcb3d11e4a 100644 --- a/crates/agent_ui/src/agent_panel.rs +++ b/crates/agent_ui/src/agent_panel.rs @@ -4810,7 +4810,7 @@ impl AgentPanel { }; let supports_logout = self .active_conversation_view() - .is_some_and(|conversation_view| conversation_view.read(cx).supports_logout(cx)); + .is_some_and(|conversation_view| conversation_view.read(cx).supports_logout()); let project_agents_md_path: Option = self .project diff --git a/crates/agent_ui/src/conversation_view.rs b/crates/agent_ui/src/conversation_view.rs index 3ef7e111fe6..ce6e2661ca4 100644 --- a/crates/agent_ui/src/conversation_view.rs +++ b/crates/agent_ui/src/conversation_view.rs @@ -530,9 +530,9 @@ impl ConversationView { }) } - pub fn supports_logout(&self, cx: &App) -> bool { + pub fn supports_logout(&self) -> bool { self.as_connected().is_some_and(|connected| { - connected.auth_state.is_ok() && connected.connection.supports_logout(cx) + connected.auth_state.is_ok() && connected.connection.supports_logout() }) } @@ -2953,7 +2953,7 @@ impl ConversationView { } pub(crate) fn logout(&mut self, window: &mut Window, cx: &mut Context) { - if !self.supports_logout(cx) { + if !self.supports_logout() { return; } @@ -4120,7 +4120,7 @@ pub(crate) mod tests { // When new_session returns AuthRequired, the server should transition // to Connected + Unauthenticated rather than getting stuck in Loading. - conversation_view.read_with(cx, |view, cx| { + conversation_view.read_with(cx, |view, _cx| { let connected = view .as_connected() .expect("Should be in Connected state even though auth is required"); @@ -4129,7 +4129,7 @@ pub(crate) mod tests { "Auth state should be Unauthenticated" ); assert!( - !view.supports_logout(cx), + !view.supports_logout(), "Logout should be hidden while unauthenticated" ); assert!( @@ -4168,7 +4168,7 @@ pub(crate) mod tests { .expect("Should still be in Connected state after auth"); assert!(connected.auth_state.is_ok(), "Auth state should be Ok"); assert!( - view.supports_logout(cx), + view.supports_logout(), "Logout should be available after authentication" ); assert!( @@ -4193,7 +4193,7 @@ pub(crate) mod tests { conversation_view.update_in(cx, |view, window, cx| view.logout(window, cx)); cx.run_until_parked(); - conversation_view.read_with(cx, |view, cx| { + conversation_view.read_with(cx, |view, _cx| { let connected = view .as_connected() .expect("Should still be in Connected state after logout"); @@ -4202,7 +4202,7 @@ pub(crate) mod tests { "Auth state should be Unauthenticated after logout" ); assert!( - !view.supports_logout(cx), + !view.supports_logout(), "Logout should be hidden after logout" ); }); @@ -5343,7 +5343,7 @@ pub(crate) mod tests { } } - fn supports_logout(&self, _cx: &App) -> bool { + fn supports_logout(&self) -> bool { true }