mirror of
https://github.com/l0ng-ai/openidea.git
synced 2026-11-07 10:16:03 -10:00
fix(agents): resolve agent commands through PATH x PATHEXT on Windows (#273)
On Windows std/tokio Command only tries .exe for a bare program name, so agents installed as npm shims (codex.cmd, claude.cmd) failed with "program not found". Bare names are now resolved through PATH x PATHEXT before spawning. - ide-core::process gains find_executable / find_executable_in (moved from ide-run::which, which re-exports them). - ide-jsonrpc Peer::spawn_framed resolves against the child's PATH (caller env, login shell, then ours): covers ACP agents and the native Codex driver. - ide-agent newest_install builds candidates with executable_names, so the native Claude driver finds .cmd shims and multi-install selection works on Windows. - ide-agent replay recorder resolves the same way. Co-authored-by: a-rookie-of-C-language <[email protected]>
This commit is contained in:
1 parent
44f01b5ce6
commit
94d209c99c
5 files changed
+129
-39
No files matched your search
@@ -21,18 +21,26 @@ use std::sync::Arc;
|
||||
/// The newest install of `command` on the `PATH` a child sees, by its `--version`.
|
||||
/// Several installs are common (npm, Homebrew, the vendor's own installer), and the first
|
||||
/// on `PATH` is often an old one that the service no longer serves the current models to.
|
||||
/// A path, or a name found once (or not at all), is used as it is.
|
||||
/// A path, or a name not found on `PATH`, is used as it is; a single match returns its
|
||||
/// resolved path so Windows `PATHEXT` shims (`claude.cmd`, `codex.cmd`) start.
|
||||
pub(crate) fn newest_install(command: &str) -> String {
|
||||
if Path::new(command).components().count() > 1 {
|
||||
let path = ide_core::process::host_var_os("PATH")
|
||||
.or_else(|| ide_core::process::shell_env().find(|(k, _)| *k == "PATH").map(|(_, v)| v.to_owned()))
|
||||
.unwrap_or_default();
|
||||
newest_install_in(command, &path)
|
||||
}
|
||||
|
||||
fn newest_install_in(command: &str, search: &std::ffi::OsStr) -> String {
|
||||
if Path::new(command).components().count() > 1 || Path::new(command).is_absolute() {
|
||||
return command.to_owned();
|
||||
}
|
||||
let path = ide_core::process::host_var_os("PATH")
|
||||
.or_else(|| ide_core::process::shell_env().find(|(k, _)| *k == "PATH").map(|(_, v)| v.to_owned()));
|
||||
let pathext = ide_core::process::host_var_os("PATHEXT");
|
||||
let names =
|
||||
ide_core::process::executable_names(command, cfg!(windows), pathext.as_deref().and_then(|p| p.to_str()));
|
||||
let mut seen = Vec::new();
|
||||
let candidates: Vec<PathBuf> = path
|
||||
.iter()
|
||||
.flat_map(std::env::split_paths)
|
||||
.map(|dir| dir.join(command))
|
||||
let candidates: Vec<PathBuf> = std::env::split_paths(search)
|
||||
.filter(|d| !d.as_os_str().is_empty())
|
||||
.flat_map(|dir| names.iter().map(move |n| dir.join(n)))
|
||||
.filter(|p| p.is_file())
|
||||
.filter(|p| {
|
||||
let real = dunce::canonicalize(p).unwrap_or_else(|_| p.clone());
|
||||
@@ -42,7 +50,7 @@ pub(crate) fn newest_install(command: &str) -> String {
|
||||
})
|
||||
.collect();
|
||||
if candidates.len() < 2 {
|
||||
return command.to_owned();
|
||||
return candidates.into_iter().next().map_or_else(|| command.to_owned(), |p| p.to_string_lossy().into_owned());
|
||||
}
|
||||
candidates
|
||||
.into_iter()
|
||||
@@ -66,16 +74,6 @@ fn parse_version(text: &str) -> Option<Vec<u64>> {
|
||||
})
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[test]
|
||||
fn versions_are_read_from_either_side_of_the_name() {
|
||||
assert_eq!(parse_version("codex-cli 0.164.0\n"), Some(vec![0, 164, 0]));
|
||||
assert_eq!(parse_version("2.1.312 (Claude Code)"), Some(vec![2, 1, 312]));
|
||||
assert_eq!(parse_version("v1.2"), Some(vec![1, 2]));
|
||||
assert_eq!(parse_version("no version here"), None);
|
||||
assert!(parse_version("codex-cli 0.164.0") > parse_version("codex-cli 0.147.0"));
|
||||
}
|
||||
|
||||
/// The agent side of an in-process ACP connection.
|
||||
pub(crate) trait Shim: Handler {
|
||||
/// Called once, before any message arrives: the connection to the client, for
|
||||
@@ -94,3 +92,36 @@ pub(crate) fn serve<S: Shim>(shim: Arc<S>, client: Arc<dyn Handler>) -> Peer {
|
||||
shim.attach(to_client);
|
||||
Peer::connect_framed(or, ow, client, Framing::Lines)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn versions_are_read_from_either_side_of_the_name() {
|
||||
assert_eq!(parse_version("codex-cli 0.164.0\n"), Some(vec![0, 164, 0]));
|
||||
assert_eq!(parse_version("2.1.312 (Claude Code)"), Some(vec![2, 1, 312]));
|
||||
assert_eq!(parse_version("v1.2"), Some(vec![1, 2]));
|
||||
assert_eq!(parse_version("no version here"), None);
|
||||
assert!(parse_version("codex-cli 0.164.0") > parse_version("codex-cli 0.147.0"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn newest_install_resolves_single_candidate_and_keeps_missing_or_paths() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let a = dir.path().join("a");
|
||||
let b = dir.path().join("b");
|
||||
std::fs::create_dir_all(&a).unwrap();
|
||||
std::fs::create_dir_all(&b).unwrap();
|
||||
std::fs::write(a.join("claude"), "#!/bin/sh\n").unwrap();
|
||||
std::fs::write(b.join("claude.cmd"), "@echo off\n").unwrap();
|
||||
let search = std::env::join_paths([&a, &b]).unwrap();
|
||||
if cfg!(windows) {
|
||||
assert_eq!(newest_install_in("claude", &search), b.join("claude.cmd").to_string_lossy());
|
||||
} else {
|
||||
assert_eq!(newest_install_in("claude", &search), a.join("claude").to_string_lossy());
|
||||
}
|
||||
assert_eq!(newest_install_in("nope", &search), "nope");
|
||||
assert_eq!(newest_install_in("bin/claude", &search), "bin/claude");
|
||||
}
|
||||
}
|
||||
@@ -185,7 +185,23 @@ pub(crate) fn recorder(out: PathBuf) -> Connector {
|
||||
let file = Arc::new(Mutex::new(String::new()));
|
||||
let connections = Arc::new(Mutex::new(0u32));
|
||||
Arc::new(move |spec, cwd, client, on_stderr| {
|
||||
let mut cmd = tokio::process::Command::new(&spec.command);
|
||||
let program = {
|
||||
let path = Path::new(&spec.command);
|
||||
if path.components().count() > 1 || path.is_absolute() {
|
||||
path.to_path_buf()
|
||||
} else {
|
||||
let search = spec
|
||||
.env
|
||||
.iter()
|
||||
.find(|(k, _)| k.eq_ignore_ascii_case("PATH"))
|
||||
.map(|(_, v)| std::ffi::OsString::from(v))
|
||||
.or_else(|| ide_core::process::shell_env().find(|(k, _)| k == "PATH").map(|(_, v)| v))
|
||||
.or_else(|| ide_core::process::host_var_os("PATH"))
|
||||
.unwrap_or_default();
|
||||
ide_core::process::find_executable(&spec.command, &search).unwrap_or_else(|| path.to_path_buf())
|
||||
}
|
||||
};
|
||||
let mut cmd = tokio::process::Command::new(&program);
|
||||
ide_core::process::prepare(cmd.as_std_mut());
|
||||
let mut child = cmd
|
||||
.args(&spec.args)
|
||||
|
||||
@@ -218,6 +218,25 @@ pub fn executable_names(name: &str, windows: bool, pathext: Option<&str>) -> Vec
|
||||
exts.iter().map(|e| format!("{name}{}", e.to_ascii_lowercase())).collect()
|
||||
}
|
||||
|
||||
/// First `search` directory holding `name` (see [`executable_names`]).
|
||||
pub fn find_executable_in(
|
||||
name: &str,
|
||||
search: &OsStr,
|
||||
windows: bool,
|
||||
pathext: Option<&str>,
|
||||
) -> Option<std::path::PathBuf> {
|
||||
let names = executable_names(name, windows, pathext);
|
||||
std::env::split_paths(search)
|
||||
.filter(|d| !d.as_os_str().is_empty())
|
||||
.find_map(|dir| names.iter().map(|n| dir.join(n)).find(|p| p.is_file()))
|
||||
}
|
||||
|
||||
/// [`find_executable_in`] for this platform (`PATHEXT` from the environment).
|
||||
pub fn find_executable(name: &str, search: &OsStr) -> Option<std::path::PathBuf> {
|
||||
let pathext = host_var_os("PATHEXT");
|
||||
find_executable_in(name, search, cfg!(windows), pathext.as_deref().and_then(|p| p.to_str()))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
@@ -363,4 +382,23 @@ mod tests {
|
||||
prepare(&mut cmd);
|
||||
assert_eq!(cmd.get_envs().find(|(k, _)| *k == "PATH").and_then(|(_, v)| v), Some(OsStr::new("/custom")));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn find_executable_finds_bare_names_on_the_given_path() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let a = dir.path().join("a");
|
||||
let b = dir.path().join("b");
|
||||
std::fs::create_dir_all(&a).unwrap();
|
||||
std::fs::create_dir_all(&b).unwrap();
|
||||
std::fs::write(a.join("agent"), "#!/bin/sh\n").unwrap();
|
||||
std::fs::write(b.join("agent.cmd"), "@echo off\n").unwrap();
|
||||
let search = std::env::join_paths([&a, &b]).unwrap();
|
||||
if cfg!(windows) {
|
||||
// The `.cmd` shim wins over a POSIX script `Command` can't start.
|
||||
assert_eq!(find_executable("agent", &search), Some(b.join("agent.cmd")));
|
||||
} else {
|
||||
assert_eq!(find_executable("agent", &search), Some(a.join("agent")));
|
||||
}
|
||||
assert_eq!(find_executable("nope", &search), None);
|
||||
}
|
||||
}
|
||||
@@ -332,6 +332,25 @@ impl Drop for KillOnDrop {
|
||||
}
|
||||
}
|
||||
|
||||
/// The program [`Peer::spawn_framed`] runs: a bare name resolved against the
|
||||
/// child's `PATH` (a caller's `env` PATH, else the login shell's, else ours), so
|
||||
/// that on Windows `PATHEXT` names (`codex` → `codex.cmd`) start; anything else
|
||||
/// is kept as given.
|
||||
fn resolve_spawn_program(command: &str, env: &[(String, String)]) -> std::path::PathBuf {
|
||||
let path = std::path::Path::new(command);
|
||||
if path.components().count() > 1 || path.is_absolute() {
|
||||
return path.to_path_buf();
|
||||
}
|
||||
let search = env
|
||||
.iter()
|
||||
.find(|(k, _)| k.eq_ignore_ascii_case("PATH"))
|
||||
.map(|(_, v)| std::ffi::OsString::from(v))
|
||||
.or_else(|| ide_core::process::shell_env().find(|(k, _)| k == "PATH").map(|(_, v)| v))
|
||||
.or_else(|| ide_core::process::host_var_os("PATH"))
|
||||
.unwrap_or_default();
|
||||
ide_core::process::find_executable(command, &search).unwrap_or_else(|| path.to_path_buf())
|
||||
}
|
||||
|
||||
impl Peer {
|
||||
/// Spawns `command` and speaks JSON-RPC over its stdio. Stderr lines, and
|
||||
/// stdout lines that are not part of a message (a launcher's own output), are
|
||||
@@ -360,7 +379,10 @@ impl Peer {
|
||||
framing: Framing,
|
||||
) -> std::io::Result<Self> {
|
||||
use std::process::Stdio;
|
||||
let mut cmd = tokio::process::Command::new(command);
|
||||
// Windows resolves a bare name only as `.exe`; find the file a child
|
||||
// would (`PATH` × `PATHEXT`, e.g. `codex` → `codex.cmd`) first.
|
||||
let resolved = resolve_spawn_program(command, env);
|
||||
let mut cmd = tokio::process::Command::new(&resolved);
|
||||
ide_core::process::prepare(cmd.as_std_mut());
|
||||
cmd.args(args)
|
||||
.current_dir(cwd)
|
||||
|
||||
@@ -2,24 +2,7 @@
|
||||
//! `PATHEXT` (`kotlinc` → `kotlinc.bat`, `npx` → `npx.cmd`), which
|
||||
//! `std::process::Command` does not do (it only appends `.exe`).
|
||||
|
||||
use std::ffi::OsStr;
|
||||
use std::path::PathBuf;
|
||||
|
||||
pub use ide_core::process::executable_names;
|
||||
|
||||
/// First `search` directory holding `name` (see [`executable_names`]).
|
||||
pub fn find_executable_in(name: &str, search: &OsStr, windows: bool, pathext: Option<&str>) -> Option<PathBuf> {
|
||||
let names = executable_names(name, windows, pathext);
|
||||
std::env::split_paths(search)
|
||||
.filter(|d| !d.as_os_str().is_empty())
|
||||
.find_map(|dir| names.iter().map(|n| dir.join(n)).find(|p| p.is_file()))
|
||||
}
|
||||
|
||||
/// [`find_executable_in`] for this platform (`PATHEXT` from the environment).
|
||||
pub fn find_executable(name: &str, search: &OsStr) -> Option<PathBuf> {
|
||||
let pathext = std::env::var("PATHEXT").ok();
|
||||
find_executable_in(name, search, cfg!(windows), pathext.as_deref())
|
||||
}
|
||||
pub use ide_core::process::{executable_names, find_executable, find_executable_in};
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
|
||||
Reference in new issue
Block a user