acp: Fix npm version spec breaking on Windows (#55938)
Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable #55770 changed the npm package version spec to `package@<=1.2.3`. On Windows this fails with `The system cannot find the file specified.` because: - `npm` resolves to `npm.cmd`, a batch file. Windows runs `.cmd` files via cmd.exe, which parses the invocation and treats unquoted `<` as input redirection. - The single quotes our shell builder emits around args are PowerShell string-literal syntax that PS strips during parsing. PS only re-adds CRT-style transport quotes around native command args containing whitespace, so `package@<=0.25.3` reaches `npm.cmd` bare and cmd.exe fails before the batch body even runs. Switch to npm's hyphen-range syntax (`0.0.0 - <version>`, equivalent to `<=<version>`), which has no `<`. Closes #55921 Release Notes: - Fixed ACP agents failing to launch on Windows with "The system cannot find the file specified"
This commit is contained in:
1 parent
2d1e20aa7d
commit
16fea4bf7a
1 file changed
+10
-3
@@ -1606,6 +1606,13 @@ impl ExternalAgentServer for LocalRegistryNpxAgent {
|
||||
/// 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.
|
||||
///
|
||||
/// We use npm's hyphen-range syntax (`0.0.0 - <version>`, equivalent to `<=<version>`) instead of
|
||||
/// the more compact `<=<version>` form because on Windows, `npm` is `npm.cmd` (a batch file run by
|
||||
/// cmd.exe), and the quotes our shell builder emits are PowerShell string-literal syntax that PS
|
||||
/// strips during parsing. PS only re-adds CRT-style transport quotes around native command args
|
||||
/// containing whitespace, so `package@<=0.25.3` reaches cmd.exe bare and the unquoted `<` is
|
||||
/// interpreted as input redirection. See zed-industries/zed#55921.
|
||||
fn bounded_npm_package_spec(package_spec: &str) -> String {
|
||||
let Some((package_name, version)) = package_spec.rsplit_once('@') else {
|
||||
return package_spec.to_string();
|
||||
@@ -1614,7 +1621,7 @@ fn bounded_npm_package_spec(package_spec: &str) -> String {
|
||||
return package_spec.to_string();
|
||||
}
|
||||
|
||||
format!("{package_name}@<={version}")
|
||||
format!("{package_name}@0.0.0 - {version}")
|
||||
}
|
||||
|
||||
struct LocalCustomAgent {
|
||||
@@ -2025,11 +2032,11 @@ mod tests {
|
||||
fn builds_bounded_npm_package_specs() {
|
||||
assert_eq!(
|
||||
bounded_npm_package_spec("agent-package@1.2.3"),
|
||||
"agent-package@<=1.2.3"
|
||||
"agent-package@0.0.0 - 1.2.3"
|
||||
);
|
||||
assert_eq!(
|
||||
bounded_npm_package_spec("@scope/agent-package@1.2.3-beta.1"),
|
||||
"@scope/agent-package@<=1.2.3-beta.1"
|
||||
"@scope/agent-package@0.0.0 - 1.2.3-beta.1"
|
||||
);
|
||||
assert_eq!(
|
||||
bounded_npm_package_spec("@scope/agent-package"),
|
||||
|
||||
Reference in new issue
Block a user