diff --git a/desktop/src-tauri/src/commands/agent_discovery.rs b/desktop/src-tauri/src/commands/agent_discovery.rs index 95534854a0b..95e9759f10e 100644 --- a/desktop/src-tauri/src/commands/agent_discovery.rs +++ b/desktop/src-tauri/src/commands/agent_discovery.rs @@ -4,6 +4,7 @@ use crate::managed_agents::{ DEFAULT_ACP_COMMAND, }; +mod forced_single_flight; mod post_install_verification; fn active_installs() -> &'static std::sync::Mutex> { @@ -49,23 +50,15 @@ pub(crate) fn plan_adapter_install<'c>( } } +/// Discover the ACP runtime catalog. `force: false` (the default) serves the +/// cheap cached path; `force: true` runs the expensive re-discovery. See +/// [`forced_single_flight`] for the split and single-flight coalescing. #[tauri::command] pub async fn discover_acp_providers( app: tauri::AppHandle, + force: Option, ) -> Result, String> { - tokio::task::spawn_blocking(move || { - use tauri::Manager; - crate::managed_agents::clear_resolve_cache(); - crate::managed_agents::refresh_login_shell_path(); - let custom_dir = app - .path() - .app_data_dir() - .ok() - .map(|d| d.join("custom_harnesses")); - crate::managed_agents::discover_acp_runtimes_from(custom_dir.as_deref()) - }) - .await - .map_err(|e| format!("spawn_blocking failed: {e}")) + forced_single_flight::discover(app, force.unwrap_or(false)).await } /// Write a user-defined harness definition to `/custom_harnesses/.json`. diff --git a/desktop/src-tauri/src/commands/agent_discovery/forced_single_flight.rs b/desktop/src-tauri/src/commands/agent_discovery/forced_single_flight.rs new file mode 100644 index 00000000000..3667d3b237e --- /dev/null +++ b/desktop/src-tauri/src/commands/agent_discovery/forced_single_flight.rs @@ -0,0 +1,80 @@ +//! Discovery execution + single-flight coalescing for the ACP runtime catalog. +//! +//! `force: false` serves from the process caches (no clear, no PATH re-fetch, no +//! CLI auth probes) — the low-millisecond path hot surfaces render from. +//! +//! `force: true` runs the expensive probe pipeline. React Query already dedups +//! the hook consumers; the single-flight here is the seatbelt for non-hook +//! invoke paths, so a burst of forced triggers coalesces onto one in-flight run +//! instead of stacking the pipeline. + +use super::AcpRuntimeCatalogEntry; + +type BoxedDiscovery = std::pin::Pin< + Box, String>> + Send>, +>; +type SharedDiscovery = futures_util::future::Shared; + +fn inflight() -> &'static std::sync::Mutex> { + use std::sync::{Mutex, OnceLock}; + static INFLIGHT: OnceLock>> = OnceLock::new(); + INFLIGHT.get_or_init(|| Mutex::new(None)) +} + +/// Discover the ACP runtime catalog. Cheap calls run directly; forced calls +/// coalesce onto a single shared run (see module docs). +pub(super) async fn discover( + app: tauri::AppHandle, + force: bool, +) -> Result, String> { + if !force { + return run(app, false).await; + } + + let shared = { + let mut guard = inflight().lock().unwrap_or_else(|e| e.into_inner()); + match guard.as_ref() { + Some(existing) => existing.clone(), + None => { + let fut: BoxedDiscovery = Box::pin(run(app, true)); + let shared = futures_util::FutureExt::shared(fut); + *guard = Some(shared.clone()); + shared + } + } + }; + + let result = shared.clone().await; + + // Clear the slot so the next forced call re-runs — but only if it still + // points at the future we just awaited (a newer run may have replaced it). + { + let mut guard = inflight().lock().unwrap_or_else(|e| e.into_inner()); + if guard + .as_ref() + .is_some_and(|current| current.ptr_eq(&shared)) + { + *guard = None; + } + } + + result +} + +async fn run(app: tauri::AppHandle, force: bool) -> Result, String> { + tokio::task::spawn_blocking(move || { + use tauri::Manager; + if force { + crate::managed_agents::clear_resolve_cache(); + crate::managed_agents::refresh_login_shell_path(); + } + let custom_dir = app + .path() + .app_data_dir() + .ok() + .map(|d| d.join("custom_harnesses")); + crate::managed_agents::discover_acp_runtimes_from(custom_dir.as_deref(), force) + }) + .await + .map_err(|e| format!("spawn_blocking failed: {e}")) +} diff --git a/desktop/src-tauri/src/managed_agents/discovery.rs b/desktop/src-tauri/src/managed_agents/discovery.rs index bc0e3a6cdae..78592357c9b 100644 --- a/desktop/src-tauri/src/managed_agents/discovery.rs +++ b/desktop/src-tauri/src/managed_agents/discovery.rs @@ -9,10 +9,18 @@ use crate::managed_agents::{ AcpAvailabilityStatus, AcpRuntimeCatalogEntry, AuthStatus, CommandAvailabilityInfo, HarnessSource, }; +mod auth_status_cache; +mod login_shell; mod presets; mod runtime_metadata; #[macro_use] mod windows_install; +pub use login_shell::{find_nvm_default_bin, login_shell_path}; +pub(crate) use login_shell::{find_via_login_shell, refresh_login_shell_path}; +#[cfg(test)] +pub(crate) use login_shell::{ + is_login_shell_path_uninit, is_safe_nvm_tag, login_shell_candidates, parse_semver_tag, +}; pub(crate) use presets::{ canonical_harness_command, command_for_runtime_id, preset_harness_definitions, preset_harness_ids, @@ -558,18 +566,40 @@ pub fn resolve_command(command: &str) -> Option { } } - // Slow path: resolve and cache. + // Slow path: resolve and cache. Negative results are cached too: an absent + // command must not re-run `resolve_command_uncached` (which spawns a login + // shell via `find_via_login_shell`) on every cheap discovery — that spawn + // on the channel-switch/composer hot path is exactly what this cache exists + // to prevent. `clear_resolve_cache` (run by every forced discovery) is the + // invalidation seam, so a newly-installed binary is still found on refresh. let result = resolve_command_uncached(command); - if result.is_some() { - if let Ok(mut guard) = cache.lock() { - guard.insert(command.to_string(), result.clone()); - } + if let Ok(mut guard) = cache.lock() { + guard.insert(command.to_string(), result.clone()); } result } +/// Cache-only command resolution for the cheap discovery path. +/// +/// Consults the Buzz-managed shim dir (a filesystem stat, never a spawn) and +/// the resolve cache; on a miss it reports the command absent rather than +/// resolving live via `resolve_command_uncached` → `find_via_login_shell`, +/// which spawns a login shell on the channel-switch / composer hot path — the +/// freeze the cheap path exists to avoid. `resolve_command` (the forced path) +/// is the sole prober and cache populator. +pub fn resolve_command_cached(command: &str) -> Option { + if let Some(managed) = resolve_buzz_managed_command(command) { + return Some(managed); + } + resolve_cache() + .lock() + .ok() + .and_then(|guard| guard.get(command).cloned()) + .flatten() +} + /// Clear the resolve_command cache so that newly-installed binaries are detected. pub fn clear_resolve_cache() { let mut guard = resolve_cache().lock().unwrap_or_else(|e| e.into_inner()); @@ -577,6 +607,9 @@ pub fn clear_resolve_cache() { // Also invalidate the adapter-availability cache so a freshly-installed // adapter is reflected the next time the summary builder checks the badge. clear_adapter_availability_cache(); + // And the auth-status cache so a forced re-discovery re-probes rather than + // reusing stale login state. + auth_status_cache::clear(); } // ── Adapter availability cache (Phase-2 badge fallback) ───────────────────── @@ -757,222 +790,10 @@ fn path_candidates_from_env_raw(basename: &str) -> Vec { .unwrap_or_default() } -/// Collect login shell candidates for the current platform. -/// -/// On Unix: `/bin/zsh`, `/bin/bash` (the historical defaults). -/// On Windows: Git Bash via `resolve_bash_path` — skips `BUZZ_SHELL` because -/// login-shell callers use bash-only `-l -c` syntax. -fn login_shell_candidates() -> Vec { - #[cfg(not(windows))] - { - vec![PathBuf::from("/bin/zsh"), PathBuf::from("/bin/bash")] - } - #[cfg(windows)] - { - super::git_bash::resolve_bash_path().into_iter().collect() - } -} - -/// Run a command in a login shell (tries zsh then bash on Unix, Git Bash on Windows). -/// Returns trimmed stdout if the command succeeds with non-empty output. -fn run_in_login_shell(args: &[&str]) -> Option { - for shell in login_shell_candidates() { - let mut cmd = Command::new(&shell); - cmd.args(args); - crate::util::configure_no_window(&mut cmd); - let Ok(output) = cmd.output() else { - continue; - }; - if !output.status.success() { - continue; - } - let stdout = String::from_utf8_lossy(&output.stdout).trim().to_string(); - if !stdout.is_empty() { - return Some(stdout); - } - } - None -} - -fn find_via_login_shell(command: &str) -> Option { - let stdout = run_in_login_shell(&["-l", "-c", r#"command -v -- "$1""#, "_", command])?; - let resolved = stdout.lines().rfind(|line| !line.trim().is_empty())?; - let path = PathBuf::from(resolved.trim()); - (path.is_absolute() && is_executable_file(&path)).then_some(path) -} - -/// Three-state backing store for the login-shell PATH cache. -#[derive(Clone)] -enum LoginShellPath { - /// Cache has never been populated; the next call will spawn a login shell. - Uninit, - /// A login shell was invoked; the inner value is the PATH it returned - /// (`None` when the shell produced no output). - Probed(Option), -} - -fn path_cache() -> &'static std::sync::Mutex { - use std::sync::{Mutex, OnceLock}; - static CACHE: OnceLock> = OnceLock::new(); - CACHE.get_or_init(|| Mutex::new(LoginShellPath::Uninit)) -} - -fn fetch_login_shell_path_inner() -> Option { - // On Windows, Git Bash's `echo $PATH` returns POSIX colon-delimited paths - // (`/mingw64/bin:/c/Users/...`) which poison native Windows children that - // split on `;`. login_shell_path() feeds agent_models, runtime, and - // cli_probe — all native processes. Return None so they inherit the real - // Windows PATH instead. - #[cfg(windows)] - { - return None; - } - - #[cfg(not(windows))] - { - let stdout = run_in_login_shell(&["-l", "-c", "echo $PATH"])?; - let last_line = stdout.lines().rfind(|l| !l.trim().is_empty())?; - Some(last_line.trim().to_string()) - } -} - -/// Return the user's full PATH from a login shell. -/// -/// The result is cached after the first call. Call [`refresh_login_shell_path`] -/// to invalidate the cache so the next call re-fetches — e.g. after the user -/// installs Node.js mid-session and clicks Retry. -/// -/// The lock is never held while the login shell spawns: we check for a cached -/// value, release the lock, run the shell, then re-lock to write. Two concurrent -/// callers may both run the shell (last-writer-wins is fine — both produce the -/// same result), but neither blocks a concurrent agent spawn on the Mutex. -pub fn login_shell_path() -> Option { - // Fast path: return cached result without spawning a shell. - { - let guard = path_cache().lock().unwrap_or_else(|e| e.into_inner()); - if let LoginShellPath::Probed(ref result) = *guard { - return result.clone(); - } - } - - // Slow path: spawn shell outside any lock. - let result = fetch_login_shell_path_inner(); - - // Write back; last-writer-wins is safe here. - { - let mut guard = path_cache().lock().unwrap_or_else(|e| e.into_inner()); - *guard = LoginShellPath::Probed(result.clone()); - } - - result -} - -/// Invalidate the login-shell PATH cache so the next [`login_shell_path`] call -/// re-fetches from a fresh login shell. -/// -/// Called before every install/retry operation and on Doctor Re-run so a -/// newly-installed tool becomes visible without restarting the app. -pub(crate) fn refresh_login_shell_path() { - let mut guard = path_cache().lock().unwrap_or_else(|e| e.into_inner()); - *guard = LoginShellPath::Uninit; -} - +/// Test-only counter for login-shell spawn attempts (see submodule). #[cfg(test)] -fn is_login_shell_path_uninit() -> bool { - matches!( - *path_cache().lock().unwrap_or_else(|e| e.into_inner()), - LoginShellPath::Uninit - ) -} - -/// Return `true` when `tag` is a safe nvm alias/version tag that can be joined -/// onto a `PathBuf` without escaping the nvm root. -/// -/// nvm uses tags like `v22.1.0` or `lts/hydrogen`. We allow ASCII alphanumeric -/// plus `. - / _` and require that no path component is `..` and that the tag -/// does not start with `/` (which would replace the base in `PathBuf::join`). -fn is_safe_nvm_tag(tag: &str) -> bool { - if tag.is_empty() { - return false; - } - // An absolute path in the alias file would let PathBuf::join silently - // replace the nvm root with an attacker-controlled path. - if tag.starts_with('/') { - return false; - } - // Reject any .. component to prevent upward traversal. - for component in tag.split('/') { - if component == ".." { - return false; - } - } - // Allow only the characters nvm uses in real tag names. - tag.chars() - .all(|c| c.is_ascii_alphanumeric() || matches!(c, '.' | '-' | '/' | '_')) -} - -/// Locate the `bin` directory for nvm's default Node.js version. -/// -/// Reads `~/.nvm/alias/default`; resolves at most one alias hop to handle -/// nvm alias chains; falls back to the highest-semver directory under -/// `~/.nvm/versions/node/`. Returns the `bin` subdirectory only when it exists. -/// -/// Cheap: at most two file reads or one `read_dir`. Never cached — computed -/// fresh per call so a mid-session `nvm install` is visible at the next spawn. -pub fn find_nvm_default_bin(home: &Path) -> Option { - let nvm_root = home.join(".nvm"); - let versions_root = nvm_root.join("versions").join("node"); - - // 1. Try alias/default, with at most one hop. - let default_alias = nvm_root.join("alias").join("default"); - if let Ok(content) = std::fs::read_to_string(&default_alias) { - let tag = content.trim().to_string(); - if is_safe_nvm_tag(&tag) { - let candidate = versions_root.join(&tag).join("bin"); - if candidate.is_dir() { - return Some(candidate); - } - // One alias hop: ~/.nvm/alias/ - let hop_file = nvm_root.join("alias").join(&tag); - if let Ok(hop_content) = std::fs::read_to_string(&hop_file) { - let hop_tag = hop_content.trim().to_string(); - if is_safe_nvm_tag(&hop_tag) { - let hop_candidate = versions_root.join(&hop_tag).join("bin"); - if hop_candidate.is_dir() { - return Some(hop_candidate); - } - } - } - } - } - - // 2. Fall back to highest-semver directory under ~/.nvm/versions/node/. - let entries = std::fs::read_dir(&versions_root).ok()?; - let best = entries - .filter_map(|e| e.ok()) - .filter_map(|e| { - let name = e.file_name(); - let s = name.to_string_lossy().into_owned(); - parse_semver_tag(&s).map(|v| (v, s)) - }) - .max_by(|(a, _), (b, _)| a.cmp(b)); - - let (_, tag) = best?; - let bin = versions_root.join(&tag).join("bin"); - bin.is_dir().then_some(bin) -} - -/// Parse a `vMAJ.MIN.PATCH` (or `vMAJ.MIN.PATCH-extra`) tag into a numeric -/// triple for semver comparison. -fn parse_semver_tag(s: &str) -> Option<(u64, u64, u64)> { - let s = s.strip_prefix('v')?; - let mut parts = s.splitn(3, '.'); - let major = parts.next()?.parse::().ok()?; - let minor = parts.next()?.parse::().ok()?; - let patch_str = parts.next()?; - let patch = patch_str.split('-').next()?.parse::().ok()?; - Some((major, minor, patch)) -} +#[path = "discovery/login_shell_spawn_probe.rs"] +pub(crate) mod login_shell_spawn_probe; pub(crate) fn find_command(command: &str) -> Option { resolve_command(command) @@ -1295,27 +1116,39 @@ struct PartialEntry { entry: AcpRuntimeCatalogEntry, } -fn discover_acp_runtime_phase1(runtime: &'static KnownAcpRuntime) -> PartialEntry { +fn discover_acp_runtime_phase1(runtime: &'static KnownAcpRuntime, force: bool) -> PartialEntry { + // Cheap path is cache-only (no login-shell spawn); forced path resolves live. + let resolve = if force { + resolve_command + } else { + resolve_command_cached + }; let adapter_result = runtime .commands .iter() - .find_map(|command| find_command(command).map(|path| (*command, path))); + .find_map(|command| resolve(command).map(|path| (*command, path))); let underlying_cli_found = runtime .underlying_cli - .map(|cli| find_command(cli).is_some()) + .map(|cli| resolve(cli).is_some()) .unwrap_or(false); let (mut availability, command, binary_path) = classify_runtime(adapter_result, runtime.underlying_cli, underlying_cli_found); - // For codex-acp: when the adapter resolves as Available, probe its full - // version. An adapter below MIN_CODEX_ACP_VERSION is treated as outdated. + // For codex-acp: when the adapter resolves as Available, determine its full + // version. A forced discovery probes the binary (spawns a subprocess); the + // cheap default path reuses the last cached availability so it stays + // process-free. An adapter below MIN_CODEX_ACP_VERSION is treated as outdated. if runtime.id == "codex" && availability == AcpAvailabilityStatus::Available && command.as_deref() == Some("codex-acp") { - if let Some(path_str) = &binary_path { - availability = codex_adapter_availability(&PathBuf::from(path_str)); + if force { + if let Some(path_str) = &binary_path { + availability = codex_adapter_availability(&PathBuf::from(path_str)); + } + } else if let Some(cached) = adapter_availability_cached() { + availability = cached; } } @@ -1328,7 +1161,7 @@ fn discover_acp_runtime_phase1(runtime: &'static KnownAcpRuntime) -> PartialEntr let underlying_cli_path = runtime .underlying_cli - .and_then(find_command) + .and_then(resolve) .map(|p| p.display().to_string()); let default_args = command @@ -1373,8 +1206,8 @@ fn discover_acp_runtime_phase1(runtime: &'static KnownAcpRuntime) -> PartialEntr AcpAvailabilityStatus::AdapterMissing | AcpAvailabilityStatus::NotInstalled ) && runtime_needs_npm(runtime) && buzz_managed_node_bin_dir().is_none() - && resolve_command("npm").is_none() - && resolve_command("node").is_none(); + && resolve("npm").is_none() + && resolve("node").is_none(); PartialEntry { runtime, @@ -1415,7 +1248,9 @@ fn discover_acp_runtime_phase1(runtime: &'static KnownAcpRuntime) -> PartialEntr /// resolves, so it should not pay the cost of authenticating every catalog entry. pub(crate) fn discover_acp_runtime_availability(runtime_id: &str) -> Option { known_acp_runtime_exact(runtime_id) - .map(discover_acp_runtime_phase1) + // Post-install verification wants fresh filesystem/version state, so + // probe rather than trust the cheap-path cache. + .map(|runtime| discover_acp_runtime_phase1(runtime, true)) .map(|partial| partial.entry.availability) } @@ -1438,47 +1273,24 @@ pub(crate) fn discover_acp_runtime_availability(runtime_id: &str) -> Option, + force: bool, ) -> Vec { + // Cheap path is cache-only (no login-shell spawn); forced path resolves live. + let resolve = if force { + resolve_command + } else { + resolve_command_cached + }; + // Phase 1: build all builtin entries (fast — no probes yet). let mut partials: Vec = KNOWN_ACP_RUNTIMES .iter() - .map(discover_acp_runtime_phase1) - .collect(); - - // Phase 2: run auth probes in parallel for entries that need them. - // Spawn one thread per probeable entry; total cost = max(probe latency). - let probe_handles: Vec<(usize, std::thread::JoinHandle)> = partials - .iter() - .enumerate() - .filter_map(|(idx, partial)| { - if partial.entry.availability != AcpAvailabilityStatus::Available { - return None; - } - let probe_args = partial.runtime.auth_probe_args?; - // Need the resolved binary path for the CLI (e.g. the actual `claude` binary). - let binary_path = resolve_command(probe_args[0])?; - let probe_args_owned: Vec = probe_args.iter().map(|s| s.to_string()).collect(); - - let handle = std::thread::spawn(move || { - let refs: Vec<&str> = probe_args_owned.iter().map(String::as_str).collect(); - probe_auth_status(&binary_path, &refs) - }); - Some((idx, handle)) - }) + .map(|runtime| discover_acp_runtime_phase1(runtime, force)) .collect(); - // Collect probe results and patch entries. - for (idx, handle) in probe_handles { - let status = handle.join().unwrap_or(AuthStatus::Unknown); - let partial = &mut partials[idx]; - partial.entry.login_hint = - if matches!(status, AuthStatus::LoggedIn | AuthStatus::NotApplicable) { - None - } else { - partial.runtime.login_hint.map(str::to_string) - }; - partial.entry.auth_status = status; - } + // Phase 2: resolve each available runtime's auth status (forced discovery + // spawns parallel CLI probes and warms the cache; the cheap path reuses it). + auth_status_cache::resolve_auth_statuses(&mut partials, force); // Fill NotApplicable / Unknown for non-probed entries. for partial in &mut partials { @@ -1508,7 +1320,7 @@ pub fn discover_acp_runtimes_from( } seen_ids.insert(def.id.to_string()); - entries.push(preset_catalog_entry(def, find_command)); + entries.push(preset_catalog_entry(def, resolve)); } // Phase 3: load and append custom harness definitions. @@ -1523,8 +1335,8 @@ pub fn discover_acp_runtimes_from( continue; } - // Availability: command on PATH → Available, else NotInstalled. - let (availability, command, binary_path) = match find_command(&def.command) { + // Availability: command resolves → Available, else NotInstalled. + let (availability, command, binary_path) = match resolve(&def.command) { Some(path) => ( AcpAvailabilityStatus::Available, Some(def.command.clone()), diff --git a/desktop/src-tauri/src/managed_agents/discovery/auth_status_cache.rs b/desktop/src-tauri/src/managed_agents/discovery/auth_status_cache.rs new file mode 100644 index 00000000000..cae0d7e2c94 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/discovery/auth_status_cache.rs @@ -0,0 +1,105 @@ +//! Auth-status cache for cheap ACP runtime discovery. +//! +//! A forced discovery (`discover_acp_providers(force: true)`) spawns one CLI +//! auth probe per available runtime — the expensive pipeline. The cheap default +//! discovery must not pay that cost, so it reuses the last known auth statuses +//! from this cache instead of probing. The cache is keyed by runtime id, warmed +//! by the forced probe phase, and cleared by `clear_resolve_cache` (which a +//! forced discovery calls before re-probing). + +use std::collections::HashMap; +use std::sync::{Mutex, OnceLock}; + +use crate::managed_agents::AuthStatus; + +fn cache() -> &'static Mutex> { + static CACHE: OnceLock>> = OnceLock::new(); + CACHE.get_or_init(|| Mutex::new(HashMap::new())) +} + +pub(super) fn clear() { + if let Ok(mut guard) = cache().lock() { + guard.clear(); + } +} + +pub(super) fn store(runtime_id: &str, status: &AuthStatus) { + if let Ok(mut guard) = cache().lock() { + guard.insert(runtime_id.to_string(), status.clone()); + } +} + +/// Last known auth status for `runtime_id`, or `AuthStatus::Unknown` when no +/// forced discovery has probed it yet. Never spawns a process. +pub(super) fn get(runtime_id: &str) -> AuthStatus { + cache() + .lock() + .ok() + .and_then(|g| g.get(runtime_id).cloned()) + .unwrap_or(AuthStatus::Unknown) +} + +#[cfg(test)] +pub(crate) fn len() -> usize { + cache().lock().map(|g| g.len()).unwrap_or(0) +} + +/// Resolve the auth status of every available, probeable runtime in `partials`, +/// patching each entry's `auth_status` + `login_hint` in place. +/// +/// Forced discovery spawns one CLI auth probe per available runtime (in +/// parallel; total cost = max(probe latency)) and warms this cache. The cheap +/// default path spawns nothing — it reuses the last cached status, falling back +/// to `Unknown` for a runtime never probed this session. +pub(super) fn resolve_auth_statuses(partials: &mut [super::PartialEntry], force: bool) { + use crate::managed_agents::AcpAvailabilityStatus; + + if force { + let probe_handles: Vec<(usize, std::thread::JoinHandle)> = partials + .iter() + .enumerate() + .filter_map(|(idx, partial)| { + if partial.entry.availability != AcpAvailabilityStatus::Available { + return None; + } + let probe_args = partial.runtime.auth_probe_args?; + // Need the resolved binary path for the CLI (e.g. the actual `claude` binary). + let binary_path = super::resolve_command(probe_args[0])?; + let probe_args_owned: Vec = + probe_args.iter().map(|s| s.to_string()).collect(); + + let handle = std::thread::spawn(move || { + let refs: Vec<&str> = probe_args_owned.iter().map(String::as_str).collect(); + super::probe_auth_status(&binary_path, &refs) + }); + Some((idx, handle)) + }) + .collect(); + + for (idx, handle) in probe_handles { + let status = handle.join().unwrap_or(AuthStatus::Unknown); + store(&partials[idx].entry.id, &status); + patch_entry(&mut partials[idx], status); + } + } else { + for partial in partials.iter_mut() { + if partial.entry.availability != AcpAvailabilityStatus::Available + || partial.runtime.auth_probe_args.is_none() + { + continue; + } + let status = get(&partial.entry.id); + patch_entry(partial, status); + } + } +} + +fn patch_entry(partial: &mut super::PartialEntry, status: AuthStatus) { + partial.entry.login_hint = if matches!(status, AuthStatus::LoggedIn | AuthStatus::NotApplicable) + { + None + } else { + partial.runtime.login_hint.map(str::to_string) + }; + partial.entry.auth_status = status; +} diff --git a/desktop/src-tauri/src/managed_agents/discovery/login_shell.rs b/desktop/src-tauri/src/managed_agents/discovery/login_shell.rs new file mode 100644 index 00000000000..d8f8e603546 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/discovery/login_shell.rs @@ -0,0 +1,236 @@ +//! Login-shell PATH discovery and nvm fallback. +//! +//! Extracted verbatim from `discovery.rs` to keep that file under the +//! file-size ratchet. Covers login-shell candidate selection, the cached +//! login-shell PATH probe, and nvm default-bin resolution. + +use std::path::{Path, PathBuf}; +use std::process::Command; + +use super::is_executable_file; + +/// Test-only spawn counter lives beside `discovery.rs`; import it here so the +/// spawn-record call site stays byte-identical to the pre-extraction source. +#[cfg(test)] +use super::login_shell_spawn_probe; + +/// Collect login shell candidates for the current platform. +/// +/// On Unix: `/bin/zsh`, `/bin/bash` (the historical defaults). +/// On Windows: Git Bash via `resolve_bash_path` — skips `BUZZ_SHELL` because +/// login-shell callers use bash-only `-l -c` syntax. +pub(crate) fn login_shell_candidates() -> Vec { + #[cfg(not(windows))] + { + vec![PathBuf::from("/bin/zsh"), PathBuf::from("/bin/bash")] + } + #[cfg(windows)] + { + super::super::git_bash::resolve_bash_path() + .into_iter() + .collect() + } +} + +/// Run a command in a login shell (tries zsh then bash on Unix, Git Bash on Windows). +/// Returns trimmed stdout if the command succeeds with non-empty output. +fn run_in_login_shell(args: &[&str]) -> Option { + #[cfg(test)] + login_shell_spawn_probe::record(); + for shell in login_shell_candidates() { + let mut cmd = Command::new(&shell); + cmd.args(args); + crate::util::configure_no_window(&mut cmd); + let Ok(output) = cmd.output() else { + continue; + }; + if !output.status.success() { + continue; + } + let stdout = String::from_utf8_lossy(&output.stdout).trim().to_string(); + if !stdout.is_empty() { + return Some(stdout); + } + } + None +} + +pub(crate) fn find_via_login_shell(command: &str) -> Option { + let stdout = run_in_login_shell(&["-l", "-c", r#"command -v -- "$1""#, "_", command])?; + let resolved = stdout.lines().rfind(|line| !line.trim().is_empty())?; + let path = PathBuf::from(resolved.trim()); + (path.is_absolute() && is_executable_file(&path)).then_some(path) +} + +/// Three-state backing store for the login-shell PATH cache. +#[derive(Clone)] +enum LoginShellPath { + /// Cache has never been populated; the next call will spawn a login shell. + Uninit, + /// A login shell was invoked; the inner value is the PATH it returned + /// (`None` when the shell produced no output). + Probed(Option), +} + +fn path_cache() -> &'static std::sync::Mutex { + use std::sync::{Mutex, OnceLock}; + static CACHE: OnceLock> = OnceLock::new(); + CACHE.get_or_init(|| Mutex::new(LoginShellPath::Uninit)) +} + +fn fetch_login_shell_path_inner() -> Option { + // On Windows, Git Bash's `echo $PATH` returns POSIX colon-delimited paths + // (`/mingw64/bin:/c/Users/...`) which poison native Windows children that + // split on `;`. login_shell_path() feeds agent_models, runtime, and + // cli_probe — all native processes. Return None so they inherit the real + // Windows PATH instead. + #[cfg(windows)] + { + return None; + } + + #[cfg(not(windows))] + { + let stdout = run_in_login_shell(&["-l", "-c", "echo $PATH"])?; + let last_line = stdout.lines().rfind(|l| !l.trim().is_empty())?; + Some(last_line.trim().to_string()) + } +} + +/// Return the user's full PATH from a login shell. +/// +/// The result is cached after the first call. Call [`refresh_login_shell_path`] +/// to invalidate the cache so the next call re-fetches — e.g. after the user +/// installs Node.js mid-session and clicks Retry. +/// +/// The lock is never held while the login shell spawns: we check for a cached +/// value, release the lock, run the shell, then re-lock to write. Two concurrent +/// callers may both run the shell (last-writer-wins is fine — both produce the +/// same result), but neither blocks a concurrent agent spawn on the Mutex. +pub fn login_shell_path() -> Option { + // Fast path: return cached result without spawning a shell. + { + let guard = path_cache().lock().unwrap_or_else(|e| e.into_inner()); + if let LoginShellPath::Probed(ref result) = *guard { + return result.clone(); + } + } + + // Slow path: spawn shell outside any lock. + let result = fetch_login_shell_path_inner(); + + // Write back; last-writer-wins is safe here. + { + let mut guard = path_cache().lock().unwrap_or_else(|e| e.into_inner()); + *guard = LoginShellPath::Probed(result.clone()); + } + + result +} + +/// Invalidate the login-shell PATH cache so the next [`login_shell_path`] call +/// re-fetches from a fresh login shell. +/// +/// Called before every install/retry operation and on Doctor Re-run so a +/// newly-installed tool becomes visible without restarting the app. +pub(crate) fn refresh_login_shell_path() { + let mut guard = path_cache().lock().unwrap_or_else(|e| e.into_inner()); + *guard = LoginShellPath::Uninit; +} + +#[cfg(test)] +pub(crate) fn is_login_shell_path_uninit() -> bool { + matches!( + *path_cache().lock().unwrap_or_else(|e| e.into_inner()), + LoginShellPath::Uninit + ) +} + +/// Return `true` when `tag` is a safe nvm alias/version tag that can be joined +/// onto a `PathBuf` without escaping the nvm root. +/// +/// nvm uses tags like `v22.1.0` or `lts/hydrogen`. We allow ASCII alphanumeric +/// plus `. - / _` and require that no path component is `..` and that the tag +/// does not start with `/` (which would replace the base in `PathBuf::join`). +pub(crate) fn is_safe_nvm_tag(tag: &str) -> bool { + if tag.is_empty() { + return false; + } + // An absolute path in the alias file would let PathBuf::join silently + // replace the nvm root with an attacker-controlled path. + if tag.starts_with('/') { + return false; + } + // Reject any .. component to prevent upward traversal. + for component in tag.split('/') { + if component == ".." { + return false; + } + } + // Allow only the characters nvm uses in real tag names. + tag.chars() + .all(|c| c.is_ascii_alphanumeric() || matches!(c, '.' | '-' | '/' | '_')) +} + +/// Locate the `bin` directory for nvm's default Node.js version. +/// +/// Reads `~/.nvm/alias/default`; resolves at most one alias hop to handle +/// nvm alias chains; falls back to the highest-semver directory under +/// `~/.nvm/versions/node/`. Returns the `bin` subdirectory only when it exists. +/// +/// Cheap: at most two file reads or one `read_dir`. Never cached — computed +/// fresh per call so a mid-session `nvm install` is visible at the next spawn. +pub fn find_nvm_default_bin(home: &Path) -> Option { + let nvm_root = home.join(".nvm"); + let versions_root = nvm_root.join("versions").join("node"); + + // 1. Try alias/default, with at most one hop. + let default_alias = nvm_root.join("alias").join("default"); + if let Ok(content) = std::fs::read_to_string(&default_alias) { + let tag = content.trim().to_string(); + if is_safe_nvm_tag(&tag) { + let candidate = versions_root.join(&tag).join("bin"); + if candidate.is_dir() { + return Some(candidate); + } + // One alias hop: ~/.nvm/alias/ + let hop_file = nvm_root.join("alias").join(&tag); + if let Ok(hop_content) = std::fs::read_to_string(&hop_file) { + let hop_tag = hop_content.trim().to_string(); + if is_safe_nvm_tag(&hop_tag) { + let hop_candidate = versions_root.join(&hop_tag).join("bin"); + if hop_candidate.is_dir() { + return Some(hop_candidate); + } + } + } + } + } + + // 2. Fall back to highest-semver directory under ~/.nvm/versions/node/. + let entries = std::fs::read_dir(&versions_root).ok()?; + let best = entries + .filter_map(|e| e.ok()) + .filter_map(|e| { + let name = e.file_name(); + let s = name.to_string_lossy().into_owned(); + parse_semver_tag(&s).map(|v| (v, s)) + }) + .max_by(|(a, _), (b, _)| a.cmp(b)); + + let (_, tag) = best?; + let bin = versions_root.join(&tag).join("bin"); + bin.is_dir().then_some(bin) +} + +/// Parse a `vMAJ.MIN.PATCH` (or `vMAJ.MIN.PATCH-extra`) tag into a numeric +/// triple for semver comparison. +pub(crate) fn parse_semver_tag(s: &str) -> Option<(u64, u64, u64)> { + let s = s.strip_prefix('v')?; + let mut parts = s.splitn(3, '.'); + let major = parts.next()?.parse::().ok()?; + let minor = parts.next()?.parse::().ok()?; + let patch_str = parts.next()?; + let patch = patch_str.split('-').next()?.parse::().ok()?; + Some((major, minor, patch)) +} diff --git a/desktop/src-tauri/src/managed_agents/discovery/login_shell_spawn_probe.rs b/desktop/src-tauri/src/managed_agents/discovery/login_shell_spawn_probe.rs new file mode 100644 index 00000000000..a716dee9f56 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/discovery/login_shell_spawn_probe.rs @@ -0,0 +1,21 @@ +//! Test-only counter for login-shell spawn attempts. +//! +//! `run_in_login_shell` is the single subprocess-spawning step on the +//! absent-command resolution path, so counting its calls proves whether a +//! cheap discovery re-spawns after a negative resolution was cached. + +use std::sync::atomic::{AtomicUsize, Ordering}; + +static COUNT: AtomicUsize = AtomicUsize::new(0); + +pub(crate) fn record() { + COUNT.fetch_add(1, Ordering::SeqCst); +} + +pub(crate) fn reset() { + COUNT.store(0, Ordering::SeqCst); +} + +pub(crate) fn count() -> usize { + COUNT.load(Ordering::SeqCst) +} diff --git a/desktop/src-tauri/src/managed_agents/discovery/presets.rs b/desktop/src-tauri/src/managed_agents/discovery/presets.rs index d86e5f33f05..fd853094515 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/presets.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/presets.rs @@ -336,7 +336,7 @@ mod tests { let _path_guard = crate::managed_agents::lock_path_mutex(); let _registry_guard = registry_test_lock(); - let entry = super::super::discover_acp_runtimes_from(None) + let entry = super::super::discover_acp_runtimes_from(None, true) .into_iter() .find(|entry| entry.id == "devin") .expect("Devin preset should appear in the runtime catalog"); diff --git a/desktop/src-tauri/src/managed_agents/discovery/tests.rs b/desktop/src-tauri/src/managed_agents/discovery/tests.rs index f7e233fbe95..2d1db692932 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/tests.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/tests.rs @@ -4,11 +4,10 @@ use super::overrides::{divergent_agent_command_override, update_time_agent_comma use super::{ apply_agent_command_update, classify_runtime, codex_adapter_availability, codex_adapter_is_outdated, create_time_agent_command_override, default_agent_command, - effective_agent_command, find_nvm_default_bin, find_via_login_shell, - is_login_shell_path_uninit, is_safe_nvm_tag, managed_agent_avatar_url, normalize_agent_args, - parse_semver_tag, probe_codex_acp_version, record_agent_command, refresh_login_shell_path, - try_record_agent_command, BUZZ_AGENT_AVATAR_URL, CLAUDE_CODE_AVATAR_URL, CODEX_AVATAR_URL, - GOOSE_AVATAR_URL, + effective_agent_command, find_nvm_default_bin, is_login_shell_path_uninit, is_safe_nvm_tag, + managed_agent_avatar_url, normalize_agent_args, parse_semver_tag, probe_codex_acp_version, + record_agent_command, refresh_login_shell_path, try_record_agent_command, + BUZZ_AGENT_AVATAR_URL, CLAUDE_CODE_AVATAR_URL, CODEX_AVATAR_URL, GOOSE_AVATAR_URL, }; use crate::managed_agents::AcpAvailabilityStatus; @@ -94,24 +93,6 @@ fn normalizes_buzz_agent_args_to_empty() { ); } -#[test] -fn login_shell_lookup_treats_command_as_data() { - let marker = - std::env::temp_dir().join(format!("buzz-discovery-marker-{}", uuid::Uuid::new_v4())); - let payload = format!("doesnotexist; touch {} #", marker.display()); - - let resolved = find_via_login_shell(&payload); - - assert!( - resolved.is_none(), - "payload should not resolve to a command" - ); - assert!( - !marker.exists(), - "shell lookup must not execute injected commands" - ); -} - #[cfg(unix)] #[test] fn explicit_path_resolution_ignores_non_executable_files() { @@ -668,8 +649,8 @@ fn apply_agent_command_update_concrete_pin_keeps_materialized_runtime() { // ── probe_codex_acp_version ─────────────────────────────────────────────────── +mod forced_discovery; mod managed_path_resolution; - #[cfg(unix)] #[test] fn probe_codex_acp_version_parses_full_semver_output() { @@ -1685,7 +1666,7 @@ fn custom_catalog_entry_carries_definition_env_for_edit_roundtrip() { ) .unwrap(); - let entries = discover_acp_runtimes_from(Some(dir.path())); + let entries = discover_acp_runtimes_from(Some(dir.path()), true); let entry = entries .iter() .find(|e| e.id == "env-harness") @@ -1715,7 +1696,7 @@ fn builtin_catalog_entry_has_empty_definition_env() { // publishes to the global registry. let _path_guard = crate::managed_agents::lock_path_mutex(); let _lock = registry_test_lock(); - let entries = discover_acp_runtimes_from(None); + let entries = discover_acp_runtimes_from(None, true); // Find any builtin entry (e.g. "goose" or "claude"). let builtin = entries .iter() @@ -1796,7 +1777,7 @@ fn discovery_publish_path_survives_mid_flight_save() { assert!(lookup_loaded_harness_by_id("mid-flight-save").is_some()); })); - let _entries = discover_acp_runtimes_from(Some(dir.path())); + let _entries = discover_acp_runtimes_from(Some(dir.path()), true); assert!( lookup_loaded_harness_by_id("mid-flight-save").is_some(), @@ -1829,7 +1810,7 @@ fn discovery_publish_path_drops_mid_flight_delete() { assert!(lookup_loaded_harness_by_id("mid-flight-delete").is_none()); })); - let _entries = discover_acp_runtimes_from(Some(dir.path())); + let _entries = discover_acp_runtimes_from(Some(dir.path()), true); assert!( lookup_loaded_harness_by_id("mid-flight-delete").is_none(), diff --git a/desktop/src-tauri/src/managed_agents/discovery/tests/forced_discovery.rs b/desktop/src-tauri/src/managed_agents/discovery/tests/forced_discovery.rs new file mode 100644 index 00000000000..cfbad365e3a --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/discovery/tests/forced_discovery.rs @@ -0,0 +1,163 @@ +// ── Cheap vs. forced discovery: the auth-probe split ──────────────────────── +// +// `discover_acp_providers(force: true)` spawns one CLI auth probe per available +// runtime; the cheap default path must reuse the last cached status and spawn +// nothing. These tests pin that split through the real `discover_acp_runtimes_from` +// pipeline with a fake `claude` CLI that records every invocation to a sentinel. + +/// Build a fake `claude` runtime on a fresh PATH: the adapter (`claude-agent-acp`) +/// and the CLI (`claude`). The CLI appends a line to `probe_log` each time it +/// runs and exits 0 (→ `LoggedIn`), so the log's existence proves whether the +/// auth probe was spawned. +#[cfg(unix)] +#[test] +fn forced_discovery_probes_auth_but_cheap_discovery_reuses_cached_status() { + use crate::managed_agents::custom_harnesses::registry_test_lock; + use crate::managed_agents::discovery::{clear_resolve_cache, discover_acp_runtimes_from}; + use crate::managed_agents::{AcpAvailabilityStatus, AuthStatus}; + use std::os::unix::fs::PermissionsExt; + + let _path_guard = crate::managed_agents::lock_path_mutex(); + let _registry_guard = registry_test_lock(); + + let dir = tempfile::tempdir().expect("tempdir"); + let probe_log = dir.path().join("claude-probe.log"); + + for name in ["claude-agent-acp", "claude"] { + let bin = dir.path().join(name); + // The adapter is never executed; only `claude` logs + exits 0. + let script = format!( + "#!/bin/sh\necho ran >> \"{}\"\nexit 0\n", + probe_log.display() + ); + std::fs::write(&bin, script).expect("write fake bin"); + std::fs::set_permissions(&bin, std::fs::Permissions::from_mode(0o755)).expect("chmod"); + } + + // Start from a clean resolve + auth cache, and a PATH that only sees our fakes. + clear_resolve_cache(); + let old_path = std::env::var_os("PATH").unwrap_or_default(); + let mut new_path = vec![dir.path().to_path_buf()]; + new_path.extend(std::env::split_paths(&old_path)); + std::env::set_var("PATH", std::env::join_paths(&new_path).expect("join PATH")); + + let result = std::panic::catch_unwind(|| { + // ── Forced: probes run, status is LoggedIn, cache is warmed. ────────── + let forced = discover_acp_runtimes_from(None, true); + let claude = forced + .iter() + .find(|e| e.id == "claude") + .expect("claude entry present"); + assert_eq!(claude.availability, AcpAvailabilityStatus::Available); + assert_eq!(claude.auth_status, AuthStatus::LoggedIn); + assert!( + probe_log.exists(), + "forced discovery must spawn the auth probe" + ); + assert!( + super::super::auth_status_cache::len() > 0, + "forced discovery must warm the auth-status cache" + ); + + // ── Cheap: no probe spawned, status reused from cache. ──────────────── + std::fs::remove_file(&probe_log).expect("clear probe log"); + let cheap = discover_acp_runtimes_from(None, false); + let claude = cheap + .iter() + .find(|e| e.id == "claude") + .expect("claude entry present"); + assert_eq!( + claude.availability, + AcpAvailabilityStatus::Available, + "cheap path keeps availability (resolved from cache)" + ); + assert_eq!( + claude.auth_status, + AuthStatus::LoggedIn, + "cheap path must reuse the cached auth status" + ); + assert!( + !probe_log.exists(), + "cheap discovery must not spawn any auth probe" + ); + }); + + // Restore global state before propagating any panic. + std::env::set_var("PATH", &old_path); + clear_resolve_cache(); + if let Err(e) = result { + std::panic::resume_unwind(e); + } +} + +/// Before any forced probe warms the resolve cache, the cheap path resolves +/// nothing live — it must not resolve a present-but-uncached binary by spawning +/// a login shell to discover it. This is the flip side of the zero-spawn +/// contract: cache-only resolution cannot see a binary the forced path has not +/// yet cached. The forced path (exercised on every surface mount) resolves it +/// and warms the cache; a subsequent cheap call then sees it Available (covered +/// by `forced_discovery_probes_auth_but_cheap_discovery_reuses_cached_status`). +/// +/// The assertion is scoped to what holds on any machine: the fake PATH-only +/// `claude` CLI must not be resolved by the cheap path (availability is never +/// `Available`, auth stays `Unknown`) and no login shell is spawned. It does +/// not pin the exact `NotInstalled` vs `CliMissing` variant, because a real +/// Buzz-managed `claude-agent-acp` shim on the host resolves via a filesystem +/// stat (production-correct, never a spawn) and yields `CliMissing` — a genuine +/// environment difference, not a regression. +#[cfg(unix)] +#[test] +fn cheap_discovery_reports_absent_before_any_forced_probe() { + use crate::managed_agents::custom_harnesses::registry_test_lock; + use crate::managed_agents::discovery::{ + clear_resolve_cache, discover_acp_runtimes_from, login_shell_spawn_probe, + }; + use crate::managed_agents::{AcpAvailabilityStatus, AuthStatus}; + use std::os::unix::fs::PermissionsExt; + + let _path_guard = crate::managed_agents::lock_path_mutex(); + let _registry_guard = registry_test_lock(); + + let dir = tempfile::tempdir().expect("tempdir"); + for name in ["claude-agent-acp", "claude"] { + let bin = dir.path().join(name); + std::fs::write(&bin, "#!/bin/sh\nexit 0\n").expect("write fake bin"); + std::fs::set_permissions(&bin, std::fs::Permissions::from_mode(0o755)).expect("chmod"); + } + + clear_resolve_cache(); // also clears the auth-status cache + login_shell_spawn_probe::reset(); + let old_path = std::env::var_os("PATH").unwrap_or_default(); + let mut new_path = vec![dir.path().to_path_buf()]; + new_path.extend(std::env::split_paths(&old_path)); + std::env::set_var("PATH", std::env::join_paths(&new_path).expect("join PATH")); + + let result = std::panic::catch_unwind(|| { + let cheap = discover_acp_runtimes_from(None, false); + let claude = cheap + .iter() + .find(|e| e.id == "claude") + .expect("claude entry present"); + assert_ne!( + claude.availability, + AcpAvailabilityStatus::Available, + "cache-only cheap discovery must not resolve the PATH-only claude CLI live" + ); + assert_eq!( + claude.auth_status, + AuthStatus::Unknown, + "an unresolved runtime with no cached status stays Unknown" + ); + assert_eq!( + login_shell_spawn_probe::count(), + 0, + "cheap discovery must not spawn a login shell to resolve the PATH-only CLI" + ); + }); + + std::env::set_var("PATH", &old_path); + clear_resolve_cache(); + if let Err(e) = result { + std::panic::resume_unwind(e); + } +} 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 0795bb2345e..5369b6321b7 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 @@ -1,5 +1,28 @@ use crate::managed_agents::discovery::{clear_resolve_cache, resolve_command}; +/// A login-shell command lookup must treat its argument as pure data — a +/// payload containing shell metacharacters must never execute. +#[test] +fn login_shell_lookup_treats_command_as_data() { + use super::super::find_via_login_shell; + + let _guard = crate::managed_agents::lock_path_mutex(); + let marker = + std::env::temp_dir().join(format!("buzz-discovery-marker-{}", uuid::Uuid::new_v4())); + let payload = format!("doesnotexist; touch {} #", marker.display()); + + let resolved = find_via_login_shell(&payload); + + assert!( + resolved.is_none(), + "payload should not resolve to a command" + ); + assert!( + !marker.exists(), + "shell lookup must not execute injected commands" + ); +} + /// The legacy Goose Windows installer wrote `%USERPROFILE%\goose\goose.exe`, /// a directory on no standard PATH. `resolve_command_uncached` finds binaries /// outside PATH only by scanning `common_binary_paths()`, so that directory @@ -88,3 +111,79 @@ fn resolve_command_prefers_buzz_managed_npm_shim_over_path() { "Buzz-managed npm shim must win over PATH/global shims" ); } + +/// The cheap discovery path must never spawn a login shell — not even on a +/// cold cache. +/// +/// `force: false` resolves commands from cache only (`resolve_command_cached`): +/// on a resolve-cache miss it reports the command absent instead of falling +/// through to `resolve_command_uncached` → `find_via_login_shell`, which spawns +/// zsh/bash. That spawn on the channel-switch/composer hot path is the exact +/// freeze source the cheap path exists to avoid, so a cold cheap call must +/// spawn zero login shells. The forced path remains the sole prober: the same +/// absent-command fixture spawns at least once under `force: true`, proving the +/// cheap-path zero is real and not a fixture that never reaches the probe. +#[cfg(unix)] +#[test] +fn cheap_discovery_never_spawns_login_shell_even_when_cold() { + use crate::managed_agents::custom_harnesses::registry_test_lock; + use crate::managed_agents::discovery::{ + clear_resolve_cache, discover_acp_runtimes_from, login_shell_spawn_probe, + }; + use std::fs; + use tempfile::tempdir; + + // Serialize with every other test that spawns a login shell: the spawn + // counter and the PATH/login-shell caches are process-global. + let _path_guard = crate::managed_agents::lock_path_mutex(); + let _registry = registry_test_lock(); + + // A custom harness whose command cannot resolve anywhere, so the resolver + // reaches `find_via_login_shell` under the forced (live) path. + let dir = tempdir().unwrap(); + fs::write( + dir.path().join("absent-harness.json"), + r#"{ + "id": "absent-harness", + "label": "Absent Harness", + "command": "buzz-absent-command-xyzzy", + "args": [] + }"#, + ) + .unwrap(); + + // Cold cache, cheap path: must spawn ZERO login shells (cache-only resolve + // reports the absent command missing without probing). + clear_resolve_cache(); + login_shell_spawn_probe::reset(); + let _ = discover_acp_runtimes_from(Some(dir.path()), false); + let cold_cheap = login_shell_spawn_probe::count(); + assert_eq!( + cold_cheap, 0, + "a cold cheap discovery must not spawn any login shell, got {cold_cheap}" + ); + + // Second cheap discovery, still cold (no forced probe populated the cache): + // still zero — cache-only resolution never probes. + login_shell_spawn_probe::reset(); + let _ = discover_acp_runtimes_from(Some(dir.path()), false); + let second_cheap = login_shell_spawn_probe::count(); + assert_eq!( + second_cheap, 0, + "a repeated cheap discovery must not spawn any login shell, got {second_cheap}" + ); + + // Forced path over the SAME absent fixture: resolves live and reaches + // `find_via_login_shell` at least once. Proves the cheap-path zero above is + // genuine — the fixture does drive the probe when live resolution runs — + // not a vacuous zero from a fixture that never reaches it. + clear_resolve_cache(); + login_shell_spawn_probe::reset(); + let _ = discover_acp_runtimes_from(Some(dir.path()), true); + let forced = login_shell_spawn_probe::count(); + clear_resolve_cache(); + assert!( + forced >= 1, + "the forced path must probe the absent command via login shell at least once, got {forced}" + ); +} diff --git a/desktop/src-tauri/src/managed_agents/runtime/cli_tests.rs b/desktop/src-tauri/src/managed_agents/runtime/cli_tests.rs new file mode 100644 index 00000000000..2d4fee340a1 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/runtime/cli_tests.rs @@ -0,0 +1,38 @@ +//! Runtime CLI configuration regression tests kept beside the configured seam. + +use super::super::configure_runtime_cli; +use crate::managed_agents::known_acp_runtime; + +#[test] +fn claude_spawn_uses_the_probed_cli_executable() { + let _guard = crate::managed_agents::lock_path_mutex(); + let temp = tempfile::tempdir().expect("temp dir"); + let cli = temp + .path() + .join(format!("claude{}", std::env::consts::EXE_SUFFIX)); + std::fs::write(&cli, "").expect("write fake cli"); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(&cli, std::fs::Permissions::from_mode(0o755)) + .expect("make fake cli executable"); + } + let original_path = std::env::var_os("PATH"); + std::env::set_var("PATH", temp.path()); + // The resolver retains negative results across tests, so the fake CLI must + // invalidate both before configuration and after restoring PATH. + crate::managed_agents::clear_resolve_cache(); + + let mut command = std::process::Command::new("buzz-acp"); + configure_runtime_cli(&mut command, known_acp_runtime("claude-agent-acp")); + + if let Some(path) = original_path { + std::env::set_var("PATH", path); + } else { + std::env::remove_var("PATH"); + } + crate::managed_agents::clear_resolve_cache(); + assert!(command + .get_envs() + .any(|(key, value)| { key == "CLAUDE_CODE_EXECUTABLE" && value == Some(cli.as_os_str()) })); +} diff --git a/desktop/src-tauri/src/managed_agents/runtime/tests.rs b/desktop/src-tauri/src/managed_agents/runtime/tests.rs index b54c0e7a050..8bedfe53207 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/tests.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/tests.rs @@ -1,5 +1,8 @@ use crate::managed_agents::known_acp_runtime; +#[path = "cli_tests.rs"] +mod cli_tests; + // ── desktop binary name tests ─────────────────────────────────────────── #[test] @@ -582,36 +585,6 @@ fn name_matches_interpreter_rejects_node_prefix() { assert!(!super::name_matches_interpreter("node-gyp")); } -#[test] -fn claude_spawn_uses_the_probed_cli_executable() { - let _guard = crate::managed_agents::lock_path_mutex(); - let temp = tempfile::tempdir().expect("temp dir"); - let cli = temp - .path() - .join(format!("claude{}", std::env::consts::EXE_SUFFIX)); - std::fs::write(&cli, "").expect("write fake cli"); - #[cfg(unix)] - { - use std::os::unix::fs::PermissionsExt; - std::fs::set_permissions(&cli, std::fs::Permissions::from_mode(0o755)) - .expect("make fake cli executable"); - } - let original_path = std::env::var_os("PATH"); - std::env::set_var("PATH", temp.path()); - - let mut command = std::process::Command::new("buzz-acp"); - super::configure_runtime_cli(&mut command, super::known_acp_runtime("claude-agent-acp")); - - if let Some(path) = original_path { - std::env::set_var("PATH", path); - } else { - std::env::remove_var("PATH"); - } - assert!(command - .get_envs() - .any(|(key, value)| { key == "CLAUDE_CODE_EXECUTABLE" && value == Some(cli.as_os_str()) })); -} - #[test] fn codex_spawn_does_not_set_a_claude_executable() { let mut command = std::process::Command::new("buzz-acp"); diff --git a/desktop/src/features/agents/acpRuntimesQuery.test.mjs b/desktop/src/features/agents/acpRuntimesQuery.test.mjs new file mode 100644 index 00000000000..c51dea05b8f --- /dev/null +++ b/desktop/src/features/agents/acpRuntimesQuery.test.mjs @@ -0,0 +1,491 @@ +/** + * Regression tests for the cheap/forced ACP runtime discovery split. + * + * Two IMPORTANT correctness contracts from the review of the split: + * + * (1) refreshAcpRuntimes() must never coalesce onto an in-flight *cheap* + * request. React Query's fetchQuery deduplicates on the shared query key, + * so a cheap fetch already running would otherwise satisfy the forced + * refresh with cached data and the forced { force: true } probe would + * never run. The fix runs the forced probe on a separate key, writes its + * result into the shared cache, then cancels the in-flight cheap query. + * This test holds a cheap request pending, fires refreshAcpRuntimes(), + * resolves the cheap request, and asserts a distinct { force: true } native + * call happened and the shared cache holds the forced result. + * + * (2) useAcpRuntimesQueryForced({ forceOnMount: false }) must consume shared + * state without mounting its own force effect. Onboarding mounts the hook + * once as the surface owner (forceOnMount default true) and once per row + * (forceOnMount false); entering the surface must cause exactly one forced + * native call before any user action. + * + * The Tauri IPC bridge is stubbed at globalThis.__TAURI_INTERNALS__.invoke so + * discoverAcpRuntimes() calls are intercepted by command name and the { force } + * payload is observed directly (same pattern as + * useLoadArchivedObserverEvents.test.mjs). + */ + +import assert from "node:assert/strict"; +import { afterEach, describe, it } from "node:test"; + +// ── Minimal DOM shim (subset used by other mounted-hook tests) ──────────────── + +function installDOMShim() { + if (globalThis.document) return; + + class MinimalEventTarget { + constructor() { + this._listeners = {}; + } + addEventListener(type, fn) { + this._listeners[type] ??= []; + this._listeners[type].push(fn); + } + removeEventListener(type, fn) { + this._listeners[type] = (this._listeners[type] ?? []).filter( + (f) => f !== fn, + ); + } + dispatchEvent(e) { + for (const fn of this._listeners[e.type] ?? []) fn(e); + return true; + } + } + + class MinimalNode extends MinimalEventTarget { + constructor(tagName) { + super(); + this.tagName = tagName; + this.children = []; + this.childNodes = []; + this.style = {}; + this.nodeType = 1; + this.parentNode = null; + } + get ownerDocument() { + return globalThis.document; + } + get firstChild() { + return this.children[0] ?? null; + } + get nextSibling() { + return null; + } + appendChild(child) { + this.children.push(child); + this.childNodes.push(child); + child.parentNode = this; + return child; + } + removeChild(child) { + this.children = this.children.filter((c) => c !== child); + this.childNodes = this.childNodes.filter((c) => c !== child); + return child; + } + insertBefore(newNode, refNode) { + if (!refNode) return this.appendChild(newNode); + const i = this.children.indexOf(refNode); + if (i < 0) return this.appendChild(newNode); + this.children.splice(i, 0, newNode); + this.childNodes.splice(i, 0, newNode); + newNode.parentNode = this; + return newNode; + } + contains(node) { + if (!node) return false; + return this === node || this.children.some((c) => c?.contains?.(node)); + } + } + + class MinimalDocument extends MinimalEventTarget { + constructor() { + super(); + this.nodeType = 9; + } + createElement(tagName) { + return new MinimalNode(tagName); + } + createTextNode(value) { + const n = new MinimalNode("#text"); + n.nodeValue = value; + n.nodeType = 3; + return n; + } + createComment(value) { + const n = new MinimalNode("#comment"); + n.nodeValue = value; + n.nodeType = 8; + return n; + } + get body() { + if (!this._body) this._body = this.createElement("body"); + return this._body; + } + get activeElement() { + return null; + } + contains(node) { + return node != null; + } + } + + globalThis.document = new MinimalDocument(); + globalThis.HTMLElement = MinimalNode; + globalThis.HTMLIFrameElement = MinimalNode; + globalThis.IS_REACT_ACT_ENVIRONMENT = true; + process.env.IS_REACT_ACT_ENVIRONMENT = "true"; + if (typeof globalThis.window === "undefined") { + Object.defineProperty(globalThis, "window", { + value: globalThis, + configurable: true, + }); + } + if (!Object.getOwnPropertyDescriptor(globalThis, "navigator")?.value) { + Object.defineProperty(globalThis, "navigator", { + value: { userAgent: "node" }, + configurable: true, + }); + } + globalThis.MutationObserver = class { + observe() {} + disconnect() {} + takeRecords() { + return []; + } + }; + globalThis.requestAnimationFrame = (fn) => setTimeout(fn, 0); +} + +installDOMShim(); + +// ── Tauri IPC interceptor ───────────────────────────────────────────────────── + +/** @type {Array<{ command: string, args: unknown }>} */ +const calls = []; +/** @type {(args: unknown) => Promise} */ +let discoverHandler = () => Promise.resolve([]); + +globalThis.__TAURI_INTERNALS__ = { + invoke: (command, args) => { + calls.push({ command, args }); + if (command === "discover_acp_providers") return discoverHandler(args); + return Promise.reject(new Error(`unmocked Tauri command: ${command}`)); + }, + transformCallback: () => Math.random(), +}; + +// ── Production imports (after shim + IPC stub) ──────────────────────────────── + +import React from "react"; +import { createRoot } from "react-dom/client"; +import { act } from "react"; +import { QueryClient } from "@tanstack/react-query"; +import { QueryClientProvider } from "@tanstack/react-query"; + +import { + acpRuntimesQueryKey, + refreshAcpRuntimes, + useAcpRuntimesQueryForced, +} from "./acpRuntimesQuery.ts"; +import { discoverAcpRuntimes } from "@/shared/api/tauriAcpDiscovery.ts"; + +// ── Wire-shape helper ───────────────────────────────────────────────────────── + +/** A raw discover_acp_providers row (snake_case wire shape). */ +function rawEntry(id, authStatusValue) { + return { + id, + label: id, + avatar_url: "", + availability: "available", + command: id, + binary_path: `/usr/bin/${id}`, + default_args: [], + mcp_command: null, + install_hint: "", + install_instructions_url: "", + can_auto_install: false, + underlying_cli_path: null, + node_required: false, + auth_status: { status: authStatusValue }, + source: "builtin", + }; +} + +function makeQueryClient() { + return new QueryClient({ defaultOptions: { queries: { retry: false } } }); +} + +/** A promise plus its resolver, for holding a request pending. */ +function deferred() { + let resolve; + const promise = new Promise((r) => { + resolve = r; + }); + return { promise, resolve }; +} + +afterEach(() => { + calls.length = 0; + discoverHandler = () => Promise.resolve([]); +}); + +describe("refreshAcpRuntimes cannot dedup onto an in-flight cheap request", () => { + it("runs a distinct force:true probe and writes it into the shared cache", async () => { + const queryClient = makeQueryClient(); + queryClient.mount(); + + // 1. A cheap request (force:false) is in flight and held pending. + const cheap = deferred(); + discoverHandler = (args) => { + if (args?.force === false) return cheap.promise; + // 2. The forced request resolves immediately with distinct data. + return Promise.resolve([rawEntry("codex", "logged_in")]); + }; + + // Start the cheap fetch through the real cheap query path and leave pending. + const cheapFetch = queryClient.fetchQuery({ + queryKey: acpRuntimesQueryKey, + queryFn: () => discoverAcpRuntimes(), + staleTime: 30 * 60_000, + }); + await new Promise((r) => setImmediate(r)); + + // 3. Forced refresh fires while the cheap fetch is still pending. + const forced = await refreshAcpRuntimes(queryClient); + + // 4. Resolve the cheap request afterward; it must not be what the caller got. + cheap.resolve([rawEntry("codex", "unknown")]); + await cheapFetch.catch(() => {}); + + const forceCalls = calls.filter( + (c) => c.command === "discover_acp_providers" && c.args?.force === true, + ); + assert.equal( + forceCalls.length, + 1, + "exactly one forced native probe must have run", + ); + assert.equal(forced[0]?.authStatus.status, "logged_in"); + assert.equal( + queryClient.getQueryData(acpRuntimesQueryKey)?.[0]?.authStatus.status, + "logged_in", + "shared cache must hold the forced result, not the later cheap one", + ); + + queryClient.unmount(); + }); +}); + +describe("useAcpRuntimesQueryForced surfaces forced-probe failures", () => { + it("projects a mount-time forced rejection into error with no unhandled rejection", async () => { + const unhandled = []; + const onUnhandled = (err) => unhandled.push(err); + process.on("unhandledRejection", onUnhandled); + + const queryClient = makeQueryClient(); + discoverHandler = (args) => + args?.force === true + ? Promise.reject(new Error("forced probe failed")) + : Promise.resolve([]); + + let latest = null; + function Consumer() { + latest = useAcpRuntimesQueryForced(); + return null; + } + + const container = document.createElement("div"); + const root = createRoot(container); + await act(async () => { + root.render( + React.createElement( + QueryClientProvider, + { client: queryClient }, + React.createElement(Consumer), + ), + ); + }); + await act(async () => { + await new Promise((r) => setTimeout(r, 50)); + }); + + assert.equal( + latest?.error instanceof Error && latest.error.message, + "forced probe failed", + "mount-time forced rejection must surface as the hook's error", + ); + assert.equal( + latest?.isError, + true, + "isError must reflect the forced failure", + ); + + // Drain the microtask queue so any stray rejection would have fired. + await new Promise((r) => setTimeout(r, 10)); + process.off("unhandledRejection", onUnhandled); + assert.deepEqual( + unhandled, + [], + "no unhandled rejection may escape the fire-and-forget mount force", + ); + + await act(async () => { + root.unmount(); + }); + }); + + it("surfaces an explicit-refresh rejection and clears it on the next success", async () => { + const unhandled = []; + const onUnhandled = (err) => unhandled.push(err); + process.on("unhandledRejection", onUnhandled); + + const queryClient = makeQueryClient(); + let failForced = true; + discoverHandler = (args) => { + if (args?.force !== true) return Promise.resolve([]); + return failForced + ? Promise.reject(new Error("refresh failed")) + : Promise.resolve([rawEntry("codex", "logged_in")]); + }; + + let latest = null; + function Consumer() { + // forceOnMount:false so the only forced probe is the explicit refresh. + latest = useAcpRuntimesQueryForced({ forceOnMount: false }); + return null; + } + + const container = document.createElement("div"); + const root = createRoot(container); + await act(async () => { + root.render( + React.createElement( + QueryClientProvider, + { client: queryClient }, + React.createElement(Consumer), + ), + ); + }); + + // Explicit refresh (button/polling shape): void-called, must not reject. + await act(async () => { + void latest.forceRefresh(); + await new Promise((r) => setTimeout(r, 50)); + }); + assert.equal( + latest?.error instanceof Error && latest.error.message, + "refresh failed", + "explicit-refresh rejection must surface as the hook's error", + ); + + // A subsequent successful refresh clears the error and delivers data. + failForced = false; + await act(async () => { + void latest.forceRefresh(); + await new Promise((r) => setTimeout(r, 50)); + }); + assert.equal( + latest?.error, + null, + "a later successful refresh clears the error", + ); + assert.equal( + queryClient.getQueryData(acpRuntimesQueryKey)?.[0]?.authStatus.status, + "logged_in", + "successful refresh writes the fresh catalog into the shared cache", + ); + + await new Promise((r) => setTimeout(r, 10)); + process.off("unhandledRejection", onUnhandled); + assert.deepEqual( + unhandled, + [], + "no unhandled rejection may escape a void forceRefresh() call", + ); + + await act(async () => { + root.unmount(); + }); + }); +}); + +describe("useAcpRuntimesQueryForced force-on-mount ownership", () => { + it("a later-mounted row does not fire a second forced probe", async () => { + const queryClient = makeQueryClient(); + discoverHandler = () => Promise.resolve([rawEntry("codex", "logged_in")]); + + // Onboarding's real sequence: the surface owner mounts and forces discovery; + // once its result renders, per-runtime rows mount. A row that shared the + // owner's default force-on-mount would fire a *second*, sequential forced + // probe (forced-key dedup cannot collapse it — the owner's fetch is already + // idle). Rows pass forceOnMount:false to consume shared state only. + function Owner() { + useAcpRuntimesQueryForced(); + return null; + } + function Row() { + useAcpRuntimesQueryForced({ forceOnMount: false }); + return null; + } + + const container = document.createElement("div"); + const root = createRoot(container); + + // 1. Owner mounts and forces once; let the probe settle. + await act(async () => { + root.render( + React.createElement( + QueryClientProvider, + { client: queryClient }, + React.createElement(Owner), + ), + ); + }); + await act(async () => { + await new Promise((r) => setTimeout(r, 50)); + }); + const afterOwner = calls.filter( + (c) => c.command === "discover_acp_providers" && c.args?.force === true, + ).length; + assert.equal(afterOwner, 1, "owner mount must force exactly once"); + + // 2. Rows mount after the owner's result settled; they must not re-probe. + await act(async () => { + root.render( + React.createElement( + QueryClientProvider, + { client: queryClient }, + React.createElement(Owner), + React.createElement(Row), + React.createElement(Row), + React.createElement(Row), + ), + ); + }); + await act(async () => { + await new Promise((r) => setTimeout(r, 50)); + }); + + const forceCalls = calls.filter( + (c) => c.command === "discover_acp_providers" && c.args?.force === true, + ); + assert.equal( + forceCalls.length, + 1, + "later-mounted rows must not trigger a second forced probe", + ); + const cheapCalls = calls.filter( + (c) => c.command === "discover_acp_providers" && c.args?.force === false, + ); + assert.equal( + cheapCalls.length, + 0, + "the forced hook must never fire a cheap fetch (enabled: false observer)", + ); + + await act(async () => { + root.unmount(); + }); + }); +}); diff --git a/desktop/src/features/agents/acpRuntimesQuery.ts b/desktop/src/features/agents/acpRuntimesQuery.ts new file mode 100644 index 00000000000..0e76e25ee76 --- /dev/null +++ b/desktop/src/features/agents/acpRuntimesQuery.ts @@ -0,0 +1,135 @@ +import * as React from "react"; +import { useQuery, useQueryClient } from "@tanstack/react-query"; + +import { discoverAcpRuntimes } from "@/shared/api/tauriAcpDiscovery"; + +/** + * Shared React Query key for the ACP runtime catalog. Every consumer (cheap or + * forced) reads and writes this one entry, so a forced refresh updates the same + * cache the hot-path `useAcpRuntimesQuery` renders from. + */ +export const acpRuntimesQueryKey = ["acp-runtimes"] as const; + +/** + * Separate key for the forced (full re-discovery) fetch. Forced refresh runs on + * *this* key, never the shared cheap key, so React Query's `fetchQuery` can + * never deduplicate a forced probe onto an in-flight cheap request for the + * shared key. The forced result is then written into the shared cache + * deliberately (see `refreshAcpRuntimes`). + */ +export const acpRuntimesForcedQueryKey = ["acp-runtimes", "forced"] as const; + +/** + * Run a forced (full re-discovery) refresh and write the result into the shared + * runtime-catalog cache. + * + * This is the only path that pays the expensive discovery pipeline (cache + * clear, PATH re-fetch, CLI auth probes). Surfaces that need fresh state call + * it deliberately: Settings/onboarding on open and on their refresh buttons, + * and the connect/install/save/delete mutations in `onSettled`. A bare + * `invalidateQueries` would only re-run the cheap query path and never + * re-probe, so the freshly-changed auth/catalog state would not be reflected. + * + * The forced fetch runs on its own key so it can never coalesce onto an + * in-flight *cheap* request for the shared key (which would satisfy the caller + * with cached availability and never run the `{ force: true }` probe). Its + * result is then written into the shared cache with `setQueryData` so hot + * surfaces rendering `useAcpRuntimesQuery` re-render with the fresh catalog. + * Concurrent forced callers still dedup on the forced key; the backend + * coalesces overlapping forced runs as a second layer. + */ +export async function refreshAcpRuntimes( + queryClient: ReturnType, +) { + try { + const result = await queryClient.fetchQuery({ + queryKey: acpRuntimesForcedQueryKey, + queryFn: () => discoverAcpRuntimes({ force: true }), + staleTime: 0, + gcTime: 0, + }); + queryClient.setQueryData(acpRuntimesQueryKey, result); + // A hot-surface cheap fetch may already be in flight on the shared key; cancel + // it so its (older, cached) result cannot land after and clobber the fresh + // forced catalog we just wrote. + await queryClient.cancelQueries({ queryKey: acpRuntimesQueryKey }); + return result; + } catch { + // The forced probe rejected. `fetchQuery` has already recorded the error in + // the forced key's query state, where `useAcpRuntimesQueryForced` projects + // it into the hook's returned `error`/`isError`. Swallow the rejection here + // — at the single source — so the many fire-and-forget callers (mount, + // sign-in polling, refresh buttons, and the four mutation `onSettled` + // paths) can keep `void refreshAcpRuntimes(...)` without ever leaking an + // unhandled rejection, and a new call site can never reintroduce one. The + // shared cache is left untouched so consumers keep the last good catalog + // alongside the surfaced error. + return undefined; + } +} + +/** + * ACP runtimes query for surfaces that need fresh auth/version state: Settings + * harness panels and onboarding. + * + * It reads the shared runtime catalog (`enabled: false`, so it never fires its + * own cheap fetch — the forced probe below is the only fetcher) and re-renders + * whenever `refreshAcpRuntimes` writes a fresh catalog into that cache. Loading + * *and error* state are taken from a disabled observer on the forced key, so + * refresh buttons and the onboarding spinner reflect the forced probe and a + * failed probe surfaces as `error`/`isError` rather than a silent empty + * catalog. `forceRefresh` drives explicit refresh buttons and sign-in + * polling. + * + * `forceOnMount` (default `true`) is the surface owner's one force-on-mount. + * Child rows that share the same surface must pass `forceOnMount: false`: they + * consume the shared query state and the `forceRefresh` callback, but must not + * mount a *second* force effect. Each mounted force effect is a distinct forced + * probe, so an owner + N rows would otherwise re-run the 20–65s pipeline N+1 + * times on entry (and race the catalog to a later state before the owner's + * first result renders). + */ +export function useAcpRuntimesQueryForced(options?: { + enabled?: boolean; + forceOnMount?: boolean; +}) { + const enabled = options?.enabled ?? true; + const forceOnMount = options?.forceOnMount ?? true; + const queryClient = useQueryClient(); + const query = useQuery({ + queryKey: acpRuntimesQueryKey, + queryFn: () => discoverAcpRuntimes(), + staleTime: 30 * 60_000, + // Read-only observer: the forced refresh is the fetcher for these surfaces, + // so this must never fire a cheap fetch (which would race and could + // overwrite the fresh forced result with cached data). + enabled: false, + }); + // Read-only observer on the forced key so the hook surfaces the forced + // probe's fetching *and error* state. `refreshAcpRuntimes` runs the fetch + // imperatively via `fetchQuery`; this disabled observer never fetches itself + // but reflects that query's state, so a rejected forced probe becomes a + // visible `error`/`isError` instead of an unhandled rejection with a silent + // empty/stale catalog. + const forcedQuery = useQuery({ + queryKey: acpRuntimesForcedQueryKey, + queryFn: () => discoverAcpRuntimes({ force: true }), + enabled: false, + }); + const forceRefresh = React.useCallback( + () => refreshAcpRuntimes(queryClient), + [queryClient], + ); + React.useEffect(() => { + if (enabled && forceOnMount) void forceRefresh(); + }, [enabled, forceOnMount, forceRefresh]); + const isFetching = query.isFetching || forcedQuery.isFetching; + return { + ...query, + error: forcedQuery.error ?? query.error, + isError: forcedQuery.isError || query.isError, + isFetching, + isLoading: isFetching && query.data === undefined, + forceRefresh, + }; +} diff --git a/desktop/src/features/agents/hooks.ts b/desktop/src/features/agents/hooks.ts index 6d8ab4f6ea8..5d0be06109e 100644 --- a/desktop/src/features/agents/hooks.ts +++ b/desktop/src/features/agents/hooks.ts @@ -25,7 +25,6 @@ import { createManagedAgent, deleteManagedAgent, deleteCustomHarness, - discoverAcpRuntimes, discoverBackendProviders, discoverGitBashPrerequisite, discoverManagedAgentPrereqs, @@ -43,6 +42,7 @@ import { updateManagedAgent, } from "@/shared/api/tauri"; import type { HarnessDefinitionInput } from "@/shared/api/tauri"; +import { discoverAcpRuntimes } from "@/shared/api/tauriAcpDiscovery"; import { setManagedAgentAutoRestart, setManagedAgentStartOnAppLaunch, @@ -50,6 +50,11 @@ import { stopManagedAgent, } from "@/shared/api/tauriManagedAgents"; import { bootstrapManagedAgentRuntimePairs } from "@/features/agents/managedAgentRuntimeHooks"; +import { + acpRuntimesQueryKey, + refreshAcpRuntimes, +} from "@/features/agents/acpRuntimesQuery"; +export { useAcpRuntimesQueryForced } from "@/features/agents/acpRuntimesQuery"; import { createPersona, deletePersona, @@ -123,7 +128,6 @@ export const managedAgentLogFocusRefetchPolicy = { export const relayAgentsQueryKey = ["relay-agents"] as const; export const managedAgentsQueryKey = ["managed-agents"] as const; export const personasQueryKey = ["personas"] as const; -export const acpRuntimesQueryKey = ["acp-runtimes"] as const; export const acpAuthMethodsQueryKey = ["acp-auth-methods"] as const; export const managedAgentPrereqsQueryKey = ["managed-agent-prereqs"] as const; export const backendProvidersQueryKey = ["backend-providers"] as const; @@ -199,12 +203,26 @@ function invalidateManagedAgentQueriesInBackground( ); } +/** + * Discover the ACP runtime catalog. + * + * This always serves the **cheap** backend path: the last cached runtime + * availability + auth statuses, no process spawns, low-millisecond. Hot + * surfaces (channel switch, composer, member bar) render from cache — a + * 30-minute `staleTime` keeps channel switches from re-triggering discovery. + * + * Fresh auth/version state (Settings, onboarding sign-in, post-mutation) comes + * from `refreshAcpRuntimes`, which runs the expensive forced path explicitly + * and writes the result into this same cache. Keeping the query's own + * `queryFn` cheap guarantees an automatic staleness refetch never re-runs the + * probe pipeline. + */ export function useAcpRuntimesQuery(options?: { enabled?: boolean }) { return useQuery({ enabled: options?.enabled ?? true, queryKey: acpRuntimesQueryKey, - queryFn: discoverAcpRuntimes, - staleTime: 60_000, + queryFn: () => discoverAcpRuntimes(), + staleTime: 30 * 60_000, }); } @@ -238,7 +256,7 @@ export function useConnectAcpRuntimeMutation() { mutationFn: (input: { runtimeId: string; methodId: string }) => connectAcpRuntime(input.runtimeId, input.methodId), onSettled: () => { - void queryClient.invalidateQueries({ queryKey: acpRuntimesQueryKey }); + void refreshAcpRuntimes(queryClient); void queryClient.invalidateQueries({ queryKey: acpAuthMethodsQueryKey }); void queryClient.invalidateQueries({ queryKey: managedAgentsQueryKey }); }, @@ -250,7 +268,7 @@ export function useInstallAcpRuntimeMutation() { return useMutation({ mutationFn: (runtimeId: string) => installAcpRuntime(runtimeId), onSettled: () => { - void queryClient.invalidateQueries({ queryKey: acpRuntimesQueryKey }); + void refreshAcpRuntimes(queryClient); void queryClient.invalidateQueries({ queryKey: managedAgentsQueryKey }); }, }); @@ -267,7 +285,7 @@ export function useSaveCustomHarnessMutation() { originalId?: string; }) => saveCustomHarness(definition, originalId), onSettled: () => { - void queryClient.invalidateQueries({ queryKey: acpRuntimesQueryKey }); + void refreshAcpRuntimes(queryClient); }, }); } @@ -277,7 +295,7 @@ export function useDeleteCustomHarnessMutation() { return useMutation({ mutationFn: (id: string) => deleteCustomHarness(id), onSettled: () => { - void queryClient.invalidateQueries({ queryKey: acpRuntimesQueryKey }); + void refreshAcpRuntimes(queryClient); }, }); } diff --git a/desktop/src/features/onboarding/ui/SetupStep.acpForcedGate.test.mjs b/desktop/src/features/onboarding/ui/SetupStep.acpForcedGate.test.mjs new file mode 100644 index 00000000000..c360256438e --- /dev/null +++ b/desktop/src/features/onboarding/ui/SetupStep.acpForcedGate.test.mjs @@ -0,0 +1,433 @@ +/** + * Mounted consumer regressions for the SetupStep forced-probe readiness gate. + * + * P1: isChecking = isFetching (not isLoading) ensures the Next button stays + * disabled while the forced probe is in flight or has rejected, even when + * cached data exists. With the old isLoading mapping, isLoading is false when + * data is present, so the button was incorrectly enabled. + * + * Mutation proof: revert only the SetupStep.tsx hunk and both tests go RED + * (button enabled in states it must block). + */ + +import assert from "node:assert/strict"; +import { after, afterEach, before, describe, it } from "node:test"; +import { JSDOM } from "jsdom"; + +const dom = new JSDOM("", { + url: "http://localhost", +}); + +Object.assign(globalThis, { + HTMLElement: dom.window.HTMLElement, + HTMLIFrameElement: dom.window.HTMLIFrameElement, + IS_REACT_ACT_ENVIRONMENT: true, + MutationObserver: dom.window.MutationObserver, + ResizeObserver: class { + observe() {} + unobserve() {} + disconnect() {} + }, + document: dom.window.document, + localStorage: dom.window.localStorage, + self: dom.window, + window: dom.window, +}); +Object.defineProperty(globalThis, "navigator", { + configurable: true, + value: dom.window.navigator, +}); +dom.window.requestAnimationFrame = (callback) => setTimeout(callback, 0); +globalThis.requestAnimationFrame = dom.window.requestAnimationFrame; +dom.window.ResizeObserver = globalThis.ResizeObserver; +dom.window.matchMedia ??= (query) => ({ + matches: false, + media: query, + onchange: null, + addListener: () => {}, + removeListener: () => {}, + addEventListener: () => {}, + removeEventListener: () => {}, + dispatchEvent: () => false, +}); +globalThis.matchMedia = dom.window.matchMedia; + +// ── Tauri IPC stub ──────────────────────────────────────────────────────────── + +let discoverHandler = () => Promise.resolve([]); + +globalThis.__TAURI_INTERNALS__ = { + invoke: (command, args) => { + if (command === "discover_acp_providers") return discoverHandler(args); + // All other commands (e.g. plugin:event|listen from useInstallOutputLine) + // reject; useInstallOutputLine catches gracefully ("event system unavailable"). + return Promise.reject(new Error(`unmocked: ${command}`)); + }, + transformCallback: () => 1, +}; +dom.window.__TAURI_INTERNALS__ = globalThis.__TAURI_INTERNALS__; + +// ── Deferred imports (must run after globalThis is configured) ──────────────── + +let React, + act, + createRoot, + QueryClient, + QueryClientProvider, + SetupStep, + acpRuntimesQueryKey, + TooltipProvider; + +before(async () => { + ({ default: React, act } = await import("react")); + ({ createRoot } = await import("react-dom/client")); + ({ QueryClient, QueryClientProvider } = await import( + "@tanstack/react-query" + )); + ({ SetupStep } = await import("./SetupStep.tsx")); + ({ acpRuntimesQueryKey } = await import( + "@/features/agents/acpRuntimesQuery.ts" + )); + ({ TooltipProvider } = await import("@/shared/ui/tooltip.tsx")); +}); + +afterEach(() => { + discoverHandler = () => Promise.resolve([]); +}); + +after(() => dom.window.close()); + +// ── Helpers ─────────────────────────────────────────────────────────────────── + +/** Camelcase AcpRuntimeCatalogEntry as stored in acpRuntimesQueryKey cache. */ +function catalogEntry(id, authStatusValue) { + return { + id, + label: id, + avatarUrl: "", + availability: "available", + command: id, + binaryPath: `/usr/bin/${id}`, + defaultArgs: [], + mcpCommand: null, + modelEnvVar: null, + providerEnvVar: null, + thinkingEnvVar: null, + maxTokensEnvVar: null, + contextLimitEnvVar: null, + maxRoundsEnvVar: null, + installHint: "", + installInstructionsUrl: "", + canAutoInstall: false, + requiresExternalCli: false, + underlyingCliPath: null, + nodeRequired: false, + authStatus: { status: authStatusValue }, + loginHint: null, + source: "builtin", + definitionEnv: {}, + }; +} + +/** Raw snake_case backend entry as `discoverAcpRuntimes` receives it before + * `fromRawAcpRuntimeCatalogEntry`. Use for values a forced probe resolves at + * the IPC boundary (vs. `catalogEntry` for values seeded directly into cache). */ +function rawReadyEntry(id) { + return { + id, + label: id, + avatar_url: "", + availability: "available", + command: id, + binary_path: `/usr/bin/${id}`, + default_args: [], + mcp_command: null, + install_hint: "", + install_instructions_url: "", + can_auto_install: false, + requires_external_cli: false, + underlying_cli_path: null, + node_required: false, + auth_status: { status: "logged_in" }, + source: "builtin", + }; +} + +function makeQueryClient() { + return new QueryClient({ defaultOptions: { queries: { retry: false } } }); +} + +function deferred() { + let resolve; + const promise = new Promise((r) => { + resolve = r; + }); + return { promise, resolve }; +} + +const NOOP = () => {}; +const ACTIONS = { back: NOOP, next: NOOP, navigateToAgentSettings: NOOP }; + +/** Mount SetupStep under the query client + tooltip provider it requires. */ +function renderSetupStep() { + const container = document.createElement("div"); + document.body.appendChild(container); + const root = createRoot(container); + return { container, root }; +} + +function setupStepTree(queryClient) { + return React.createElement( + QueryClientProvider, + { client: queryClient }, + React.createElement( + TooltipProvider, + null, + React.createElement(SetupStep, { + actions: ACTIONS, + direction: "forward", + onReadyRuntimeIdsChange: NOOP, + }), + ), + ); +} + +// ── Tests ───────────────────────────────────────────────────────────────────── + +describe("SetupStep Next button readiness gate — P1 regression (mounted consumer)", () => { + it("onboarding-setup-next is disabled while forced probe is pending over cached data", async () => { + const queryClient = makeQueryClient(); + // Pre-seed cache with a ready runtime. getReadyOnboardingRuntimes + // will return it, so readyRuntimeIds.length > 0 — proving the button + // is blocked by isChecking, not by an empty ready set. + queryClient.setQueryData(acpRuntimesQueryKey, [ + catalogEntry("codex", "logged_in"), + ]); + + const pending = deferred(); + discoverHandler = (args) => + args?.force === true ? pending.promise : Promise.resolve([]); + + const container = document.createElement("div"); + document.body.appendChild(container); + const root = createRoot(container); + + await act(async () => { + root.render( + React.createElement( + QueryClientProvider, + { client: queryClient }, + React.createElement( + TooltipProvider, + null, + React.createElement(SetupStep, { + actions: ACTIONS, + direction: "forward", + onReadyRuntimeIdsChange: NOOP, + }), + ), + ), + ); + }); + // Let the mount-time forceRefresh dispatch (but not resolve). + await act(async () => { + await new Promise((r) => setTimeout(r, 10)); + }); + + const button = container.querySelector( + '[data-testid="onboarding-setup-next"]', + ); + assert.ok(button, "onboarding-setup-next button must be present"); + assert.ok( + button.disabled, + "Next button must be disabled while forced probe is in flight over cached data", + ); + + // Resolve the pending probe inside act so React Query drains its state + // update before unmount — prevents "Promise resolution still pending" + // from the dangling deferred. + await act(async () => { + pending.resolve([]); + await new Promise((r) => setTimeout(r, 0)); + }); + await act(async () => { + root.unmount(); + }); + container.remove(); + queryClient.clear(); + }); + + it("onboarding-setup-next is disabled after forced probe rejects over cached data", async () => { + const queryClient = makeQueryClient(); + queryClient.setQueryData(acpRuntimesQueryKey, [ + catalogEntry("codex", "logged_in"), + ]); + + discoverHandler = (args) => + args?.force === true + ? Promise.reject(new Error("forced probe rejected")) + : Promise.resolve([]); + + const container = document.createElement("div"); + document.body.appendChild(container); + const root = createRoot(container); + + await act(async () => { + root.render( + React.createElement( + QueryClientProvider, + { client: queryClient }, + React.createElement( + TooltipProvider, + null, + React.createElement(SetupStep, { + actions: ACTIONS, + direction: "forward", + onReadyRuntimeIdsChange: NOOP, + }), + ), + ), + ); + }); + await act(async () => { + await new Promise((r) => setTimeout(r, 50)); + }); + + const button = container.querySelector( + '[data-testid="onboarding-setup-next"]', + ); + assert.ok(button, "onboarding-setup-next button must be present"); + assert.ok( + button.disabled, + "Next button must be disabled after forced probe rejects, even with cached data", + ); + + const errorEl = container.querySelector( + '[data-testid="onboarding-setup-error"]', + ); + assert.ok( + errorEl, + "the forced rejection error must be rendered after the probe rejects", + ); + assert.match( + errorEl.textContent ?? "", + /forced probe rejected/, + "rendered error must surface the forced rejection message", + ); + + await act(async () => { + root.unmount(); + }); + container.remove(); + queryClient.clear(); + }); +}); + +describe("SetupStep cached-ready revalidation — P4 regression (mounted consumer)", () => { + it("cached READY is replaced by a CHECKING indicator while a warm forced probe is pending", async () => { + const queryClient = makeQueryClient(); + queryClient.setQueryData(acpRuntimesQueryKey, [ + catalogEntry("codex", "logged_in"), + ]); + + const pending = deferred(); + discoverHandler = (args) => + args?.force === true ? pending.promise : Promise.resolve([]); + + const { container, root } = renderSetupStep(); + await act(async () => { + root.render(setupStepTree(queryClient)); + }); + await act(async () => { + await new Promise((r) => setTimeout(r, 10)); + }); + + assert.ok( + container.querySelector( + '[data-testid="onboarding-runtime-rechecking-codex"]', + ), + "a pending warm recheck over a cached-ready runtime must show CHECKING…", + ); + assert.equal( + container.querySelector('[data-testid="onboarding-runtime-ready-codex"]'), + null, + "cached READY must not be presented as current while the recheck is in flight", + ); + + // Success restores READY. + await act(async () => { + pending.resolve([rawReadyEntry("codex")]); + await new Promise((r) => setTimeout(r, 50)); + }); + assert.ok( + container.querySelector('[data-testid="onboarding-runtime-ready-codex"]'), + "READY returns once the warm recheck succeeds", + ); + assert.equal( + container.querySelector( + '[data-testid="onboarding-runtime-rechecking-codex"]', + ), + null, + "the CHECKING indicator clears on success", + ); + const button = container.querySelector( + '[data-testid="onboarding-setup-next"]', + ); + assert.ok(button && !button.disabled, "Next is enabled after success"); + + await act(async () => { + root.unmount(); + }); + container.remove(); + queryClient.clear(); + }); + + it("cached READY is replaced by a recheck affordance after a warm forced probe rejects", async () => { + const queryClient = makeQueryClient(); + queryClient.setQueryData(acpRuntimesQueryKey, [ + catalogEntry("codex", "logged_in"), + ]); + + discoverHandler = (args) => + args?.force === true + ? Promise.reject(new Error("warm recheck failed")) + : Promise.resolve([]); + + const { container, root } = renderSetupStep(); + await act(async () => { + root.render(setupStepTree(queryClient)); + }); + await act(async () => { + await new Promise((r) => setTimeout(r, 50)); + }); + + assert.ok( + container.querySelector( + '[data-testid="onboarding-runtime-recheck-codex"]', + ), + "a warm rejection over a cached-ready runtime must offer a recheck, not claim READY", + ); + assert.equal( + container.querySelector('[data-testid="onboarding-runtime-ready-codex"]'), + null, + "cached READY must not be presented as current after the recheck rejects", + ); + assert.ok( + container.querySelector('[data-testid="onboarding-setup-error"]'), + "the warm rejection error stays visible alongside the retained card", + ); + const button = container.querySelector( + '[data-testid="onboarding-setup-next"]', + ); + assert.ok( + button && button.disabled, + "Next stays gated while readiness is unconfirmed", + ); + + await act(async () => { + root.unmount(); + }); + container.remove(); + queryClient.clear(); + }); +}); diff --git a/desktop/src/features/onboarding/ui/SetupStep.tsx b/desktop/src/features/onboarding/ui/SetupStep.tsx index 2a3476b2eaf..aac9b53846a 100644 --- a/desktop/src/features/onboarding/ui/SetupStep.tsx +++ b/desktop/src/features/onboarding/ui/SetupStep.tsx @@ -4,7 +4,7 @@ import { Check, Info } from "lucide-react"; import { useAcpAuthMethodsQuery, - useAcpRuntimesQuery, + useAcpRuntimesQueryForced, useConnectAcpRuntimeMutation, useInstallAcpRuntimeMutation, } from "@/features/agents/hooks"; @@ -51,9 +51,9 @@ type InstallResultState = { type InstallResultsState = Record; function useSetupStepState(): SetupStepState { - const runtimesQuery = useAcpRuntimesQuery(); + const runtimesQuery = useAcpRuntimesQueryForced(); const items = runtimesQuery.data ?? []; - const isChecking = runtimesQuery.isLoading; + const isChecking = runtimesQuery.isFetching; const errorMessage = runtimesQuery.error instanceof Error ? runtimesQuery.error.message : null; @@ -109,7 +109,11 @@ function RuntimeStatus({ runtime.authStatus.status === "logged_out", }); const connectMutation = useConnectAcpRuntimeMutation(); - const runtimesQuery = useAcpRuntimesQuery(); + // Child rows share the surface owner's forced query state + refresh callback + // (`useSetupStepState` owns the single force-on-mount). Each row must not + // mount its own force effect, or onboarding entry re-runs discovery once per + // row instead of once for the surface. + const runtimesQuery = useAcpRuntimesQueryForced({ forceOnMount: false }); const [isWaitingForSignIn, setIsWaitingForSignIn] = React.useState(false); const [didSignInCheckTimeOut, setDidSignInCheckTimeOut] = React.useState(false); @@ -125,7 +129,7 @@ function RuntimeStatus({ if (!isWaitingForSignIn) return; const interval = window.setInterval(() => { - void runtimesQuery.refetch(); + void runtimesQuery.forceRefresh(); }, 2_000); const timeout = window.setTimeout(() => { setIsWaitingForSignIn(false); @@ -136,7 +140,7 @@ function RuntimeStatus({ window.clearInterval(interval); window.clearTimeout(timeout); }; - }, [isWaitingForSignIn, runtimesQuery.refetch]); + }, [isWaitingForSignIn, runtimesQuery.forceRefresh]); const authMethods = getOnboardingAuthMethods( runtime, methodsQuery.data?.methods ?? [], @@ -157,7 +161,7 @@ function RuntimeStatus({ if (didSignInCheckTimeOut) { setDidSignInCheckTimeOut(false); setIsWaitingForSignIn(true); - void runtimesQuery.refetch(); + void runtimesQuery.forceRefresh(); return; } if (!authMethod) { @@ -215,6 +219,40 @@ function RuntimeStatus({ } if (runtimeIsReadyForOnboarding(runtime)) { + // Cached readiness must not read as freshly confirmed while a warm forced + // probe is revalidating (or has rejected) over it. `runtimesQuery` shares + // the surface owner's forced-query state, so its fetching/error flags track + // the in-flight recheck. Pending → a visible CHECKING… state; a warm + // rejection → a recheck affordance (never an unqualified READY). On success + // both clear and READY returns. Next stays gated by isChecking/errorMessage + // in SetupStepContent, so this only governs the per-card claim. + if (runtimesQuery.isFetching) { + return ( +
+ + CHECKING… +
+ ); + } + if (runtimesQuery.isError) { + return ( + + ); + } return ( @@ -244,7 +282,7 @@ function RuntimeStatus({ aria-label={`Check ${runtime.label} again`} className="buzz-onboarding-runtime-setup h-5 rounded-full bg-[var(--buzz-welcome-chartreuse)]/30 px-2.5 font-mono !text-badge font-normal uppercase text-foreground hover:bg-[var(--buzz-welcome-chartreuse)]/40" disabled={runtimesQuery.isFetching} - onClick={() => void runtimesQuery.refetch()} + onClick={() => void runtimesQuery.forceRefresh()} type="button" variant="ghost" > @@ -653,7 +691,10 @@ function RuntimeProvidersSection({ )} {errorMessage ? ( -

+

{errorMessage}

) : null} @@ -724,7 +765,11 @@ function SetupStepContent({ + + ) : null} {isLoading ? ( + ) : isColdError ? ( +
+ Couldn't load runtimes. + +
) : filtered.length === 0 ? (

- No runtimes match. + {isSearching ? "No runtimes match." : "No runtimes found."}

) : ( <> diff --git a/desktop/src/features/settings/ui/HarnessesSettingsPanel.tsx b/desktop/src/features/settings/ui/HarnessesSettingsPanel.tsx index e7254c5e05c..2ce769e6462 100644 --- a/desktop/src/features/settings/ui/HarnessesSettingsPanel.tsx +++ b/desktop/src/features/settings/ui/HarnessesSettingsPanel.tsx @@ -3,7 +3,7 @@ import { ExternalLink, Plus, RefreshCw } from "lucide-react"; import { openUrl } from "@tauri-apps/plugin-opener"; import { - useAcpRuntimesQuery, + useAcpRuntimesQueryForced, useGitBashPrerequisiteQuery, } from "@/features/agents/hooks"; import type { AcpRuntimeCatalogEntry } from "@/shared/api/types"; @@ -85,7 +85,7 @@ function GitBashCard({ * needs multi-step setup, plus the custom-harness form. */ export function HarnessesSettingsPanel() { - const runtimesQuery = useAcpRuntimesQuery(); + const runtimesQuery = useAcpRuntimesQueryForced(); const gitBashQuery = useGitBashPrerequisiteQuery(); const [catalogOpen, setCatalogOpen] = React.useState(false); // Incremented each time the user clicks "Check again" so HarnessRow @@ -119,7 +119,7 @@ export function HarnessesSettingsPanel() { disabled={isRefreshing} onClick={() => { setResetEpoch((e) => e + 1); - void runtimesQuery.refetch(); + void runtimesQuery.forceRefresh(); void gitBashQuery.refetch(); }} size="sm" diff --git a/desktop/src/shared/api/tauri.ts b/desktop/src/shared/api/tauri.ts index b31b8fe9776..b4f0df6fc09 100644 --- a/desktop/src/shared/api/tauri.ts +++ b/desktop/src/shared/api/tauri.ts @@ -863,12 +863,6 @@ export async function discoverGitBashPrerequisite(): Promise { - return ( - await invokeTauri("discover_acp_providers") - ).map(fromRawAcpRuntimeCatalogEntry); -} - /** Input shape for creating or updating a custom harness. */ export type HarnessDefinitionInput = { id: string; diff --git a/desktop/src/shared/api/tauriAcpDiscovery.ts b/desktop/src/shared/api/tauriAcpDiscovery.ts new file mode 100644 index 00000000000..6dd4a53dbbe --- /dev/null +++ b/desktop/src/shared/api/tauriAcpDiscovery.ts @@ -0,0 +1,24 @@ +import type { AcpRuntimeCatalogEntry } from "@/shared/api/types"; +import { + fromRawAcpRuntimeCatalogEntry, + invokeTauri, + type RawAcpRuntimeCatalogEntry, +} from "@/shared/api/tauri"; + +/** + * Discover the ACP runtime catalog. + * + * `force` defaults to `false` — the cheap backend path that serves cached + * availability + auth statuses with no process spawns. Pass `{ force: true }` + * only from surfaces that need fresh auth/version state (Settings, onboarding, + * post-mutation refresh); that path runs the expensive probe pipeline. + */ +export async function discoverAcpRuntimes(options?: { + force?: boolean; +}): Promise { + const raw = await invokeTauri( + "discover_acp_providers", + { force: options?.force ?? false }, + ); + return raw.map(fromRawAcpRuntimeCatalogEntry); +}