From cdce765aa0a42984d8b9a63c65fc824ab23034e7 Mon Sep 17 00:00:00 2001 From: Assis Ngolo Date: Fri, 2 Oct 2026 17:10:02 +0000 Subject: [PATCH 1/4] fix(claude): install managed CLI from source card --- .ai/lessons.md | 10 ++ docs/developer/architecture-guide.md | 8 + jean-core/src/claude_cli/commands.rs | 46 +++++- jean-core/src/lib.rs | 70 ++++++--- .../preferences/BackendCliSourceCards.tsx | 32 ++-- .../ClaudeManagedInstallButton.test.tsx | 137 ++++++++++++++++++ .../ClaudeManagedInstallButton.tsx | 78 ++++++++++ .../preferences/panes/GeneralPane.tsx | 16 +- src/lib/claude-cli-status.test.ts | 42 ++++++ src/lib/claude-cli-status.ts | 19 +++ src/types/claude-cli.ts | 1 + 11 files changed, 425 insertions(+), 34 deletions(-) create mode 100644 src/components/preferences/ClaudeManagedInstallButton.test.tsx create mode 100644 src/components/preferences/ClaudeManagedInstallButton.tsx create mode 100644 src/lib/claude-cli-status.test.ts create mode 100644 src/lib/claude-cli-status.ts diff --git a/.ai/lessons.md b/.ai/lessons.md index 3a5dd3892..d6eff8e3b 100644 --- a/.ai/lessons.md +++ b/.ai/lessons.md @@ -37,3 +37,13 @@ - A new streaming backend needs two parser paths: the live response parser and the run-log reconstruction parser. - Route persisted runs by the per-run backend or model prefix before using a generic fallback parser. - Test history reload with the backend's real NDJSON format. Live streaming success does not prove that the response survives a query refresh or app reload. + +## Make managed installation reachable before source selection + +- Offer installation directly on the managed-source card even when PATH is selected; do not require selecting an absent installation first. +- Track managed installation separately from effective execution availability. Settings can show the selected source as missing without hiding a usable fallback backend elsewhere. + +## Typecheck test mocks before handing back + +- Verify module exports before referencing fixtures in partial mocks. Missing exports can silently return undefined at runtime. +- Run `bun run typecheck` alongside targeted tests; passing runtime tests alone does not validate typed mock setup. diff --git a/docs/developer/architecture-guide.md b/docs/developer/architecture-guide.md index 5dfc5d555..d46c4d9b9 100644 --- a/docs/developer/architecture-guide.md +++ b/docs/developer/architecture-guide.md @@ -558,6 +558,14 @@ When adding entirely new systems: 6. **Test everything** - Use quality gates to maintain code health 7. **Document patterns** - Keep docs current as patterns evolve +### CLI source preferences and installation status + +Auto-detect a system CLI source only when its source field is absent from saved preferences. Never overwrite an explicit managed or PATH choice during loading. + +Claude's installation status describes the binary available for execution, including fallback. Its separate `managed_installed` field describes the Jean-managed copy. Settings derives selected-source display status without changing backend availability for onboarding or chat. + +The managed Claude card can install while PATH is selected. Installation resolves the latest stable version through the existing installer, then selects the managed source only after success. A backend mutex rejects concurrent installations across all entry points. + ### Cross-platform CLI resolution and launch When resolving external CLIs from PATH, use `crate::platform::detect_cli_in_path()` or diff --git a/jean-core/src/claude_cli/commands.rs b/jean-core/src/claude_cli/commands.rs index e28512d6f..b17eff223 100644 --- a/jean-core/src/claude_cli/commands.rs +++ b/jean-core/src/claude_cli/commands.rs @@ -13,7 +13,7 @@ use tokio::sync::Mutex as AsyncMutex; use super::config::{ ensure_cli_dir, get_cli_binary_path, get_cli_dir, get_wsl_cli_binary_path, get_wsl_cli_dir, - resolve_cli_binary, + jean_managed_installed, resolve_cli_binary, }; use crate::http_server::EmitExt; #[cfg(target_os = "macos")] @@ -55,6 +55,7 @@ const CLAUDE_USAGE_CACHE_TTL_SECS: u64 = 5 * 60; /// Stale cache is still served on 429 / transient API failures (OpenUsage pattern). const CLAUDE_USAGE_STALE_CACHE_MAX_SECS: u64 = 6 * 60 * 60; const CLAUDE_USAGE_USER_AGENT: &str = "claude-code/2.1.69"; +static CLAUDE_INSTALL_LOCK: AsyncMutex<()> = AsyncMutex::const_new(()); static CLAUDE_USAGE_FETCH_LOCK: OnceLock> = OnceLock::new(); fn claude_usage_fetch_lock() -> &'static AsyncMutex<()> { @@ -66,6 +67,8 @@ fn claude_usage_fetch_lock() -> &'static AsyncMutex<()> { pub struct ClaudeCliStatus { /// Whether Claude CLI is installed pub installed: bool, + #[serde(default)] + pub managed_installed: bool, /// Installed version (if any) pub version: Option, /// Path to the CLI binary (if installed) @@ -103,6 +106,7 @@ pub struct InstallProgress { pub async fn check_claude_cli_installed(app: AppHandle) -> Result { log::trace!("Checking Claude CLI installation status"); + let managed_installed = jean_managed_installed(&app); let wsl = crate::platform::get_wsl_config(); let binary_path = resolve_cli_binary(&app); @@ -122,6 +126,7 @@ pub async fn check_claude_cli_installed(app: AppHandle) -> Result Result Result Result Result<(), String> { Ok(()) } +fn try_acquire_claude_install() -> Result, String> { + CLAUDE_INSTALL_LOCK + .try_lock() + .map_err(|_| "Claude CLI installation is already in progress".to_string()) +} + /// Install Claude CLI by downloading the binary directly from Anthropic's distribution bucket pub async fn install_claude_cli(app: AppHandle, version: Option) -> Result<(), String> { + let _install_guard = try_acquire_claude_install()?; log::trace!("Installing Claude CLI, version: {:?}", version); // Check if any Claude processes are running - cannot replace binary while in use @@ -1520,6 +1535,35 @@ fn emit_progress(app: &AppHandle, stage: &str, message: &str, percent: u8) { mod tests { use super::*; + #[test] + fn claude_install_lock_rejects_concurrent_install_and_releases() { + let guard = try_acquire_claude_install().unwrap(); + assert_eq!( + try_acquire_claude_install().unwrap_err(), + "Claude CLI installation is already in progress" + ); + drop(guard); + assert!(try_acquire_claude_install().is_ok()); + } + + #[test] + fn status_serializes_managed_installation_independently_of_selected_source() { + for installed in [false, true] { + for managed_installed in [false, true] { + let status = ClaudeCliStatus { + installed, + managed_installed, + version: None, + path: None, + supports_auth_command: false, + }; + let value = serde_json::to_value(status).unwrap(); + assert_eq!(value["installed"], installed); + assert_eq!(value["managed_installed"], managed_installed); + } + } + } + #[test] fn wsl_credentials_path_uses_wsl_home() { assert_eq!( diff --git a/jean-core/src/lib.rs b/jean-core/src/lib.rs index bd883dafc..7d0ec8f30 100644 --- a/jean-core/src/lib.rs +++ b/jean-core/src/lib.rs @@ -727,10 +727,7 @@ fn maybe_auto_select_system_coderabbit( preferences: &mut AppPreferences, raw_preferences: Option<&Value>, ) -> bool { - let coderabbit_source_missing = raw_preferences - .and_then(Value::as_object) - .map(|object| !object.contains_key("coderabbit_cli_source")) - .unwrap_or(true); + let coderabbit_source_missing = cli_source_missing(raw_preferences, "coderabbit_cli_source"); if coderabbit_source_missing && coderabbit_cli::should_auto_use_system_coderabbit(app) { preferences.coderabbit_cli_source = "path".to_string(); @@ -740,27 +737,38 @@ fn maybe_auto_select_system_coderabbit( false } -/// When Jean-managed Claude/Codex/OpenCode is missing but a system PATH install -/// exists, switch the preference to `"path"` so Settings UI, auth, and status -/// checks agree with the binary actually used (issue #387). -/// -/// Runtime `resolve_cli_binary` also falls back to PATH when Jean-managed is -/// missing; this persists the source so the UI does not show a misleading -/// "Jean" selection. -fn maybe_auto_select_system_cli_sources(app: &AppHandle, preferences: &mut AppPreferences) -> bool { +fn cli_source_missing(raw_preferences: Option<&Value>, field: &str) -> bool { + raw_preferences + .and_then(Value::as_object) + .map(|object| !object.contains_key(field)) + .unwrap_or(true) +} + +/// Auto-select PATH only for preferences without an explicit source choice. +fn maybe_auto_select_system_cli_sources( + app: &AppHandle, + preferences: &mut AppPreferences, + raw_preferences: Option<&Value>, +) -> bool { let mut changed = false; - if preferences.claude_cli_source == "jean" && claude_cli::should_auto_use_system(app) { + if cli_source_missing(raw_preferences, "claude_cli_source") + && claude_cli::should_auto_use_system(app) + { log::info!("Auto-selecting Claude CLI source=path (Jean-managed missing, system found)"); preferences.claude_cli_source = "path".to_string(); changed = true; } - if preferences.codex_cli_source == "jean" && codex_cli::should_auto_use_system(app) { + if cli_source_missing(raw_preferences, "codex_cli_source") + && codex_cli::should_auto_use_system(app) + { log::info!("Auto-selecting Codex CLI source=path (Jean-managed missing, system found)"); preferences.codex_cli_source = "path".to_string(); changed = true; } - if preferences.opencode_cli_source == "jean" && opencode_cli::should_auto_use_system(app) { + if cli_source_missing(raw_preferences, "opencode_cli_source") + && opencode_cli::should_auto_use_system(app) + { log::info!("Auto-selecting OpenCode CLI source=path (Jean-managed missing, system found)"); preferences.opencode_cli_source = "path".to_string(); changed = true; @@ -776,7 +784,7 @@ fn maybe_auto_select_system_cli_preferences( raw_preferences: Option<&Value>, ) -> bool { let mut changed = maybe_auto_select_system_coderabbit(app, preferences, raw_preferences); - changed |= maybe_auto_select_system_cli_sources(app, preferences); + changed |= maybe_auto_select_system_cli_sources(app, preferences, raw_preferences); changed } @@ -905,13 +913,35 @@ fn resolve_http_server_bind_host(prefs: &AppPreferences) -> String { #[cfg(test)] mod tests { use super::{ - default_global_system_prompt, default_model, migrate_smoke_test_preferences, - parse_cli_args_from, resolve_headless_bind_host, resolve_headless_token_required, - resolve_http_server_bind_host, server_preferences_value, validate_headless_security, - AppPreferences, + cli_source_missing, default_global_system_prompt, default_model, + migrate_smoke_test_preferences, parse_cli_args_from, resolve_headless_bind_host, + resolve_headless_token_required, resolve_http_server_bind_host, server_preferences_value, + validate_headless_security, AppPreferences, }; use serde_json::json; + #[test] + fn cli_auto_selection_only_applies_to_absent_raw_source_fields() { + for field in [ + "claude_cli_source", + "codex_cli_source", + "opencode_cli_source", + "coderabbit_cli_source", + ] { + assert!(cli_source_missing(None, field)); + assert!(cli_source_missing(Some(&json!({})), field)); + for source in [json!("jean"), json!("path"), json!(""), json!(null)] { + let mut raw = json!({}); + raw[field] = source; + assert!(!cli_source_missing(Some(&raw), field)); + } + assert!(cli_source_missing(Some(&json!({"theme": "dark"})), field)); + } + let raw = json!({"claude_cli_source": "jean"}); + assert!(!cli_source_missing(Some(&raw), "claude_cli_source")); + assert!(cli_source_missing(Some(&raw), "codex_cli_source")); + } + #[test] fn server_preferences_exclude_client_fields_and_redact_secrets() { let mut preferences = AppPreferences::default(); diff --git a/src/components/preferences/BackendCliSourceCards.tsx b/src/components/preferences/BackendCliSourceCards.tsx index b12499d66..8915d431d 100644 --- a/src/components/preferences/BackendCliSourceCards.tsx +++ b/src/components/preferences/BackendCliSourceCards.tsx @@ -1,3 +1,4 @@ +import type { ReactNode } from 'react' import { Label } from '@/components/ui/label' import { RadioGroup, RadioGroupItem } from '@/components/ui/radio-group' @@ -5,6 +6,7 @@ interface BackendCliSourceCardsProps { value: 'jean' | 'path' onValueChange: (value: 'jean' | 'path') => void backendName: string + managedAction?: ReactNode managedDescription?: string path: string | null | undefined pathVersion?: string | null @@ -16,6 +18,7 @@ export function BackendCliSourceCards({ onValueChange, backendName, managedDescription, + managedAction, path, pathVersion, pathFound, @@ -29,19 +32,24 @@ export function BackendCliSourceCards({ }} className="w-full gap-3" > -