acp: Fix spawning login task (#38567)

Cole Miller created 1 month ago

Reverts #38175, which is not correct, since in fact we do need to
pre-quote the command and arguments for the shell when using
`SpawnInTerminal` (although we should probably change the API so that
this isn't necessary). Then, applies the same fix as #38565 to fix the
root cause of being unable to spawn the login task on macOS, or in any
case where the command/args contain spaces.

Release Notes:

- Fixed being unable to login with Claude Code or Gemini using the
terminal.

Change summary

Cargo.lock                             |  1 
crates/agent_ui/Cargo.toml             |  1 
crates/agent_ui/src/acp/thread_view.rs | 34 ++++++++++++---------------
3 files changed, 17 insertions(+), 19 deletions(-)

Detailed changes

Cargo.lock

@@ -416,6 +416,7 @@ dependencies = [
  "serde_json",
  "serde_json_lenient",
  "settings",
+ "shlex",
  "smol",
  "streaming_diff",
  "task",

crates/agent_ui/Cargo.toml

@@ -80,6 +80,7 @@ serde.workspace = true
 serde_json.workspace = true
 serde_json_lenient.workspace = true
 settings.workspace = true
+shlex.workspace = true
 smol.workspace = true
 streaming_diff.workspace = true
 task.workspace = true

crates/agent_ui/src/acp/thread_view.rs

@@ -9,7 +9,7 @@ use agent_client_protocol::{self as acp, PromptCapabilities};
 use agent_servers::{AgentServer, AgentServerDelegate};
 use agent_settings::{AgentProfileId, AgentSettings, CompletionMode, NotifyWhenAgentWaiting};
 use agent2::{DbThreadMetadata, HistoryEntry, HistoryEntryId, HistoryStore, NativeAgentServer};
-use anyhow::{Result, anyhow, bail};
+use anyhow::{Context as _, Result, anyhow, bail};
 use arrayvec::ArrayVec;
 use audio::{Audio, Sound};
 use buffer_diff::BufferDiff;
@@ -1582,6 +1582,19 @@ impl AcpThreadView {
 
         window.spawn(cx, async move |cx| {
             let mut task = login.clone();
+            task.command = task
+                .command
+                .map(|command| anyhow::Ok(shlex::try_quote(&command)?.to_string()))
+                .transpose()?;
+            task.args = task
+                .args
+                .iter()
+                .map(|arg| {
+                    Ok(shlex::try_quote(arg)
+                        .context("Failed to quote argument")?
+                        .to_string())
+                })
+                .collect::<Result<Vec<_>>>()?;
             task.full_label = task.label.clone();
             task.id = task::TaskId(format!("external-agent-{}-login", task.label));
             task.command_label = task.label.clone();
@@ -1591,7 +1604,7 @@ impl AcpThreadView {
             task.shell = shell;
 
             let terminal = terminal_panel.update_in(cx, |terminal_panel, window, cx| {
-                terminal_panel.spawn_task(&login, window, cx)
+                terminal_panel.spawn_task(&task, window, cx)
             })?;
 
             let terminal = terminal.await?;
@@ -5669,23 +5682,6 @@ pub(crate) mod tests {
         });
     }
 
-    #[gpui::test]
-    async fn test_spawn_external_agent_login_handles_spaces(cx: &mut TestAppContext) {
-        init_test(cx);
-
-        // Verify paths with spaces aren't pre-quoted
-        let path_with_spaces = "/Users/test/Library/Application Support/Zed/cli.js";
-        let login_task = task::SpawnInTerminal {
-            command: Some("node".to_string()),
-            args: vec![path_with_spaces.to_string(), "/login".to_string()],
-            ..Default::default()
-        };
-
-        // Args should be passed as-is, not pre-quoted
-        assert!(!login_task.args[0].starts_with('"'));
-        assert!(!login_task.args[0].starts_with('\''));
-    }
-
     #[gpui::test]
     async fn test_notification_for_tool_authorization(cx: &mut TestAppContext) {
         init_test(cx);