From 3389f1d551d4683fcfdcf8f18fa52bdf09c4a723 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Tue, 14 Apr 2026 07:30:32 -0400 Subject: [PATCH] chore: remove dead system dependency check infrastructure Now that Graphviz is vendored, remove the DepSpec/probe_system_deps/ check_system_deps infrastructure from doctor.rs (empty since the vendoring), the no-op pre-flight check from install.rs, and the stale hardcoded "dot" check from demo diagnostics. Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/crates/fabro-cli/src/commands/doctor.rs | 200 ------------------- lib/crates/fabro-cli/src/commands/install.rs | 29 +-- lib/crates/fabro-server/src/demo/mod.rs | 6 - 3 files changed, 2 insertions(+), 233 deletions(-) diff --git a/lib/crates/fabro-cli/src/commands/doctor.rs b/lib/crates/fabro-cli/src/commands/doctor.rs index 5ca10f6ba..5747632cd 100644 --- a/lib/crates/fabro-cli/src/commands/doctor.rs +++ b/lib/crates/fabro-cli/src/commands/doctor.rs @@ -1,5 +1,4 @@ use std::path::{Path, PathBuf}; -use std::sync::LazyLock; use anyhow::Result; use fabro_api::types as api_types; @@ -14,125 +13,12 @@ pub(crate) use fabro_util::check_report::{ use fabro_util::printer::Printer; use fabro_util::terminal::Styles; use fabro_util::version::FABRO_VERSION; -use regex::Regex; -use semver::Version; -use tokio::process::Command as TokioCommand; use crate::args::{DoctorArgs, GlobalArgs}; use crate::command_context::CommandContext; use crate::shared::print_json_pretty; use crate::user_config; -pub(crate) struct DepSpec { - pub name: &'static str, - command: &'static [&'static str], - pub required: bool, - pub min_version: Version, - pattern: &'static LazyLock, -} - -#[derive(Debug, Clone, PartialEq)] -pub(crate) enum ProbeOutcome { - NotFound, - Failed, - Ok { version: Option }, -} - -pub(crate) const DEP_SPECS: &[DepSpec] = &[]; - -fn parse_version(re: &Regex, output: &str) -> Option { - let caps = re.captures(output)?; - Some(Version::new( - caps[1].parse().ok()?, - caps[2].parse().ok()?, - caps[3].parse().ok()?, - )) -} - -pub(crate) async fn probe_system_deps() -> Vec { - let mut outcomes = Vec::with_capacity(DEP_SPECS.len()); - for spec in DEP_SPECS { - let result = TokioCommand::new(spec.command[0]) - .args(&spec.command[1..]) - .output() - .await - .ok(); - - let outcome = match result { - None => ProbeOutcome::NotFound, - Some(output) if !output.status.success() => ProbeOutcome::Failed, - Some(output) => { - let stdout = String::from_utf8_lossy(&output.stdout); - let stderr = String::from_utf8_lossy(&output.stderr); - let version = parse_version(spec.pattern, &stdout) - .or_else(|| parse_version(spec.pattern, &stderr)); - ProbeOutcome::Ok { version } - } - }; - outcomes.push(outcome); - } - outcomes -} - -fn dep_issue(name: &str, issue: &str, required: bool) -> (CheckStatus, String) { - let severity = if required { "required" } else { "optional" }; - let status = if required { - CheckStatus::Error - } else { - CheckStatus::Warning - }; - (status, format!("{name}: {issue} ({severity})")) -} - -pub(crate) fn check_system_deps(specs: &[DepSpec], outcomes: &[ProbeOutcome]) -> CheckResult { - let mut details = Vec::new(); - let mut worst_status = CheckStatus::Pass; - - for (spec, outcome) in specs.iter().zip(outcomes) { - let (status, text) = match outcome { - ProbeOutcome::NotFound => dep_issue(spec.name, "not found", spec.required), - ProbeOutcome::Failed => dep_issue(spec.name, "command failed", spec.required), - ProbeOutcome::Ok { version: None } => { - (CheckStatus::Pass, format!("{}: version unknown", spec.name)) - } - ProbeOutcome::Ok { - version: Some(version), - } => { - if version < &spec.min_version { - ( - CheckStatus::Warning, - format!("{}: {version} (minimum {})", spec.name, spec.min_version), - ) - } else { - (CheckStatus::Pass, format!("{}: {version}", spec.name)) - } - } - }; - - worst_status = worst_status.max(status); - details.push(CheckDetail::new(text)); - } - - let summary = match worst_status { - CheckStatus::Pass => "all found".to_string(), - CheckStatus::Warning => "some issues".to_string(), - CheckStatus::Error => "missing required tools".to_string(), - }; - - let remediation = match worst_status { - CheckStatus::Pass => None, - _ => Some("Install missing system dependencies".to_string()), - }; - - CheckResult { - name: "System dependencies".to_string(), - status: worst_status, - summary, - details, - remediation, - } -} - pub(crate) fn check_config( settings_path: Option, legacy_paths: &[PathBuf], @@ -678,90 +564,4 @@ mod tests { assert!(rendered.contains("Fabro Doctor")); assert!(rendered.contains("[✓] Configuration (loaded)")); } - - static TEST_RE: LazyLock = - LazyLock::new(|| Regex::new(r"version (\d+)\.(\d+)\.(\d+)").unwrap()); - - fn spec(name: &'static str, required: bool, min_version: Version) -> DepSpec { - DepSpec { - name, - command: &["echo", "unused"], - required, - min_version, - pattern: &TEST_RE, - } - } - - #[test] - fn check_system_deps_all_present() { - let specs = [spec("dot", false, Version::new(2, 0, 0))]; - let outcomes = [ProbeOutcome::Ok { - version: Some(Version::new(12, 2, 1)), - }]; - let result = check_system_deps(&specs, &outcomes); - assert_eq!(result.status, CheckStatus::Pass); - assert_eq!(result.summary, "all found"); - } - - #[test] - fn check_system_deps_required_missing_is_error() { - let specs = [spec("required-tool", true, Version::new(3, 0, 0))]; - let outcomes = [ProbeOutcome::NotFound]; - let result = check_system_deps(&specs, &outcomes); - assert_eq!(result.status, CheckStatus::Error); - } - - #[test] - fn check_system_deps_optional_missing_is_warning() { - let specs = [spec("dot", false, Version::new(2, 0, 0))]; - let outcomes = [ProbeOutcome::NotFound]; - let result = check_system_deps(&specs, &outcomes); - assert_eq!(result.status, CheckStatus::Warning); - } - - #[test] - fn check_system_deps_outdated_is_warning() { - let specs = [spec("required-tool", true, Version::new(3, 0, 0))]; - let outcomes = [ProbeOutcome::Ok { - version: Some(Version::new(1, 1, 1)), - }]; - let result = check_system_deps(&specs, &outcomes); - assert_eq!(result.status, CheckStatus::Warning); - } - - #[test] - fn check_system_deps_unparseable_success_is_pass() { - let specs = [spec("required-tool", true, Version::new(3, 0, 0))]; - let outcomes = [ProbeOutcome::Ok { version: None }]; - let result = check_system_deps(&specs, &outcomes); - assert_eq!(result.status, CheckStatus::Pass); - assert!(result.details[0].text.contains("version unknown")); - } - - #[test] - fn check_system_deps_required_command_failed_is_error() { - let specs = [spec("required-tool", true, Version::new(3, 0, 0))]; - let outcomes = [ProbeOutcome::Failed]; - let result = check_system_deps(&specs, &outcomes); - assert_eq!(result.status, CheckStatus::Error); - } - - #[test] - fn check_system_deps_optional_command_failed_is_warning() { - let specs = [spec("dot", false, Version::new(2, 0, 0))]; - let outcomes = [ProbeOutcome::Failed]; - let result = check_system_deps(&specs, &outcomes); - assert_eq!(result.status, CheckStatus::Warning); - } - - #[test] - fn check_system_deps_error_beats_warning() { - let specs = [ - spec("required-tool", true, Version::new(3, 0, 0)), - spec("dot", false, Version::new(2, 0, 0)), - ]; - let outcomes = [ProbeOutcome::NotFound, ProbeOutcome::NotFound]; - let result = check_system_deps(&specs, &outcomes); - assert_eq!(result.status, CheckStatus::Error); - } } diff --git a/lib/crates/fabro-cli/src/commands/install.rs b/lib/crates/fabro-cli/src/commands/install.rs index 9c8f560de..0e31e8f3e 100644 --- a/lib/crates/fabro-cli/src/commands/install.rs +++ b/lib/crates/fabro-cli/src/commands/install.rs @@ -1304,33 +1304,8 @@ async fn run_install_inner( } } - // Pre-flight checks - let facts = { - let dep_outcomes = doctor::probe_system_deps().await; - let dep_check = doctor::check_system_deps(doctor::DEP_SPECS, &dep_outcomes); - - fabro_util::printerr!( - printer, - " {}", - s.dim.apply_to("[Pre-flight] System dependency checks") - ); - - if dep_check.status == doctor::CheckStatus::Error { - fabro_util::printerr!(printer, " Missing required system dependencies:"); - for detail in &dep_check.details { - fabro_util::printerr!(printer, " {}", detail.text); - } - bail!("Install missing required tools before running setup"); - } - - for detail in &dep_check.details { - fabro_util::printerr!(printer, " {}", detail.text); - } - fabro_util::printerr!(printer, ""); - - InstallFacts { - codex_detected: detect_binary_on_path("codex").await, - } + let facts = InstallFacts { + codex_detected: detect_binary_on_path("codex").await, }; // Step 1: LLM Providers diff --git a/lib/crates/fabro-server/src/demo/mod.rs b/lib/crates/fabro-server/src/demo/mod.rs index 8ee460bd0..81e6b4aa7 100644 --- a/lib/crates/fabro-server/src/demo/mod.rs +++ b/lib/crates/fabro-server/src/demo/mod.rs @@ -396,12 +396,6 @@ pub(crate) async fn run_diagnostics( { "name": "Brave Search", "status": "warning", "summary": "not configured", "details": [], "remediation": "Set BRAVE_SEARCH_API_KEY to enable web search" } ] }, - { - "title": "System", - "checks": [ - { "name": "dot", "status": "pass", "summary": "dot available", "details": [], "remediation": null } - ] - }, { "title": "Configuration", "checks": [