From c3d0c542db5542477678c09e41954825c19865f6 Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Tue, 5 May 2026 15:21:37 +0200 Subject: [PATCH 1/2] Implement fallback retry --- crates/agent_servers/src/acp.rs | 149 +++++++++++++++++++++-- crates/agent_servers/src/custom.rs | 17 ++- crates/project/src/agent_server_store.rs | 144 ++++++++++++++++++---- crates/proto/proto/ai.proto | 1 + 4 files changed, 264 insertions(+), 47 deletions(-) diff --git a/crates/agent_servers/src/acp.rs b/crates/agent_servers/src/acp.rs index 93efddb03d81db..f4ff3d91995320 100644 --- a/crates/agent_servers/src/acp.rs +++ b/crates/agent_servers/src/acp.rs @@ -225,6 +225,26 @@ fn exited_load_error_with_stderr(status: ExitStatus, debug_log: &AcpDebugLog) -> } } +fn should_retry_with_fallback_command(error: &anyhow::Error) -> bool { + let Some(LoadError::Exited { + stderr: Some(stderr), + .. + }) = error.downcast_ref::() + else { + return false; + }; + + is_npm_release_age_resolution_failure(stderr) +} + +fn is_npm_release_age_resolution_failure(stderr: &str) -> bool { + let stderr = stderr.to_lowercase(); + stderr.contains("min-release-age") + || stderr.contains("minimum release age") + || stderr.contains("with a date before") + || (stderr.contains("no matching version found") && stderr.contains("before")) +} + /// Awaits the response to an ACP request from a GPUI foreground task. /// /// The ACP SDK offers two ways to consume a [`SentRequest`]: @@ -551,28 +571,122 @@ impl AgentSessionList for AcpSessionList { } } +async fn agent_server_command( + agent_server_store: WeakEntity, + agent_id: AgentId, + fallback: bool, + extra_args: Vec, + extra_env: HashMap, + cx: &mut AsyncApp, +) -> Result> { + let command = agent_server_store.update(cx, |store, cx| { + let agent = store + .get_external_agent(&agent_id) + .with_context(|| format!("agent server `{agent_id}` is not registered"))?; + if fallback { + anyhow::Ok(agent.get_fallback_command(extra_args, extra_env, &mut cx.to_async())) + } else { + anyhow::Ok(Some(agent.get_command( + extra_args, + extra_env, + &mut cx.to_async(), + ))) + } + })??; + + match command { + Some(command) => Ok(Some(command.await?)), + None => Ok(None), + } +} + pub async fn connect( agent_id: AgentId, project: Entity, - command: AgentServerCommand, agent_server_store: WeakEntity, + extra_args: Vec, + extra_env: HashMap, default_mode: Option, default_model: Option, default_config_options: HashMap, cx: &mut AsyncApp, ) -> Result> { - let conn = AcpConnection::stdio( - agent_id, - project, - command.clone(), - agent_server_store, - default_mode, - default_model, - default_config_options, + let command = agent_server_command( + agent_server_store.clone(), + agent_id.clone(), + false, + extra_args.clone(), + extra_env.clone(), + cx, + ) + .await? + .context("agent server did not provide a command")?; + + let primary_command = command.clone(); + let primary_error = match AcpConnection::stdio( + agent_id.clone(), + project.clone(), + command, + agent_server_store.clone(), + default_mode.clone(), + default_model.clone(), + default_config_options.clone(), cx, ) - .await?; - Ok(Rc::new(conn) as _) + .await + { + Ok(conn) => return Ok(Rc::new(conn) as _), + Err(error) => error, + }; + + if should_retry_with_fallback_command(&primary_error) { + let fallback_command = match agent_server_command( + agent_server_store.clone(), + agent_id.clone(), + true, + extra_args, + extra_env, + cx, + ) + .await + { + Ok(Some(command)) => command, + Ok(None) => return Err(primary_error), + Err(error) => { + log::warn!( + "Failed to generate fallback command for agent {}: {error:#}", + agent_id + ); + return Err(primary_error); + } + }; + + if fallback_command == primary_command { + return Err(primary_error); + } + + log::warn!( + "Retrying agent server `{}` with fallback command after startup failed: {primary_error:#}", + agent_id + ); + let conn = AcpConnection::stdio( + agent_id, + project, + fallback_command, + agent_server_store, + default_mode, + default_model, + default_config_options, + cx, + ) + .await + .with_context(|| { + format!("fallback agent command failed after primary command failed: {primary_error}") + })?; + return Ok(Rc::new(conn) as _); + } + + Err(primary_error) } const MINIMUM_SUPPORTED_VERSION: acp::ProtocolVersion = acp::ProtocolVersion::V1; @@ -2308,6 +2422,19 @@ mod tests { use super::*; + #[test] + fn detects_npm_release_age_resolution_failures() { + assert!(is_npm_release_age_resolution_failure( + "npm error notarget No matching version found for @agentclientprotocol/claude-agent-acp@0.32.0 with a date before 4/28/2026, 12:11:38 PM." + )); + assert!(is_npm_release_age_resolution_failure( + "npm error min-release-age prevented selecting a matching version" + )); + assert!(!is_npm_release_age_resolution_failure( + "npm error notarget No matching version found for missing-package@1.2.3." + )); + } + #[test] fn terminal_auth_task_builds_spawn_from_prebuilt_command() { let command = AgentServerCommand { diff --git a/crates/agent_servers/src/custom.rs b/crates/agent_servers/src/custom.rs index b3574f6e81a5a1..8dfbecee775501 100644 --- a/crates/agent_servers/src/custom.rs +++ b/crates/agent_servers/src/custom.rs @@ -355,22 +355,21 @@ impl AgentServer for CustomAgentServer { extra_env.insert("GEMINI_API_KEY".into(), api_key); } } - let command = store - .update(cx, |store, cx| { + if let Some(new_version_available_tx) = delegate.new_version_available { + store.update(cx, |store, _| { let agent = store.get_external_agent(&agent_id).with_context(|| { format!("Custom agent server `{}` is not registered", agent_id) })?; - if let Some(new_version_available_tx) = delegate.new_version_available { - agent.set_new_version_available_tx(new_version_available_tx); - } - anyhow::Ok(agent.get_command(vec![], extra_env, &mut cx.to_async())) - })?? - .await?; + agent.set_new_version_available_tx(new_version_available_tx); + anyhow::Ok(()) + })??; + } let connection = crate::acp::connect( agent_id, project, - command, store.clone(), + vec![], + extra_env, default_mode, default_model, default_config_options, diff --git a/crates/project/src/agent_server_store.rs b/crates/project/src/agent_server_store.rs index 103a44197aefd0..68d7ac701639b5 100644 --- a/crates/project/src/agent_server_store.rs +++ b/crates/project/src/agent_server_store.rs @@ -18,6 +18,7 @@ use rpc::{ proto::{self, ExternalExtensionAgent}, }; use schemars::JsonSchema; +use semver::Version; use serde::{Deserialize, Serialize}; use settings::{RegisterSetting, SettingsStore}; use sha2::{Digest, Sha256}; @@ -121,6 +122,15 @@ pub trait ExternalAgentServer { cx: &mut AsyncApp, ) -> Task>; + fn get_fallback_command( + &self, + _extra_args: Vec, + _extra_env: HashMap, + _cx: &mut AsyncApp, + ) -> Option>> { + None + } + fn version(&self) -> Option<&SharedString> { None } @@ -804,7 +814,15 @@ impl AgentServerStore { if let Some(new_version_available_tx) = new_version_available_tx { agent.set_new_version_available_tx(new_version_available_tx); } - anyhow::Ok(agent.get_command(vec![], extra_env, &mut cx.to_async())) + if envelope.payload.fallback.unwrap_or(false) { + agent + .get_fallback_command(vec![], extra_env, &mut cx.to_async()) + .with_context(|| { + format!("agent `{}` has no fallback command", envelope.payload.name) + }) + } else { + Ok(agent.get_command(vec![], extra_env, &mut cx.to_async())) + } })? .await?; Ok(proto::AgentServerCommand { @@ -976,17 +994,10 @@ struct RemoteExternalAgentServer { new_version_available_tx: Option>>, } -impl ExternalAgentServer for RemoteExternalAgentServer { - fn take_new_version_available_tx(&mut self) -> Option>> { - self.new_version_available_tx.take() - } - - fn set_new_version_available_tx(&mut self, tx: watch::Sender>) { - self.new_version_available_tx = Some(tx); - } - - fn get_command( +impl RemoteExternalAgentServer { + fn command( &self, + fallback: bool, extra_args: Vec, extra_env: HashMap, cx: &mut AsyncApp, @@ -1011,6 +1022,7 @@ impl ExternalAgentServer for RemoteExternalAgentServer { project_id, name, root_dir, + fallback: Some(fallback), }) })? .await?; @@ -1024,6 +1036,34 @@ impl ExternalAgentServer for RemoteExternalAgentServer { }) }) } +} + +impl ExternalAgentServer for RemoteExternalAgentServer { + fn take_new_version_available_tx(&mut self) -> Option>> { + self.new_version_available_tx.take() + } + + fn set_new_version_available_tx(&mut self, tx: watch::Sender>) { + self.new_version_available_tx = Some(tx); + } + + fn get_command( + &self, + extra_args: Vec, + extra_env: HashMap, + cx: &mut AsyncApp, + ) -> Task> { + self.command(false, extra_args, extra_env, cx) + } + + fn get_fallback_command( + &self, + extra_args: Vec, + extra_env: HashMap, + cx: &mut AsyncApp, + ) -> Option>> { + Some(self.command(true, extra_args, extra_env, cx)) + } fn as_any(&self) -> &dyn Any { self @@ -1102,6 +1142,15 @@ fn sanitize_path_component(input: &str) -> String { } } +fn npm_package_spec_with_version_ceiling(package_spec: &str) -> Option { + let (package_name, version) = package_spec.rsplit_once('@')?; + if package_name.is_empty() || Version::parse(version).is_err() { + return None; + } + + Some(format!("{package_name}@<={version}")) +} + fn versioned_archive_cache_dir( base_dir: &Path, version: Option<&str>, @@ -1512,21 +1561,10 @@ struct LocalRegistryNpxAgent { new_version_available_tx: Option>>, } -impl ExternalAgentServer for LocalRegistryNpxAgent { - fn version(&self) -> Option<&SharedString> { - Some(&self.version) - } - - fn take_new_version_available_tx(&mut self) -> Option>> { - self.new_version_available_tx.take() - } - - fn set_new_version_available_tx(&mut self, tx: watch::Sender>) { - self.new_version_available_tx = Some(tx); - } - - fn get_command( +impl LocalRegistryNpxAgent { + fn command_for_package( &self, + package: String, extra_args: Vec, extra_env: HashMap, cx: &mut AsyncApp, @@ -1535,7 +1573,6 @@ impl ExternalAgentServer for LocalRegistryNpxAgent { let node_runtime = self.node_runtime.clone(); let project_environment = self.project_environment.downgrade(); let registry_id = self.registry_id.clone(); - let package = self.package.clone(); let args = self.args.clone(); let distribution_env = self.distribution_env.clone(); let settings_env = self.settings_env.clone(); @@ -1554,7 +1591,7 @@ impl ExternalAgentServer for LocalRegistryNpxAgent { .join(sanitize_path_component(®istry_id)); fs.create_dir(&prefix_dir).await?; - let mut exec_args = vec!["--yes".to_string(), "--".to_string(), package.to_string()]; + let mut exec_args = vec!["--yes".to_string(), "--".to_string(), package]; exec_args.extend(args); let npm_command = node_runtime @@ -1582,6 +1619,39 @@ impl ExternalAgentServer for LocalRegistryNpxAgent { Ok(command) }) } +} + +impl ExternalAgentServer for LocalRegistryNpxAgent { + fn version(&self) -> Option<&SharedString> { + Some(&self.version) + } + + fn take_new_version_available_tx(&mut self) -> Option>> { + self.new_version_available_tx.take() + } + + fn set_new_version_available_tx(&mut self, tx: watch::Sender>) { + self.new_version_available_tx = Some(tx); + } + + fn get_command( + &self, + extra_args: Vec, + extra_env: HashMap, + cx: &mut AsyncApp, + ) -> Task> { + self.command_for_package(self.package.to_string(), extra_args, extra_env, cx) + } + + fn get_fallback_command( + &self, + extra_args: Vec, + extra_env: HashMap, + cx: &mut AsyncApp, + ) -> Option>> { + let fallback_package = npm_package_spec_with_version_ceiling(&self.package)?; + Some(self.command_for_package(fallback_package, extra_args, extra_env, cx)) + } fn as_any(&self) -> &dyn Any { self @@ -1996,6 +2066,26 @@ mod tests { }) } + #[test] + fn builds_bounded_npm_package_fallback_specs() { + assert_eq!( + npm_package_spec_with_version_ceiling("agent-package@1.2.3").as_deref(), + Some("agent-package@<=1.2.3") + ); + assert_eq!( + npm_package_spec_with_version_ceiling("@scope/agent-package@1.2.3-beta.1").as_deref(), + Some("@scope/agent-package@<=1.2.3-beta.1") + ); + assert_eq!( + npm_package_spec_with_version_ceiling("@scope/agent-package"), + None + ); + assert_eq!( + npm_package_spec_with_version_ceiling("agent-package@latest"), + None + ); + } + #[test] fn detects_supported_archive_suffixes() { assert!(matches!( diff --git a/crates/proto/proto/ai.proto b/crates/proto/proto/ai.proto index 20c87d830a68c3..1ca269249ff18b 100644 --- a/crates/proto/proto/ai.proto +++ b/crates/proto/proto/ai.proto @@ -7,6 +7,7 @@ message GetAgentServerCommand { uint64 project_id = 1; string name = 2; optional string root_dir = 3; + optional bool fallback = 4; } message GetContextServerCommand { From 943bfa74d8ce5d886680dafa27cbd7a7d1937861 Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Tue, 5 May 2026 15:39:13 +0200 Subject: [PATCH 2/2] Bound npm package versions in agent commands Replace fallback command retries with bounded npm package specs when building local registry agent commands, and remove the unused fallback command path from ACP and proto APIs. --- crates/agent_servers/src/acp.rs | 149 ++------------------- crates/agent_servers/src/custom.rs | 17 +-- crates/project/src/agent_server_store.rs | 163 ++++++++--------------- crates/proto/proto/ai.proto | 1 - 4 files changed, 79 insertions(+), 251 deletions(-) diff --git a/crates/agent_servers/src/acp.rs b/crates/agent_servers/src/acp.rs index f4ff3d91995320..93efddb03d81db 100644 --- a/crates/agent_servers/src/acp.rs +++ b/crates/agent_servers/src/acp.rs @@ -225,26 +225,6 @@ fn exited_load_error_with_stderr(status: ExitStatus, debug_log: &AcpDebugLog) -> } } -fn should_retry_with_fallback_command(error: &anyhow::Error) -> bool { - let Some(LoadError::Exited { - stderr: Some(stderr), - .. - }) = error.downcast_ref::() - else { - return false; - }; - - is_npm_release_age_resolution_failure(stderr) -} - -fn is_npm_release_age_resolution_failure(stderr: &str) -> bool { - let stderr = stderr.to_lowercase(); - stderr.contains("min-release-age") - || stderr.contains("minimum release age") - || stderr.contains("with a date before") - || (stderr.contains("no matching version found") && stderr.contains("before")) -} - /// Awaits the response to an ACP request from a GPUI foreground task. /// /// The ACP SDK offers two ways to consume a [`SentRequest`]: @@ -571,122 +551,28 @@ impl AgentSessionList for AcpSessionList { } } -async fn agent_server_command( - agent_server_store: WeakEntity, - agent_id: AgentId, - fallback: bool, - extra_args: Vec, - extra_env: HashMap, - cx: &mut AsyncApp, -) -> Result> { - let command = agent_server_store.update(cx, |store, cx| { - let agent = store - .get_external_agent(&agent_id) - .with_context(|| format!("agent server `{agent_id}` is not registered"))?; - if fallback { - anyhow::Ok(agent.get_fallback_command(extra_args, extra_env, &mut cx.to_async())) - } else { - anyhow::Ok(Some(agent.get_command( - extra_args, - extra_env, - &mut cx.to_async(), - ))) - } - })??; - - match command { - Some(command) => Ok(Some(command.await?)), - None => Ok(None), - } -} - pub async fn connect( agent_id: AgentId, project: Entity, + command: AgentServerCommand, agent_server_store: WeakEntity, - extra_args: Vec, - extra_env: HashMap, default_mode: Option, default_model: Option, default_config_options: HashMap, cx: &mut AsyncApp, ) -> Result> { - let command = agent_server_command( - agent_server_store.clone(), - agent_id.clone(), - false, - extra_args.clone(), - extra_env.clone(), - cx, - ) - .await? - .context("agent server did not provide a command")?; - - let primary_command = command.clone(); - let primary_error = match AcpConnection::stdio( - agent_id.clone(), - project.clone(), - command, - agent_server_store.clone(), - default_mode.clone(), - default_model.clone(), - default_config_options.clone(), + let conn = AcpConnection::stdio( + agent_id, + project, + command.clone(), + agent_server_store, + default_mode, + default_model, + default_config_options, cx, ) - .await - { - Ok(conn) => return Ok(Rc::new(conn) as _), - Err(error) => error, - }; - - if should_retry_with_fallback_command(&primary_error) { - let fallback_command = match agent_server_command( - agent_server_store.clone(), - agent_id.clone(), - true, - extra_args, - extra_env, - cx, - ) - .await - { - Ok(Some(command)) => command, - Ok(None) => return Err(primary_error), - Err(error) => { - log::warn!( - "Failed to generate fallback command for agent {}: {error:#}", - agent_id - ); - return Err(primary_error); - } - }; - - if fallback_command == primary_command { - return Err(primary_error); - } - - log::warn!( - "Retrying agent server `{}` with fallback command after startup failed: {primary_error:#}", - agent_id - ); - let conn = AcpConnection::stdio( - agent_id, - project, - fallback_command, - agent_server_store, - default_mode, - default_model, - default_config_options, - cx, - ) - .await - .with_context(|| { - format!("fallback agent command failed after primary command failed: {primary_error}") - })?; - return Ok(Rc::new(conn) as _); - } - - Err(primary_error) + .await?; + Ok(Rc::new(conn) as _) } const MINIMUM_SUPPORTED_VERSION: acp::ProtocolVersion = acp::ProtocolVersion::V1; @@ -2422,19 +2308,6 @@ mod tests { use super::*; - #[test] - fn detects_npm_release_age_resolution_failures() { - assert!(is_npm_release_age_resolution_failure( - "npm error notarget No matching version found for @agentclientprotocol/claude-agent-acp@0.32.0 with a date before 4/28/2026, 12:11:38 PM." - )); - assert!(is_npm_release_age_resolution_failure( - "npm error min-release-age prevented selecting a matching version" - )); - assert!(!is_npm_release_age_resolution_failure( - "npm error notarget No matching version found for missing-package@1.2.3." - )); - } - #[test] fn terminal_auth_task_builds_spawn_from_prebuilt_command() { let command = AgentServerCommand { diff --git a/crates/agent_servers/src/custom.rs b/crates/agent_servers/src/custom.rs index 8dfbecee775501..b3574f6e81a5a1 100644 --- a/crates/agent_servers/src/custom.rs +++ b/crates/agent_servers/src/custom.rs @@ -355,21 +355,22 @@ impl AgentServer for CustomAgentServer { extra_env.insert("GEMINI_API_KEY".into(), api_key); } } - if let Some(new_version_available_tx) = delegate.new_version_available { - store.update(cx, |store, _| { + let command = store + .update(cx, |store, cx| { let agent = store.get_external_agent(&agent_id).with_context(|| { format!("Custom agent server `{}` is not registered", agent_id) })?; - agent.set_new_version_available_tx(new_version_available_tx); - anyhow::Ok(()) - })??; - } + if let Some(new_version_available_tx) = delegate.new_version_available { + agent.set_new_version_available_tx(new_version_available_tx); + } + anyhow::Ok(agent.get_command(vec![], extra_env, &mut cx.to_async())) + })?? + .await?; let connection = crate::acp::connect( agent_id, project, + command, store.clone(), - vec![], - extra_env, default_mode, default_model, default_config_options, diff --git a/crates/project/src/agent_server_store.rs b/crates/project/src/agent_server_store.rs index 68d7ac701639b5..cdde687ec63233 100644 --- a/crates/project/src/agent_server_store.rs +++ b/crates/project/src/agent_server_store.rs @@ -122,15 +122,6 @@ pub trait ExternalAgentServer { cx: &mut AsyncApp, ) -> Task>; - fn get_fallback_command( - &self, - _extra_args: Vec, - _extra_env: HashMap, - _cx: &mut AsyncApp, - ) -> Option>> { - None - } - fn version(&self) -> Option<&SharedString> { None } @@ -814,15 +805,7 @@ impl AgentServerStore { if let Some(new_version_available_tx) = new_version_available_tx { agent.set_new_version_available_tx(new_version_available_tx); } - if envelope.payload.fallback.unwrap_or(false) { - agent - .get_fallback_command(vec![], extra_env, &mut cx.to_async()) - .with_context(|| { - format!("agent `{}` has no fallback command", envelope.payload.name) - }) - } else { - Ok(agent.get_command(vec![], extra_env, &mut cx.to_async())) - } + anyhow::Ok(agent.get_command(vec![], extra_env, &mut cx.to_async())) })? .await?; Ok(proto::AgentServerCommand { @@ -994,10 +977,17 @@ struct RemoteExternalAgentServer { new_version_available_tx: Option>>, } -impl RemoteExternalAgentServer { - fn command( +impl ExternalAgentServer for RemoteExternalAgentServer { + fn take_new_version_available_tx(&mut self) -> Option>> { + self.new_version_available_tx.take() + } + + fn set_new_version_available_tx(&mut self, tx: watch::Sender>) { + self.new_version_available_tx = Some(tx); + } + + fn get_command( &self, - fallback: bool, extra_args: Vec, extra_env: HashMap, cx: &mut AsyncApp, @@ -1022,7 +1012,6 @@ impl RemoteExternalAgentServer { project_id, name, root_dir, - fallback: Some(fallback), }) })? .await?; @@ -1036,34 +1025,6 @@ impl RemoteExternalAgentServer { }) }) } -} - -impl ExternalAgentServer for RemoteExternalAgentServer { - fn take_new_version_available_tx(&mut self) -> Option>> { - self.new_version_available_tx.take() - } - - fn set_new_version_available_tx(&mut self, tx: watch::Sender>) { - self.new_version_available_tx = Some(tx); - } - - fn get_command( - &self, - extra_args: Vec, - extra_env: HashMap, - cx: &mut AsyncApp, - ) -> Task> { - self.command(false, extra_args, extra_env, cx) - } - - fn get_fallback_command( - &self, - extra_args: Vec, - extra_env: HashMap, - cx: &mut AsyncApp, - ) -> Option>> { - Some(self.command(true, extra_args, extra_env, cx)) - } fn as_any(&self) -> &dyn Any { self @@ -1142,15 +1103,6 @@ fn sanitize_path_component(input: &str) -> String { } } -fn npm_package_spec_with_version_ceiling(package_spec: &str) -> Option { - let (package_name, version) = package_spec.rsplit_once('@')?; - if package_name.is_empty() || Version::parse(version).is_err() { - return None; - } - - Some(format!("{package_name}@<={version}")) -} - fn versioned_archive_cache_dir( base_dir: &Path, version: Option<&str>, @@ -1561,10 +1513,21 @@ struct LocalRegistryNpxAgent { new_version_available_tx: Option>>, } -impl LocalRegistryNpxAgent { - fn command_for_package( +impl ExternalAgentServer for LocalRegistryNpxAgent { + fn version(&self) -> Option<&SharedString> { + Some(&self.version) + } + + fn take_new_version_available_tx(&mut self) -> Option>> { + self.new_version_available_tx.take() + } + + fn set_new_version_available_tx(&mut self, tx: watch::Sender>) { + self.new_version_available_tx = Some(tx); + } + + fn get_command( &self, - package: String, extra_args: Vec, extra_env: HashMap, cx: &mut AsyncApp, @@ -1573,6 +1536,7 @@ impl LocalRegistryNpxAgent { let node_runtime = self.node_runtime.clone(); let project_environment = self.project_environment.downgrade(); let registry_id = self.registry_id.clone(); + let package = bounded_npm_package_spec(&self.package); let args = self.args.clone(); let distribution_env = self.distribution_env.clone(); let settings_env = self.settings_env.clone(); @@ -1619,39 +1583,6 @@ impl LocalRegistryNpxAgent { Ok(command) }) } -} - -impl ExternalAgentServer for LocalRegistryNpxAgent { - fn version(&self) -> Option<&SharedString> { - Some(&self.version) - } - - fn take_new_version_available_tx(&mut self) -> Option>> { - self.new_version_available_tx.take() - } - - fn set_new_version_available_tx(&mut self, tx: watch::Sender>) { - self.new_version_available_tx = Some(tx); - } - - fn get_command( - &self, - extra_args: Vec, - extra_env: HashMap, - cx: &mut AsyncApp, - ) -> Task> { - self.command_for_package(self.package.to_string(), extra_args, extra_env, cx) - } - - fn get_fallback_command( - &self, - extra_args: Vec, - extra_env: HashMap, - cx: &mut AsyncApp, - ) -> Option>> { - let fallback_package = npm_package_spec_with_version_ceiling(&self.package)?; - Some(self.command_for_package(fallback_package, extra_args, extra_env, cx)) - } fn as_any(&self) -> &dyn Any { self @@ -1662,6 +1593,30 @@ impl ExternalAgentServer for LocalRegistryNpxAgent { } } +/// People are using min-release-age more frequently. Which means a fresh registry will likely have +/// new package versions than the user can install. +/// We set the version to now be a ceiling and not an exact pin instead. This allows npm to resolve +/// the latest version it can find that satisfies the constraint. npm seems to check regularly enough +/// that new versions are available. This does have a few downsides: +/// - The user might have an older cached version of the package that satisfies the constraint, until +/// npm checks for updates again. +/// - The registry args/env may not be valid for the resolved version. +/// +/// This is a best-effort attempt to install a version that works without overriding the user's +/// security settings, as the args don't change often. The registry will need to support this better +/// at some point, but until then, this is a best-effort workaround that hopefully solves the issue +/// for most users. +fn bounded_npm_package_spec(package_spec: &str) -> String { + let Some((package_name, version)) = package_spec.rsplit_once('@') else { + return package_spec.to_string(); + }; + if package_name.is_empty() || Version::parse(version).is_err() { + return package_spec.to_string(); + } + + format!("{package_name}@<={version}") +} + struct LocalCustomAgent { project_environment: Entity, command: AgentServerCommand, @@ -2067,22 +2022,22 @@ mod tests { } #[test] - fn builds_bounded_npm_package_fallback_specs() { + fn builds_bounded_npm_package_specs() { assert_eq!( - npm_package_spec_with_version_ceiling("agent-package@1.2.3").as_deref(), - Some("agent-package@<=1.2.3") + bounded_npm_package_spec("agent-package@1.2.3"), + "agent-package@<=1.2.3" ); assert_eq!( - npm_package_spec_with_version_ceiling("@scope/agent-package@1.2.3-beta.1").as_deref(), - Some("@scope/agent-package@<=1.2.3-beta.1") + bounded_npm_package_spec("@scope/agent-package@1.2.3-beta.1"), + "@scope/agent-package@<=1.2.3-beta.1" ); assert_eq!( - npm_package_spec_with_version_ceiling("@scope/agent-package"), - None + bounded_npm_package_spec("@scope/agent-package"), + "@scope/agent-package" ); assert_eq!( - npm_package_spec_with_version_ceiling("agent-package@latest"), - None + bounded_npm_package_spec("agent-package@latest"), + "agent-package@latest" ); } diff --git a/crates/proto/proto/ai.proto b/crates/proto/proto/ai.proto index 1ca269249ff18b..20c87d830a68c3 100644 --- a/crates/proto/proto/ai.proto +++ b/crates/proto/proto/ai.proto @@ -7,7 +7,6 @@ message GetAgentServerCommand { uint64 project_id = 1; string name = 2; optional string root_dir = 3; - optional bool fallback = 4; } message GetContextServerCommand {