Move login metehods out of AgentServer

Richard Feldman created

Change summary

crates/agent/src/native_agent_server.rs   |  16 ---
crates/agent_servers/src/agent_servers.rs |  44 ++++---
crates/agent_servers/src/claude.rs        |  36 +++---
crates/agent_servers/src/codex.rs         |  36 +++---
crates/agent_servers/src/custom.rs        |  40 ++++---
crates/agent_servers/src/gemini.rs        |  40 ++++---
crates/agent_ui/src/acp/thread_view.rs    | 129 ++++++++++++++----------
7 files changed, 180 insertions(+), 161 deletions(-)

Detailed changes

crates/agent/src/native_agent_server.rs 🔗

@@ -34,22 +34,6 @@ impl AgentServer for NativeAgentServer {
         ui::IconName::ZedAgent
     }
 
-    fn local_login_commands(&self) -> Vec<String> {
-        vec![]
-    }
-
-    fn remote_login_commands(&self) -> Vec<String> {
-        vec![]
-    }
-
-    fn local_logout_commands(&self) -> Vec<String> {
-        vec![]
-    }
-
-    fn remote_logout_commands(&self) -> Vec<String> {
-        vec![]
-    }
-
     fn connect(
         &self,
         _root_dir: Option<&Path>,

crates/agent_servers/src/agent_servers.rs 🔗

@@ -68,26 +68,6 @@ pub trait AgentServer: Send {
     ) {
     }
 
-    /// Returns the list of slash commands that should trigger Zed's authentication UI
-    /// when running locally (e.g., "/login").
-    /// These commands will be intercepted by Zed to show the auth method selection UI.
-    fn local_login_commands(&self) -> Vec<String>;
-
-    /// Returns the list of slash commands that should trigger Zed's authentication UI
-    /// when running remotely (e.g., "/login").
-    /// These commands will be intercepted by Zed to show the auth method selection UI.
-    fn remote_login_commands(&self) -> Vec<String>;
-
-    /// Returns the list of logout-related slash commands that should be sent to the agent
-    /// when running locally to let it reset internal state (e.g., "/logout").
-    /// These commands will be added to available_commands and passed through to the agent.
-    fn local_logout_commands(&self) -> Vec<String>;
-
-    /// Returns the list of logout-related slash commands that should be sent to the agent
-    /// when running remotely to let it reset internal state (e.g., "/logout").
-    /// These commands will be added to available_commands and passed through to the agent.
-    fn remote_logout_commands(&self) -> Vec<String>;
-
     fn connect(
         &self,
         root_dir: Option<&Path>,
@@ -109,6 +89,30 @@ impl dyn AgentServer {
     }
 }
 
+/// Extension trait for ACP-specific agent capabilities.
+/// This trait is only implemented by agents that use the Agent Client Protocol (ACP).
+pub trait AcpAgentServer: AgentServer {
+    /// Returns the list of slash commands that should trigger Zed's authentication UI
+    /// when running locally (e.g., "/login").
+    /// These commands will be intercepted by Zed to show the auth method selection UI.
+    fn local_login_commands(&self) -> Vec<String>;
+
+    /// Returns the list of slash commands that should trigger Zed's authentication UI
+    /// when running remotely (e.g., "/login").
+    /// These commands will be intercepted by Zed to show the auth method selection UI.
+    fn remote_login_commands(&self) -> Vec<String>;
+
+    /// Returns the list of logout-related slash commands that should be sent to the agent
+    /// when running locally to let it reset internal state (e.g., "/logout").
+    /// These commands will be added to available_commands and passed through to the agent.
+    fn local_logout_commands(&self) -> Vec<String>;
+
+    /// Returns the list of logout-related slash commands that should be sent to the agent
+    /// when running remotely to let it reset internal state (e.g., "/logout").
+    /// These commands will be added to available_commands and passed through to the agent.
+    fn remote_logout_commands(&self) -> Vec<String>;
+}
+
 /// Load the default proxy environment variables to pass through to the agent
 pub fn load_proxy_env(cx: &mut App) -> HashMap<String, String> {
     let proxy_url = cx

crates/agent_servers/src/claude.rs 🔗

@@ -11,7 +11,7 @@ use collections::HashMap;
 use gpui::{App, AppContext as _, SharedString, Task};
 use project::agent_server_store::{AllAgentServersSettings, CLAUDE_CODE_NAME};
 
-use crate::{AgentServer, AgentServerDelegate, load_proxy_env};
+use crate::{AcpAgentServer, AgentServer, AgentServerDelegate, load_proxy_env};
 use acp_thread::AgentConnection;
 
 #[derive(Clone)]
@@ -56,22 +56,6 @@ impl AgentServer for ClaudeCode {
         });
     }
 
-    fn local_login_commands(&self) -> Vec<String> {
-        vec!["login".to_string()]
-    }
-
-    fn remote_login_commands(&self) -> Vec<String> {
-        vec!["login".to_string()]
-    }
-
-    fn local_logout_commands(&self) -> Vec<String> {
-        vec!["logout".to_string()]
-    }
-
-    fn remote_logout_commands(&self) -> Vec<String> {
-        vec!["logout".to_string()]
-    }
-
     fn connect(
         &self,
         root_dir: Option<&Path>,
@@ -122,3 +106,21 @@ impl AgentServer for ClaudeCode {
         self
     }
 }
+
+impl AcpAgentServer for ClaudeCode {
+    fn local_login_commands(&self) -> Vec<String> {
+        vec!["login".to_string()]
+    }
+
+    fn remote_login_commands(&self) -> Vec<String> {
+        vec!["login".to_string()]
+    }
+
+    fn local_logout_commands(&self) -> Vec<String> {
+        vec!["logout".to_string()]
+    }
+
+    fn remote_logout_commands(&self) -> Vec<String> {
+        vec!["logout".to_string()]
+    }
+}

crates/agent_servers/src/codex.rs 🔗

@@ -11,7 +11,7 @@ use gpui::{App, AppContext as _, SharedString, Task};
 use project::agent_server_store::{AllAgentServersSettings, CODEX_NAME};
 use settings::{SettingsStore, update_settings_file};
 
-use crate::{AgentServer, AgentServerDelegate, load_proxy_env};
+use crate::{AcpAgentServer, AgentServer, AgentServerDelegate, load_proxy_env};
 
 #[derive(Clone)]
 pub struct Codex;
@@ -36,22 +36,6 @@ impl AgentServer for Codex {
         ui::IconName::AiOpenAi
     }
 
-    fn local_login_commands(&self) -> Vec<String> {
-        vec![]
-    }
-
-    fn remote_login_commands(&self) -> Vec<String> {
-        vec![]
-    }
-
-    fn local_logout_commands(&self) -> Vec<String> {
-        vec![]
-    }
-
-    fn remote_logout_commands(&self) -> Vec<String> {
-        vec![]
-    }
-
     fn default_mode(&self, cx: &mut App) -> Option<acp::SessionModeId> {
         let settings = cx.read_global(|settings: &SettingsStore, _| {
             settings.get::<AllAgentServersSettings>(None).codex.clone()
@@ -126,3 +110,21 @@ impl AgentServer for Codex {
         self
     }
 }
+
+impl AcpAgentServer for Codex {
+    fn local_login_commands(&self) -> Vec<String> {
+        vec![]
+    }
+
+    fn remote_login_commands(&self) -> Vec<String> {
+        vec![]
+    }
+
+    fn local_logout_commands(&self) -> Vec<String> {
+        vec![]
+    }
+
+    fn remote_logout_commands(&self) -> Vec<String> {
+        vec![]
+    }
+}

crates/agent_servers/src/custom.rs 🔗

@@ -1,4 +1,4 @@
-use crate::{AgentServerDelegate, load_proxy_env};
+use crate::{AcpAgentServer, AgentServerDelegate, load_proxy_env};
 use acp_thread::AgentConnection;
 use agent_client_protocol as acp;
 use anyhow::{Context as _, Result};
@@ -7,7 +7,7 @@ use fs::Fs;
 use gpui::{App, AppContext as _, SharedString, Task};
 use project::agent_server_store::{AllAgentServersSettings, ExternalAgentServerName};
 use settings::{SettingsStore, update_settings_file};
-use std::{path::Path, rc::Rc, sync::Arc};
+use std::{any::Any, path::Path, rc::Rc, sync::Arc};
 use ui::IconName;
 
 /// A generic agent server implementation for custom user-defined agents
@@ -34,22 +34,6 @@ impl crate::AgentServer for CustomAgentServer {
         IconName::Terminal
     }
 
-    fn local_login_commands(&self) -> Vec<String> {
-        vec![]
-    }
-
-    fn remote_login_commands(&self) -> Vec<String> {
-        vec![]
-    }
-
-    fn local_logout_commands(&self) -> Vec<String> {
-        vec![]
-    }
-
-    fn remote_logout_commands(&self) -> Vec<String> {
-        vec![]
-    }
-
     fn default_mode(&self, cx: &mut App) -> Option<acp::SessionModeId> {
         let settings = cx.read_global(|settings: &SettingsStore, _| {
             settings
@@ -125,7 +109,25 @@ impl crate::AgentServer for CustomAgentServer {
         })
     }
 
-    fn into_any(self: Rc<Self>) -> Rc<dyn std::any::Any> {
+    fn into_any(self: Rc<Self>) -> Rc<dyn Any> {
         self
     }
 }
+
+impl AcpAgentServer for CustomAgentServer {
+    fn local_login_commands(&self) -> Vec<String> {
+        vec![]
+    }
+
+    fn remote_login_commands(&self) -> Vec<String> {
+        vec![]
+    }
+
+    fn local_logout_commands(&self) -> Vec<String> {
+        vec![]
+    }
+
+    fn remote_logout_commands(&self) -> Vec<String> {
+        vec![]
+    }
+}

crates/agent_servers/src/gemini.rs 🔗

@@ -1,7 +1,7 @@
 use std::rc::Rc;
 use std::{any::Any, path::Path};
 
-use crate::{AgentServer, AgentServerDelegate, load_proxy_env};
+use crate::{AcpAgentServer, AgentServer, AgentServerDelegate, load_proxy_env};
 use acp_thread::AgentConnection;
 use anyhow::{Context as _, Result};
 use collections::HashMap;
@@ -25,24 +25,6 @@ impl AgentServer for Gemini {
         ui::IconName::AiGemini
     }
 
-    fn local_login_commands(&self) -> Vec<String> {
-        vec!["login".to_string()]
-    }
-
-    fn remote_login_commands(&self) -> Vec<String> {
-        // When remote, OAuth doesn't work, so login is handled via the
-        // auth_commands mapping (oauth-personal -> spawn-gemini-cli)
-        vec![]
-    }
-
-    fn local_logout_commands(&self) -> Vec<String> {
-        vec![]
-    }
-
-    fn remote_logout_commands(&self) -> Vec<String> {
-        vec![]
-    }
-
     fn connect(
         &self,
         root_dir: Option<&Path>,
@@ -104,6 +86,26 @@ impl AgentServer for Gemini {
     }
 }
 
+impl AcpAgentServer for Gemini {
+    fn local_login_commands(&self) -> Vec<String> {
+        vec!["login".to_string()]
+    }
+
+    fn remote_login_commands(&self) -> Vec<String> {
+        // When remote, OAuth doesn't work, so login is handled via the
+        // auth_commands mapping (oauth-personal -> spawn-gemini-cli)
+        vec![]
+    }
+
+    fn local_logout_commands(&self) -> Vec<String> {
+        vec![]
+    }
+
+    fn remote_logout_commands(&self) -> Vec<String> {
+        vec![]
+    }
+}
+
 #[cfg(test)]
 pub(crate) mod tests {
     use project::agent_server_store::AgentServerCommand;

crates/agent_ui/src/acp/thread_view.rs 🔗

@@ -260,6 +260,7 @@ impl ThreadFeedbackState {
 
 pub struct AcpThreadView {
     agent: Rc<dyn AgentServer>,
+    acp_agent: Option<Rc<dyn agent_servers::AcpAgentServer>>,
     workspace: WeakEntity<Workspace>,
     project: Entity<Project>,
     thread_state: ThreadState,
@@ -403,8 +404,38 @@ impl AcpThreadView {
         let show_codex_windows_warning = crate::ExternalAgent::parse_built_in(agent.as_ref())
             == Some(crate::ExternalAgent::Codex);
 
+        // Try to downcast to AcpAgentServer for ACP-specific functionality
+        let acp_agent = agent
+            .clone()
+            .into_any()
+            .downcast::<agent_servers::Gemini>()
+            .map(|a| a as Rc<dyn agent_servers::AcpAgentServer>)
+            .or_else(|_| {
+                agent
+                    .clone()
+                    .into_any()
+                    .downcast::<agent_servers::ClaudeCode>()
+                    .map(|a| a as Rc<dyn agent_servers::AcpAgentServer>)
+            })
+            .or_else(|_| {
+                agent
+                    .clone()
+                    .into_any()
+                    .downcast::<agent_servers::Codex>()
+                    .map(|a| a as Rc<dyn agent_servers::AcpAgentServer>)
+            })
+            .or_else(|_| {
+                agent
+                    .clone()
+                    .into_any()
+                    .downcast::<agent_servers::CustomAgentServer>()
+                    .map(|a| a as Rc<dyn agent_servers::AcpAgentServer>)
+            })
+            .ok();
+
         Self {
             agent: agent.clone(),
+            acp_agent,
             workspace: workspace.clone(),
             project: project.clone(),
             entry_view_state,
@@ -1053,18 +1084,23 @@ impl AcpThreadView {
         let text = self.message_editor.read(cx).text(cx);
         let text = text.trim();
 
-        // Check if this is a login or logout command
+        // Check if this is a login or logout command (only for ACP agents)
         let command_name = text.strip_prefix('/');
         let is_remote = self.project.read(cx).is_via_remote_server();
-        let login_commands = if is_remote {
-            self.agent.remote_login_commands()
-        } else {
-            self.agent.local_login_commands()
-        };
-        let logout_commands = if is_remote {
-            self.agent.remote_logout_commands()
+        let (login_commands, logout_commands) = if let Some(acp_agent) = &self.acp_agent {
+            let login = if is_remote {
+                acp_agent.remote_login_commands()
+            } else {
+                acp_agent.local_login_commands()
+            };
+            let logout = if is_remote {
+                acp_agent.remote_logout_commands()
+            } else {
+                acp_agent.local_logout_commands()
+            };
+            (login, logout)
         } else {
-            self.agent.local_logout_commands()
+            (vec![], vec![])
         };
         let is_login_command = if let Some(cmd) = command_name {
             login_commands.iter().any(|c| c == cmd)
@@ -1449,36 +1485,39 @@ impl AcpThreadView {
             AcpThreadEvent::AvailableCommandsUpdated(available_commands) => {
                 let mut available_commands = available_commands.clone();
 
-                let is_remote = self.project.read(cx).is_via_remote_server();
-                let login_commands = if is_remote {
-                    self.agent.remote_login_commands()
-                } else {
-                    self.agent.local_login_commands()
-                };
-                let logout_commands = if is_remote {
-                    self.agent.remote_logout_commands()
-                } else {
-                    self.agent.local_logout_commands()
-                };
+                // Add auth commands only for ACP agents
+                if let Some(acp_agent) = &self.acp_agent {
+                    let is_remote = self.project.read(cx).is_via_remote_server();
+                    let login_commands = if is_remote {
+                        acp_agent.remote_login_commands()
+                    } else {
+                        acp_agent.local_login_commands()
+                    };
+                    let logout_commands = if is_remote {
+                        acp_agent.remote_logout_commands()
+                    } else {
+                        acp_agent.local_logout_commands()
+                    };
 
-                // Add login commands from the agent
-                for command_name in login_commands {
-                    available_commands.push(acp::AvailableCommand {
-                        name: command_name,
-                        description: "Authenticate".to_owned(),
-                        input: None,
-                        meta: None,
-                    });
-                }
+                    // Add login commands from the agent
+                    for command_name in login_commands {
+                        available_commands.push(acp::AvailableCommand {
+                            name: command_name,
+                            description: "Authenticate".to_owned(),
+                            input: None,
+                            meta: None,
+                        });
+                    }
 
-                // Add logout commands from the agent
-                for command_name in logout_commands {
-                    available_commands.push(acp::AvailableCommand {
-                        name: command_name,
-                        description: "Authenticate".to_owned(),
-                        input: None,
-                        meta: None,
-                    });
+                    // Add logout commands from the agent
+                    for command_name in logout_commands {
+                        available_commands.push(acp::AvailableCommand {
+                            name: command_name,
+                            description: "Authenticate".to_owned(),
+                            input: None,
+                            meta: None,
+                        });
+                    }
                 }
 
                 self.available_commands.replace(available_commands);
@@ -6050,22 +6089,6 @@ pub(crate) mod tests {
             "Test".into()
         }
 
-        fn local_login_commands(&self) -> Vec<String> {
-            vec![]
-        }
-
-        fn remote_login_commands(&self) -> Vec<String> {
-            vec![]
-        }
-
-        fn local_logout_commands(&self) -> Vec<String> {
-            vec![]
-        }
-
-        fn remote_logout_commands(&self) -> Vec<String> {
-            vec![]
-        }
-
         fn connect(
             &self,
             _root_dir: Option<&Path>,