fix: accept reformatted launchd service ownership

CHANGELOG.md

@@ -39,6 +39,7 @@ All notable changes to OCM are documented here.
 
 ### Fixed
 
+- Accept reformatted and binary macOS LaunchAgent plists for the same OCM store while still rejecting foreign or invalid owners. Thanks @TheAngryPit (#147, #149).
 - Keep npm-owned executable updates with npm, prevent temporary npx caches from
   owning background services, and preserve process identity during npm-launched
   gateway refreshes. Protect managed and symlinked installer destinations even

Cargo.toml

@@ -23,6 +23,7 @@ path = "src/main.rs"
 [dependencies]
 serde = { version = "1.0", features = ["derive"] }
 serde_json = { version = "1.0", features = ["preserve_order"] }
+plist = { version = "1.10", default-features = false }
 json5 = "1.3"
 serde_yaml = "0.9"
 sha2 = "0.11"

docs/USAGE.md

@@ -966,6 +966,11 @@ Background services:
 
 Windows service support is not implemented yet.
 
+On macOS, service ownership is read from `EnvironmentVariables.OCM_HOME` in
+the LaunchAgent plist. Plist-aware editors can reformat the definition or
+convert it to binary without changing ownership. A different, missing, or
+invalid owner still prevents OCM from replacing or controlling that service.
+
 ## Safety notes
 
 `ocm` keeps safety checks around destructive actions.

src/service/platform.rs

@@ -292,27 +292,47 @@ pub(crate) fn managed_service_owner_matches(
     ocm_home: &str,
     env: &BTreeMap<String, String>,
 ) -> Result<bool, String> {
-    let raw = fs::read_to_string(definition_path).map_err(|error| {
+    let raw = fs::read(definition_path).map_err(|error| {
         format!(
             "failed to read existing service definition {}: {error}",
             display_path(definition_path)
         )
     })?;
-    let owner_markers = match service_manager_kind(env) {
-        ServiceManagerKind::Launchd => vec![format!(
-            "<key>OCM_HOME</key>\n      <string>{}</string>",
-            plist_escape(ocm_home)
-        )],
-        ServiceManagerKind::SystemdUser => vec![
-            format!("Environment=\"OCM_HOME={}\"", systemd_escape(ocm_home)),
-            format!(
-                "Environment=\"OCM_HOME={}\"",
-                systemd_legacy_escape(ocm_home)
-            ),
-        ],
-        ServiceManagerKind::Unsupported => return Ok(true),
-    };
-    Ok(owner_markers.iter().any(|marker| raw.contains(marker)))
+    match service_manager_kind(env) {
+        ServiceManagerKind::Launchd => {
+            let value = plist::Value::from_reader(std::io::Cursor::new(&raw)).map_err(|error| {
+                format!(
+                    "failed to parse existing service definition {}: {error}",
+                    display_path(definition_path)
+                )
+            })?;
+            Ok(value
+                .as_dictionary()
+                .and_then(|dict| dict.get("EnvironmentVariables"))
+                .and_then(plist::Value::as_dictionary)
+                .and_then(|dict| dict.get("OCM_HOME"))
+                .and_then(plist::Value::as_string)
+                == Some(ocm_home))
+        }
+        ServiceManagerKind::SystemdUser => {
+            let raw = std::str::from_utf8(&raw).map_err(|error| {
+                format!(
+                    "failed to read existing service definition {}: {error}",
+                    display_path(definition_path)
+                )
+            })?;
+            Ok([
+                format!("Environment=\"OCM_HOME={}\"", systemd_escape(ocm_home)),
+                format!(
+                    "Environment=\"OCM_HOME={}\"",
+                    systemd_legacy_escape(ocm_home)
+                ),
+            ]
+            .iter()
+            .any(|marker| raw.contains(marker)))
+        }
+        ServiceManagerKind::Unsupported => Ok(true),
+    }
 }
 
 pub(crate) fn validate_managed_service_executable(
@@ -1725,6 +1745,70 @@ mod tests {
         fs::remove_dir_all(&root).unwrap();
     }
 
+    #[test]
+    fn launchd_service_owner_accepts_reformatted_plist() {
+        let root = tempfile::tempdir().unwrap();
+        let env = BTreeMap::from([(
+            "OCM_INTERNAL_SERVICE_MANAGER".to_string(),
+            "launchd".to_string(),
+        )]);
+        let definition = ManagedServiceDefinition {
+            label: OCM_SERVICE_LABEL.to_string(),
+            description: "owner fixture".to_string(),
+            definition_path: root.path().join("owner.plist"),
+            program_arguments: vec!["/bin/true".to_string()],
+            working_directory: root.path().to_path_buf(),
+            stdout_path: root.path().join("stdout.log"),
+            stderr_path: root.path().join("stderr.log"),
+            environment: BTreeMap::from([(
+                "OCM_HOME".to_string(),
+                "/tmp/store & data".to_string(),
+            )]),
+        };
+        write_managed_service_definition(&definition, &env).unwrap();
+        let original = fs::read_to_string(&definition.definition_path).unwrap();
+        let reformatted = original.replace("      <string>", "\t<string>");
+        assert_ne!(original, reformatted);
+        fs::write(&definition.definition_path, &reformatted).unwrap();
+        super::validate_managed_service_owner(&definition, &env).unwrap();
+        assert_eq!(
+            fs::read_to_string(&definition.definition_path).unwrap(),
+            reformatted
+        );
+        let value = plist::Value::from_reader_xml(reformatted.as_bytes()).unwrap();
+        value.to_file_binary(&definition.definition_path).unwrap();
+        super::validate_managed_service_owner(&definition, &env).unwrap();
+
+        // Only EnvironmentVariables.OCM_HOME owns the service. A matching string
+        // elsewhere (including comments) must never authorize another store.
+        for body in [
+            "<dict><key>EnvironmentVariables</key><dict><key>OCM_HOME</key><string>/tmp/other</string></dict></dict>",
+            "<dict><key>OCM_HOME</key><string>/tmp/store &amp; data</string></dict>",
+            "<dict><key>EnvironmentVariables</key><dict><key>OCM_HOME</key><integer>7</integer></dict></dict>",
+            "<dict><key>EnvironmentVariables</key><string>/tmp/store &amp; data</string></dict>",
+            "<dict><!-- <key>OCM_HOME</key>\n      <string>/tmp/store &amp; data</string> --></dict>",
+            "<dict/>",
+        ] {
+            let fixture = format!("<?xml version=\"1.0\"?><plist version=\"1.0\">{body}</plist>");
+            fs::write(&definition.definition_path, &fixture).unwrap();
+            let error = write_managed_service_definition(&definition, &env).unwrap_err();
+            assert!(
+                error.contains("already bound to a different OCM_HOME"),
+                "{error}"
+            );
+            assert_eq!(
+                fs::read_to_string(&definition.definition_path).unwrap(),
+                fixture
+            );
+        }
+        fs::write(&definition.definition_path, "not a plist").unwrap();
+        let error = super::validate_managed_service_owner(&definition, &env).unwrap_err();
+        assert!(
+            error.contains("failed to parse existing service definition"),
+            "{error}"
+        );
+    }
+
     #[test]
     fn stable_service_identity_rejects_a_different_store_owner() {
         let unique = SystemTime::now()

tests/daemon_runtime_tests.rs

@@ -1794,6 +1794,7 @@ fn daemon_defers_a_saved_service_start_until_source_watch_releases_the_env() {
     let source_was_started = launcher_marker.exists();
     drop(source_watch);
     let after = wait_for_runtime_children(&runtime_path, 2, Some("demo"), Duration::from_secs(10));
+    let source_was_resumed = wait_for_file(&launcher_marker, Duration::from_secs(5));
     stop_process(&mut daemon);
 
     assert!(during.is_some(), "unwatched sibling should remain runnable");
@@ -1803,7 +1804,10 @@ fn daemon_defers_a_saved_service_start_until_source_watch_releases_the_env() {
         after.is_some(),
         "service did not resume after watch released"
     );
-    assert!(launcher_marker.exists());
+    assert!(
+        source_was_resumed,
+        "resumed child did not write its startup marker"
+    );
 }
 
 #[test]

tests/service_command_tests.rs

@@ -715,6 +715,53 @@ fn service_stop_keeps_the_daemon_while_a_sibling_env_is_running() {
     assert!(managed_service_definition_path(&env, &cwd, "ocm").exists());
 }
 
+#[test]
+fn service_start_accepts_reformatted_same_store_plist() {
+    let root = TestDir::new("service-start-reformatted-owner");
+    let cwd = root.child("workspace");
+    fs::create_dir_all(&cwd).unwrap();
+    let env = launchd_env(&root);
+    setup_launcher_env(&cwd, &env);
+    let started = run_ocm(&cwd, &env, &["service", "start", "demo"]);
+    assert!(started.status.success(), "{}", stderr(&started));
+
+    let path = managed_service_definition_path(&env, &cwd, "ocm");
+    let original = fs::read_to_string(&path).unwrap();
+    let reformatted = original.replace("      <string>", "\t<string>");
+    assert_ne!(original, reformatted);
+    fs::write(&path, &reformatted).unwrap();
+    #[cfg(target_os = "macos")]
+    {
+        let edited = Command::new("/usr/libexec/PlistBuddy")
+            .args(["-c", "Set :Comment reformatted-owner-fixture"])
+            .arg(&path)
+            .output()
+            .unwrap();
+        assert!(edited.status.success(), "{}", stderr(&edited));
+    }
+    let restarted = run_ocm(&cwd, &env, &["service", "start", "demo"]);
+    assert!(restarted.status.success(), "{}", stderr(&restarted));
+
+    let mut foreign_env = env.clone();
+    foreign_env.insert(
+        "OCM_HOME".to_string(),
+        path_string(&root.child("foreign-store")),
+    );
+    setup_launcher_env(&cwd, &foreign_env);
+    let before = fs::read(&path).unwrap();
+    let rejected = run_ocm(&cwd, &foreign_env, &["service", "start", "demo"]);
+    assert!(!rejected.status.success());
+    assert!(
+        stderr(&rejected).contains("already bound to a different OCM_HOME"),
+        "{}",
+        stderr(&rejected)
+    );
+    assert_eq!(fs::read(&path).unwrap(), before);
+
+    let stopped = run_ocm(&cwd, &env, &["service", "stop", "demo"]);
+    assert!(stopped.status.success(), "{}", stderr(&stopped));
+}
+
 #[test]
 fn service_start_rejects_a_daemon_owned_by_another_store() {
     let root = TestDir::new("service-start-other-store-owner");

tests/support/mod.rs

@@ -411,7 +411,7 @@ pub fn npm_fixture(package: &Path) -> Option<(PathBuf, PathBuf)> {
     );
     let binary = payload.join(format!("vendor/{target}/bin/ocm"));
     fs::create_dir_all(binary.parent().unwrap()).unwrap();
-    fs::hard_link(ocm_test_binary_path(), &binary).unwrap();
+    fs::copy(ocm_test_binary_path(), &binary).unwrap();
     let entrypoint = package.join("bin/ocm.cjs");
     write_executable_script(&entrypoint, include_str!("../../npm/ocm.cjs"));
     Some((entrypoint, binary))

tests/upgrade_command_tests.rs

@@ -1184,15 +1184,8 @@ fn upgrade_rolls_back_when_gateway_rpc_is_not_ready() {
     let cwd = root.child("workspace");
     fs::create_dir_all(&cwd).unwrap();
 
-    let health_server =
-        TestHttpServer::serve_bytes_times("/health", "application/json", br#"{"ok":true}"#, 8);
-    let health_url = health_server.url();
-    let health_port = health_url
-        .split(':')
-        .nth(2)
-        .and_then(|value| value.split('/').next())
-        .and_then(|value| value.parse::<u32>().ok())
-        .unwrap();
+    let (health_port, health_requests, health_stop, health_handle) =
+        spawn_converging_health_server();
 
     let old_tarball =
         openclaw_package_tarball(&recording_openclaw_script("2026.3.24"), "2026.3.24");
@@ -1356,6 +1349,7 @@ fn upgrade_rolls_back_when_gateway_rpc_is_not_ready() {
     let upgrade = run_ocm(&cwd, &env, &["upgrade", "demo"]);
     observer_done.store(true, Ordering::Relaxed);
     let (stop_count, start_count, target_entered_backoff) = restart_observer.join().unwrap();
+    stop_converging_health_server(health_port, &health_stop, health_handle);
 
     assert!(!upgrade.status.success(), "{}", stdout(&upgrade));
     let output = stdout(&upgrade);
@@ -1365,7 +1359,7 @@ fn upgrade_rolls_back_when_gateway_rpc_is_not_ready() {
         output.contains("post-upgrade gateway readiness failed: gateway RPC is not ready"),
         "{output}"
     );
-    assert!(!health_server.requests().is_empty());
+    assert!(health_requests.load(Ordering::SeqCst) > 0);
     assert!(target_entered_backoff);
     assert!(stop_count >= 2, "stop_count={stop_count}");
     assert!(start_count >= 2, "start_count={start_count}");