acp: Improve handling of invalid external agent server downloads (#37465)

Cole Miller created 2 months ago

Related to #37213, #37150

When listing previously-downloaded versions of an external agent, don't
try to use any downloads that are missing the agent entrypoint
(indicating that they're corrupt/unusable), and delete those versions,
so that we can attempt to download the latest version again.

Also report clearer errors when failing to start a session due to an
agent server entrypoint or root directory not existing.

Release Notes:

- N/A

Change summary

crates/agent_servers/src/agent_servers.rs | 27 +++++++++++++++---------
crates/agent_servers/src/claude.rs        |  8 +++++++
crates/agent_servers/src/gemini.rs        | 12 +++++++++-
3 files changed, 35 insertions(+), 12 deletions(-)

Detailed changes

crates/agent_servers/src/agent_servers.rs

@@ -111,9 +111,11 @@ impl AgentServerDelegate {
                         continue;
                     };
 
-                    if let Some(version) = file_name
-                        .to_str()
-                        .and_then(|name| semver::Version::from_str(&name).ok())
+                    if let Some(name) = file_name.to_str()
+                        && let Some(version) = semver::Version::from_str(name).ok()
+                        && fs
+                            .is_file(&dir.join(file_name).join(&entrypoint_path))
+                            .await
                     {
                         versions.push((version, file_name.to_owned()));
                     } else {
@@ -156,6 +158,7 @@ impl AgentServerDelegate {
                     cx.background_spawn({
                         let file_name = file_name.clone();
                         let dir = dir.clone();
+                        let fs = fs.clone();
                         async move {
                             let latest_version =
                                 node_runtime.npm_package_latest_version(&package_name).await;
@@ -184,7 +187,7 @@ impl AgentServerDelegate {
                     }
                     let dir = dir.clone();
                     cx.background_spawn(Self::download_latest_version(
-                        fs,
+                        fs.clone(),
                         dir.clone(),
                         node_runtime,
                         package_name,
@@ -192,14 +195,18 @@ impl AgentServerDelegate {
                     .await?
                     .into()
                 };
+
+                let agent_server_path = dir.join(version).join(entrypoint_path);
+                let agent_server_path_exists = fs.is_file(&agent_server_path).await;
+                anyhow::ensure!(
+                    agent_server_path_exists,
+                    "Missing entrypoint path {} after installation",
+                    agent_server_path.to_string_lossy()
+                );
+
                 anyhow::Ok(AgentServerCommand {
                     path: node_path,
-                    args: vec![
-                        dir.join(version)
-                            .join(entrypoint_path)
-                            .to_string_lossy()
-                            .to_string(),
-                    ],
+                    args: vec![agent_server_path.to_string_lossy().to_string()],
                     env: Default::default(),
                 })
             })

crates/agent_servers/src/claude.rs

@@ -76,6 +76,7 @@ impl AgentServer for ClaudeCode {
         cx: &mut App,
     ) -> Task<Result<Rc<dyn AgentConnection>>> {
         let root_dir = root_dir.to_path_buf();
+        let fs = delegate.project().read(cx).fs().clone();
         let server_name = self.name();
         let settings = cx.read_global(|settings: &SettingsStore, _| {
             settings.get::<AllAgentServersSettings>(None).claude.clone()
@@ -109,6 +110,13 @@ impl AgentServer for ClaudeCode {
                     .insert("ANTHROPIC_API_KEY".to_owned(), api_key.key);
             }
 
+            let root_dir_exists = fs.is_dir(&root_dir).await;
+            anyhow::ensure!(
+                root_dir_exists,
+                "Session root {} does not exist or is not a directory",
+                root_dir.to_string_lossy()
+            );
+
             crate::acp::connect(server_name, command.clone(), &root_dir, cx).await
         })
     }

crates/agent_servers/src/gemini.rs

@@ -36,6 +36,7 @@ impl AgentServer for Gemini {
         cx: &mut App,
     ) -> Task<Result<Rc<dyn AgentConnection>>> {
         let root_dir = root_dir.to_path_buf();
+        let fs = delegate.project().read(cx).fs().clone();
         let server_name = self.name();
         let settings = cx.read_global(|settings: &SettingsStore, _| {
             settings.get::<AllAgentServersSettings>(None).gemini.clone()
@@ -74,6 +75,13 @@ impl AgentServer for Gemini {
                     .insert("GEMINI_API_KEY".to_owned(), api_key.key);
             }
 
+            let root_dir_exists = fs.is_dir(&root_dir).await;
+            anyhow::ensure!(
+                root_dir_exists,
+                "Session root {} does not exist or is not a directory",
+                root_dir.to_string_lossy()
+            );
+
             let result = crate::acp::connect(server_name, command.clone(), &root_dir, cx).await;
             match &result {
                 Ok(connection) => {
@@ -92,7 +100,7 @@ impl AgentServer for Gemini {
                         log::error!("connected to gemini, but missing prompt_capabilities.image (version is {current_version})");
                         return Err(LoadError::Unsupported {
                             current_version: current_version.into(),
-                            command: command.path.to_string_lossy().to_string().into(),
+                            command: (command.path.to_string_lossy().to_string() + " " + &command.args.join(" ")).into(),
                             minimum_version: Self::MINIMUM_VERSION.into(),
                         }
                         .into());
@@ -129,7 +137,7 @@ impl AgentServer for Gemini {
                     if !supported {
                         return Err(LoadError::Unsupported {
                             current_version: current_version.into(),
-                            command: command.path.to_string_lossy().to_string().into(),
+                            command: (command.path.to_string_lossy().to_string() + " " + &command.args.join(" ")).into(),
                             minimum_version: Self::MINIMUM_VERSION.into(),
                         }
                         .into());