Provide More Intuitive Behaviour for Launchd Services

CHANGELOG.md

@@ -7,6 +7,22 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## [0.10.0] - 2025-12-14
+
+### Changed
+
+- **BREAKING CHANGE**: Launchd services with restart policies (`RestartPolicy::Always`, `OnFailure`, or `OnSuccess`) no longer auto-start when `install()` is called. Services must now be explicitly started using `start()`. This provides cross-platform consistency where `install()` registers the service definition without starting it, matching the behavior of systemd and other service managers.
+  - Services with `KeepAlive` configured are now installed with `Disabled=true` in the plist
+  - The `start()` function removes the `Disabled` key and reloads the service
+  - The `autostart` parameter continues to control only `RunAtLoad` (whether service starts on OS boot), not initial install behavior
+  - Migration: Add explicit `manager.start(ctx)?` call after `manager.install(ctx)?` if you need the service to start immediately
+
+### Fixed
+
+- Fixed incorrect Launchd restart policy implementation for `RestartPolicy::OnFailure` and `RestartPolicy::OnSuccess`:
+  - `OnFailure` now correctly uses `KeepAlive` dictionary with `SuccessfulExit=false` (restart on non-zero exit) instead of `KeepAlive=true` (always restart)
+  - `OnSuccess` now correctly uses `SuccessfulExit=true` (restart on zero exit) instead of `SuccessfulExit=false`
+
 ## [0.9.0] - 2025-11-22
 
 ### Changed

Cargo.toml

@@ -3,7 +3,7 @@ name = "service-manager"
 description = "Provides adapters to communicate with various operating system service managers"
 categories = ["config"]
 keywords = ["generator"]
-version = "0.9.0"
+version = "0.10.0"
 authors = ["Chip Senkbeil <chip@senkbeil.org>"]
 edition = "2021"
 homepage = "https://github.com/chipsenkbeil/service-manager-rs"

README.md

@@ -31,7 +31,7 @@ Add the following to your `Cargo.toml`:
 
 ```toml
 [dependencies]
-service-manager = "0.8"
+service-manager = "0.10"
 ```
 
 ## Examples

src/launchd.rs

@@ -138,7 +138,9 @@ impl ServiceManager for LaunchdServiceManager {
         )?;
 
         // Load the service.
-        // If "KeepAlive" is set to true, the service will immediately start.
+        // Services with KeepAlive configured will have Disabled=true set, preventing auto-start
+        // until explicitly started via start(). This provides cross-platform consistency where
+        // install() never auto-starts services.
         wrap_output(launchctl("load", plist_path.to_string_lossy().as_ref())?)?;
 
         Ok(())
@@ -153,8 +155,49 @@ impl ServiceManager for LaunchdServiceManager {
     }
 
     fn start(&self, ctx: ServiceStartCtx) -> io::Result<()> {
-        // To start services that do not have "KeepAlive" set to true
-        wrap_output(launchctl("start", ctx.label.to_qualified_name().as_str())?)?;
+        let qualified_name = ctx.label.to_qualified_name();
+        let plist_path = self.get_plist_path(qualified_name.clone());
+
+        if !plist_path.exists() {
+            return Err(io::Error::new(
+                io::ErrorKind::NotFound,
+                format!("Service {} is not installed", qualified_name),
+            ));
+        }
+
+        let plist_data = std::fs::read(&plist_path)?;
+        let mut plist: Value = plist::from_reader(std::io::Cursor::new(plist_data))
+            .map_err(|e| io::Error::new(io::ErrorKind::InvalidData, e))?;
+        let is_disabled = if let Value::Dictionary(ref dict) = plist {
+            dict.get("Disabled")
+                .and_then(|v| v.as_boolean())
+                .unwrap_or(false)
+        } else {
+            false
+        };
+
+        if is_disabled {
+            // Service was disable to prevent automatic start when KeepAlive is used. Now the
+            // disabled key will be removed. This makes the services behave in a more sane way like
+            // service managers on other platforms.
+            if let Value::Dictionary(ref mut dict) = plist {
+                dict.remove("Disabled");
+            }
+
+            let mut buffer = Vec::new();
+            plist
+                .to_writer_xml(&mut buffer)
+                .map_err(|e| io::Error::new(io::ErrorKind::Other, e))?;
+            utils::write_file(plist_path.as_path(), &buffer, PLIST_FILE_PERMISSIONS)?;
+
+            let _ = launchctl("unload", plist_path.to_string_lossy().as_ref());
+            wrap_output(launchctl("load", plist_path.to_string_lossy().as_ref())?)?;
+        } else {
+            // Service is not disabled, use regular start command
+            // This works for non-KeepAlive services
+            wrap_output(launchctl("start", qualified_name.as_str())?)?;
+        }
+
         Ok(())
     }
 
@@ -312,6 +355,8 @@ fn make_plist<'a>(
                 // Don't set KeepAlive
             }
             RestartPolicy::Always { delay_secs } => {
+                // KeepAlive *without* the SuccessfulExit construct will keep the service alive
+                // whether the process exits successfully or not.
                 dict.insert("KeepAlive".to_string(), Value::Boolean(true));
                 if delay_secs.is_some() {
                     log::warn!(
@@ -321,11 +366,12 @@ fn make_plist<'a>(
                 }
             }
             RestartPolicy::OnFailure { delay_secs } => {
-                dict.insert("KeepAlive".to_string(), Value::Boolean(true));
-                log::warn!(
-                    "Right now we don't have more granular restart support for Launchd so the service will always restart; using KeepAlive=true for service '{}'",
-                    label
-                );
+                // Create KeepAlive dictionary with SuccessfulExit=false
+                // This means: restart when exit is NOT successful
+                let mut keep_alive_dict = Dictionary::new();
+                keep_alive_dict.insert("SuccessfulExit".to_string(), Value::Boolean(false));
+                dict.insert("KeepAlive".to_string(), Value::Dictionary(keep_alive_dict));
+
                 if delay_secs.is_some() {
                     log::warn!(
                         "Launchd does not support restart delays; delay_secs will be ignored for service '{}'",
@@ -334,10 +380,10 @@ fn make_plist<'a>(
                 }
             }
             RestartPolicy::OnSuccess { delay_secs } => {
-                // Create KeepAlive dictionary with SuccessfulExit=false
-                // This means: restart when exit is successful (exit code 0)
+                // Create KeepAlive dictionary with SuccessfulExit=true
+                // This means: restart when exit is successful
                 let mut keep_alive_dict = Dictionary::new();
-                keep_alive_dict.insert("SuccessfulExit".to_string(), Value::Boolean(false));
+                keep_alive_dict.insert("SuccessfulExit".to_string(), Value::Boolean(true));
                 dict.insert("KeepAlive".to_string(), Value::Dictionary(keep_alive_dict));
 
                 if delay_secs.is_some() {
@@ -378,6 +424,19 @@ fn make_plist<'a>(
         dict.insert("RunAtLoad".to_string(), Value::Boolean(false));
     }
 
+    let has_keep_alive = if let Some(keep_alive) = config.keep_alive {
+        keep_alive
+    } else {
+        !matches!(restart_policy, RestartPolicy::Never)
+    };
+
+    // Set Disabled key to prevent the service automatically starting on load when KeepAlive is present.
+    // This provides consistent cross-platform behaviour which is much more intuitive.
+    // The service must be explicitly started via start().
+    if has_keep_alive {
+        dict.insert("Disabled".to_string(), Value::Boolean(true));
+    }
+
     let plist = Value::Dictionary(dict);
 
     let mut buffer = Vec::new();

system-tests/tests/runner.rs

@@ -108,7 +108,6 @@ pub fn run_test(manager: &TypedServiceManager, username: Option<String>) -> Opti
         wait();
     }
 
-    eprintln!("Checking status of service");
     assert!(
         matches!(
             manager
@@ -133,24 +132,26 @@ pub fn run_test(manager: &TypedServiceManager, username: Option<String>) -> Opti
             working_directory: None,
             environment: None,
             autostart: false,
-            restart_policy: RestartPolicy::Never,
+            restart_policy: RestartPolicy::OnFailure { delay_secs: None },
         })
         .unwrap();
 
     // Wait for service to be installed
     wait();
 
-    eprintln!("Checking status of service");
+    // On Launchd the status will still be reported as `NotInstalled` because the `Disabled` key
+    // was used when a restart policy was applied.
+    eprintln!("Checking status of service after install with autostart=false");
     assert!(
         matches!(
             manager
                 .status(ServiceStatusCtx {
                     label: service_label.clone(),
                 })
                 .unwrap(),
-            ServiceStatus::Stopped(_)
+            ServiceStatus::Stopped(_) | ServiceStatus::NotInstalled
         ),
-        "service should be stopped"
+        "service should be stopped or not installed after install with autostart=false and restart policy"
     );
 
     let is_user_specified =