From 5fb5f6f567dfb4eb66ed7c55225726b2bcc2722c Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Thu, 9 Apr 2026 23:01:33 +0200 Subject: [PATCH] acp: Use npm --prefix for registry npx agents Because we weren't going through the normal npm subcommand route, we weren't getting a prefix flag applied. Which meant some users were seeing errors of incorrect package managers when used with a JS project. --- crates/node_runtime/src/node_runtime.rs | 96 ++++++++++++++++++++---- crates/project/src/agent_server_store.rs | 10 +++ 2 files changed, 92 insertions(+), 14 deletions(-) diff --git a/crates/node_runtime/src/node_runtime.rs b/crates/node_runtime/src/node_runtime.rs index 8d838cbe7505a696fbfbeebfa53e96dbd3b19e29..b0a7f82a40a507e341b4d3e1141f925be350f5aa 100644 --- a/crates/node_runtime/src/node_runtime.rs +++ b/crates/node_runtime/src/node_runtime.rs @@ -238,11 +238,16 @@ impl NodeRuntime { .await } - pub async fn npm_command(&self, subcommand: &str, args: &[&str]) -> Result { + pub async fn npm_command( + &self, + prefix_dir: Option<&Path>, + subcommand: &str, + args: &[&str], + ) -> Result { let http = self.0.lock().await.http.clone(); self.instance() .await - .npm_command(http.proxy(), subcommand, args) + .npm_command(prefix_dir, http.proxy(), subcommand, args) .await } @@ -372,6 +377,7 @@ trait NodeRuntimeTrait: Send + Sync { async fn npm_command( &self, + prefix_dir: Option<&Path>, proxy: Option<&Url>, subcommand: &str, args: &[&str], @@ -554,11 +560,13 @@ impl NodeRuntimeTrait for ManagedNodeRuntime { args: &[&str], ) -> Result { let attempt = || async { - let npm_command = self.npm_command(proxy, subcommand, args).await?; + let npm_command = self.npm_command(directory, proxy, subcommand, args).await?; let mut command = util::command::new_command(npm_command.path); command.args(npm_command.args); command.envs(npm_command.env); - configure_npm_command(&mut command, directory); + if let Some(directory) = directory { + command.current_dir(directory); + } command.output().await.map_err(|e| anyhow!("{e}")) }; @@ -586,6 +594,7 @@ impl NodeRuntimeTrait for ManagedNodeRuntime { async fn npm_command( &self, + prefix_dir: Option<&Path>, proxy: Option<&Url>, subcommand: &str, args: &[&str], @@ -604,6 +613,7 @@ impl NodeRuntimeTrait for ManagedNodeRuntime { let command_args = build_npm_command_args( Some(&npm_file), + prefix_dir, &self.installation_path.join("cache"), Some(&self.installation_path.join("blank_user_npmrc")), Some(&self.installation_path.join("blank_global_npmrc")), @@ -722,11 +732,13 @@ impl NodeRuntimeTrait for SystemNodeRuntime { subcommand: &str, args: &[&str], ) -> anyhow::Result { - let npm_command = self.npm_command(proxy, subcommand, args).await?; + let npm_command = self.npm_command(directory, proxy, subcommand, args).await?; let mut command = util::command::new_command(npm_command.path); command.args(npm_command.args); command.envs(npm_command.env); - configure_npm_command(&mut command, directory); + if let Some(directory) = directory { + command.current_dir(directory); + } let output = command.output().await?; anyhow::ensure!( output.status.success(), @@ -739,12 +751,14 @@ impl NodeRuntimeTrait for SystemNodeRuntime { async fn npm_command( &self, + prefix_dir: Option<&Path>, proxy: Option<&Url>, subcommand: &str, args: &[&str], ) -> Result { let command_args = build_npm_command_args( None, + prefix_dir, &self.scratch_dir.join("cache"), None, None, @@ -825,6 +839,7 @@ impl NodeRuntimeTrait for UnavailableNodeRuntime { async fn npm_command( &self, + _: Option<&Path>, _proxy: Option<&Url>, _subcommand: &str, _args: &[&str], @@ -841,13 +856,6 @@ impl NodeRuntimeTrait for UnavailableNodeRuntime { } } -fn configure_npm_command(command: &mut util::command::Command, directory: Option<&Path>) { - if let Some(directory) = directory { - command.current_dir(directory); - command.args(["--prefix".into(), directory.to_path_buf()]); - } -} - fn proxy_argument(proxy: Option<&Url>) -> Option { let mut proxy = proxy.cloned()?; // Map proxy settings from `http://localhost:10809` to `http://127.0.0.1:10809` @@ -865,6 +873,7 @@ fn proxy_argument(proxy: Option<&Url>) -> Option { fn build_npm_command_args( entrypoint: Option<&Path>, + prefix_dir: Option<&Path>, cache_dir: &Path, user_config: Option<&Path>, global_config: Option<&Path>, @@ -876,6 +885,10 @@ fn build_npm_command_args( if let Some(entrypoint) = entrypoint { command_args.push(entrypoint.to_string_lossy().into_owned()); } + if let Some(prefix_dir) = prefix_dir { + command_args.push("--prefix".into()); + command_args.push(prefix_dir.to_string_lossy().into_owned()); + } command_args.push(subcommand.to_string()); command_args.push(format!("--cache={}", cache_dir.display())); if let Some(user_config) = user_config { @@ -928,9 +941,11 @@ fn npm_command_env(node_binary: Option<&Path>) -> HashMap { #[cfg(test)] mod tests { + use std::path::Path; + use http_client::Url; - use super::proxy_argument; + use super::{build_npm_command_args, proxy_argument}; // Map localhost to 127.0.0.1 // NodeRuntime without environment information can not parse `localhost` correctly. @@ -960,4 +975,57 @@ mod tests { ); } } + + #[test] + fn test_build_npm_command_args_inserts_prefix_before_subcommand() { + let args = build_npm_command_args( + None, + Some(Path::new("/tmp/zed-prefix")), + Path::new("/tmp/cache"), + None, + None, + None, + "exec", + &["--yes", "--", "agent-package"], + ); + + assert_eq!( + args, + vec![ + "--prefix".to_string(), + "/tmp/zed-prefix".to_string(), + "exec".to_string(), + "--cache=/tmp/cache".to_string(), + "--yes".to_string(), + "--".to_string(), + "agent-package".to_string(), + ] + ); + } + + #[test] + fn test_build_npm_command_args_keeps_entrypoint_before_prefix() { + let args = build_npm_command_args( + Some(Path::new("/tmp/npm-cli.js")), + Some(Path::new("/tmp/zed-prefix")), + Path::new("/tmp/cache"), + None, + None, + None, + "exec", + &["--yes"], + ); + + assert_eq!( + args, + vec![ + "/tmp/npm-cli.js".to_string(), + "--prefix".to_string(), + "/tmp/zed-prefix".to_string(), + "exec".to_string(), + "--cache=/tmp/cache".to_string(), + "--yes".to_string(), + ] + ); + } } diff --git a/crates/project/src/agent_server_store.rs b/crates/project/src/agent_server_store.rs index 5a9721d827cf3d189c7954f0698b662e5aaf4852..4cc3e38d3b9de708d749a31a4deb9e6c5ceafd48 100644 --- a/crates/project/src/agent_server_store.rs +++ b/crates/project/src/agent_server_store.rs @@ -568,6 +568,7 @@ impl AgentServerStore { agent_name.clone(), ExternalAgentEntry::new( Box::new(LocalRegistryNpxAgent { + fs: fs.clone(), node_runtime: node_runtime.clone(), project_environment: project_environment.clone(), version: agent.metadata.version.clone(), @@ -1498,6 +1499,7 @@ impl ExternalAgentServer for LocalRegistryArchiveAgent { } struct LocalRegistryNpxAgent { + fs: Arc, node_runtime: NodeRuntime, project_environment: Entity, version: SharedString, @@ -1527,6 +1529,7 @@ impl ExternalAgentServer for LocalRegistryNpxAgent { extra_env: HashMap, cx: &mut AsyncApp, ) -> Task> { + let fs = self.fs.clone(); let node_runtime = self.node_runtime.clone(); let project_environment = self.project_environment.downgrade(); let package = self.package.clone(); @@ -1542,11 +1545,18 @@ impl ExternalAgentServer for LocalRegistryNpxAgent { .await .unwrap_or_default(); + let prefix_dir = paths::external_agents_dir() + .join("registry") + .join("npx") + .join(Path::new(package.as_str())); + fs.create_dir(&prefix_dir).await?; + let mut exec_args = vec!["--yes".to_string(), "--".to_string(), package.to_string()]; exec_args.extend(args); let npm_command = node_runtime .npm_command( + Some(&prefix_dir), "exec", &exec_args.iter().map(|a| a.as_str()).collect::>(), )