diff --git a/desktop/src-tauri/src/commands/agent_discovery.rs b/desktop/src-tauri/src/commands/agent_discovery.rs index e3d9b22189..929d86d03c 100644 --- a/desktop/src-tauri/src/commands/agent_discovery.rs +++ b/desktop/src-tauri/src/commands/agent_discovery.rs @@ -154,26 +154,13 @@ fn install_acp_runtime_blocking(runtime_id: &str) -> Result steps.extend(cli_steps), + Err(failed) => return Ok(failed), } // Phase 2: Install adapter if missing (or outdated) and commands are available. @@ -1042,6 +1029,7 @@ fn floor_char_boundary(s: &str, mut index: usize) -> usize { } // ── managed Node/npm runtime ────────────────────────────────────────────────── +mod cli_install; mod managed_node; use managed_node::{ ensure_managed_node_runtime_blocking, managed_node_runtime_supported, managed_npm_command, diff --git a/desktop/src-tauri/src/commands/agent_discovery/cli_install.rs b/desktop/src-tauri/src/commands/agent_discovery/cli_install.rs new file mode 100644 index 0000000000..9777e6bf95 --- /dev/null +++ b/desktop/src-tauri/src/commands/agent_discovery/cli_install.rs @@ -0,0 +1,63 @@ +//! Phase-1 underlying CLI install / repair for ACP runtime setup. + +use crate::managed_agents::{ + readiness::cli_probe, InstallRuntimeResult, InstallStepResult, KnownAcpRuntime, +}; + +use super::run_install_command_with_retry; + +/// Install a missing underlying CLI, or repair an outdated Claude Code build. +/// +/// Returns `Err(result)` when a step fails (caller should return that result +/// immediately). Returns `Ok(steps)` with zero or more completed install steps. +pub(super) fn install_or_repair_underlying_cli( + runtime_id: &str, + runtime: &KnownAcpRuntime, +) -> Result, InstallRuntimeResult> { + let Some(cli) = runtime.underlying_cli else { + return Ok(Vec::new()); + }; + + let mut steps = Vec::new(); + match crate::managed_agents::resolve_command(cli) { + None => { + for cmd in runtime.cli_install_commands_for_os() { + let result = run_install_command_with_retry("cli", cmd); + let success = result.success; + steps.push(result); + if !success { + return Err(InstallRuntimeResult { + success: false, + steps, + restarted_count: 0, + failed_restart_count: 0, + }); + } + } + } + // Claude Code may already be present but too old to expose + // `claude auth status` (older builds treat those args as a prompt). + // Repair with `claude update` before adapter install so onboarding + // auth probes can succeed. + Some(cli_path) if runtime_id == "claude" => { + let augmented = cli_probe::augmented_path(); + if cli_probe::claude_auth_status_needs_upgrade(&cli_path, augmented.as_deref()) { + let result = run_install_command_with_retry("cli", "claude update"); + let success = result.success; + steps.push(result); + if !success { + return Err(InstallRuntimeResult { + success: false, + steps, + restarted_count: 0, + failed_restart_count: 0, + }); + } + crate::managed_agents::clear_resolve_cache(); + } + } + Some(_) => {} + } + + Ok(steps) +} diff --git a/desktop/src-tauri/src/managed_agents/discovery.rs b/desktop/src-tauri/src/managed_agents/discovery.rs index a2e47d1281..9624a8ac00 100644 --- a/desktop/src-tauri/src/managed_agents/discovery.rs +++ b/desktop/src-tauri/src/managed_agents/discovery.rs @@ -126,7 +126,9 @@ const KNOWN_ACP_RUNTIMES: &[KnownAcpRuntime] = &[ max_tokens_env_var: None, context_limit_env_var: None, required_normalized_fields: &[], - login_hint: Some("Run the Claude CLI to complete authentication."), + login_hint: Some( + "Sign in with Claude Code, or run `claude update` if status checks fail.", + ), auth_probe_args: Some(&["claude", "auth", "status"]), }, KnownAcpRuntime { @@ -905,6 +907,10 @@ pub(crate) fn is_npm_global_install(cmd: &str) -> bool { || t.starts_with("npm uninstall -g ") } +fn unknown_auth_status(diagnostic: Option) -> AuthStatus { + AuthStatus::Unknown { diagnostic } +} + /// Run a CLI auth probe with a 10-second process-level timeout. /// /// Spawns the probe CLI as a child process. Stdout and stderr are drained on @@ -915,6 +921,9 @@ fn probe_auth_status(binary_path: &Path, probe_args: &[&str]) -> AuthStatus { use crate::managed_agents::readiness::cli_probe; let augmented_path = cli_probe::augmented_path(); + let claude_probe = cli_probe::is_claude_auth_status_probe(probe_args); + let timeout_diagnostic = + claude_probe.then(|| cli_probe::CLAUDE_AUTH_PROBE_UPDATE_HINT.to_string()); let mut command = std::process::Command::new(binary_path); command.args(&probe_args[1..]); @@ -929,7 +938,7 @@ fn probe_auth_status(binary_path: &Path, probe_args: &[&str]) -> AuthStatus { let mut child = match command.spawn() { Ok(c) => c, - Err(_) => return AuthStatus::Unknown, + Err(_) => return unknown_auth_status(timeout_diagnostic), }; // Drain stdout/stderr on background threads to prevent pipe-buffer deadlock. @@ -941,6 +950,7 @@ fn probe_auth_status(binary_path: &Path, probe_args: &[&str]) -> AuthStatus { if let Some(mut pipe) = stdout_pipe { let _ = pipe.read_to_end(&mut buf); } + buf }); let stderr_thread = std::thread::spawn(move || { let mut buf = Vec::new(); @@ -972,7 +982,7 @@ fn probe_auth_status(binary_path: &Path, probe_args: &[&str]) -> AuthStatus { let _ = wait_thread.join(); let _ = stdout_thread.join(); let _ = stderr_thread.join(); - return AuthStatus::Unknown; + return unknown_auth_status(timeout_diagnostic); } match rx.recv_timeout(Duration::from_millis(100).min(remaining)) { Ok(Ok(status)) => break status, @@ -980,27 +990,35 @@ fn probe_auth_status(binary_path: &Path, probe_args: &[&str]) -> AuthStatus { let _ = wait_thread.join(); let _ = stdout_thread.join(); let _ = stderr_thread.join(); - return AuthStatus::Unknown; + return unknown_auth_status(timeout_diagnostic); } Err(std::sync::mpsc::RecvTimeoutError::Timeout) => continue, Err(std::sync::mpsc::RecvTimeoutError::Disconnected) => { let _ = stdout_thread.join(); let _ = stderr_thread.join(); - return AuthStatus::Unknown; + return unknown_auth_status(timeout_diagnostic); } } }; let _ = wait_thread.join(); - let _ = stdout_thread.join(); + let stdout_bytes = stdout_thread.join().unwrap_or_default(); let stderr_bytes = stderr_thread.join().unwrap_or_default(); - match cli_probe::classify_probe_output(&stderr_bytes, exit_status.success()) { + match cli_probe::classify_auth_probe_output( + probe_args, + &stdout_bytes, + &stderr_bytes, + exit_status.success(), + ) { cli_probe::ProbeOutcome::LoggedIn => AuthStatus::LoggedIn, cli_probe::ProbeOutcome::LoggedOut => AuthStatus::LoggedOut, cli_probe::ProbeOutcome::ConfigInvalid { stderr_excerpt } => AuthStatus::ConfigInvalid { diagnostic: stderr_excerpt, }, + cli_probe::ProbeOutcome::Unsupported { diagnostic } => { + unknown_auth_status(Some(diagnostic)) + } } } @@ -1284,7 +1302,7 @@ fn discover_acp_runtime_phase1(runtime: &'static KnownAcpRuntime) -> PartialEntr underlying_cli_path, node_required, // Filled in by the auth-probe phase in full catalog discovery. - auth_status: AuthStatus::Unknown, + auth_status: AuthStatus::Unknown { diagnostic: None }, login_hint: None, }, } @@ -1331,27 +1349,33 @@ pub fn discover_acp_runtimes() -> Vec { // Collect probe results and patch entries. for (idx, handle) in probe_handles { - let status = handle.join().unwrap_or(AuthStatus::Unknown); + let status = handle + .join() + .unwrap_or(AuthStatus::Unknown { diagnostic: None }); 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.login_hint = match &status { + AuthStatus::LoggedIn | AuthStatus::NotApplicable => None, + AuthStatus::Unknown { + diagnostic: Some(diagnostic), + } => Some(diagnostic.clone()), + _ => partial.runtime.login_hint.map(str::to_string), + }; partial.entry.auth_status = status; } // Fill NotApplicable / Unknown for non-probed entries. for partial in &mut partials { - if partial.entry.auth_status == AuthStatus::Unknown { + if matches!( + partial.entry.auth_status, + AuthStatus::Unknown { diagnostic: None } + ) { partial.entry.auth_status = if partial.entry.availability == AcpAvailabilityStatus::Available && partial.runtime.auth_probe_args.is_none() { AuthStatus::NotApplicable } else { - AuthStatus::Unknown + AuthStatus::Unknown { diagnostic: None } }; } } diff --git a/desktop/src-tauri/src/managed_agents/readiness/cli_login.rs b/desktop/src-tauri/src/managed_agents/readiness/cli_login.rs index 4036d9f239..833fdf6b1b 100644 --- a/desktop/src-tauri/src/managed_agents/readiness/cli_login.rs +++ b/desktop/src-tauri/src/managed_agents/readiness/cli_login.rs @@ -61,6 +61,15 @@ pub(super) fn requirements( diagnostic: stderr_excerpt, }] } + // Outdated Claude Code (etc.): surface the update hint via the + // same CliLogin nudge so Doctor / setup can point at repair. + cli_probe::ProbeOutcome::Unsupported { diagnostic } => { + vec![missing_requirement( + probe_args, + &diagnostic, + AcpAvailabilityStatus::Available, + )] + } } } other => vec![missing_requirement(probe_args, setup_copy, other)], diff --git a/desktop/src-tauri/src/managed_agents/readiness/cli_probe.rs b/desktop/src-tauri/src/managed_agents/readiness/cli_probe.rs index 513da4e2a8..24f93eabc7 100644 --- a/desktop/src-tauri/src/managed_agents/readiness/cli_probe.rs +++ b/desktop/src-tauri/src/managed_agents/readiness/cli_probe.rs @@ -36,8 +36,17 @@ pub(crate) enum ProbeOutcome { /// A trimmed excerpt of the stderr message to surface in the nudge. stderr_excerpt: String, }, + /// The probe command is missing or incompatible with this CLI build + /// (e.g. older Claude Code treating `auth status` as a prompt). + Unsupported { + /// Actionable guidance for the user (typically "run `claude update`"). + diagnostic: String, + }, } +/// Hint shown when Claude Code is too old to expose `claude auth status`. +pub(crate) const CLAUDE_AUTH_PROBE_UPDATE_HINT: &str = "This Claude Code build doesn’t support `claude auth status`. Run `claude update`, then try again."; + /// Signals emitted to stderr by codex (and related CLI tools) when they /// fail to parse their config file. We check these to distinguish a /// config-parse failure from a genuine "not authenticated" exit. @@ -49,6 +58,39 @@ pub(crate) enum ProbeOutcome { /// one term. const CONFIG_PARSE_SIGNALS: &[&str] = &["error loading configuration", "unknown variant"]; +/// Whether this probe is Claude's JSON `auth status` command. +/// +/// Older Claude Code builds lack the `auth` subcommand and treat +/// `auth status` as a free-form prompt, so callers must validate JSON +/// (`loggedIn`) instead of trusting exit status alone. +pub(crate) fn is_claude_auth_status_probe(probe_args: &[&str]) -> bool { + probe_args.len() >= 3 + && probe_args[0] == "claude" + && probe_args[probe_args.len() - 2] == "auth" + && probe_args[probe_args.len() - 1] == "status" +} + +/// Parse Claude's `auth status` JSON for the `loggedIn` boolean. +fn parse_claude_logged_in(stdout: &[u8]) -> Option { + let text = String::from_utf8_lossy(stdout); + let trimmed = text.trim(); + if trimmed.is_empty() { + return None; + } + // Prefer a full-document parse; fall back to the first JSON object if the + // CLI wraps status with banners/logging. + if let Ok(value) = serde_json::from_str::(trimmed) { + return value.get("loggedIn").and_then(|v| v.as_bool()); + } + let start = trimmed.find('{')?; + let end = trimmed.rfind('}')?; + if end < start { + return None; + } + let value = serde_json::from_str::(&trimmed[start..=end]).ok()?; + value.get("loggedIn").and_then(|v| v.as_bool()) +} + /// Run the probe at the resolved absolute path so the GUI-PATH gap is /// bypassed. Injects the same augmented PATH used for launched agents so /// script shims with `/usr/bin/env ` shebangs can find runtimes @@ -66,8 +108,7 @@ pub(crate) fn login_probe( crate::util::configure_no_window(&mut command); match command.output() { - Ok(o) if o.status.success() => ProbeOutcome::LoggedIn, - Ok(o) => classify_probe_output(&o.stderr, false), + Ok(o) => classify_auth_probe_output(probe_args, &o.stdout, &o.stderr, o.status.success()), Err(_) => ProbeOutcome::LoggedOut, } } @@ -75,8 +116,28 @@ pub(crate) fn login_probe( /// Classify collected probe output into a `ProbeOutcome`. /// /// Shared between `login_probe` (which has the full `Output`) and the -/// process-level timeout path in `probe_auth_status` (which drains stderr -/// on a background thread and collects it separately). +/// process-level timeout path in `probe_auth_status` (which drains stdout / +/// stderr on background threads and collects them separately). +pub(crate) fn classify_auth_probe_output( + probe_args: &[&str], + stdout_bytes: &[u8], + stderr_bytes: &[u8], + exit_success: bool, +) -> ProbeOutcome { + if is_claude_auth_status_probe(probe_args) { + return match parse_claude_logged_in(stdout_bytes) { + Some(true) => ProbeOutcome::LoggedIn, + Some(false) => ProbeOutcome::LoggedOut, + None => ProbeOutcome::Unsupported { + diagnostic: CLAUDE_AUTH_PROBE_UPDATE_HINT.to_string(), + }, + }; + } + + classify_probe_output(stderr_bytes, exit_success) +} + +/// Classify non-Claude probe output from stderr + exit status. pub(crate) fn classify_probe_output(stderr_bytes: &[u8], exit_success: bool) -> ProbeOutcome { if exit_success { return ProbeOutcome::LoggedIn; @@ -96,9 +157,141 @@ pub(crate) fn classify_probe_output(stderr_bytes: &[u8], exit_success: bool) -> } } +/// Returns true when Claude's `--help` output does not advertise an `auth` +/// subcommand. +/// +/// Older builds omit `auth` and treat `claude auth status` as a free-form +/// prompt (which can hang for tens of seconds). Prefer this cheap help parse +/// over running the status probe during install repair. +pub(crate) fn claude_help_missing_auth_command(help_stdout: &str) -> bool { + let lower = help_stdout.to_lowercase(); + // Prefer the Commands section when present so option text like + // `--chrome` / "authentication" prose does not false-positive. + let commands_section = lower.split("commands:").nth(1).unwrap_or(lower.as_str()); + // Match a commands-list line like `auth Manage authentication`. + for line in commands_section.lines() { + let trimmed = line.trim_start(); + if let Some(rest) = trimmed.strip_prefix("auth") { + if rest.is_empty() || rest.starts_with(char::is_whitespace) { + return false; + } + } + } + true +} + +/// Returns true when the Claude binary's `auth status` probe is missing or +/// incompatible (so install should run `claude update`). +/// +/// Uses `claude --help` (fast, non-interactive) rather than `auth status`, +/// which older builds may interpret as a prompt and hang on. +pub(crate) fn claude_auth_status_needs_upgrade( + binary_path: &Path, + augmented_path: Option<&str>, +) -> bool { + let mut command = std::process::Command::new(binary_path); + command.arg("--help"); + if let Some(path) = augmented_path { + command.env("PATH", path); + } + crate::util::configure_no_window(&mut command); + + match command.output() { + Ok(output) => { + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); + // Some CLIs print help on stderr. + claude_help_missing_auth_command(&format!("{stdout}\n{stderr}")) + } + // If we cannot run --help, attempt the upgrade path rather than + // leaving onboarding stuck on a hanging auth probe. + Err(_) => true, + } +} + #[cfg(test)] mod tests { - use super::{ProbeOutcome, CONFIG_PARSE_SIGNALS}; + use super::{ + classify_auth_probe_output, is_claude_auth_status_probe, ProbeOutcome, + CLAUDE_AUTH_PROBE_UPDATE_HINT, CONFIG_PARSE_SIGNALS, + }; + + #[test] + fn detects_claude_auth_status_probe_args() { + assert!(is_claude_auth_status_probe(&["claude", "auth", "status"])); + assert!(!is_claude_auth_status_probe(&["codex", "login", "status"])); + assert!(!is_claude_auth_status_probe(&["claude", "doctor"])); + } + + #[test] + fn claude_auth_status_json_logged_in() { + let stdout = br#"{ + "loggedIn": true, + "authMethod": "claude.ai", + "email": "user@example.com" +}"#; + assert_eq!( + classify_auth_probe_output(&["claude", "auth", "status"], stdout, b"", true), + ProbeOutcome::LoggedIn + ); + } + + #[test] + fn claude_auth_status_json_logged_out() { + let stdout = br#"{"loggedIn":false}"#; + assert_eq!( + classify_auth_probe_output(&["claude", "auth", "status"], stdout, b"", false), + ProbeOutcome::LoggedOut + ); + } + + #[test] + fn claude_auth_status_prompt_hijack_is_unsupported() { + // Older Claude Code builds lack `auth` and treat the args as a prompt. + let stdout = b"I'd be happy to help with authentication in Buzz!"; + let outcome = classify_auth_probe_output(&["claude", "auth", "status"], stdout, b"", true); + assert_eq!( + outcome, + ProbeOutcome::Unsupported { + diagnostic: CLAUDE_AUTH_PROBE_UPDATE_HINT.to_string(), + } + ); + } + + #[test] + fn claude_auth_status_empty_success_is_unsupported() { + let outcome = classify_auth_probe_output(&["claude", "auth", "status"], b"", b"", true); + assert!(matches!(outcome, ProbeOutcome::Unsupported { .. })); + } + + #[test] + fn claude_help_detects_missing_auth_command() { + let old_help = r#" +Commands: + doctor Check the health of your Claude Code + install [options] [target] Install Claude Code native build + mcp Configure and manage MCP servers + setup-token Set up a long-lived authentication token + update|upgrade Check for updates and install if available +"#; + assert!(super::claude_help_missing_auth_command(old_help)); + + let new_help = r#" +Commands: + auth Manage authentication + doctor Check the health of your Claude Code + update|upgrade Check for updates and install if available +"#; + assert!(!super::claude_help_missing_auth_command(new_help)); + } + + #[test] + fn non_claude_probe_still_trusts_exit_status() { + assert_eq!( + classify_auth_probe_output(&["codex", "login", "status"], b"", b"", true), + ProbeOutcome::LoggedIn + ); + } #[cfg(unix)] #[test] @@ -233,6 +426,32 @@ mod tests { ); } + #[cfg(unix)] + #[test] + fn login_probe_claude_json_uses_stdout_not_bare_exit() { + use std::fs; + use std::os::unix::fs::PermissionsExt; + + let temp = tempfile::tempdir().expect("temp dir"); + let bin_dir = temp.path().join("bin"); + fs::create_dir_all(&bin_dir).expect("bin dir"); + + // Exit 0 with conversational stdout — must NOT count as logged in. + let script_path = bin_dir.join("claude"); + fs::write( + &script_path, + "#!/bin/sh\necho 'Sure, I can help with auth status!'\nexit 0\n", + ) + .expect("write script"); + fs::set_permissions(&script_path, fs::Permissions::from_mode(0o755)).expect("chmod script"); + + let outcome = super::login_probe(&script_path, &["claude", "auth", "status"], None); + assert!( + matches!(outcome, ProbeOutcome::Unsupported { .. }), + "prompt-hijack stdout must be Unsupported; got {outcome:?}" + ); + } + /// Verify that every string in CONFIG_PARSE_SIGNALS is lowercased so the /// case-insensitive match works correctly. #[test] diff --git a/desktop/src-tauri/src/managed_agents/types.rs b/desktop/src-tauri/src/managed_agents/types.rs index af4908911b..6a28975868 100644 --- a/desktop/src-tauri/src/managed_agents/types.rs +++ b/desktop/src-tauri/src/managed_agents/types.rs @@ -561,8 +561,15 @@ pub enum AuthStatus { }, /// This runtime does not have a login step (e.g. goose, buzz-agent). NotApplicable, - /// Probe was not attempted (runtime unavailable or probe timed out). - Unknown, + /// Probe was not attempted, timed out, or is incompatible with this CLI. + /// + /// `diagnostic` carries actionable guidance when we can identify the + /// failure mode (e.g. outdated Claude Code lacking `auth status`). + Unknown { + /// Optional guidance shown in Doctor / onboarding tooltips. + #[serde(default, skip_serializing_if = "Option::is_none")] + diagnostic: Option, + }, } #[derive(Debug, Clone, Serialize)] diff --git a/desktop/src/features/agents/hooks.ts b/desktop/src/features/agents/hooks.ts index 128ae9083b..352054b6bc 100644 --- a/desktop/src/features/agents/hooks.ts +++ b/desktop/src/features/agents/hooks.ts @@ -63,6 +63,7 @@ import { import { teamsQueryKey } from "@/features/agents/teamHooks"; import type { AcpRuntime, + AcpRuntimeCatalogEntry, AgentPersona, Channel, CreateManagedAgentInput, @@ -229,10 +230,39 @@ export function useConnectAcpRuntimeMutation() { export function useInstallAcpRuntimeMutation() { const queryClient = useQueryClient(); return useMutation({ - mutationFn: (runtimeId: string) => installAcpRuntime(runtimeId), - onSettled: () => { - void queryClient.invalidateQueries({ queryKey: acpRuntimesQueryKey }); - void queryClient.invalidateQueries({ queryKey: managedAgentsQueryKey }); + mutationFn: async (runtimeId: string) => { + const result = await installAcpRuntime(runtimeId); + // Await the active catalog refetch inside mutationFn so `isPending` + // stays true until discovery reflects the new install. Fire-and-forget + // invalidation in onSettled left a gap where onboarding reverted to + // UPDATE CLI / INSTALL on stale `unknown` catalog data. + await queryClient.invalidateQueries({ queryKey: acpRuntimesQueryKey }); + if (result.success) { + const catalog = + queryClient.getQueryData( + acpRuntimesQueryKey, + ); + const entry = catalog?.find((item) => item.id === runtimeId); + // CLI repair can exit before the replacement binary is visible to the + // next auth probe. Retry once when discovery still reports an update + // diagnostic (catalog-driven — not a harness-id special case). + if ( + entry?.availability === "available" && + entry.authStatus.status === "unknown" && + entry.authStatus.diagnostic + ) { + await new Promise((resolve) => { + window.setTimeout(resolve, 1_500); + }); + await queryClient.invalidateQueries({ + queryKey: acpRuntimesQueryKey, + }); + } + } + await queryClient.invalidateQueries({ + queryKey: managedAgentsQueryKey, + }); + return result; }, }); } diff --git a/desktop/src/features/onboarding/ui/SetupStep.tsx b/desktop/src/features/onboarding/ui/SetupStep.tsx index 288ca30259..cfac3fe535 100644 --- a/desktop/src/features/onboarding/ui/SetupStep.tsx +++ b/desktop/src/features/onboarding/ui/SetupStep.tsx @@ -201,14 +201,19 @@ function RuntimeStatus({ } if (isInstalling) { + const isUpdating = runtimeNeedsCliUpdate(runtime); return (
- INSTALLING + {isUpdating ? "UPDATING" : "INSTALLING"}
); } @@ -238,6 +243,22 @@ function RuntimeStatus({ runtime.availability === "available" && runtime.authStatus.status === "unknown" ) { + // Only offer UPDATE CLI when discovery attached an update diagnostic + // (e.g. outdated Claude Code). Generic unknown stays on CHECK AGAIN. + if (runtimeNeedsCliUpdate(runtime) && runtime.canAutoInstall) { + return ( + + ); + } return (