acp: Improve error reporting and log more information when failing to launch gemini (#37178)

Cole Miller created 2 months ago

In the case where we fail to create an ACP connection to Gemini, only
report the "unsupported version" error if the version for the found
binary is at least our minimum version. That means we'll surface the
real error in this situation.

This also fixes incorrect sorting of downloaded Gemini versions--as @kpe
pointed out we were effectively using the version string as a key. Now
we'll correctly use the parsed semver::Version instead.

Release Notes:

- N/A

Change summary

crates/agent_servers/src/agent_servers.rs | 11 ++++--
crates/agent_servers/src/gemini.rs        | 39 +++++++++++++++---------
2 files changed, 31 insertions(+), 19 deletions(-)

Detailed changes

crates/agent_servers/src/agent_servers.rs

@@ -105,22 +105,23 @@ impl AgentServerDelegate {
                         .to_str()
                         .and_then(|name| semver::Version::from_str(&name).ok())
                     {
-                        versions.push((file_name.to_owned(), version));
+                        versions.push((version, file_name.to_owned()));
                     } else {
                         to_delete.push(file_name.to_owned())
                     }
                 }
 
                 versions.sort();
-                let newest_version = if let Some((file_name, version)) = versions.last().cloned()
-                    && minimum_version.is_none_or(|minimum_version| version > minimum_version)
+                let newest_version = if let Some((version, file_name)) = versions.last().cloned()
+                    && minimum_version.is_none_or(|minimum_version| version >= minimum_version)
                 {
                     versions.pop();
                     Some(file_name)
                 } else {
                     None
                 };
-                to_delete.extend(versions.into_iter().map(|(file_name, _)| file_name));
+                log::debug!("existing version of {package_name}: {newest_version:?}");
+                to_delete.extend(versions.into_iter().map(|(_, file_name)| file_name));
 
                 cx.background_spawn({
                     let fs = fs.clone();
@@ -200,6 +201,8 @@ impl AgentServerDelegate {
         node_runtime: NodeRuntime,
         package_name: SharedString,
     ) -> Result<String> {
+        log::debug!("downloading latest version of {package_name}");
+
         let tmp_dir = tempfile::tempdir_in(&dir)?;
 
         node_runtime

crates/agent_servers/src/gemini.rs

@@ -63,7 +63,9 @@ impl AgentServer for Gemini {
                 })?
                 .await?
             };
-            command.args.push("--experimental-acp".into());
+            if !command.args.contains(&ACP_ARG.into()) {
+                command.args.push(ACP_ARG.into());
+            }
 
             if let Some(api_key) = cx.update(GoogleLanguageModelProvider::api_key)?.await.ok() {
                 command
@@ -86,17 +88,17 @@ impl AgentServer for Gemini {
                             .await;
                         let current_version =
                             String::from_utf8(version_output?.stdout)?.trim().to_owned();
-                        if !connection.prompt_capabilities().image {
-                            return Err(LoadError::Unsupported {
-                                current_version: current_version.into(),
-                                command: command.path.to_string_lossy().to_string().into(),
-                                minimum_version: Self::MINIMUM_VERSION.into(),
-                            }
-                            .into());
+
+                        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(),
+                            minimum_version: Self::MINIMUM_VERSION.into(),
                         }
+                        .into());
                     }
                 }
-                Err(_) => {
+                Err(e) => {
                     let version_fut = util::command::new_smol_command(&command.path)
                         .args(command.args.iter())
                         .arg("--version")
@@ -111,12 +113,19 @@ impl AgentServer for Gemini {
 
                     let (version_output, help_output) =
                         futures::future::join(version_fut, help_fut).await;
-
-                    let current_version = std::str::from_utf8(&version_output?.stdout)?
-                        .trim()
-                        .to_string();
-                    let supported = String::from_utf8(help_output?.stdout)?.contains(ACP_ARG);
-
+                    let Some(version_output) = version_output.ok().and_then(|output| String::from_utf8(output.stdout).ok()) else {
+                        return result;
+                    };
+                    let Some((help_stdout, help_stderr)) = help_output.ok().and_then(|output| String::from_utf8(output.stdout).ok().zip(String::from_utf8(output.stderr).ok())) else  {
+                        return result;
+                    };
+
+                    let current_version = version_output.trim().to_string();
+                    let supported = help_stdout.contains(ACP_ARG) || current_version.parse::<semver::Version>().is_ok_and(|version| version >= Self::MINIMUM_VERSION.parse::<semver::Version>().unwrap());
+
+                    log::error!("failed to create ACP connection to gemini (version is {current_version}, supported: {supported}): {e}");
+                    log::debug!("gemini --help stdout: {help_stdout:?}");
+                    log::debug!("gemini --help stderr: {help_stderr:?}");
                     if !supported {
                         return Err(LoadError::Unsupported {
                             current_version: current_version.into(),