From c039d192a8146b610803b53090db8f3e17f8b37a Mon Sep 17 00:00:00 2001 From: andrexibiza <84248988+andrexibiza@users.noreply.github.com> Date: Sun, 2 Aug 2026 22:14:36 -0500 Subject: [PATCH] fix(desktop): resolve agent CLIs via Windows registry PATH when process env is stale The managed-agent resolver probes for CLI binaries only against the inherited process PATH. On Windows, apps relaunched by an updater, service, or long-lived parent keep the environment they were launched with, so PATH entries written to the registry later are invisible and installed runtimes report as NotInstalled even though a freshly spawned process resolves them. Hermes Agent is the concrete case: its hermes-acp launcher lives in the install's venv Scripts dir, which the 0.5.3 auto-updater relaunch no longer carried. The registry Environment\Path (HKLM + HKCU) still has the entry. Fall back to the machine and per-user registry Path values when the process PATH scan misses: read both hives, expand %VAR% references, and scan those dirs for the target basenames. Mirrors the existing git_bash_from_registry pattern. discovery.rs stays under its file-size ratchet by moving the shim scan into the new registry_path module. Signed-off-by: andrexibiza <84248988+andrexibiza@users.noreply.github.com> --- desktop/src-tauri/Cargo.toml | 2 +- .../src-tauri/src/managed_agents/discovery.rs | 28 +-- .../managed_agents/discovery/registry_path.rs | 163 ++++++++++++++++++ .../tests/managed_path_resolution.rs | 35 ++++ 4 files changed, 205 insertions(+), 23 deletions(-) create mode 100644 desktop/src-tauri/src/managed_agents/discovery/registry_path.rs diff --git a/desktop/src-tauri/Cargo.toml b/desktop/src-tauri/Cargo.toml index fd58f27878..1467d6c2e6 100644 --- a/desktop/src-tauri/Cargo.toml +++ b/desktop/src-tauri/Cargo.toml @@ -57,7 +57,7 @@ user-idle = { version = "0.6", default-features = false } plist = "1" [target.'cfg(windows)'.dependencies] -windows-sys = { version = "0.61", features = ["Win32_Security", "Win32_Storage_FileSystem", "Win32_System_JobObjects", "Win32_System_Registry", "Win32_System_Threading", "Win32_Foundation"] } +windows-sys = { version = "0.61", features = ["Win32_Security", "Win32_Storage_FileSystem", "Win32_System_Environment", "Win32_System_JobObjects", "Win32_System_Registry", "Win32_System_Threading", "Win32_Foundation"] } keyring = { version = "3.6.3", default-features = false, features = ["windows-native", "vendored"], optional = true } user-idle = { version = "0.6", default-features = false } diff --git a/desktop/src-tauri/src/managed_agents/discovery.rs b/desktop/src-tauri/src/managed_agents/discovery.rs index 8d1b8a5013..5eb4614b39 100644 --- a/desktop/src-tauri/src/managed_agents/discovery.rs +++ b/desktop/src-tauri/src/managed_agents/discovery.rs @@ -11,6 +11,7 @@ use crate::managed_agents::{ }; mod presets; +mod registry_path; mod runtime_metadata; use presets::{preset_catalog_entry, PRESET_HARNESSES}; @@ -715,16 +716,12 @@ fn resolve_command_uncached(command: &str) -> Option { } } - // On Windows, also scan PATH for .cmd/.bat shims (npm globals). + // On Windows, scan PATH for .cmd/.bat shims (npm globals) and fall back + // to the registry PATH when the inherited process env is a stale + // snapshot (updater relaunches, services — see registry_path). #[cfg(windows)] - { - for basename in command_basenames(command).iter().skip(1) { - for candidate in path_candidates_from_env_raw(basename) { - if candidate.is_file() { - return Some(candidate); - } - } - } + if let Some(path) = registry_path::resolve_windows_fallbacks(&basenames) { + return Some(path); } if let Some(path) = find_via_login_shell(command) { @@ -767,19 +764,6 @@ fn path_candidates_from_env(command: &str) -> Vec { .unwrap_or_default() } -/// Like `path_candidates_from_env` but joins `basename` as-is (no `.exe` suffix). -/// Used for `.cmd`/`.bat` shim resolution on Windows. -#[cfg(windows)] -fn path_candidates_from_env_raw(basename: &str) -> Vec { - std::env::var_os("PATH") - .map(|paths| { - std::env::split_paths(&paths) - .map(|dir| dir.join(basename)) - .collect::>() - }) - .unwrap_or_default() -} - /// Collect login shell candidates for the current platform. /// /// On Unix: `/bin/zsh`, `/bin/bash` (the historical defaults). diff --git a/desktop/src-tauri/src/managed_agents/discovery/registry_path.rs b/desktop/src-tauri/src/managed_agents/discovery/registry_path.rs new file mode 100644 index 0000000000..8adbc292a8 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/discovery/registry_path.rs @@ -0,0 +1,163 @@ +//! Windows fallbacks for command resolution: PATH shim scanning and the +//! registry `Environment\Path` fallback. +//! +//! The inherited process `PATH` can be a stale snapshot: apps relaunched by +//! an updater, services, and long-lived parents keep the environment they +//! were launched with, so PATH entries written to the registry later (e.g. a +//! freshly installed agent CLI) stay invisible even though a newly spawned +//! process would see them. Reading `Environment\Path` from both hives +//! restores the authoritative post-login PATH and mirrors the resolver's own +//! `git_bash_from_registry` fallback. + +#[cfg(windows)] +use std::path::PathBuf; + +/// Windows-only resolution steps run after the process-env `.exe` scan. +/// +/// 1. Scan the process PATH for `.cmd`/`.bat` shims (npm globals). +/// 2. Fall back to the machine and per-user registry `Path` values. +#[cfg(windows)] +pub(super) fn resolve_windows_fallbacks(basenames: &[String]) -> Option { + for basename in basenames.iter().skip(1) { + for candidate in path_candidates_from_env_raw(basename) { + if candidate.is_file() { + return Some(candidate); + } + } + } + resolve_via_registry_path(basenames) +} + +/// Like `path_candidates_from_env` but joins `basename` as-is (no `.exe` +/// suffix). Used for `.cmd`/`.bat` shim resolution on Windows. +#[cfg(windows)] +fn path_candidates_from_env_raw(basename: &str) -> Vec { + std::env::var_os("PATH") + .map(|paths| { + std::env::split_paths(&paths) + .map(|dir| dir.join(basename)) + .collect::>() + }) + .unwrap_or_default() +} + +/// Resolve against the machine and per-user registry `Path` values. +/// +/// Registry values are REG_EXPAND_SZ; `%VAR%` references are resolved so +/// entries like `%SystemRoot%\system32` become real directories. On +/// expansion failure, the raw value is skipped. +#[cfg(windows)] +fn resolve_via_registry_path(basenames: &[String]) -> Option { + use std::ffi::OsString; + use std::os::windows::ffi::{OsStrExt, OsStringExt}; + use windows_sys::Win32::Foundation::{ERROR_MORE_DATA, ERROR_SUCCESS}; + use windows_sys::Win32::System::Environment::ExpandEnvironmentStringsW; + use windows_sys::Win32::System::Registry::{ + RegCloseKey, RegOpenKeyExW, RegQueryValueExW, HKEY_CURRENT_USER, HKEY_LOCAL_MACHINE, + KEY_READ, + }; + + const VALUE: &str = "Path"; + let value: Vec = VALUE.encode_utf16().chain(Some(0)).collect(); + + // Machine value first, then per-user — matches the effective merge order + // of the process environment (`HKLM` entries precede `HKCU` entries). + for (hive, key) in [ + ( + HKEY_LOCAL_MACHINE, + "SYSTEM\\CurrentControlSet\\Control\\Session Manager\\Environment", + ), + (HKEY_CURRENT_USER, "Environment"), + ] { + let key: Vec = key.encode_utf16().chain(Some(0)).collect(); + + // SAFETY: key/value are null-terminated UTF-16 for the duration of + // each call, and every successfully opened handle is closed before + // the next hive is tried. + unsafe { + let mut handle = std::ptr::null_mut(); + if RegOpenKeyExW(hive, key.as_ptr(), 0, KEY_READ, &mut handle) != ERROR_SUCCESS { + continue; + } + + let mut byte_len = 0; + let status = RegQueryValueExW( + handle, + value.as_ptr(), + std::ptr::null(), + std::ptr::null_mut(), + std::ptr::null_mut(), + &mut byte_len, + ); + if (status != ERROR_SUCCESS && status != ERROR_MORE_DATA) || byte_len == 0 { + RegCloseKey(handle); + continue; + } + + let mut data = vec![0u16; (byte_len as usize).div_ceil(2)]; + let status = RegQueryValueExW( + handle, + value.as_ptr(), + std::ptr::null(), + std::ptr::null_mut(), + data.as_mut_ptr().cast(), + &mut byte_len, + ); + RegCloseKey(handle); + if status != ERROR_SUCCESS { + continue; + } + + while data.last() == Some(&0) { + data.pop(); + } + let raw = OsString::from_wide(&data); + + let expanded: Vec = raw.encode_wide().chain(Some(0)).collect(); + let mut buf = vec![0u16; 1024]; + let expanded_len = + ExpandEnvironmentStringsW(expanded.as_ptr(), buf.as_mut_ptr(), buf.len() as u32); + if expanded_len == 0 { + continue; + } + if expanded_len > buf.len() as u32 { + buf.resize(expanded_len as usize, 0); + let _ = ExpandEnvironmentStringsW( + expanded.as_ptr(), + buf.as_mut_ptr(), + buf.len() as u32, + ); + } + while buf.last() == Some(&0) { + buf.pop(); + } + let expanded = OsString::from_wide(&buf); + + if let Some(path) = resolve_in_dirs(basenames, std::env::split_paths(&expanded)) { + return Some(path); + } + } + } + + None +} + +/// Scan `dirs` for any basename in `basenames` that exists as a file. +/// +/// Extracted from the registry fallback so the dir-scanning logic is +/// testable without touching the real registry. +#[cfg(windows)] +pub(super) fn resolve_in_dirs( + basenames: &[String], + dirs: impl Iterator, +) -> Option { + for dir in dirs { + for basename in basenames { + let candidate = dir.join(basename); + if candidate.is_file() { + return Some(candidate); + } + } + } + None +} diff --git a/desktop/src-tauri/src/managed_agents/discovery/tests/managed_path_resolution.rs b/desktop/src-tauri/src/managed_agents/discovery/tests/managed_path_resolution.rs index 0795bb2345..aa78695f11 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/tests/managed_path_resolution.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/tests/managed_path_resolution.rs @@ -88,3 +88,38 @@ fn resolve_command_prefers_buzz_managed_npm_shim_over_path() { "Buzz-managed npm shim must win over PATH/global shims" ); } + +/// A binary that exists only in a directory from the registry PATH (not the +/// inherited process PATH) must still resolve: the registry fallback exists +/// precisely for processes whose environment predates a PATH update (e.g. +/// apps relaunched by an updater), which is the Hermes Agent on Windows case. +#[cfg(windows)] +#[test] +fn resolve_in_dirs_finds_binary_from_registry_style_dir_list() { + use std::path::PathBuf; + + let temp = tempfile::tempdir().expect("tempdir"); + let registry_dir = temp + .path() + .join("hermes-agent") + .join("venv") + .join("Scripts"); + std::fs::create_dir_all(®istry_dir).expect("create registry-style dir"); + + let binary = registry_dir.join("hermes-acp.exe"); + std::fs::write(&binary, b"placeholder").expect("write fake hermes-acp.exe"); + + let basenames = vec!["hermes-acp".to_string(), "hermes-acp.exe".to_string()]; + let dirs = [ + PathBuf::from("C:\\definitely\\missing"), + registry_dir.clone(), + ]; + + let resolved = super::super::registry_path::resolve_in_dirs(&basenames, dirs.into_iter()); + + assert_eq!( + resolved.as_deref(), + Some(binary.as_path()), + "registry-style dir scan must find hermes-acp.exe even though the process PATH misses it" + ); +}