doctor: a service is "installed" only if the OS will actually start it

CHANGELOG.md

@@ -257,6 +257,16 @@ EXPERIMENTAL, so on-disk formats and the CLI may change without notice.
 
 ### Fixed
 
+- **`fabric doctor` reports whether the service is ENABLED, not just that its
+  unit file exists.** It read `service_installed` from the unit file's presence,
+  so a service disabled during an incident with its unit left in place said
+  "installed and managed by the OS" — and after a reboot no daemon started. The
+  CA trust check was fixed for exactly this mistake; the service check was not.
+  Doctor now queries the manager (`systemctl --user is-enabled`, or whether
+  launchd has the label loaded) and reads three states: enabled, present but not
+  enabled (a problem, because a reboot leaves no daemon), and not installed.
+  Finding 10 of the 2026-08-29 review.
+
 - **`fabric doctor` on a non-default home asks the right daemon which build a
   peer runs.** Doctor shells out to `fabric exec <peer> -- fabric --version`
   without `--home`, so `fabric --home X doctor` asked the DEFAULT daemon about a

src/doctor.rs

@@ -139,8 +139,9 @@ pub struct Facts {
     /// first-run report red, which is the one thing this design was for.
     pub manages_service: bool,
     pub daemon_running: bool,
-    /// `None` when it could not be determined, which is different from `false`.
-    pub service_installed: Option<bool>,
+    /// Whether the managed service will start on boot, not merely whether its
+    /// unit file exists. Presence is not enablement.
+    pub service: ServiceEnablement,
     pub own_version: String,
     pub peers: Vec<PeerFact>,
     pub syncs: Vec<SyncFact>,
@@ -155,7 +156,11 @@ impl Facts {
     fn never_configured(&self) -> bool {
         !self.has_identity
             && self.peers.is_empty()
-            && !(self.manages_service && self.service_installed == Some(true))
+            && !(self.manages_service
+                && matches!(
+                    self.service,
+                    ServiceEnablement::Enabled | ServiceEnablement::PresentNotEnabled
+                ))
     }
 }
 
@@ -182,18 +187,29 @@ pub fn diagnose(facts: &Facts) -> Vec<Finding> {
             "this home is not the managed one, so no OS service applies to it",
         )
     } else {
-        match facts.service_installed {
-        Some(true) => Finding::new("service", Verdict::Ok, "installed and managed by the OS"),
-        Some(false) => Finding::new(
+        match facts.service {
+        ServiceEnablement::Enabled => {
+            Finding::new("service", Verdict::Ok, "installed, enabled, and will start on boot")
+        }
+        // The unit file exists but the manager will not start it on boot. This
+        // is NOT the same as "installed": a reboot leaves no daemon. It reads
+        // differently from "not installed" because the repair differs.
+        ServiceEnablement::PresentNotEnabled => Finding::new(
+            "service",
+            if fresh { Verdict::Setup } else { Verdict::Problem },
+            "the service is installed but not enabled, so it will not start after a reboot",
+        )
+        .with_action("fabric service install"),
+        ServiceEnablement::NotInstalled => Finding::new(
             "service",
             if fresh { Verdict::Setup } else { Verdict::Problem },
             "fabric is not installed as a service, so it will not come back after a reboot",
         )
         .with_action("fabric service install"),
-        None => Finding::new(
+        ServiceEnablement::Unknown => Finding::new(
             "service",
             Verdict::Unknown,
-            "could not tell whether fabric is installed as a service",
+            "could not tell whether fabric is enabled as a service",
         ),
         }
     });
@@ -550,7 +566,7 @@ mod tests {
             has_identity: true,
             manages_service: true,
             daemon_running: true,
-            service_installed: Some(true),
+            service: ServiceEnablement::Enabled,
             own_version: "0.2.0+abc".to_string(),
             peers: vec![PeerFact {
                 label: "hetz".to_string(),
@@ -603,7 +619,7 @@ mod tests {
             has_identity: false,
             manages_service: true,
             daemon_running: false,
-            service_installed: Some(false),
+            service: ServiceEnablement::NotInstalled,
             own_version: "0.2.0+abc".to_string(),
             peers: Vec::new(),
             syncs: Vec::new(),
@@ -649,7 +665,7 @@ mod tests {
             manages_service: false,
             // ...but the unit is on disk for the prod home, and this is exactly
             // what the gatherer sees.
-            service_installed: Some(true),
+            service: ServiceEnablement::Enabled,
             daemon_running: false,
             own_version: "0.2.0+abc".to_string(),
             peers: Vec::new(),
@@ -679,7 +695,7 @@ mod tests {
     #[test]
     fn the_same_gap_on_a_configured_machine_is_a_problem() {
         let mut facts = configured();
-        facts.service_installed = Some(false);
+        facts.service = ServiceEnablement::NotInstalled;
         let findings = diagnose(&facts);
         let service = find(&findings, "service");
         assert_eq!(service[0].verdict, Verdict::Problem);
@@ -690,7 +706,7 @@ mod tests {
     #[test]
     fn a_check_that_could_not_look_says_unknown_and_counts() {
         let mut facts = configured();
-        facts.service_installed = None;
+        facts.service = ServiceEnablement::Unknown;
         facts.peers[0].reachable = None;
         facts.ca.present = true;
         facts.ca.installed = None;
@@ -775,6 +791,38 @@ mod tests {
         assert_eq!(argv.get(exec_at + 1).map(String::as_str), Some("hetz"));
     }
 
+    /// Finding 10: a unit file left in place by a `disable` is NOT "installed
+    /// and managed by the OS". It will not start after a reboot, and it reads
+    /// as a problem with a different repair from "never installed".
+    #[test]
+    fn a_present_but_not_enabled_service_is_a_problem_not_ok() {
+        let mut facts = configured();
+        facts.service = ServiceEnablement::PresentNotEnabled;
+        let findings = diagnose(&facts);
+        let service = find(&findings, "service");
+        assert!(
+            service
+                .iter()
+                .any(|f| f.verdict == Verdict::Problem && f.detail.contains("not enabled")),
+            "a disabled-but-present service did not read as a problem: {:?}",
+            service.iter().map(|f| (&f.verdict, &f.detail)).collect::<Vec<_>>()
+        );
+        assert!(
+            !service.iter().any(|f| f.detail.contains("installed, enabled")),
+            "a disabled service was still called enabled"
+        );
+
+        // And the healthy case still reads Ok, so the test is about the state
+        // and not about the check being broken.
+        facts.service = ServiceEnablement::Enabled;
+        let findings = diagnose(&facts);
+        assert!(
+            find(&findings, "service")
+                .iter()
+                .any(|f| f.verdict == Verdict::Ok && f.detail.contains("will start on boot"))
+        );
+    }
+
     #[test]
     fn a_denied_sync_and_an_unreachable_one_read_differently() {
         let mut facts = configured();
@@ -991,20 +1039,11 @@ mod tests {
 use anyhow::Result;
 
 use crate::ca;
+use crate::service::ServiceEnablement;
 use crate::config::FabricHome;
 use crate::control::{ControlRequest, ControlResponse, PeerReachability, SyncEntryStatus};
 
 /// Where the OS service definition would live on this platform.
-fn service_unit_path() -> Result<PathBuf> {
-    #[cfg(target_os = "macos")]
-    {
-        crate::service::launch_agent_path()
-    }
-    #[cfg(not(target_os = "macos"))]
-    {
-        crate::service::systemd_user_unit_path()
-    }
-}
 
 /// Collect everything the checks reason about.
 ///
@@ -1019,11 +1058,7 @@ where
     let has_identity = home.identity_path().exists();
     let manages_service = home.is_default_state_root();
 
-    let service_installed = match service_unit_path() {
-        Ok(path) => Some(path.exists()),
-        // Could not even work out where it would be. Not "no".
-        Err(_) => None,
-    };
+    let service = crate::service::service_enablement();
 
     let mut own_version = env!("CARGO_PKG_VERSION").to_string();
     let mut daemon_running = false;
@@ -1090,7 +1125,7 @@ where
         has_identity,
         manages_service,
         daemon_running,
-        service_installed,
+        service,
         own_version,
         peers,
         ca: gather_ca(home, &syncs),

src/service.rs

@@ -297,6 +297,71 @@ fn resolve_memory_max_mb(home: &FabricHome, requested: Option<Option<u64>>) -> R
     Ok(config.memory_max_mb())
 }
 
+/// Whether the managed service will actually start on boot — not merely whether
+/// its unit file exists.
+///
+/// Presence is not enablement. A service disabled during an incident, with its
+/// unit file left in place, read as "installed and managed by the OS" and never
+/// came back after a reboot. The CA trust check was fixed for exactly this
+/// mistake; the service check was not. Finding 10 of the 2026-08-29 review.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum ServiceEnablement {
+    /// Enabled: the manager will start it on boot.
+    Enabled,
+    /// The unit or plist is present, but the manager will not start it on boot.
+    PresentNotEnabled,
+    /// No unit or plist at all.
+    NotInstalled,
+    /// Could not determine (the manager could not be queried).
+    Unknown,
+}
+
+/// Query the OS service manager for the real enablement state.
+pub fn service_enablement() -> ServiceEnablement {
+    match ServiceManager::current() {
+        Ok(manager) => manager.enablement(),
+        Err(_) => ServiceEnablement::Unknown,
+    }
+}
+
+/// Interpret `systemctl --user is-enabled fabric.service`.
+///
+/// `present` is whether the unit file exists, `ran` whether the query executed,
+/// and `stdout` its first word. Kept free of `cfg` so it is tested on any host.
+fn interpret_systemd_is_enabled(present: bool, ran: bool, stdout: &str) -> ServiceEnablement {
+    if !present {
+        return ServiceEnablement::NotInstalled;
+    }
+    if !ran {
+        return ServiceEnablement::Unknown;
+    }
+    match stdout.split_whitespace().next().unwrap_or("") {
+        "enabled" | "enabled-runtime" => ServiceEnablement::Enabled,
+        // The unit exists but will not be started on boot. `static`, `masked`,
+        // `linked`, `indirect`, `generated`, `transient` all mean "not enabled
+        // to start" for our purposes, and so does an empty answer with the file
+        // present.
+        "disabled" | "static" | "masked" | "linked" | "indirect" | "generated"
+        | "transient" | "" => ServiceEnablement::PresentNotEnabled,
+        _ => ServiceEnablement::Unknown,
+    }
+}
+
+/// Interpret launchd signals: whether the plist is present and whether the
+/// label is currently loaded (`launchctl print` succeeds).
+fn interpret_launchd(present: bool, loaded: bool) -> ServiceEnablement {
+    if !present {
+        return ServiceEnablement::NotInstalled;
+    }
+    if loaded {
+        ServiceEnablement::Enabled
+    } else {
+        // The plist is on disk but launchd has not loaded it, so a reboot will
+        // not start it. This is the disabled-during-an-incident case.
+        ServiceEnablement::PresentNotEnabled
+    }
+}
+
 enum ServiceManager {
     #[cfg(target_os = "linux")]
     SystemdUser,
@@ -319,6 +384,36 @@ impl ServiceManager {
             bail!("fabric service is currently supported on Linux systemd-user and macOS launchd");
         }
     }
+
+    fn enablement(&self) -> ServiceEnablement {
+        match self {
+            #[cfg(target_os = "linux")]
+            ServiceManager::SystemdUser => {
+                let present = systemd_user_unit_path()
+                    .map(|path| path.exists())
+                    .unwrap_or(false);
+                let query = Command::new("systemctl")
+                    .args(["--user", "is-enabled", SERVICE_NAME])
+                    .output();
+                let (ran, stdout) = match &query {
+                    Ok(output) => (true, String::from_utf8_lossy(&output.stdout).into_owned()),
+                    Err(_) => (false, String::new()),
+                };
+                interpret_systemd_is_enabled(present, ran, &stdout)
+            }
+            #[cfg(target_os = "macos")]
+            ServiceManager::LaunchdUser => {
+                let present = launch_agent_path().map(|path| path.exists()).unwrap_or(false);
+                let target = launchd_service_target();
+                let loaded = Command::new("launchctl")
+                    .args(["print", &target])
+                    .output()
+                    .map(|output| output.status.success())
+                    .unwrap_or(false);
+                interpret_launchd(present, loaded)
+            }
+        }
+    }
 }
 
 #[cfg(target_os = "linux")]
@@ -817,6 +912,46 @@ fn xml_escape(value: &str) -> String {
 
 #[cfg(test)]
 mod tests {
+    use super::{ServiceEnablement, interpret_launchd, interpret_systemd_is_enabled};
+
+    #[test]
+    fn systemd_is_enabled_reading_tells_enabled_from_merely_present() {
+        // The bug this replaces: a disabled service with its unit file present
+        // read as installed and never came back after a reboot.
+        assert_eq!(
+            interpret_systemd_is_enabled(true, true, "enabled\n"),
+            ServiceEnablement::Enabled
+        );
+        for present_not_enabled in ["disabled", "static", "masked", "linked", ""] {
+            assert_eq!(
+                interpret_systemd_is_enabled(true, true, present_not_enabled),
+                ServiceEnablement::PresentNotEnabled,
+                "is-enabled={present_not_enabled:?} must not read as enabled"
+            );
+        }
+        // No unit file at all, whatever the query says.
+        assert_eq!(
+            interpret_systemd_is_enabled(false, true, "enabled"),
+            ServiceEnablement::NotInstalled
+        );
+        // Could not run the query, but the file is there: not a claim either way.
+        assert_eq!(
+            interpret_systemd_is_enabled(true, false, ""),
+            ServiceEnablement::Unknown
+        );
+    }
+
+    #[test]
+    fn launchd_reading_tells_loaded_from_merely_present() {
+        assert_eq!(interpret_launchd(true, true), ServiceEnablement::Enabled);
+        assert_eq!(
+            interpret_launchd(true, false),
+            ServiceEnablement::PresentNotEnabled,
+            "a plist on disk that launchd has not loaded will not start on boot"
+        );
+        assert_eq!(interpret_launchd(false, false), ServiceEnablement::NotInstalled);
+    }
+
     use super::*;
 
     /// The unit must name the binary it was GIVEN, not the one rendering it.