Run ACP login from same cwd as agent server (#42038)

Richard Feldman created

This makes it possible to do login via things like `cmd: "node", args:
["my-node-file.js", "login"]`

Also, that command will now use Zed's managed `node` instance.

Release Notes:

- ACP extensions can now run terminal login commands using relative
paths

Change summary

crates/agent_ui/src/acp/thread_view.rs     | 123 +++++++++++++++++------
crates/extension/src/extension_manifest.rs |   4 
crates/project/src/agent_server_store.rs   |  54 ++++++++++
3 files changed, 146 insertions(+), 35 deletions(-)

Detailed changes

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

@@ -1506,6 +1506,12 @@ impl AcpThreadView {
                             })
                             .unwrap_or_default();
 
+                        // Run SpawnInTerminal in the same dir as the ACP server
+                        let cwd = connection
+                            .clone()
+                            .downcast::<agent_servers::AcpConnection>()
+                            .map(|acp_conn| acp_conn.root_dir().to_path_buf());
+
                         // Build SpawnInTerminal from _meta
                         let login = task::SpawnInTerminal {
                             id: task::TaskId(format!("external-agent-{}-login", label)),
@@ -1514,6 +1520,7 @@ impl AcpThreadView {
                             command: Some(command.to_string()),
                             args,
                             command_label: label.to_string(),
+                            cwd,
                             env,
                             use_new_terminal: true,
                             allow_concurrent_runs: true,
@@ -1526,8 +1533,9 @@ impl AcpThreadView {
                         pending_auth_method.replace(method.clone());
 
                         if let Some(workspace) = self.workspace.upgrade() {
+                            let project = self.project.clone();
                             let authenticate = Self::spawn_external_agent_login(
-                                login, workspace, false, window, cx,
+                                login, workspace, project, false, true, window, cx,
                             );
                             cx.notify();
                             self.auth_task = Some(cx.spawn_in(window, {
@@ -1671,7 +1679,10 @@ impl AcpThreadView {
             && let Some(login) = self.login.clone()
         {
             if let Some(workspace) = self.workspace.upgrade() {
-                Self::spawn_external_agent_login(login, workspace, false, window, cx)
+                let project = self.project.clone();
+                Self::spawn_external_agent_login(
+                    login, workspace, project, false, false, window, cx,
+                )
             } else {
                 Task::ready(Ok(()))
             }
@@ -1721,17 +1732,40 @@ impl AcpThreadView {
     fn spawn_external_agent_login(
         login: task::SpawnInTerminal,
         workspace: Entity<Workspace>,
+        project: Entity<Project>,
         previous_attempt: bool,
+        check_exit_code: bool,
         window: &mut Window,
         cx: &mut App,
     ) -> Task<Result<()>> {
         let Some(terminal_panel) = workspace.read(cx).panel::<TerminalPanel>(cx) else {
             return Task::ready(Ok(()));
         };
-        let project = workspace.read(cx).project().clone();
 
         window.spawn(cx, async move |cx| {
             let mut task = login.clone();
+            if let Some(cmd) = &task.command {
+                // Have "node" command use Zed's managed Node runtime by default
+                if cmd == "node" {
+                    let resolved_node_runtime = project
+                        .update(cx, |project, cx| {
+                            let agent_server_store = project.agent_server_store().clone();
+                            agent_server_store.update(cx, |store, cx| {
+                                store.node_runtime().map(|node_runtime| {
+                                    cx.background_spawn(async move {
+                                        node_runtime.binary_path().await
+                                    })
+                                })
+                            })
+                        });
+
+                    if let Ok(Some(resolve_task)) = resolved_node_runtime {
+                        if let Ok(node_path) = resolve_task.await {
+                            task.command = Some(node_path.to_string_lossy().to_string());
+                        }
+                    }
+                }
+            }
             task.shell = task::Shell::WithArguments {
                 program: task.command.take().expect("login command should be set"),
                 args: std::mem::take(&mut task.args),
@@ -1749,44 +1783,65 @@ impl AcpThreadView {
             })?;
 
             let terminal = terminal.await?;
-            let mut exit_status = terminal
-                .read_with(cx, |terminal, cx| terminal.wait_for_completed_task(cx))?
-                .fuse();
-
-            let logged_in = cx
-                .spawn({
-                    let terminal = terminal.clone();
-                    async move |cx| {
-                        loop {
-                            cx.background_executor().timer(Duration::from_secs(1)).await;
-                            let content =
-                                terminal.update(cx, |terminal, _cx| terminal.get_content())?;
-                            if content.contains("Login successful")
-                                || content.contains("Type your message")
-                            {
-                                return anyhow::Ok(());
+
+            if check_exit_code {
+                // For extension-based auth, wait for the process to exit and check exit code
+                let exit_status = terminal
+                    .read_with(cx, |terminal, cx| terminal.wait_for_completed_task(cx))?
+                    .await;
+
+                match exit_status {
+                    Some(status) if status.success() => {
+                        Ok(())
+                    }
+                    Some(status) => {
+                        Err(anyhow!("Login command failed with exit code: {:?}", status.code()))
+                    }
+                    None => {
+                        Err(anyhow!("Login command terminated without exit status"))
+                    }
+                }
+            } else {
+                // For hardcoded agents (claude-login, gemini-cli): look for specific output
+                let mut exit_status = terminal
+                    .read_with(cx, |terminal, cx| terminal.wait_for_completed_task(cx))?
+                    .fuse();
+
+                let logged_in = cx
+                    .spawn({
+                        let terminal = terminal.clone();
+                        async move |cx| {
+                            loop {
+                                cx.background_executor().timer(Duration::from_secs(1)).await;
+                                let content =
+                                    terminal.update(cx, |terminal, _cx| terminal.get_content())?;
+                                if content.contains("Login successful")
+                                    || content.contains("Type your message")
+                                {
+                                    return anyhow::Ok(());
+                                }
                             }
                         }
+                    })
+                    .fuse();
+                futures::pin_mut!(logged_in);
+                futures::select_biased! {
+                    result = logged_in => {
+                        if let Err(e) = result {
+                            log::error!("{e}");
+                            return Err(anyhow!("exited before logging in"));
+                        }
                     }
-                })
-                .fuse();
-            futures::pin_mut!(logged_in);
-            futures::select_biased! {
-                result = logged_in => {
-                    if let Err(e) = result {
-                        log::error!("{e}");
+                    _ = exit_status => {
+                        if !previous_attempt && project.read_with(cx, |project, _| project.is_via_remote_server())? && login.label.contains("gemini") {
+                            return cx.update(|window, cx| Self::spawn_external_agent_login(login, workspace, project.clone(), true, false, window, cx))?.await
+                        }
                         return Err(anyhow!("exited before logging in"));
                     }
                 }
-                _ = exit_status => {
-                    if !previous_attempt && project.read_with(cx, |project, _| project.is_via_remote_server())? && login.label.contains("gemini") {
-                        return cx.update(|window, cx| Self::spawn_external_agent_login(login, workspace, true, window, cx))?.await
-                    }
-                    return Err(anyhow!("exited before logging in"));
-                }
+                terminal.update(cx, |terminal, _| terminal.kill_active_task())?;
+                Ok(())
             }
-            terminal.update(cx, |terminal, _| terminal.kill_active_task())?;
-            Ok(())
         })
     }
 

crates/extension/src/extension_manifest.rs 🔗

@@ -173,6 +173,10 @@ pub struct AgentServerManifestEntry {
     /// cmd = "node"
     /// args = ["index.js", "--port", "3000"]
     /// ```
+    ///
+    /// Note: All commands are executed with the archive extraction directory as the
+    /// working directory, so relative paths in args (like "index.js") will resolve
+    /// relative to the extracted archive contents.
     pub targets: HashMap<String, TargetConfig>,
 }
 

crates/project/src/agent_server_store.rs 🔗

@@ -438,6 +438,13 @@ impl AgentServerStore {
         cx.emit(AgentServersUpdated);
     }
 
+    pub fn node_runtime(&self) -> Option<NodeRuntime> {
+        match &self.state {
+            AgentServerStoreState::Local { node_runtime, .. } => Some(node_runtime.clone()),
+            _ => None,
+        }
+    }
+
     pub fn local(
         node_runtime: NodeRuntime,
         fs: Arc<dyn Fs>,
@@ -1560,7 +1567,7 @@ impl ExternalAgentServer for LocalExtensionArchiveAgent {
                 env: Some(env),
             };
 
-            Ok((command, root_dir.to_string_lossy().into_owned(), None))
+            Ok((command, version_dir.to_string_lossy().into_owned(), None))
         })
     }
 
@@ -1946,6 +1953,51 @@ mod extension_agent_tests {
         assert_eq!(target.args, vec!["index.js"]);
     }
 
+    #[gpui::test]
+    async fn test_commands_run_in_extraction_directory(cx: &mut TestAppContext) {
+        let fs = fs::FakeFs::new(cx.background_executor.clone());
+        let http_client = http_client::FakeHttpClient::with_404_response();
+        let node_runtime = NodeRuntime::unavailable();
+        let worktree_store = cx.new(|_| WorktreeStore::local(false, fs.clone()));
+        let project_environment = cx.new(|cx| {
+            crate::ProjectEnvironment::new(None, worktree_store.downgrade(), None, false, cx)
+        });
+
+        let agent = LocalExtensionArchiveAgent {
+            fs: fs.clone(),
+            http_client,
+            node_runtime,
+            project_environment,
+            extension_id: Arc::from("test-ext"),
+            agent_id: Arc::from("test-agent"),
+            targets: {
+                let mut map = HashMap::default();
+                map.insert(
+                    "darwin-aarch64".to_string(),
+                    extension::TargetConfig {
+                        archive: "https://example.com/test.zip".into(),
+                        cmd: "node".into(),
+                        args: vec![
+                            "server.js".into(),
+                            "--config".into(),
+                            "./config.json".into(),
+                        ],
+                        sha256: None,
+                    },
+                );
+                map
+            },
+            env: HashMap::default(),
+        };
+
+        // Verify the agent is configured with relative paths in args
+        let target = agent.targets.get("darwin-aarch64").unwrap();
+        assert_eq!(target.args[0], "server.js");
+        assert_eq!(target.args[2], "./config.json");
+        // These relative paths will resolve relative to the extraction directory
+        // when the command is executed
+    }
+
     #[test]
     fn test_tilde_expansion_in_settings() {
         let settings = settings::BuiltinAgentServerSettings {