Provide More Intuitive Behaviour for Launchd Services (#40)

* fix: use correct logic for launchd restart policies

For the `OnSuccess` case, the logic was actually inverted, so the service was restarting when the
exit was *not* successful. This has now been corrected.

The logic we were previously using for the `OnSuccess` case has now been applied to the `OnFailure`
case, so we now support that scenario rather than emitting a warning saying we don't support it.

A comment clarifies that when the `SuccessfulExit` construct is *not* used, this is effectively the
`Always` policy.

* feat: prevent launchd services starting when keepalive is used

When the service definition includes the `KeepAlive` setting it causes services to automatically
start when `launchctl load` is used. This is quite unintuitive and is not how services behave on
other platforms.

We now make use of the `Disabled` setting to prevent the service starting automatically and make it
behave like service managers on other platforms. This is desirable when you want to add many
services first and not have them all start at the same time, which is the case for our application.
It also seems to be the behaviour most of our users expect and I personally think makes most sense.

* chore(release): bump to version 0.10.0

Author: jacderida

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 =