From eedfc5be5aad0a043aec03ebb922e7ee5e5fc9b3 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Wed, 3 Sep 2025 15:47:39 -0400 Subject: [PATCH] acp: Improve handling of invalid external agent server downloads (#37465) 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 --- 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(-) diff --git a/crates/agent_servers/src/agent_servers.rs b/crates/agent_servers/src/agent_servers.rs index 6ac81639ca4e6e94cab22fc7cb7ee5344c92983e..e214dabfc763c2a46f1f4665c3d1f881d5ce406e 100644 --- a/crates/agent_servers/src/agent_servers.rs +++ b/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(), }) }) diff --git a/crates/agent_servers/src/claude.rs b/crates/agent_servers/src/claude.rs index 0a4f152e8afd991fed90af12aa5bbff909c8aa2d..a02d8c37c1ebaa87acdbeee4ce384787758f12b7 100644 --- a/crates/agent_servers/src/claude.rs +++ b/crates/agent_servers/src/claude.rs @@ -76,6 +76,7 @@ impl AgentServer for ClaudeCode { cx: &mut App, ) -> Task>> { 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::(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 }) } diff --git a/crates/agent_servers/src/gemini.rs b/crates/agent_servers/src/gemini.rs index a1553d288ab44d96bdfe08723a092ce231ba005b..b58ad703cda496c4413f30decbfa5e0b1d1b0735 100644 --- a/crates/agent_servers/src/gemini.rs +++ b/crates/agent_servers/src/gemini.rs @@ -36,6 +36,7 @@ impl AgentServer for Gemini { cx: &mut App, ) -> Task>> { 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::(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());