From 92f739dbb9f2b46a1d825b39c0ea2c521dae0dbc Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Fri, 29 Aug 2025 13:40:39 -0400 Subject: [PATCH] acp: Improve error reporting and log more information when failing to launch gemini (#37178) 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 --- crates/agent_servers/src/agent_servers.rs | 11 ++++--- crates/agent_servers/src/gemini.rs | 35 ++++++++++++++--------- 2 files changed, 29 insertions(+), 17 deletions(-) diff --git a/crates/agent_servers/src/agent_servers.rs b/crates/agent_servers/src/agent_servers.rs index c1fc7b91ae..c610c53ea8 100644 --- a/crates/agent_servers/src/agent_servers.rs +++ b/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 { + log::debug!("downloading latest version of {package_name}"); + let tmp_dir = tempfile::tempdir_in(&dir)?; node_runtime diff --git a/crates/agent_servers/src/gemini.rs b/crates/agent_servers/src/gemini.rs index 5e958f6869..a1553d288a 100644 --- a/crates/agent_servers/src/gemini.rs +++ b/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 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 = std::str::from_utf8(&version_output?.stdout)? - .trim() - .to_string(); - let supported = String::from_utf8(help_output?.stdout)?.contains(ACP_ARG); + let current_version = version_output.trim().to_string(); + let supported = help_stdout.contains(ACP_ARG) || current_version.parse::().is_ok_and(|version| version >= Self::MINIMUM_VERSION.parse::().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(),