From 2e444f9f4332497b1435a23fdb5f1b88c7194665 Mon Sep 17 00:00:00 2001 From: Tyler Fletcher Date: Fri, 24 Jul 2026 18:28:45 -0500 Subject: [PATCH 1/6] fix(desktop): recover from outdated Claude Code auth status probes Older Claude Code builds lack `claude auth status` and treat those args as a prompt, so onboarding auth checks time out and leave Next disabled. Validate JSON `loggedIn`, surface an update hint, and repair via `claude update` on install. Co-authored-by: Cursor Signed-off-by: Tyler Fletcher --- .../src-tauri/src/commands/agent_discovery.rs | 22 +- .../commands/agent_discovery/cli_install.rs | 63 +++++ .../src-tauri/src/managed_agents/discovery.rs | 58 +++-- .../src/managed_agents/readiness/cli_login.rs | 9 + .../src/managed_agents/readiness/cli_probe.rs | 229 +++++++++++++++++- desktop/src-tauri/src/managed_agents/types.rs | 11 +- .../src/features/onboarding/ui/SetupStep.tsx | 21 +- .../settings/ui/DoctorSettingsPanel.tsx | 9 + desktop/src/shared/api/types.ts | 2 +- 9 files changed, 383 insertions(+), 41 deletions(-) create mode 100644 desktop/src-tauri/src/commands/agent_discovery/cli_install.rs diff --git a/desktop/src-tauri/src/commands/agent_discovery.rs b/desktop/src-tauri/src/commands/agent_discovery.rs index f080272914..537a53cb29 100644 --- a/desktop/src-tauri/src/commands/agent_discovery.rs +++ b/desktop/src-tauri/src/commands/agent_discovery.rs @@ -144,26 +144,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. @@ -1031,6 +1018,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 f40eed5a13..317af24acc 100644 --- a/desktop/src-tauri/src/managed_agents/discovery.rs +++ b/desktop/src-tauri/src/managed_agents/discovery.rs @@ -122,7 +122,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 { @@ -899,6 +901,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 @@ -909,6 +915,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..]); @@ -923,7 +932,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. @@ -935,6 +944,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(); @@ -966,7 +976,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, @@ -974,27 +984,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)) + } } } @@ -1256,7 +1274,7 @@ pub fn discover_acp_runtimes() -> Vec { underlying_cli_path, node_required, // Filled in by the probe phase below. - auth_status: AuthStatus::Unknown, + auth_status: AuthStatus::Unknown { diagnostic: None }, login_hint: None, }, } @@ -1287,27 +1305,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..7d42ad6cc6 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,12 +108,35 @@ 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, } } +/// 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 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 collected probe output into a `ProbeOutcome`. /// /// Shared between `login_probe` (which has the full `Output`) and the @@ -96,9 +161,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 +430,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 90da759af0..776cf7e6d4 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/onboarding/ui/SetupStep.tsx b/desktop/src/features/onboarding/ui/SetupStep.tsx index aa02486706..a43d9a428b 100644 --- a/desktop/src/features/onboarding/ui/SetupStep.tsx +++ b/desktop/src/features/onboarding/ui/SetupStep.tsx @@ -238,6 +238,21 @@ function RuntimeStatus({ runtime.availability === "available" && runtime.authStatus.status === "unknown" ) { + // Outdated Claude Code (etc.): Install repairs the CLI via `claude update`. + if (runtime.canAutoInstall) { + return ( + + ); + } return (