From 2ad979af6609dbeadb913b2081f39a6fde495ac7 Mon Sep 17 00:00:00 2001 From: arookieofc <2128194521@qq.com> Date: Fri, 30 Oct 2026 12:22:04 +0800 Subject: [PATCH 1/7] feat(agents): complete OpenCode's /compact builtin (#257) OpenCode handles /compact through its ACP session but does not list it in availableCommands, so the composer never offered it. Agent builtins the agent implements without advertising are now merged into slash-command completion without duplicating listed commands; the table holds only commands verified against the agent (OpenCode /compact). Claude Code already advertises /compact. Executable lookup keeps using the shared PATHEXT-aware resolution on main, with a test that the newest install across PATH entries is chosen. Co-authored-by: a-rookie-of-C-language <186315048+a-rookie-of-C-language@users.noreply.github.com> --- crates/ide-agent/src/lib.rs | 22 +++++++++++++++-- crates/ide-agent/src/native/mod.rs | 20 ++++++++++++++++ crates/ide-agent/src/tests.rs | 38 +++++++++++++++++++++++++++++- src/platform/locales/text/zh.json | 1 + 4 files changed, 78 insertions(+), 3 deletions(-) diff --git a/crates/ide-agent/src/lib.rs b/crates/ide-agent/src/lib.rs index a1cdaeab..af22f4ac 100644 --- a/crates/ide-agent/src/lib.rs +++ b/crates/ide-agent/src/lib.rs @@ -314,8 +314,8 @@ impl AgentManager { } /// The slash commands to complete for `provider`, in `task`'s directory or (without one) - /// in the project: what the agent offered last time there, plus, for Claude Code, the - /// skills and custom commands on disk it has not listed. + /// in the project: what the agent offered last time there, unadvertised builtins, + /// and, for Claude Code, skills and custom commands on disk it has not listed. pub async fn commands(&self, provider: &str, task: Option<&str>) -> IdeResult> { let (provider, offered, cwd) = match task { Some(id) => { @@ -325,6 +325,14 @@ impl AgentManager { } None => (provider.to_owned(), self.shared.known_commands(provider), self.shared.paths.project_root.clone()), }; + // Commands the agent implements itself and never lists (its CLI builtins): + // merged without duplicating what `availableCommands` or the disk offer. + let mut offered = offered; + for &(name, description) in builtin_commands(&provider) { + if !offered.iter().any(|c| c.name == name) { + offered.push(AgentCommand { name: name.to_owned(), description: description.to_owned(), hint: None }); + } + } if provider != "claude" && provider != "claude-acp" { return Ok(offered); } @@ -1629,6 +1637,16 @@ fn model_of(config: &[AgentConfigOption]) -> Option { config.iter().find(|o| o.category.as_deref() == Some("model")).and_then(|o| o.current.clone()) } +/// Commands the agent implements on its own and never lists in `availableCommands` — +/// its CLI builtins, passed through as prompt text (e.g. `/compact`). `(name, description)`. +fn builtin_commands(provider: &str) -> &'static [(&'static str, &'static str)] { + match provider { + // OpenCode ACP handles `/compact` via session.summarize but omits it from its command list. + "opencode" => &[("compact", "compact the conversation")], + _ => &[], + } +} + fn file_name(path: &Path) -> String { path.file_name().map_or_else(|| path.to_string_lossy().into_owned(), |n| n.to_string_lossy().into_owned()) } diff --git a/crates/ide-agent/src/native/mod.rs b/crates/ide-agent/src/native/mod.rs index 10f11309..665e9a04 100644 --- a/crates/ide-agent/src/native/mod.rs +++ b/crates/ide-agent/src/native/mod.rs @@ -124,4 +124,24 @@ mod tests { assert_eq!(newest_install_in("nope", &search), "nope"); assert_eq!(newest_install_in("bin/claude", &search), "bin/claude"); } + + #[cfg(unix)] + #[test] + fn newest_install_compares_versions_across_path_entries() { + use std::os::unix::fs::PermissionsExt; + + let dir = tempfile::tempdir().unwrap(); + let old = dir.path().join("old"); + let new = dir.path().join("new"); + for (path, version) in [(&old, "2.9.0"), (&new, "2.10.0")] { + std::fs::create_dir_all(path).unwrap(); + let executable = path.join("agent"); + std::fs::write(&executable, format!("#!/bin/sh\necho {version}\n")).unwrap(); + std::fs::set_permissions(&executable, std::fs::Permissions::from_mode(0o755)).unwrap(); + } + for paths in [[&old, &new], [&new, &old]] { + let search = std::env::join_paths(paths).unwrap(); + assert_eq!(newest_install_in("agent", &search), new.join("agent").to_string_lossy()); + } + } } diff --git a/crates/ide-agent/src/tests.rs b/crates/ide-agent/src/tests.rs index f30fc2b5..0acf9ffc 100644 --- a/crates/ide-agent/src/tests.rs +++ b/crates/ide-agent/src/tests.rs @@ -210,7 +210,7 @@ async fn commands_are_remembered_per_project_and_merged_with_skills_on_disk() { let names = |c: Vec| c.into_iter().map(|c| c.name).collect::>(); let mgr_a = manager_at(&a, &models, Arc::new(FakeAgent::default())); - // Before any session: the skills on disk. + // Before any session: only the skills on disk. assert_eq!(names(mgr_a.commands("claude", None).await.unwrap()), ["triage"]); let task = mgr_a .create(NewAgentTask { @@ -239,6 +239,42 @@ async fn commands_are_remembered_per_project_and_merged_with_skills_on_disk() { assert_eq!(names(mgr_a.providers().into_iter().find(|p| p.id == "claude").unwrap().commands), ["review"]); } +#[tokio::test(flavor = "multi_thread")] +async fn builtin_commands_only_fill_verified_gaps_and_preserve_advertised_metadata() { + let tmp = tempfile::tempdir().unwrap(); + let agent = Arc::new(FakeAgent::default()); + let mgr = manager_at(tmp.path(), &tmp.path().join("models.json"), agent); + for provider in ["claude", "claude-acp", "opencode2", "codex", "custom"] { + assert!(mgr.commands(provider, None).await.unwrap().is_empty(), "{provider}"); + } + let commands = mgr.commands("opencode", None).await.unwrap(); + assert_eq!(commands.iter().map(|c| c.name.as_str()).collect::>(), ["compact"]); + let task = mgr + .create(NewAgentTask { + prompt: text("Hello"), + provider: "opencode".into(), + config: Vec::new(), + isolation: Isolation::Local, + base: None, + }) + .await + .unwrap(); + wait_for(&mgr, &task.id, AgentStatus::NeedsInput).await; + let advertised = AgentCommand { + name: "compact".into(), + description: "Agent supplied description".into(), + hint: Some("instructions".into()), + }; + mgr.task(&task.id).unwrap().state.lock().task.commands = vec![advertised.clone()]; + // The task's provider wins over the caller's provider; metadata is not replaced. + let commands = mgr.commands("claude", Some(&task.id)).await.unwrap(); + assert_eq!(commands.len(), 1); + assert_eq!(commands[0].name, advertised.name); + assert_eq!(commands[0].description, advertised.description); + assert_eq!(commands[0].hint, advertised.hint); + mgr.shutdown(); +} + async fn wait_for(mgr: &AgentManager, id: &str, status: AgentStatus) { for _ in 0..500 { let task = mgr.task(id).unwrap(); diff --git a/src/platform/locales/text/zh.json b/src/platform/locales/text/zh.json index 8c396967..db7eb770 100644 --- a/src/platform/locales/text/zh.json +++ b/src/platform/locales/text/zh.json @@ -2278,6 +2278,7 @@ "cargo run configurations and rust-analyzer use this toolchain (PATH, RUSTUP_TOOLCHAIN).": "cargo 运行配置和 rust-analyzer 使用此工具链（PATH、RUSTUP_TOOLCHAIN）。", "chord prefix": "组合键前缀", "command line tool. Install it, then run": "命令行工具。请安装它，然后运行", + "compact the conversation": "压缩对话上下文", "configured in": "配置位置：", "const declaration": "const 声明", "current branch": "当前分支", From 7a46774fdac346d5186572ffa6a97591ae355e82 Mon Sep 17 00:00:00 2001 From: Mykhailo Smolinskyi Date: Fri, 30 Oct 2026 06:23:25 +0200 Subject: [PATCH 2/7] fix(angular): find ngserver installed through version managers (#260) The Angular language server (ngserver) was not found when Node and global npm packages came from a version manager (nvm, fnm, volta, asdf, mise) whose bin directories are not on the GUI app's PATH. Tool lookup now searches those directories too through a shared ide-core tool_path module used by the LSP launcher, SDK queries and tool detection; explicit relative commands resolve against the project root, empty PATH entries are ignored, and a non-executable file earlier on PATH no longer shadows the real command. The process query test that flaked under load now uses a generous deadline for the success case, with the timeout covered by a separate test of a child that never exits. Co-authored-by: nntndfrk <2458463+nntndfrk@users.noreply.github.com> --- Cargo.lock | 2 - crates/ide-agent/src/registry.rs | 2 +- crates/ide-core/src/lib.rs | 1 + crates/ide-core/src/tool_path.rs | 193 +++++++++++++++++++++++++++++ crates/ide-lsp/Cargo.toml | 1 - crates/ide-lsp/src/launch.rs | 92 +++----------- crates/ide-lsp/src/lib.rs | 17 ++- crates/ide-sdk/Cargo.toml | 1 - crates/ide-sdk/src/angular.rs | 11 +- crates/ide-sdk/src/apply.rs | 4 +- crates/ide-sdk/src/exec.rs | 87 +++++++------ crates/ide-sdk/src/godot.rs | 2 +- crates/ide-sdk/src/lib.rs | 1 - crates/ide-sdk/src/npm.rs | 203 ++++++++++++++++++++++++++++++- crates/ide-sdk/src/tests.rs | 154 +++++++++++++++++++++++ crates/ide-tools/src/jdk.rs | 2 +- crates/ide-tools/src/lib.rs | 43 +++---- crates/ide-tools/src/server.rs | 4 +- crates/ide-tools/src/tests.rs | 5 +- 19 files changed, 657 insertions(+), 168 deletions(-) create mode 100644 crates/ide-core/src/tool_path.rs diff --git a/Cargo.lock b/Cargo.lock index b73037d5..61acdb2b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4380,7 +4380,6 @@ dependencies = [ "tokio", "toml 0.9.12+spec-1.1.0", "tracing", - "which", ] [[package]] @@ -4490,7 +4489,6 @@ dependencies = [ "tempfile", "toml 0.9.12+spec-1.1.0", "tracing", - "which", "zip", ] diff --git a/crates/ide-agent/src/registry.rs b/crates/ide-agent/src/registry.rs index 6de90d78..c9bef601 100644 --- a/crates/ide-agent/src/registry.rs +++ b/crates/ide-agent/src/registry.rs @@ -335,7 +335,7 @@ pub fn mark_updates(paths: &RegistryPaths, providers: &mut [AgentProvider]) { /// HTTPS only, redirects included. Tests serve downloads over `file://`. fn http() -> IdeResult { - let http = Http::new(&ide_tools::default_search_path())?; + let http = Http::new(&ide_core::tool_path::search_path())?; Ok(if cfg!(test) { http } else { http.https_only() }) } diff --git a/crates/ide-core/src/lib.rs b/crates/ide-core/src/lib.rs index 1b58ba68..fcb39a1f 100644 --- a/crates/ide-core/src/lib.rs +++ b/crates/ide-core/src/lib.rs @@ -20,6 +20,7 @@ pub mod remote_settings; pub mod settings; pub mod task; pub mod text; +pub mod tool_path; pub mod version_managers; pub mod workflow_settings; diff --git a/crates/ide-core/src/tool_path.rs b/crates/ide-core/src/tool_path.rs new file mode 100644 index 00000000..6abe16a4 --- /dev/null +++ b/crates/ide-core/src/tool_path.rs @@ -0,0 +1,193 @@ +//! Where the IDE looks up the executables it runs: language servers, the tools +//! their settings come from, installers. An app started from the desktop gets a +//! minimal `PATH`, and version managers set theirs up in shell rc files. + +use crate::process::{host_var_os, login_env}; +use crate::version_managers; +use std::collections::HashSet; +use std::ffi::{OsStr, OsString}; +use std::path::{Path, PathBuf}; + +/// Where executables are looked up: this process's `PATH` (the login shell's in the +/// app), the login shell's `PATH` as last read, then per-user directories and mise's, +/// asdf's and pyenv's shims, for when the shell's setup didn't reach the app. Language server +/// launchers, their settings hooks and tool detection all look here, so a settings +/// hook finds the executable its server is launched from. +pub fn search_path() -> OsString { + let home = std::env::var_os("HOME").or_else(|| std::env::var_os("USERPROFILE")).map(PathBuf::from); + let shell_path = login_env().and_then(|env| env.iter().find(|(k, _)| k == "PATH").map(|(_, v)| v.clone())); + let shims: Vec = version_managers::managers().iter().map(|m| m.shims()).collect(); + let dirs = search_dirs(host_var_os("PATH").as_deref(), shell_path.as_deref(), home.as_deref(), &shims); + std::env::join_paths(dirs).unwrap_or_default() +} + +/// Resolves a command on the shared path, or an explicit path relative to `root`. +pub fn find_command(command: &str, search: &OsStr, root: &Path) -> Option { + let path = Path::new(command); + let explicit = path.parent().is_some_and(|p| !p.as_os_str().is_empty()); + let pathext = host_var_os("PATHEXT"); + let names = crate::process::executable_names(command, cfg!(windows), pathext.as_deref().and_then(OsStr::to_str)); + let dirs = if explicit { + vec![root.to_path_buf()] + } else { + std::env::split_paths(search).filter(|p| !p.as_os_str().is_empty()).map(|p| root.join(p)).collect() + }; + dirs.into_iter().find_map(|dir| names.iter().map(|name| dir.join(name)).find(|p| is_executable(p))) +} + +fn is_executable(path: &Path) -> bool { + let Ok(metadata) = path.metadata() else { return false }; + if !metadata.is_file() { + return false; + } + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + metadata.permissions().mode() & 0o111 != 0 + } + #[cfg(not(unix))] + true +} + +/// [`search_path`]'s directories, in order and once each. Only absolute ones: a +/// relative entry (`.`, `node_modules/.bin`) resolves against the project, which +/// could then supply the tools the IDE runs. One that can't be joined (a `:` in a +/// directory name) is left out rather than failing the whole path. +fn search_dirs( + process_path: Option<&OsStr>, + shell_path: Option<&OsStr>, + home: Option<&Path>, + shims: &[PathBuf], +) -> Vec { + let mut dirs: Vec = process_path.into_iter().chain(shell_path).flat_map(std::env::split_paths).collect(); + if let Some(home) = home { + dirs.extend(USER_DIRS.iter().map(|d| home.join(d))); + dirs.extend(nvm_bins(&home.join(".nvm/versions/node"))); + } + // Version managers' shims after real tool directories: a shim exists for a command any + // installed version has, and fails when the selected one lacks it. Before the system's, + // as the managers' shell setup puts them. + dirs.extend(home.map(|h| h.join(".pyenv/shims"))); + dirs.extend(shims.iter().cloned()); + dirs.extend(SYSTEM_DIRS.iter().map(PathBuf::from)); + let mut seen = HashSet::new(); + dirs.retain(|d| d.is_absolute() && std::env::join_paths([d]).is_ok() && seen.insert(d.clone())); + dirs +} + +/// Tool directories below the home directory. +const USER_DIRS: &[&str] = &[ + ".cargo/bin", + ".local/bin", + "go/bin", + ".bun/bin", + ".deno/bin", + ".npm-global/bin", + ".volta/bin", + // Set up in shell rc files: fnm's default Node and pnpm's global bin (Linux, then macOS). + ".local/share/fnm/aliases/default/bin", + "Library/Application Support/fnm/aliases/default/bin", + ".fnm/aliases/default/bin", + ".local/share/pnpm/bin", + ".local/share/pnpm", + "Library/pnpm/bin", + "Library/pnpm", +]; + +const SYSTEM_DIRS: &[&str] = &["/opt/homebrew/bin", "/usr/local/go/bin", "/usr/local/bin", "/usr/bin", "/bin"]; + +/// The `bin` directories of nvm's Node versions in `dir`, newest first. +fn nvm_bins(dir: &Path) -> Vec { + let mut versions: Vec<(Vec, PathBuf)> = std::fs::read_dir(dir) + .into_iter() + .flatten() + .flatten() + .map(|e| (version_key(&e.file_name().to_string_lossy()), e.path().join("bin"))) + .collect(); + versions.sort(); + versions.into_iter().rev().map(|(_, bin)| bin).collect() +} + +/// `v22.10.1` as `[22, 10, 1]`: v22 is newer than v9, v20.10 newer than v20.9. +fn version_key(name: &str) -> Vec { + name.trim_start_matches('v').split('.').map(|n| n.parse().unwrap_or(0)).collect() +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn commands_resolve_on_path_and_explicit_paths_under_root() { + let dir = tempfile::tempdir().unwrap(); + let bin = dir.path().join("bin"); + std::fs::create_dir(&bin).unwrap(); + let name = if cfg!(windows) { "ngserver.cmd" } else { "ngserver" }; + std::fs::write(bin.join(name), "").unwrap(); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(bin.join(name), std::fs::Permissions::from_mode(0o755)).unwrap(); + } + let search = std::env::join_paths([&bin]).unwrap(); + assert_eq!(find_command("ngserver", &search, dir.path()), Some(bin.join(name))); + assert_eq!(find_command("bin/ngserver", &search, dir.path()), Some(bin.join(name))); + assert_eq!(find_command(bin.join(name).to_str().unwrap(), &search, dir.path()), Some(bin.join(name))); + assert_eq!(find_command("absent", &search, dir.path()), None); + for command in ["bin/ngserver", bin.join(name).to_str().unwrap()] { + assert_eq!(find_command(command, OsStr::new(""), dir.path()), Some(bin.join(name))); + } + } + + #[cfg(unix)] + #[test] + fn non_executable_files_do_not_shadow_commands() { + use std::os::unix::fs::PermissionsExt; + let tmp = tempfile::tempdir().unwrap(); + let dirs = [tmp.path().join("first"), tmp.path().join("second")]; + for (dir, mode) in dirs.iter().zip([0o644, 0o755]) { + std::fs::create_dir(dir).unwrap(); + let file = dir.join("ngserver"); + std::fs::write(&file, "#!/bin/sh\n").unwrap(); + std::fs::set_permissions(file, std::fs::Permissions::from_mode(mode)).unwrap(); + } + let path = std::env::join_paths(&dirs).unwrap(); + assert_eq!(find_command("ngserver", &path, tmp.path()), Some(dirs[1].join("ngserver"))); + assert_eq!(find_command("first/ngserver", &path, tmp.path()), None); + } + + #[cfg(unix)] + #[test] + fn search_dirs_put_the_process_path_first_and_shims_after_real_tools() { + let shims = [PathBuf::from("/home/me/.local/share/mise/shims"), "/home/me/.asdf/shims".into()]; + let dirs = search_dirs( + Some(OsStr::new("/usr/bin:.:node_modules/.bin:/opt/tools")), + // nushell prints `$PATH` as is. + Some(OsStr::new("$PATH:/opt/tools:/home/me/.asdf/shims")), + Some(Path::new("/home/me")), + &shims, + ); + let at = |d: &str| dirs.iter().position(|p| p == Path::new(d)).unwrap(); + assert_eq!(dirs[..3], [PathBuf::from("/usr/bin"), "/opt/tools".into(), "/home/me/.asdf/shims".into()]); + assert!(at("/home/me/.npm-global/bin") < at("/home/me/.pyenv/shims")); + assert!(at("/home/me/.local/share/mise/shims") < at("/usr/local/bin")); + assert!(dirs.iter().all(|d| d.is_absolute())); + let unique: HashSet<&PathBuf> = dirs.iter().collect(); + assert_eq!(unique.len(), dirs.len()); + // A `:` can't be in a `PATH` entry: those below such a home are left out. + let dirs = search_dirs(Some(OsStr::new("/usr/bin")), None, Some(Path::new("/home/a:b")), &[]); + assert!(dirs.contains(&PathBuf::from("/usr/bin")) && !dirs.iter().any(|d| d.starts_with("/home"))); + } + + #[test] + fn nvm_versions_newest_first() { + let dir = std::env::temp_dir().join(format!("openidea-nvm-{}", std::process::id())); + for v in ["v9.11.2", "v20.9.0", "v22.1.0", "v20.10.0"] { + std::fs::create_dir_all(dir.join(v).join("bin")).unwrap(); + } + let bins = nvm_bins(&dir); + std::fs::remove_dir_all(&dir).unwrap(); + let names: Vec<_> = bins.iter().map(|b| b.parent().unwrap().file_name().unwrap().to_owned()).collect(); + assert_eq!(names, ["v22.1.0", "v20.10.0", "v20.9.0", "v9.11.2"]); + } +} diff --git a/crates/ide-lsp/Cargo.toml b/crates/ide-lsp/Cargo.toml index 317f9319..ed05a90b 100644 --- a/crates/ide-lsp/Cargo.toml +++ b/crates/ide-lsp/Cargo.toml @@ -21,7 +21,6 @@ futures.workspace = true globset.workspace = true toml.workspace = true percent-encoding = "2" -which = "8" [dev-dependencies] tempfile.workspace = true diff --git a/crates/ide-lsp/src/launch.rs b/crates/ide-lsp/src/launch.rs index 782df3b8..3343763d 100644 --- a/crates/ide-lsp/src/launch.rs +++ b/crates/ide-lsp/src/launch.rs @@ -1,11 +1,12 @@ //! Starting server processes: binary resolution and the [`Launcher`] seam. use ide_core::CancellationToken; +use ide_core::tool_path::search_path; use ide_jsonrpc::{Handler, Peer}; use ide_language::LanguageServerConfig; use std::ffi::OsString; use std::path::{Path, PathBuf}; -use std::sync::{Arc, OnceLock}; +use std::sync::Arc; #[derive(Debug)] pub enum LaunchError { @@ -72,8 +73,7 @@ impl Launcher for ProcessLauncher { cancel: &CancellationToken, ) -> Result { let search = server_search_path(config); - let binary = which::which_in(&config.command, Some(&search), root) - .ok() + let binary = ide_core::tool_path::find_command(&config.command, &search, root) .and_then(|b| resolve_rustup_proxy(&config.command, b)); let mut env: Vec<(String, String)> = vec![("PATH".into(), search.to_string_lossy().into_owned())]; env.extend(config.env.iter().filter(|(k, _)| *k != "PATH").map(|(k, v)| (k.clone(), v.clone()))); @@ -95,7 +95,7 @@ impl Launcher for ProcessLauncher { } fn available(&self, config: &LanguageServerConfig, root: &Path) -> bool { - which::which_in(&config.command, Some(server_search_path(config)), root).is_ok() + ide_core::tool_path::find_command(&config.command, &server_search_path(config), root).is_some() } } @@ -104,9 +104,6 @@ fn server_search_path(config: &LanguageServerConfig) -> OsString { config.env.get("PATH").map(OsString::from).unwrap_or_else(search_path) } -/// `PATH` plus the login shell's `PATH` and common tool directories, so -/// servers resolve even when the app is launched from a GUI shell with a -/// minimal environment. /// `~/.cargo/bin/` is usually a rustup proxy that fails when the active /// toolchain lacks the component (common for rust-analyzer). In that case use /// the newest installed toolchain that ships the binary; with none, the tool is @@ -153,74 +150,6 @@ fn resolve_rustup_proxy(command: &str, binary: PathBuf) -> Option { } } -pub fn search_path() -> OsString { - static PATH: OnceLock = OnceLock::new(); - PATH.get_or_init(|| { - let mut dirs: Vec = - std::env::var_os("PATH").map(|p| std::env::split_paths(&p).collect()).unwrap_or_default(); - if let Some(shell_path) = login_shell_path() { - dirs.extend(std::env::split_paths(&shell_path)); - } - if let Some(home) = std::env::var_os("HOME").or_else(|| std::env::var_os("USERPROFILE")).map(PathBuf::from) { - for rel in [".cargo/bin", ".local/bin", "go/bin", ".bun/bin", ".deno/bin", ".npm-global/bin", ".volta/bin"] - { - dirs.push(home.join(rel)); - } - // nvm: newest installed node's bin. - if let Ok(entries) = std::fs::read_dir(home.join(".nvm/versions/node")) { - let mut versions: Vec = entries.flatten().map(|e| e.path().join("bin")).collect(); - versions.sort(); - dirs.extend(versions.into_iter().rev()); - } - } - for d in ["/opt/homebrew/bin", "/usr/local/bin", "/usr/bin", "/bin"] { - dirs.push(PathBuf::from(d)); - } - let mut seen = std::collections::HashSet::new(); - dirs.retain(|d| !d.as_os_str().is_empty() && seen.insert(d.clone())); - std::env::join_paths(dirs).unwrap_or_default() - }) - .clone() -} - -#[cfg(unix)] -fn login_shell_path() -> Option { - use std::io::Read; - use std::process::{Command, Stdio}; - use std::time::{Duration, Instant}; - let shell = std::env::var_os("SHELL")?; - const MARK: &str = "__OPENIDEA_PATH__"; - let mut child = ide_core::process::prepare(&mut Command::new(shell)) - .args(["-l", "-c", &format!("printf '{MARK}%s{MARK}' \"$PATH\"")]) - .stdin(Stdio::null()) - .stdout(Stdio::piped()) - .stderr(Stdio::null()) - .spawn() - .ok()?; - let deadline = Instant::now() + Duration::from_secs(3); - loop { - match child.try_wait() { - Ok(Some(_)) => break, - Ok(None) if Instant::now() < deadline => std::thread::sleep(Duration::from_millis(10)), - _ => { - let _ = child.kill(); - let _ = child.wait(); - return None; - } - } - } - let mut out = String::new(); - child.stdout.take()?.read_to_string(&mut out).ok()?; - let start = out.find(MARK)? + MARK.len(); - let end = start + out[start..].find(MARK)?; - Some(OsString::from(&out[start..end])) -} - -#[cfg(not(unix))] -fn login_shell_path() -> Option { - None -} - /// Workspace root for `file`: the outermost ancestor within `project_root` /// containing a root marker (so a Cargo/npm workspace gets one server, not one /// per member), else `project_root`. `None` when `file` is outside the project. @@ -258,6 +187,19 @@ mod tests { std::fs::remove_dir_all(&tmp).unwrap(); } + #[test] + fn marked_root_needs_a_marker() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + let file = root.join("apps/web/src/app.ts"); + std::fs::create_dir_all(file.parent().unwrap()).unwrap(); + let markers = vec!["angular.json".to_string(), "node_modules/@angular/core".to_string()]; + assert_eq!(marked_root(root, &file, &markers), None); + std::fs::create_dir_all(root.join("apps/web/node_modules/@angular/core")).unwrap(); + assert_eq!(marked_root(root, &file, &markers), Some(root.join("apps/web"))); + assert_eq!(marked_root(root, &root.join("apps/api/main.ts"), &markers), None); + } + #[test] fn search_path_contains_env_path() { let p = search_path(); diff --git a/crates/ide-lsp/src/lib.rs b/crates/ide-lsp/src/lib.rs index 785a1d79..403a8219 100644 --- a/crates/ide-lsp/src/lib.rs +++ b/crates/ide-lsp/src/lib.rs @@ -24,7 +24,7 @@ mod uri; mod virtual_doc; pub use inspect::{FileInspection, InspectHost, InspectOptions}; -pub use launch::{LaunchError, Launcher, ProcessLauncher, search_path}; +pub use launch::{LaunchError, Launcher, ProcessLauncher}; pub use scope::{CandidateFiles, CandidateFn, CandidateQuery, MAX_CANDIDATES}; pub use uri::{is_virtual_uri, path_to_uri_string, uri_target, uri_to_path}; @@ -1921,9 +1921,22 @@ impl LspManager { /// Servers no longer enabled stop; newly enabled ones attach to open documents. pub async fn reconfigure(&self) { self.stop_unlisted().await; + let Some(me) = self.me.upgrade() else { return }; let servers: Vec> = self.servers.lock().clone(); for s in servers { - let next = self.configured(&s); + // The settings hook may run tools (`npm root -g`), as when the server starts. + let next = match tokio::task::spawn_blocking({ + let (me, s) = (me.clone(), s.clone()); + move || me.configured(&s) + }) + .await + { + Ok(next) => next, + Err(e) => { + tracing::warn!("{}: settings hook failed: {e}", s.name); + continue; + } + }; let current = s.effective_config(); if next == current { continue; diff --git a/crates/ide-sdk/Cargo.toml b/crates/ide-sdk/Cargo.toml index d4225ada..8d9a24e7 100644 --- a/crates/ide-sdk/Cargo.toml +++ b/crates/ide-sdk/Cargo.toml @@ -17,7 +17,6 @@ serde_json.workspace = true specta.workspace = true toml.workspace = true tracing.workspace = true -which = "8" zip.workspace = true [dev-dependencies] diff --git a/crates/ide-sdk/src/angular.rs b/crates/ide-sdk/src/angular.rs index d7566418..54c5591a 100644 --- a/crates/ide-sdk/src/angular.rs +++ b/crates/ide-sdk/src/angular.rs @@ -1,15 +1,18 @@ //! Angular Language Server (`ngserver`): its package, where TypeScript and //! `@angular/language-service` come from, and the project's Angular version. -use crate::npm::package_dir; +use crate::npm::{package_dir, shim_package}; +use std::collections::BTreeMap; +use std::ffi::OsStr; use std::path::{Path, PathBuf}; pub(crate) const SERVER: &str = "angular-language-server"; const PACKAGE: &str = "@angular/language-server"; -/// `@angular/language-server`'s directory for an `ngserver` executable. -pub(crate) fn server_package(bin: &Path) -> Option { - package_dir(bin, PACKAGE) +/// `@angular/language-server`'s directory for an `ngserver` executable found on +/// `path` and run in `cwd` with `env`. +pub(crate) fn server_package(bin: &Path, path: &OsStr, cwd: &Path, env: &BTreeMap) -> Option { + package_dir(bin, PACKAGE).or_else(|| shim_package(bin, PACKAGE, path, cwd, env)) } /// Why an `ngserver` whose package isn't found can't start: it loads TypeScript diff --git a/crates/ide-sdk/src/apply.rs b/crates/ide-sdk/src/apply.rs index acd00692..527e1924 100644 --- a/crates/ide-sdk/src/apply.rs +++ b/crates/ide-sdk/src/apply.rs @@ -519,8 +519,8 @@ fn configure_vue_typescript(cfg: &mut LanguageServerConfig, root: &Path, server_ fn configure_angular(cfg: &mut LanguageServerConfig, root: &Path, server_root: &Path, s: &Settings) { // Found as the launcher finds it; missing, it asks to be installed. let path = cfg.env.get("PATH").map_or_else(search_path, OsString::from); - let Ok(bin) = which::which_in(&cfg.command, Some(path), server_root) else { return }; - let Some(package) = angular::server_package(&bin) else { + let Some(bin) = ide_core::tool_path::find_command(&cfg.command, &path, server_root) else { return }; + let Some(package) = angular::server_package(&bin, &path, server_root, &cfg.env) else { cfg.launch_error = Some(angular::missing_package_message(&bin)); return; }; diff --git a/crates/ide-sdk/src/exec.rs b/crates/ide-sdk/src/exec.rs index 03333c50..aef10b05 100644 --- a/crates/ide-sdk/src/exec.rs +++ b/crates/ide-sdk/src/exec.rs @@ -3,35 +3,13 @@ use ide_core::output_encoding::decode_tool_output; use ide_core::{CancellationToken, IdeError, IdeResult}; -use std::ffi::OsString; use std::io::{BufRead, BufReader, Read}; use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; use std::sync::mpsc; use std::time::{Duration, Instant}; -/// The process `PATH` (already the login shell's in the app) plus common tool dirs. -pub fn search_path() -> OsString { - let mut dirs: Vec = - std::env::var_os("PATH").map(|p| std::env::split_paths(&p).collect()).unwrap_or_default(); - if let Some(home) = crate::home_dir() { - for rel in [".cargo/bin", ".local/bin", "go/bin", ".pyenv/shims", ".bun/bin", ".npm-global/bin", ".volta/bin"] { - dirs.push(home.join(rel)); - } - // nvm: newest installed node's bin, as language servers are launched. - if let Ok(entries) = std::fs::read_dir(home.join(".nvm/versions/node")) { - let mut versions: Vec = entries.flatten().map(|e| e.path().join("bin")).collect(); - versions.sort(); - dirs.extend(versions.into_iter().rev()); - } - } - for d in ["/opt/homebrew/bin", "/usr/local/bin", "/usr/bin", "/bin"] { - dirs.push(PathBuf::from(d)); - } - let mut seen = std::collections::HashSet::new(); - dirs.retain(|d| !d.as_os_str().is_empty() && seen.insert(d.clone())); - std::env::join_paths(dirs).unwrap_or_default() -} +pub(crate) use ide_core::tool_path::search_path; /// `dir` in front of the search path, as a `PATH` value. pub(crate) fn path_with(dirs: &[&Path]) -> String { @@ -45,7 +23,7 @@ pub(crate) fn path_with(dirs: &[&Path]) -> String { /// Resolves a command name on the search path. pub(crate) fn which(name: &str) -> Option { let path = search_path(); - std::env::split_paths(&path).map(|d| d.join(crate::exe(name))).find(|p| p.is_file()) + ide_core::process::find_executable(name, &path) } /// Runs `program args` and returns trimmed stdout (stderr when stdout is empty) @@ -66,27 +44,41 @@ pub(crate) fn query_in( if let Some(cwd) = cwd { cmd.current_dir(cwd); } + if !env.iter().any(|(k, _)| *k == "PATH") { + cmd.env("PATH", search_path()); + } let mut child = ide_core::process::prepare(&mut cmd) .args(args) - .env("PATH", search_path()) .envs(env.iter().copied()) .stdin(Stdio::null()) .stdout(Stdio::piped()) .stderr(Stdio::piped()) .spawn() .ok()?; - let mut stdout = child.stdout.take()?; - let mut stderr = child.stderr.take()?; - let out = std::thread::spawn(move || { - let mut s = Vec::new(); - let _ = stdout.read_to_end(&mut s); - decode_tool_output(&s) - }); - let err = std::thread::spawn(move || { - let mut s = Vec::new(); - let _ = stderr.read_to_end(&mut s); - decode_tool_output(&s) - }); + // Chunks as they come, `None` at the end of a pipe. A reader stops once the query + // has returned, closing the pipe on whatever still writes to it. + let (tx, rx) = mpsc::channel::<(bool, Option>)>(); + let pipes: [(bool, Box); 2] = + [(true, Box::new(child.stdout.take()?)), (false, Box::new(child.stderr.take()?))]; + for (is_out, mut pipe) in pipes { + let tx = tx.clone(); + std::thread::spawn(move || { + let mut buf = [0u8; 4096]; + loop { + let chunk = match pipe.read(&mut buf) { + Ok(0) => None, + Ok(n) => Some(buf[..n].to_vec()), + Err(e) if e.kind() == std::io::ErrorKind::Interrupted => continue, + Err(_) => None, + }; + let end = chunk.is_none(); + if tx.send((is_out, chunk)).is_err() || end { + return; + } + } + }); + } + drop(tx); let deadline = Instant::now() + timeout; let status = loop { match child.try_wait() { @@ -99,12 +91,27 @@ pub(crate) fn query_in( } } }; - let (out, err) = (out.join().unwrap_or_default(), err.join().unwrap_or_default()); if !status.success() { return None; } - let text = if out.trim().is_empty() { err } else { out }; - Some(text.trim().to_owned()) + // Up to the deadline, or briefly past it for a program that exited just before: + // a process it left running (a wrapper's background job) can hold the pipes + // open, or keep writing to them, after it exits. + let until = deadline.max(Instant::now() + Duration::from_millis(100)); + let (mut out, mut err, mut open) = (Vec::new(), Vec::new(), 2); + while open > 0 { + let left = until.saturating_duration_since(Instant::now()); + if left.is_zero() { + break; + } + match rx.recv_timeout(left) { + Ok((is_out, Some(chunk))) => if is_out { &mut out } else { &mut err }.extend(chunk), + Ok((_, None)) => open -= 1, + Err(_) => break, + } + } + let text = if out.trim_ascii().is_empty() { err } else { out }; + Some(decode_tool_output(&text).trim().to_owned()) } /// Runs a command to completion, reporting every output line (stdout and diff --git a/crates/ide-sdk/src/godot.rs b/crates/ide-sdk/src/godot.rs index 31999cbe..bdc888da 100644 --- a/crates/ide-sdk/src/godot.rs +++ b/crates/ide-sdk/src/godot.rs @@ -23,7 +23,7 @@ pub fn find(custom: Option) -> Option { if let Some(p) = custom { return Some(p); } - find_in(&crate::search_path(), &install_locations(crate::home_dir().as_deref())) + find_in(&crate::exec::search_path(), &install_locations(crate::home_dir().as_deref())) } pub(crate) fn find_in(search: &OsStr, locations: &[PathBuf]) -> Option { diff --git a/crates/ide-sdk/src/lib.rs b/crates/ide-sdk/src/lib.rs index 66ebdbbf..3d178c62 100644 --- a/crates/ide-sdk/src/lib.rs +++ b/crates/ide-sdk/src/lib.rs @@ -25,7 +25,6 @@ use specta::Type; use std::path::{Path, PathBuf}; pub use discover::{detect, jdk_sdk}; -pub use exec::search_path; /// What [`detect`] looks for. #[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize, Type)] diff --git a/crates/ide-sdk/src/npm.rs b/crates/ide-sdk/src/npm.rs index 15a58ec2..89c94edb 100644 --- a/crates/ide-sdk/src/npm.rs +++ b/crates/ide-sdk/src/npm.rs @@ -1,7 +1,9 @@ //! Node-based language servers installed with npm: the package behind a command. +use std::collections::BTreeMap; use std::ffi::OsStr; use std::path::{Path, PathBuf}; +use std::time::Duration; /// `command` as given (a path) or found on `path`, npm shims included. pub(crate) fn find_command(command: &str, path: &OsStr) -> Option { @@ -9,15 +11,13 @@ pub(crate) fn find_command(command: &str, path: &OsStr) -> Option { if p.is_absolute() { return p.is_file().then(|| p.to_path_buf()); } - let exts: &[&str] = if cfg!(windows) { &[".cmd", ".exe", ""] } else { &[""] }; - std::env::split_paths(path) - .flat_map(|d| exts.iter().map(move |e| d.join(format!("{command}{e}")))) - .find(|c| c.is_file()) + ide_core::process::find_executable(command, path) } /// The npm package an executable comes from: next to a `node_modules/.bin` shim /// (the IDE's tools dir, a project), a Windows global shim (`%APPDATA%\npm`), a -/// Unix global `/bin` or a symlink into the package. +/// Unix global `/bin`, a symlink into the package, Volta's package image +/// or the package a shim script runs (pnpm). pub(crate) fn package_dir(bin: &Path, package: &str) -> Option { let dir = bin.parent()?; let mut candidates = vec![dir.join("node_modules").join(package)]; @@ -31,6 +31,197 @@ pub(crate) fn package_dir(bin: &Path, package: &str) -> Option { } if let Some(prefix) = dir.parent() { candidates.push(prefix.join("lib").join("node_modules").join(package)); + // Volta: `/bin/` runs `/tools/image/packages/`. + let image = prefix.join("tools").join("image").join("packages").join(package); + candidates.push(image.join("lib").join("node_modules").join(package)); + candidates.push(image.join("node_modules").join(package)); + } + let installed = |c: &PathBuf| c.join("package.json").is_file(); + candidates.into_iter().find(installed).or_else(|| script_targets(bin, package).into_iter().find(installed)) +} + +/// How shim scripts name their own directory: sh and PowerShell `$basedir` (pnpm +/// 11's sh shims also `$basedir_abs` and `$basedir_win`), cmd `%~dp0` (pnpm's +/// `%dp0%` is set from it). +const SCRIPT_DIR: [&str; 8] = + ["${basedir}", "$basedir", "${basedir_abs}", "$basedir_abs", "${basedir_win}", "$basedir_win", "%~dp0", "%dp0%"]; + +/// `path` after the [`SCRIPT_DIR`] it starts with. sh reads `$basedir_abs` as one +/// name, not as `$basedir` followed by `_abs`. +fn after_script_dir(path: &str) -> Option<&str> { + SCRIPT_DIR.iter().find_map(|d| { + let rest = path.strip_prefix(d)?; + let longer_name = d.starts_with('$') + && !d.ends_with('}') + && rest.starts_with(|c: char| c == '_' || c.is_ascii_alphanumeric()); + (!longer_name).then_some(rest) + }) +} + +/// The `package` directories a shim script runs from: relative to the script's +/// directory, e.g. pnpm's `"$basedir/../global/v11//node_modules//bin/"` +/// or `"%~dp0\..\global\..."`, or absolute. +fn script_targets(bin: &Path, package: &str) -> Vec { + const MAX_SCRIPT: u64 = 64 * 1024; + if std::fs::metadata(bin).map_or(true, |m| m.len() > MAX_SCRIPT) { + return Vec::new(); + } + let Ok(bytes) = std::fs::read(bin) else { return Vec::new() }; + let text = String::from_utf8_lossy(&bytes).replace('\\', "/"); + let bases: Vec = [ + bin.parent().map(Path::to_path_buf), + dunce::canonicalize(bin).ok().and_then(|r| r.parent().map(Path::to_path_buf)), + ] + .into_iter() + .flatten() + .collect(); + let needle = format!("node_modules/{package}"); + // What sh and cmd split words on; a folder name may contain a no-break space. + let separator = |c: char| c == '"' || c == '\'' || c.is_ascii_whitespace(); + let mut targets = Vec::new(); + for (at, _) in text.match_indices(&needle) { + let end = at + needle.len(); + if !text[end..].chars().next().is_none_or(|c| c == '/' || separator(c)) { + continue; + } + // Within quotes the path starts at the opening one and may contain spaces. + let line = text[..at].rfind('\n').map_or(0, |i| i + 1); + let start = match ['"', '\''].into_iter().find(|q| text[line..at].matches(*q).count() % 2 == 1) { + Some(quote) => text[..at].rfind(quote), + None => text[..at].rfind(separator), + } + .map_or(0, |i| i + 1); + let path = &text[start..end]; + // `"$basedir"/../...`: the directory quoted on its own. + let after_dir = text[..start] + .strip_suffix(['"', '\'']) + .is_some_and(|before| SCRIPT_DIR.iter().any(|d| before.ends_with(d))); + match after_script_dir(path).or(after_dir.then_some(path)) { + Some(rest) => targets.extend(bases.iter().map(|b| b.join(rest.trim_start_matches('/')))), + None if Path::new(path).is_absolute() => targets.push(PathBuf::from(path)), + // `$HOME/...`, `%APPDATA%\...` or relative to wherever the script runs. + None => {} + } + } + targets.into_iter().map(|t| dunce::canonicalize(&t).unwrap_or(t)).collect() +} + +/// For a version manager's shim (`~/.asdf/shims`, mise's `shims`), which names +/// only the command: `package` in the global `node_modules` of the manager's +/// npm shim, run as the server is (in `cwd`, with its `env` and `path`), so the +/// manager picks the same Node. The shims go first on `path`, as the manager's +/// shell setup puts them: npm (`#!/usr/bin/env node`) then runs on the manager's +/// Node, not on a system one earlier on `path` that has another global root. +pub(crate) fn shim_package( + bin: &Path, + package: &str, + path: &OsStr, + cwd: &Path, + env: &BTreeMap, +) -> Option { + let shims = bin.parent()?; + if shims.file_name() != Some(OsStr::new("shims")) { + return None; + } + // Only the npm shim next to it: any other npm may be another installation's. + let exts: &[&str] = if cfg!(windows) { &[".cmd", ".exe"] } else { &[""] }; + let npm = exts.iter().map(|e| shims.join(format!("npm{e}"))).find(|p| p.is_file())?; + let path = std::env::join_paths(std::iter::once(shims.to_path_buf()).chain(std::env::split_paths(path))).ok()?; + // The login shell's variables (`ASDF_DATA_DIR`, `MISE_ENV`...) under the server's own. + let shell_env: Vec<_> = ide_core::process::shell_env().collect(); + let mut env: Vec<(&str, &str)> = shell_env + .iter() + .filter_map(|(k, v)| Some((k.to_str()?, v.to_str()?))) + .chain(env.iter().map(|(k, v)| (k.as_str(), v.as_str()))) + .collect(); + env.push(("PATH", path.to_str()?)); + let root = crate::exec::query_in(&npm, &["root", "-g"], Some(cwd), &env, Duration::from_secs(10))?; + let dir = PathBuf::from(root.lines().last()?.trim()).join(package); + dir.join("package.json").is_file().then_some(dir) +} + +#[cfg(test)] +mod tests { + use super::*; + + const PACKAGE: &str = "@angular/language-server"; + + /// `/bin/ngserver` running `script`. + fn shim(dir: &Path, script: &str) -> PathBuf { + std::fs::create_dir_all(dir.join("bin")).unwrap(); + let shim = dir.join("bin/ngserver"); + std::fs::write(&shim, script).unwrap(); + shim + } + + /// The package installed in `/node_modules`. + fn install(dir: &Path) -> PathBuf { + let package = dir.join("node_modules").join(PACKAGE); + std::fs::create_dir_all(&package).unwrap(); + std::fs::write(package.join("package.json"), "{}").unwrap(); + dunce::canonicalize(package).unwrap() + } + + #[test] + fn shim_scripts_name_the_package_relative_to_their_directory() { + let tmp = tempfile::tempdir().unwrap(); + let package = install(&tmp.path().join("global/v11/abc")); + for script in [ + "#!/bin/sh\nexec node \"$basedir/../global/v11/abc/node_modules/@angular/language-server/bin/ngserver\" \"$@\"\n", + "#!/bin/sh\nexec node \"${basedir}/../global/v11/abc/node_modules/@angular/language-server/bin/ngserver\" \"$@\"\n", + "#!/bin/sh\nexec node \"$basedir\"/../global/v11/abc/node_modules/@angular/language-server/bin/ngserver \"$@\"\n", + // pnpm 11. + "#!/bin/sh\nexec \"$basedir/node\" \"$basedir_abs/../global/v11/abc/node_modules/@angular/language-server/bin/ngserver\" \"$@\"\n", + "#!/bin/sh\nexec node \"${basedir_abs}/../global/v11/abc/node_modules/@angular/language-server/bin/ngserver\" \"$@\"\n", + "#!/bin/sh\nexec node.exe \"$basedir_win/../global/v11/abc/node_modules/@angular/language-server/bin/ngserver\" \"$@\"\n", + "@\"%~dp0\\..\\global\\v11\\abc\\node_modules\\@angular\\language-server\\bin\\ngserver\" %*\r\n", + "@\"%~dp0..\\global\\v11\\abc\\node_modules\\@angular\\language-server\\bin\\ngserver\" %*\r\n", + "@SET dp0=%~dp0\r\n\"%dp0%\\..\\global\\v11\\abc\\node_modules\\@angular\\language-server\\bin\\ngserver\" %*\r\n", + ] { + assert_eq!(package_dir(&shim(tmp.path(), script), PACKAGE), Some(package.clone()), "{script}"); + } + } + + #[test] + fn quoted_shim_targets_keep_their_spaces() { + let tmp = tempfile::tempdir().unwrap(); + let package = install(&tmp.path().join("Global Packages/abc")); + let absolute = package.display().to_string(); + for script in [ + "#!/bin/sh\nexec node \"$basedir/../Global Packages/abc/node_modules/@angular/language-server/bin/ngserver\" \"$@\"\n" + .to_owned(), + "@\"%~dp0\\..\\Global Packages\\abc\\node_modules\\@angular\\language-server\\bin\\ngserver\" %*\r\n".to_owned(), + format!("#!/bin/sh\nexec node \"{absolute}/bin/ngserver\" \"$@\"\n"), + format!("#!/bin/sh\nexec node '{absolute}/bin/ngserver' \"$@\"\n"), + ] { + assert_eq!(package_dir(&shim(tmp.path(), &script), PACKAGE), Some(package.clone()), "{script}"); + } + } + + #[test] + fn shim_scripts_with_an_absolute_path_and_non_ascii_spaces() { + let tmp = tempfile::tempdir().unwrap(); + let package = install(&tmp.path().join("山田\u{3000}太郎\u{a0}x")); + let script = format!("#!/bin/sh\nexec node \"{}/bin/ngserver\" \"$@\"\n", package.display()); + assert_eq!(package_dir(&shim(tmp.path(), &script), PACKAGE), Some(package)); + } + + #[test] + fn shim_scripts_skip_other_packages_and_other_directories() { + let tmp = tempfile::tempdir().unwrap(); + install(&tmp.path().join("home")); + for script in [ + "exec node \"$basedir/../home/node_modules/@angular/language-server-next/bin/ngserver\"\n", + "exec node \"$HOME/node_modules/@angular/language-server/bin/ngserver\"\n", + "exec node \"$basedirx/../home/node_modules/@angular/language-server/bin/ngserver\"\n", + "exec node ../home/node_modules/@angular/language-server/bin/ngserver\n", + ] { + assert_eq!(script_targets(&shim(tmp.path(), script), PACKAGE), Vec::::new(), "{script}"); + } + let large = format!( + "{}\n\"$basedir/../home/node_modules/@angular/language-server/bin/ngserver\"\n", + "#".repeat(64 * 1024) + ); + assert_eq!(script_targets(&shim(tmp.path(), &large), PACKAGE), Vec::::new()); } - candidates.into_iter().find(|c| c.join("package.json").is_file()) } diff --git a/crates/ide-sdk/src/tests.rs b/crates/ide-sdk/src/tests.rs index 92fda6da..7af792e0 100644 --- a/crates/ide-sdk/src/tests.rs +++ b/crates/ide-sdk/src/tests.rs @@ -673,6 +673,160 @@ fn angular_language_server_probes_its_own_package_then_the_project() { assert_eq!((cfg.launch_error, cfg.args), (None, vec!["--stdio".to_owned()])); } +#[test] +fn angular_language_server_is_found_behind_version_manager_shims() { + let tmp = tempfile::tempdir().unwrap(); + let project = tmp.path().join("app"); + std::fs::create_dir_all(&project).unwrap(); + let install = |node_modules: &Path| { + let package = node_modules.join("@angular/language-server"); + std::fs::create_dir_all(&package).unwrap(); + std::fs::write(package.join("package.json"), "{}").unwrap(); + package + }; + let probes = |command: &Path, s: &Settings, env: &[(&str, &str)]| { + let mut cfg = server("angular-language-server"); + cfg.command = command.to_string_lossy().into_owned(); + cfg.env.extend(env.iter().map(|(k, v)| (k.to_string(), v.to_string()))); + configure_server(&mut cfg, &project, &project, s); + cfg.args.iter().position(|a| a == "--ngProbeLocations").map(|i| cfg.args[i + 1].clone()) + }; + let with_project = |package: &Path| Some(format!("{},{}", package.to_string_lossy(), project.to_string_lossy())); + + // pnpm: the shim script runs the package from pnpm's global directory. + let pnpm = tmp.path().join("pnpm"); + let package = install(&pnpm.join("global/v11/abc/node_modules")); + let shim = npm_shim(&pnpm.join("bin"), "ngserver"); + let script = "exec node \"$basedir/../global/v11/abc/node_modules/@angular/language-server/bin/ngserver\" \"$@\"\n"; + std::fs::write(&shim, format!("#!/bin/sh\n{script}")).unwrap(); + assert_eq!(probes(&shim, &Settings::default(), &[]), with_project(&dunce::canonicalize(&package).unwrap())); + + // Volta: the package's image, next to the shim's directory. Joined as the lookup + // joins it: the probe locations compare as text, and Windows keeps a `/` as given. + let volta = tmp.path().join("volta"); + let image = volta.join("tools").join("image").join("packages").join("@angular/language-server"); + let package = install(&image.join("lib").join("node_modules")); + let shim = npm_shim(&volta.join("bin"), "ngserver"); + assert_eq!(probes(&shim, &Settings::default(), &[]), with_project(&package)); + + // asdf, mise: the shim names only the command; the manager's npm shim knows + // where global packages are, run as the server is: in the project (whose + // `.tool-versions` picks the Node), with the login shell's and its own variables. + let root_of = |package: &Path| package.parent().unwrap().parent().unwrap().display().to_string(); + let package = install(&tmp.path().join("asdf/installs/nodejs/24/lib/node_modules")); + std::fs::write(project.join(".tool-versions"), "nodejs 24\n").unwrap(); + // The only writer in this test binary. + ide_core::process::set_shell_env(vec![("OPENIDEA_TEST_SHELL".into(), "1".into())]); + let shims = tmp.path().join(".asdf/shims"); + // The manager's npm runs on the first Node on PATH (`#!/usr/bin/env node`). + std::fs::write( + npm_shim(&shims, "npm"), + if cfg!(windows) { + "@if exist .tool-versions if \"%OPENIDEA_TEST_SHELL%\"==\"1\" call node\r\n" + } else { + "#!/bin/sh\n[ -f .tool-versions ] && [ \"$OPENIDEA_TEST_SHELL\" = 1 ] && exec node\n" + }, + ) + .unwrap(); + std::fs::write( + npm_shim(&shims, "node"), + if cfg!(windows) { + "@echo %OPENIDEA_TEST_NPM_ROOT%\r\n" + } else { + "#!/bin/sh\necho \"$OPENIDEA_TEST_NPM_ROOT\"\n" + }, + ) + .unwrap(); + // Another installation's npm and Node, first on PATH. + let node = tmp.path().join("node/bin"); + let other = root_of(&install(&tmp.path().join("node/lib/node_modules"))); + for name in ["npm", "node"] { + std::fs::write( + npm_shim(&node, name), + if cfg!(windows) { format!("@echo {other}\r\n") } else { format!("#!/bin/sh\necho '{other}'\n") }, + ) + .unwrap(); + } + let mut s = Settings::default(); + s.languages.node.interpreter = node.join("node").to_string_lossy().into_owned(); + let root = root_of(&package); + let env = [("OPENIDEA_TEST_NPM_ROOT", root.as_str())]; + assert_eq!(probes(&npm_shim(&shims, "ngserver"), &s, &env), with_project(&package)); + // Not without the manager's own npm, nor for an executable elsewhere: the npm + // on PATH may be another installation's. + assert_eq!(probes(&npm_shim(&tmp.path().join("mise/shims"), "ngserver"), &s, &env), None); + assert_eq!(probes(&npm_shim(&tmp.path().join("bin"), "ngserver"), &s, &env), None); +} + +#[cfg(unix)] +#[test] +fn query_output_is_read_within_the_deadline() { + // Keep process startup outside a tight timing budget even under workspace load. + let timeout = std::time::Duration::from_secs(60); + let env = [("PATH", "/usr/bin:/bin")]; + let started = std::time::Instant::now(); + let out = crate::exec::query_in(Path::new("/bin/sh"), &["-c", "printf /global/root"], None, &env, timeout); + assert_eq!(out.as_deref(), Some("/global/root")); + // EOF must return promptly, without waiting for the generous deadline. + assert!(started.elapsed() < std::time::Duration::from_secs(30)); +} + +#[cfg(unix)] +#[test] +fn query_drains_both_pipes_before_the_child_exits() { + let out = crate::exec::query_in( + Path::new("/bin/sh"), + &["-c", "head -c 262144 /dev/zero; head -c 262144 /dev/zero >&2"], + None, + &[("PATH", "/usr/bin:/bin")], + std::time::Duration::from_secs(60), + ) + .unwrap(); + assert_eq!(out.len(), 262144); +} + +#[cfg(unix)] +#[test] +fn query_is_bounded_when_descendants_keep_pipes_open() { + let tmp = tempfile::tempdir().unwrap(); + let pid = tmp.path().join("pid"); + // The descendant outlives the query; clean it up even if an assertion fails. + struct Descendant(std::path::PathBuf); + impl Drop for Descendant { + fn drop(&mut self) { + if let Ok(pid) = std::fs::read_to_string(&self.0) { + let _ = std::process::Command::new("/bin/kill").arg(pid.trim()).status(); + } + } + } + let _descendant = Descendant(pid.clone()); + let started = std::time::Instant::now(); + let out = crate::exec::query_in( + Path::new("/bin/sh"), + &["-c", "sleep 120 & echo $! > \"$1\"; printf /global/root", "wrapper", pid.to_str().unwrap()], + None, + &[("PATH", "/usr/bin:/bin")], + std::time::Duration::from_secs(30), + ); + assert_eq!(out.as_deref(), Some("/global/root")); + // The descendant keeps both pipes open for longer than the query deadline. + assert!(started.elapsed() < std::time::Duration::from_secs(90)); +} + +#[cfg(unix)] +#[test] +fn query_times_out_when_the_child_does_not_exit() { + // exec avoids leaving a descendant behind when the timed-out child is killed. + let out = crate::exec::query_in( + Path::new("/bin/sh"), + &["-c", "printf /global/root; exec sleep 120"], + None, + &[("PATH", "/usr/bin:/bin")], + std::time::Duration::from_millis(100), + ); + assert_eq!(out, None); +} + #[test] fn max_java_picks_an_older_jdk() { let dir = tempfile::tempdir().unwrap(); diff --git a/crates/ide-tools/src/jdk.rs b/crates/ide-tools/src/jdk.rs index 279df197..b5bcf6db 100644 --- a/crates/ide-tools/src/jdk.rs +++ b/crates/ide-tools/src/jdk.rs @@ -448,7 +448,7 @@ mod tests { _ => None, }; if let Some(platform) = other { - let tools = Tools::with_paths(crate::tools_dir(tmp.path()), crate::default_search_path(), platform); + let tools = Tools::with_paths(crate::tools_dir(tmp.path()), ide_core::tool_path::search_path(), platform); let jdk = tools.download_jdk(TEMURIN, feature, None, &CancellationToken::new(), &|_| {}).unwrap(); let home = jdk.home.into_path_buf(); eprintln!("{}", home.display()); diff --git a/crates/ide-tools/src/lib.rs b/crates/ide-tools/src/lib.rs index ccbeb890..e32d1002 100644 --- a/crates/ide-tools/src/lib.rs +++ b/crates/ide-tools/src/lib.rs @@ -174,28 +174,6 @@ pub fn path_with_tools(app_data: &Path, path: &std::ffi::OsStr) -> OsString { std::env::join_paths(dirs).unwrap_or_else(|_| path.to_owned()) } -/// `PATH` plus the usual per-user tool locations (GUI apps on macOS start with a minimal `PATH`). -pub fn default_search_path() -> OsString { - let mut dirs: Vec = - ide_core::process::host_var_os("PATH").map(|p| std::env::split_paths(&p).collect()).unwrap_or_default(); - if let Some(home) = std::env::var_os("HOME").or_else(|| std::env::var_os("USERPROFILE")).map(PathBuf::from) { - for rel in [".cargo/bin", ".local/bin", "go/bin", ".bun/bin", ".volta/bin", ".npm-global/bin", "Library/pnpm"] { - dirs.push(home.join(rel)); - } - if let Ok(entries) = std::fs::read_dir(home.join(".nvm/versions/node")) { - let mut versions: Vec = entries.flatten().map(|e| e.path().join("bin")).collect(); - versions.sort(); - dirs.extend(versions.into_iter().rev()); - } - } - for d in ["/opt/homebrew/bin", "/usr/local/go/bin", "/usr/local/bin", "/usr/bin", "/bin"] { - dirs.push(PathBuf::from(d)); - } - let mut seen = HashSet::new(); - dirs.retain(|d| !d.as_os_str().is_empty() && seen.insert(d.clone())); - std::env::join_paths(dirs).unwrap_or_default() -} - static INSTALLING: Mutex>> = Mutex::new(None); struct InstallGuard(String); @@ -331,12 +309,13 @@ pub struct Tools { custom: BTreeMap, /// Executables found outside the search path (app bundles), by command. found: BTreeMap, + version_timeout: Duration, } impl Tools { - /// Tools under `/tools` using [`default_search_path`]. + /// Tools under `/tools` using [`ide_core::tool_path::search_path`]. pub fn new(app_data: &Path) -> Self { - Self::with_paths(tools_dir(app_data), default_search_path(), Platform::current()) + Self::with_paths(tools_dir(app_data), ide_core::tool_path::search_path(), Platform::current()) } pub fn with_paths(root: PathBuf, search_path: OsString, platform: Platform) -> Self { @@ -344,7 +323,15 @@ impl Tools { let system: Vec = std::env::split_paths(&search_path).filter(|d| !ours.contains(d)).collect(); let system_path = std::env::join_paths(&system).unwrap_or_default(); let child_path = std::env::join_paths(ours.iter().chain(system.iter())).unwrap_or_default(); - Self { root, system_path, child_path, platform, custom: BTreeMap::new(), found: BTreeMap::new() } + Self { + root, + system_path, + child_path, + platform, + custom: BTreeMap::new(), + found: BTreeMap::new(), + version_timeout: Duration::from_secs(4), + } } /// Reports the executables chosen in settings (command → path) as the tools' location. @@ -364,7 +351,7 @@ impl Tools { } fn which(&self, cmd: &str) -> Option { - which::which_in(cmd, Some(&self.system_path), &self.root).ok() + ide_core::tool_path::find_command(cmd, &self.system_path, &self.root) } fn which_any(&self, cmds: &[&str]) -> Option { @@ -428,7 +415,7 @@ impl Tools { let program = sibling.or_else(|| self.which(cmd))?; let mut c = self.command(program); c.args(args); - let (ok, out) = run_quiet(c, Duration::from_secs(4))?; + let (ok, out) = run_quiet(c, self.version_timeout)?; if !ok { return None; } @@ -548,7 +535,7 @@ impl Tools { if cmd == spec.commands[0] { path.clone() } else { path.parent().and_then(|d| self.exe_in(d, cmd))? }; let mut c = self.command(program); c.args(args); - let (ok, out) = run_quiet(c, Duration::from_secs(4))?; + let (ok, out) = run_quiet(c, self.version_timeout)?; if ok { parse_version(&out) } else { None } }); info.status = ToolStatus::Custom; diff --git a/crates/ide-tools/src/server.rs b/crates/ide-tools/src/server.rs index 3e3481c4..d62f2af6 100644 --- a/crates/ide-tools/src/server.rs +++ b/crates/ide-tools/src/server.rs @@ -2,7 +2,7 @@ //! version publishes one per supported host platform (see `release.yml`). use crate::process::Http; -use crate::{InstallEvent, default_search_path, fetch_verified, sources}; +use crate::{InstallEvent, fetch_verified, sources}; use ide_core::CancellationToken; use ide_core::{IdeError, IdeResult}; use std::path::{Path, PathBuf}; @@ -46,7 +46,7 @@ pub fn fetch( std::fs::create_dir_all(dir)?; let staging = tempfile_dir(dir)?; let result = (|| { - let http = Http::new(&default_search_path())?; + let http = Http::new(&ide_core::tool_path::search_path())?; // The binary runs on the remote host: never install it unverified. let checksum_url = format!("{url}.sha256"); let sha256 = sources::parse_checksum(&http.get_text(&checksum_url, cancel)?, &name) diff --git a/crates/ide-tools/src/tests.rs b/crates/ide-tools/src/tests.rs index b9a2ec48..02226cff 100644 --- a/crates/ide-tools/src/tests.rs +++ b/crates/ide-tools/src/tests.rs @@ -24,7 +24,10 @@ impl Env { fn tools(&self) -> Tools { let mut dirs = vec![self.sys.clone()]; dirs.extend(bin_dirs_in(&self.root, Platform::current())); - Tools::with_paths(self.root.clone(), std::env::join_paths(dirs).unwrap(), Platform::current()) + let mut tools = Tools::with_paths(self.root.clone(), std::env::join_paths(dirs).unwrap(), Platform::current()); + // These fixtures test detection and version parsing, not process startup latency. + tools.version_timeout = Duration::from_secs(60); + tools } #[cfg(unix)] From 3563f70810fa7f28e260e16edaea6cf3d9fa56ef Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Fri, 30 Oct 2026 13:16:44 +0800 Subject: [PATCH 3/7] fix(settings): keep the settings window on screen on Windows and Linux secondary displays (#357) Fitting the settings window to the monitor work area left it invisible on a Windows secondary display. The fitting now applies on macOS only; Windows and Linux centre the window over its owner as before. Fixes #325 --- src-tauri/src/windows.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src-tauri/src/windows.rs b/src-tauri/src/windows.rs index 74edba5f..133e3b70 100644 --- a/src-tauri/src/windows.rs +++ b/src-tauri/src/windows.rs @@ -713,14 +713,26 @@ pub fn open_settings_window( .map_err(|e| IdeError::internal(format!("cannot create settings window: {e}")))?; apply_zoom(app, &window); // Restore runs during build; fit its size to the parent's current display before showing. + #[cfg(target_os = "macos")] if let Err(e) = fit_settings_window(&window, &parent) { tracing::warn!("cannot fit settings window: {e}"); } + // Elsewhere the remembered size is kept and the position follows the parent. + #[cfg(not(target_os = "macos"))] + if let (Ok(pos), Ok(size), Ok(own), Ok(scale)) = + (parent.outer_position(), parent.outer_size(), window.outer_size(), parent.scale_factor()) + { + let (pos, size, own) = + (pos.to_logical::(scale), size.to_logical::(scale), own.to_logical::(scale)); + let at = centered(pos.x, pos.y, size.width, size.height, own.width, own.height); + let _ = window.set_position(tauri::LogicalPosition::new(at.0, at.1)); + } focus(&window); Ok(()) } /// Fit outer bounds, including native decorations, using one monitor's logical coordinate space. +#[cfg(target_os = "macos")] fn fit_settings_window(window: &WebviewWindow, parent: &WebviewWindow) -> tauri::Result<()> { let Some(monitor) = parent.current_monitor()?.or(parent.primary_monitor()?) else { return Ok(()); @@ -758,6 +770,7 @@ fn fit_settings_window(window: &WebviewWindow, parent: &WebviewWindow) -> tauri: )) } +#[cfg_attr(not(target_os = "macos"), allow(dead_code))] fn fit_bounds(parent: (f64, f64, f64, f64), size: (f64, f64), work: (f64, f64, f64, f64)) -> (f64, f64, f64, f64) { let width = size.0.min(work.2); let height = size.1.min(work.3); From 27bef6956e677f16aa81497c48aafd7006903b8f Mon Sep 17 00:00:00 2001 From: arookieofc <2128194521@qq.com> Date: Fri, 30 Oct 2026 13:18:55 +0800 Subject: [PATCH 4/7] fix(tools): validate zip symlink entries on every platform (#350) Zip symlink entries were only validated on Unix: on Windows they fell through to the regular-file path, so a link escaping the destination was written as a file instead of being rejected (and the escape test failed on Windows CI). The symlink branch now validates on every platform; Unix creates the in-archive link, while Windows skips it, since creating symlinks needs a privilege most users lack. Co-authored-by: a-rookie-of-C-language <186315048+a-rookie-of-C-language@users.noreply.github.com> --- crates/ide-tools/src/archive.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/crates/ide-tools/src/archive.rs b/crates/ide-tools/src/archive.rs index 11a36926..5318c5f7 100644 --- a/crates/ide-tools/src/archive.rs +++ b/crates/ide-tools/src/archive.rs @@ -121,13 +121,16 @@ fn extract_zip(archive: &Path, dest: &Path, cancel: &CancellationToken) -> IdeRe if let Some(parent) = out.parent() { fs::create_dir_all(parent)?; } - #[cfg(unix)] if file.is_symlink() { let mut target = String::new(); io::Read::read_to_string(&mut file, &mut target)?; validate_link(dest, &rel, Path::new(&target))?; let _ = fs::remove_file(&out); - std::os::unix::fs::symlink(target, &out)?; + #[cfg(unix)] + std::os::unix::fs::symlink(&target, &out)?; + // Windows can't create symlinks without SeCreateSymbolicLinkPrivilege + // (os error 1314), so we drop in-archive links there rather than fail + // the whole extraction. Validation above still rejects escapes. continue; } let mut w = File::create(&out)?; @@ -485,6 +488,11 @@ mod tests { assert!(extract(&archive, ArchiveKind::Zip, &dest, "x", &CancellationToken::new()).is_err()); assert!(fs::symlink_metadata(dest.join("link")).is_err()); } + } + + #[cfg(unix)] + #[test] + fn zip_internal_symlink_is_created() { // A link inside the archive (e.g. a framework's Versions/Current) still extracts. let tmp = tempfile::tempdir().unwrap(); let archive = tmp.path().join("ok.zip"); From 53fd3683e2ee3407ecdb70670361d0df681fd206 Mon Sep 17 00:00:00 2001 From: DIMAS FEBRIYANTO <62858781+dimassfeb-09@users.noreply.github.com> Date: Fri, 30 Oct 2026 12:21:11 +0700 Subject: [PATCH 5/7] fix(agent): keep the selected model when starting a new chat (#351) After choosing a model and starting a new chat, the picker still showed that model but the session fell back to the agent's default. The remembered configuration was recorded from the agent's reported defaults before the user's choices were applied, and changing an option during a session did not update it. A new session now remembers its options after applying the user's choices, and changing an option updates the remembered configuration. Co-authored-by: dimassfeb-09 <62858781+dimassfeb-09@users.noreply.github.com> --- crates/ide-agent/src/lib.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/ide-agent/src/lib.rs b/crates/ide-agent/src/lib.rs index af22f4ac..464bbc04 100644 --- a/crates/ide-agent/src/lib.rs +++ b/crates/ide-agent/src/lib.rs @@ -547,6 +547,10 @@ impl AgentManager { o.current = Some(value); } s.task.model = model_of(&s.task.config).or(s.task.model.take()); + let provider = s.task.provider.clone(); + let config = s.task.config.clone(); + drop(s); + task.shared.remember(&provider, Some(&config), None); } task.approve_pending(); task.persist(); @@ -1191,7 +1195,6 @@ impl Task { (if known { reported } else { s.task.config.clone() }, s.wanted.clone()) }; config::add_client_options(&provider_id, &mut options); - self.shared.remember(&provider_id, Some(&options), None); for choice in &wanted { // An option the session doesn't offer yet is set once it does (`apply_late_choices`). let Some(o) = options.iter().find(|o| o.id == choice.id).cloned() else { continue }; @@ -1220,6 +1223,7 @@ impl Task { Err(e) => tracing::warn!("agent task {}: setting {} to {} failed: {e}", self.id, o.id, choice.value), } } + self.shared.remember(&provider_id, Some(&options), None); { let mut s = self.state.lock(); s.task.model = model_of(&options).or(s.task.model.take()); From 97b8928e4fa441a82a8d08530ff200e2debebab3 Mon Sep 17 00:00:00 2001 From: DIMAS FEBRIYANTO <62858781+dimassfeb-09@users.noreply.github.com> Date: Fri, 30 Oct 2026 12:21:15 +0700 Subject: [PATCH 6/7] feat(ui): search in the agent model dropdown (#352) Select gains an optional search box that filters its options; the agent model pickers use it, so long model lists can be filtered instead of scrolled. Keyboard navigation and type-ahead work on the filtered list, and the popup keeps its width while filtering. Co-authored-by: dimassfeb-09 <62858781+dimassfeb-09@users.noreply.github.com> --- src/features/agents/ConfigPickers.tsx | 1 + src/platform/locales/text/zh.json | 1 + src/ui/Select.module.css | 13 ++++++++++ src/ui/Select.tsx | 34 +++++++++++++++++++++++---- 4 files changed, 45 insertions(+), 4 deletions(-) diff --git a/src/features/agents/ConfigPickers.tsx b/src/features/agents/ConfigPickers.tsx index bcc562f6..c9d26a00 100644 --- a/src/features/agents/ConfigPickers.tsx +++ b/src/features/agents/ConfigPickers.tsx @@ -44,6 +44,7 @@ export function ConfigPickers({ : undefined, detail: v.description ?? undefined, }))} + searchable={o.category === "model"} onChange={(v) => onChange(o.id, v)} // Effort and other levels say what they are: "Effort Medium". renderValue={ diff --git a/src/platform/locales/text/zh.json b/src/platform/locales/text/zh.json index db7eb770..38824211 100644 --- a/src/platform/locales/text/zh.json +++ b/src/platform/locales/text/zh.json @@ -3431,6 +3431,7 @@ "Chart Settings": "图表设置", "No numeric values to plot": "没有可绘制的数值", "Search": "搜索", + "Search…": "搜索…", "Statements": "语句", "No statements": "没有语句", "Statement": "语句", diff --git a/src/ui/Select.module.css b/src/ui/Select.module.css index 785f7dcc..a784be74 100644 --- a/src/ui/Select.module.css +++ b/src/ui/Select.module.css @@ -123,3 +123,16 @@ border: 1px solid ButtonText; } } + +.searchWrap { + padding: 4px 6px 6px; +} +.search { + width: 100%; + height: var(--h-field, 28px); + padding: 0 8px; + border-radius: var(--radius-control, 4px); + background: var(--bg-field, var(--bg-button)); + box-shadow: inset 0 0 0 1px var(--border-button); + color: var(--fg-strong); +} diff --git a/src/ui/Select.tsx b/src/ui/Select.tsx index 48a55fb2..d6fb9043 100644 --- a/src/ui/Select.tsx +++ b/src/ui/Select.tsx @@ -4,6 +4,7 @@ import { Icon, type IconName } from "./icons"; import { Popup } from "./Popup"; import { List, listNavigate } from "./List"; import { useTx } from "@/platform/i18n"; +import { SearchField } from "./Input"; import styles from "./Select.module.css"; export interface SelectOption { @@ -26,6 +27,8 @@ export interface SelectProps { "aria-label"?: string; /** Custom trigger content. */ renderValue?: (option: SelectOption | undefined) => ReactNode; + /** Show a search box in the popup (useful for long option lists). */ + searchable?: boolean; } const TYPE_AHEAD_MS = 800; @@ -40,6 +43,7 @@ export function Select({ variant = "framed", className, renderValue, + searchable = false, ...aria }: SelectProps) { const tx = useTx(); @@ -47,10 +51,16 @@ export function Select({ const triggerRef = useRef(null); const current = options.find((o) => o.value === value); const [active, setActive] = useState(0); + const [query, setQuery] = useState(""); const typeAhead = useRef({ text: "", at: -Infinity }); const isSection = (o: SelectOption) => !!o.disabled; + const filtered = + searchable && query.trim() + ? options.filter((o) => !o.disabled && tx(o.label).toLowerCase().includes(query.trim().toLowerCase())) + : options; const openPopup = () => { + setQuery(""); setActive( Math.max( 0, @@ -69,7 +79,7 @@ export function Select({ const onPopupKey = (e: KeyboardEvent) => { if (e.key === "Enter" || e.key === " ") { e.preventDefault(); - choose(options[active]); + choose(filtered[active]); return; } const next = listNavigate(e.key, { items: options, active, isSection, wrap: true }); @@ -85,10 +95,10 @@ export function Select({ ahead.at = e.timeStamp; const repeat = ahead.text.length > 1 && [...ahead.text].every((c) => c === ahead.text[0]); const prefix = repeat ? ahead.text[0]! : ahead.text; - const n = options.length; + const n = filtered.length; for (let s = prefix.length > 1 ? 0 : 1; s <= n; s++) { const i = (active + s) % n; - if (!options[i]!.disabled && tx(options[i]!.label).toLowerCase().startsWith(prefix)) { + if (!filtered[i]!.disabled && tx(filtered[i]!.label).toLowerCase().startsWith(prefix)) { setActive(i); break; } @@ -135,6 +145,22 @@ export function Select({ className={styles.popup} >
+ {searchable && ( +
+ { + setQuery(v); + setActive(0); + }} + size="normal" + variant="framed" + className={styles.search} + /> +
+ )} {/* Virtualised rows are absolutely positioned; this sizes the popup to the widest label. */}
{options.map((o) => ( @@ -147,7 +173,7 @@ export function Select({ ))}
o.value} isSection={isSection} renderSection={(o) => {tx(o.label)}} From 0a27d02f4d6fd03b5c389133f3e08c198bef8799 Mon Sep 17 00:00:00 2001 From: HePengZhou <35765776+Hepengzhou@users.noreply.github.com> Date: Fri, 30 Oct 2026 13:22:12 +0800 Subject: [PATCH 7/7] fix(terminal): stop fcitx5 commits from being sent twice (#336) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With WebKitGTK and fcitx5, committed IME text reached the terminal twice: the IME commits without firing compositionstart, so xterm's composition bookkeeping never advanced and every commit was emitted both as the committed text and as a stale textarea diff. An IME commit gate now lets only the first emission of a commit through (with a 250 ms safety timeout) and empties the hidden textarea once the commit ends. It engages only for input that looks like a composition (keyCode 229 / composition input events) and stands down for IMEs that report compositionstart (macOS, IBus, Windows), leaving xterm's own handling unchanged there. Co-authored-by: Hepengzhou <35765776+Hepengzhou@users.noreply.github.com> 未在 Linux（fcitx5）实机验证。 --- src/features/terminal/imeCommit.test.ts | 99 +++++++++++++++++++++++++ src/features/terminal/imeCommit.ts | 84 +++++++++++++++++++++ src/features/terminal/session.ts | 46 +++++++++++- 3 files changed, 228 insertions(+), 1 deletion(-) create mode 100644 src/features/terminal/imeCommit.test.ts create mode 100644 src/features/terminal/imeCommit.ts diff --git a/src/features/terminal/imeCommit.test.ts b/src/features/terminal/imeCommit.test.ts new file mode 100644 index 00000000..76753353 --- /dev/null +++ b/src/features/terminal/imeCommit.test.ts @@ -0,0 +1,99 @@ +import { describe, expect, it } from "vitest"; +import { IME_COMMIT_TIMEOUT_MS, ImeCommitGate, type ImeScheduler } from "./imeCommit"; + +/** Runs queued callbacks in order, the way setTimeout(0) drains after the current event. */ +function manualScheduler() { + let next = 1; + const tasks = new Map void>(); + const scheduler: ImeScheduler = { + after: (callback) => { + const id = next++; + tasks.set(id, callback); + return id; + }, + cancel: (id) => { + tasks.delete(id); + }, + }; + const run = () => { + for (const [id, task] of [...tasks]) { + tasks.delete(id); + task(); + } + }; + return { scheduler, run, pending: () => tasks.size }; +} + +function gateWithRebase() { + const { scheduler, run, pending } = manualScheduler(); + let rebased = 0; + const gate = new ImeCommitGate(() => rebased++, scheduler); + return { gate, run, pending, rebases: () => rebased }; +} + +describe("ImeCommitGate", () => { + it("takes the first emission of a commit and drops the repeated one", () => { + const { gate } = gateWithRebase(); + gate.begin(); + expect(gate.accept()).toBe(true); + expect(gate.accept()).toBe(false); + }); + + it("keeps the real text of the pi case and drops the stale suffix", () => { + // xterm sends the committed diff first, then a stale `value.substring(start)`. + const { gate, run } = gateWithRebase(); + const sent: string[] = []; + const emit = (data: string) => { + if (gate.accept()) sent.push(data); + }; + gate.begin(); + emit("在吗"); + gate.finish(); + emit("好在吗"); + run(); + expect(sent).toEqual(["在吗"]); + }); + + it("accepts every emission once the commit is over", () => { + const { gate, run } = gateWithRebase(); + gate.begin(); + expect(gate.accept()).toBe(true); + expect(gate.accept()).toBe(false); + gate.finish(); + run(); + expect(gate.accept()).toBe(true); + expect(gate.accept()).toBe(true); + }); + + it("rebases the textarea when the commit closes, not before", () => { + const { gate, run, rebases } = gateWithRebase(); + gate.begin(); + gate.finish(); + expect(rebases()).toBe(0); + run(); + expect(rebases()).toBe(1); + }); + + it("stands down once a real compositionstart arrives", () => { + const { gate, rebases } = gateWithRebase(); + gate.begin(); + gate.compositionStart(); + expect(gate.accept()).toBe(true); + expect(gate.accept()).toBe(true); + expect(rebases()).toBe(0); + // And a later commit is not gated either. + gate.begin(); + expect(gate.accept()).toBe(true); + expect(gate.accept()).toBe(true); + }); + + it("closes a commit whose compositionend never arrives", () => { + const { gate, run, pending, rebases } = gateWithRebase(); + gate.begin(); + expect(pending()).toBe(1); + run(); + expect(IME_COMMIT_TIMEOUT_MS).toBeGreaterThan(0); + expect(rebases()).toBe(1); + expect(gate.accept()).toBe(true); + }); +}); diff --git a/src/features/terminal/imeCommit.ts b/src/features/terminal/imeCommit.ts new file mode 100644 index 00000000..baeca349 --- /dev/null +++ b/src/features/terminal/imeCommit.ts @@ -0,0 +1,84 @@ +/** + * Collapses the repeated data emissions of one IME commit. + * + * WebKitGTK + fcitx5 commit text without firing `compositionstart`, so xterm's `CompositionHelper` + * never advances `_compositionPosition.start` and keeps appending the committed text to its hidden + * textarea. A commit then reaches `onData` twice — once with the committed text, once with a stale + * `value.substring(start)` — and the same text shows up in the terminal twice. + * + * One commit must send one chunk: take the first emission and drop the rest, then rebase the + * textarea so the next commit starts from an empty value. IMEs that do report `compositionstart` + * (macOS, IBus) keep xterm's own bookkeeping, so the gate stands down for them. + */ + +/** How long a commit may stay open when its `compositionend` never arrives. */ +export const IME_COMMIT_TIMEOUT_MS = 250; + +export interface ImeScheduler { + after(callback: () => void, ms: number): number; + cancel(id: number): void; +} + +const WINDOW_SCHEDULER: ImeScheduler = { + after: (callback, ms) => window.setTimeout(callback, ms), + cancel: (id) => window.clearTimeout(id), +}; + +export class ImeCommitGate { + private open = false; + private sent = false; + private timer: number | null = null; + private native = false; + private readonly rebase: () => void; + private readonly scheduler: ImeScheduler; + + constructor(rebase: () => void, scheduler: ImeScheduler = WINDOW_SCHEDULER) { + this.rebase = rebase; + this.scheduler = scheduler; + } + + /** A commit starts: the IME's own keystroke (code 229) or its composition input event. */ + begin(): void { + if (this.native || this.open) return; + this.open = true; + this.sent = false; + this.timer = this.scheduler.after(() => this.close(), IME_COMMIT_TIMEOUT_MS); + } + + /** The commit ended: let the emissions both of xterm's senders queued go out, then rebase. */ + finish(): void { + if (!this.open) return; + this.cancelTimer(); + this.timer = this.scheduler.after(() => this.close(), 0); + } + + /** Whether this emission should reach the PTY. */ + accept(): boolean { + if (!this.open) return true; + if (this.sent) return false; + this.sent = true; + return true; + } + + /** The IME reports composition events, so xterm needs no help — stay out of its way. */ + compositionStart(): void { + this.native = true; + this.cancelTimer(); + this.open = false; + this.sent = false; + } + + /** Ends the commit and rebases the textarea to an empty value. */ + close(): void { + this.cancelTimer(); + if (!this.open) return; + this.open = false; + this.rebase(); + } + + private cancelTimer(): void { + if (this.timer === null) return; + this.scheduler.cancel(this.timer); + this.timer = null; + } +} diff --git a/src/features/terminal/session.ts b/src/features/terminal/session.ts index 9c7c0f8c..cdc92247 100644 --- a/src/features/terminal/session.ts +++ b/src/features/terminal/session.ts @@ -11,6 +11,7 @@ import { Channel } from "@tauri-apps/api/core"; import { call, commands, isTauri, type TerminalId } from "@/platform/ipc"; import { editorService, notifications } from "@/workbench/contributions"; import { fitScheduler, settledResize } from "./fit"; +import { ImeCommitGate } from "./imeCommit"; import { findTerminalLinks, resolvePath, type TerminalLink } from "./links"; import { splitCommandLine } from "@/platform/commandLine"; import { useProject } from "@/platform/project"; @@ -176,6 +177,10 @@ export class TerminalSession { private lastSize = { cols: 0, rows: 0 }; /** Bumped on restart so late output from a previous PTY is ignored. */ private generation = 0; + /** WebKitGTK delivers one IME commit' worth of text more than once; see `ImeCommitGate`. */ + private readonly ime = new ImeCommitGate(() => { + if (this.term.textarea) this.term.textarea.value = ""; + }); constructor(opts: SessionOptions) { this.opts = opts; @@ -227,7 +232,7 @@ export class TerminalSession { this.searchAddon.onDidChangeResults((r) => { for (const l of this.searchListeners) l({ index: r.resultIndex, count: r.resultCount }); }), - this.term.onData((data) => this.input(data)), + this.term.onData((data) => this.fromTerminal(data)), this.term.onBinary((data) => this.input(data)), this.term.onTitleChange((title) => this.update({ appTitle: title.trim() || null })), // OSC 7: `file://host/path` — shells report their cwd so relative links resolve correctly. @@ -347,6 +352,7 @@ export class TerminalSession { () => this.scheduleFit(), ); this.term.textarea?.addEventListener("focus", () => this.opts.onFocus(this.key)); + this.watchImeCommit(); } this.resizeObserver?.disconnect(); this.resizeObserver = new ResizeObserver(() => this.scheduleFit()); @@ -364,6 +370,44 @@ export class TerminalSession { this.fitter.schedule(); } + /** + * Routes a WebKitGTK / fcitx5 IME commit through `ImeCommitGate`: the platform reports the + * composition's own keystroke (code 229) and its input event but never `compositionstart`, + * which is what xterm's own bookkeeping is built on. + */ + private watchImeCommit() { + const textarea = this.term.textarea; + if (!textarea) return; + const onKeyDown = (e: KeyboardEvent) => { + if (e.keyCode === 229) this.ime.begin(); + }; + const onInput = (e: InputEvent) => { + if (e.inputType === "insertFromComposition" || e.inputType === "insertCompositionText") this.ime.begin(); + }; + const onCompositionStart = () => this.ime.compositionStart(); + // Bubble phase, i.e. after xterm's own listener: the rebase must land behind the emissions + // that xterm's composition and textarea-diff paths already scheduled. + const onCompositionEnd = () => this.ime.finish(); + this.host.addEventListener("keydown", onKeyDown, true); + textarea.addEventListener("input", onInput, true); + textarea.addEventListener("compositionstart", onCompositionStart, true); + textarea.addEventListener("compositionend", onCompositionEnd); + this.disposables.push({ + dispose: () => { + this.ime.close(); + this.host.removeEventListener("keydown", onKeyDown, true); + textarea.removeEventListener("input", onInput, true); + textarea.removeEventListener("compositionstart", onCompositionStart, true); + textarea.removeEventListener("compositionend", onCompositionEnd); + }, + }); + } + + /** xterm's data event, with one IME commit's repeated emissions collapsed. */ + private fromTerminal(data: string) { + if (this.ime.accept()) this.input(data); + } + private doFit() { if (this.disposed || !this.host.isConnected) return; if (this.host.offsetWidth === 0 || this.host.offsetHeight === 0) return;