From 23f2fb6089b014910ce47c49c2ccd17a5070f728 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Wed, 5 Nov 2025 18:17:50 -0500 Subject: [PATCH] Run ACP login from same cwd as agent server (#42038) 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 --- 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(-) diff --git a/crates/agent_ui/src/acp/thread_view.rs b/crates/agent_ui/src/acp/thread_view.rs index 8ef7c7bbb0597e704a4b3b98b694601c27cacd5e..db43fe5f8a27d85d5b566900125d493accc82187 100644 --- a/crates/agent_ui/src/acp/thread_view.rs +++ b/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::() + .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, + project: Entity, previous_attempt: bool, + check_exit_code: bool, window: &mut Window, cx: &mut App, ) -> Task> { let Some(terminal_panel) = workspace.read(cx).panel::(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(()) }) } diff --git a/crates/extension/src/extension_manifest.rs b/crates/extension/src/extension_manifest.rs index ab620ec6dbfc023dfdab073e3d4b3aad9413597f..7e074ffcab77ceb2a63fd92448faa2e13f4ec8c4 100644 --- a/crates/extension/src/extension_manifest.rs +++ b/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, } diff --git a/crates/project/src/agent_server_store.rs b/crates/project/src/agent_server_store.rs index c710d96efa275290565a3c31dacf21cc7f7fb17c..2fc218cf0c1930b5e06e91f84486d563bec7c8fc 100644 --- a/crates/project/src/agent_server_store.rs +++ b/crates/project/src/agent_server_store.rs @@ -438,6 +438,13 @@ impl AgentServerStore { cx.emit(AgentServersUpdated); } + pub fn node_runtime(&self) -> Option { + match &self.state { + AgentServerStoreState::Local { node_runtime, .. } => Some(node_runtime.clone()), + _ => None, + } + } + pub fn local( node_runtime: NodeRuntime, fs: Arc, @@ -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 {