From 60649227715aaaa3312f2bb495ae69c4bff26507 Mon Sep 17 00:00:00 2001 From: Afonso Jorge Ramos Date: Thu, 17 Sep 2026 01:15:13 +0200 Subject: [PATCH 1/3] fix(daemon): reconcile macOS launch-agent startup Reuse an unchanged running launch agent and reload stopped or changed registrations before replacing an unmanaged daemon. Propagate launchctl failures so unavailable launchd is not mistaken for a missing job. --- rust/crates/spicetify/src/daemon/manager.rs | 280 +++++++++++++++++++- 1 file changed, 266 insertions(+), 14 deletions(-) diff --git a/rust/crates/spicetify/src/daemon/manager.rs b/rust/crates/spicetify/src/daemon/manager.rs index d54ca23063..32fe9fcf18 100644 --- a/rust/crates/spicetify/src/daemon/manager.rs +++ b/rust/crates/spicetify/src/daemon/manager.rs @@ -15,6 +15,9 @@ pub enum DaemonManagerError { #[error("systemctl error: {0}")] Systemctl(String), + #[error("launchctl error: {0}")] + Launchctl(String), + #[error("failed to spawn daemon: {0}")] Spawn(#[from] super::process::DaemonSpawnError), } @@ -168,14 +171,54 @@ fn registry_err(e: impl std::fmt::Display) -> DaemonManagerError { #[cfg(target_os = "macos")] #[derive(Debug, Clone, Copy)] pub struct MacosDaemonManager; +#[cfg(target_os = "macos")] +const LAUNCH_AGENT_LABEL: &str = "app.spicetify.daemon"; + +#[cfg(target_os = "macos")] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum LaunchAgentInstallAction { + Noop, + Load, + Reload, +} + +#[cfg(target_os = "macos")] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum LaunchAgentState { + Unloaded, + LoadedStopped, + Running, +} + +#[cfg(target_os = "macos")] +fn launch_agent_install_action( + existing_plist: Option<&str>, + desired_plist: &str, + state: LaunchAgentState, +) -> LaunchAgentInstallAction { + if state == LaunchAgentState::Running && existing_plist == Some(desired_plist) { + LaunchAgentInstallAction::Noop + } else if state == LaunchAgentState::Unloaded { + LaunchAgentInstallAction::Load + } else { + LaunchAgentInstallAction::Reload + } +} + +#[cfg(target_os = "macos")] +fn launch_agent_path(home: &std::path::Path) -> PathBuf { + home.join("Library/LaunchAgents").join(format!("{LAUNCH_AGENT_LABEL}.plist")) +} + #[cfg(target_os = "macos")] impl MacosDaemonManager { fn install() -> Result<(), DaemonManagerError> { - let plist_dir = home_dir()?.join("Library/LaunchAgents"); + let home = home_dir()?; + let plist_dir = home.join("Library/LaunchAgents"); std::fs::create_dir_all(&plist_dir)?; let exe = current_exe()?; let daemon_exe = super::daemon_binary_for(&exe); - let plist_path = plist_dir.join("app.spicetify.daemon.plist"); + let plist_path = launch_agent_path(&home); let plist = format!( r#" @@ -183,7 +226,7 @@ impl MacosDaemonManager { Label - app.spicetify.daemon + {} ProgramArguments {} @@ -194,18 +237,41 @@ impl MacosDaemonManager { "#, + LAUNCH_AGENT_LABEL, xml_escape(&daemon_exe.display().to_string()) ); - std::fs::write(&plist_path, plist)?; - run_launchctl(&["load", "-w"], &plist_path); - Ok(()) + let existing_plist = std::fs::read_to_string(&plist_path).ok(); + let plist_changed = existing_plist.as_deref() != Some(&plist); + let status = launch_agent_status()?; + let action = launch_agent_install_action(existing_plist.as_deref(), &plist, status); + + match action { + LaunchAgentInstallAction::Noop => Ok(()), + LaunchAgentInstallAction::Load => { + stop_unmanaged_daemon(); + if plist_changed { + std::fs::write(&plist_path, &plist)?; + } + run_launchctl(&["load", "-w"], Some(&plist_path)).map(|_| ()) + } + LaunchAgentInstallAction::Reload => { + let _ = run_launchctl(&["remove", LAUNCH_AGENT_LABEL], None)?; + stop_unmanaged_daemon(); + if plist_changed { + std::fs::write(&plist_path, &plist)?; + } + run_launchctl(&["load", "-w"], Some(&plist_path)).map(|_| ()) + } + } } fn uninstall() { if let Ok(home) = home_dir() { - let plist_path = home.join("Library/LaunchAgents/app.spicetify.daemon.plist"); + let plist_path = launch_agent_path(&home); if plist_path.exists() { - run_launchctl(&["unload", "-w"], &plist_path); + if let Err(e) = run_launchctl(&["unload", "-w"], Some(&plist_path)) { + tracing::warn!(error = %e, "failed to unload daemon auto-start"); + } if let Err(e) = std::fs::remove_file(&plist_path) && e.kind() != std::io::ErrorKind::NotFound { @@ -216,7 +282,7 @@ impl MacosDaemonManager { } fn is_installed() -> bool { - home_dir().is_ok_and(|h| h.join("Library/LaunchAgents/app.spicetify.daemon.plist").exists()) + home_dir().is_ok_and(|home| launch_agent_path(&home).exists()) } } @@ -308,14 +374,67 @@ fn home_dir() -> Result { } #[cfg(target_os = "macos")] -fn run_launchctl(args: &[&str], plist: &std::path::Path) { - match std::process::Command::new("launchctl").args(args).arg(plist).status() { - Ok(s) if !s.success() => tracing::warn!("launchctl exited with {s}"), - Err(e) => tracing::warn!(error = %e, "failed to run launchctl"), - _ => {} +fn launch_agent_status() -> Result { + let output = run_launchctl(&["list"], None)?; + Ok(parse_launch_agent_status(&String::from_utf8_lossy(&output))) +} + +#[cfg(target_os = "macos")] +fn parse_launch_agent_status(list: &str) -> LaunchAgentState { + for line in list.lines() { + let mut fields = line.split_whitespace(); + let pid = fields.next(); + let _ = fields.next(); + if fields.next() == Some(LAUNCH_AGENT_LABEL) { + return if pid.is_some_and(|pid| pid.parse::().is_ok_and(|pid| pid > 0)) { + LaunchAgentState::Running + } else { + LaunchAgentState::LoadedStopped + }; + } + } + LaunchAgentState::Unloaded +} + +#[cfg(target_os = "macos")] +fn stop_unmanaged_daemon() { + if super::is_daemon_running() { + super::shutdown_daemon(); } } +#[cfg(target_os = "macos")] +fn run_launchctl( + args: &[&str], + plist: Option<&std::path::Path>, +) -> Result, DaemonManagerError> { + let mut command = std::process::Command::new("launchctl"); + let _ = command.args(args); + if let Some(plist) = plist { + let _ = command.arg(plist); + } + + launchctl_output(args, command.output()?) +} + +#[cfg(target_os = "macos")] +fn launchctl_output( + args: &[&str], + output: std::process::Output, +) -> Result, DaemonManagerError> { + if output.status.success() { + return Ok(output.stdout); + } + + let stderr = String::from_utf8_lossy(&output.stderr).trim().to_owned(); + let detail = if stderr.is_empty() { + format!("{} exited with {}", args.join(" "), output.status) + } else { + format!("{}: {stderr}", args.join(" ")) + }; + Err(DaemonManagerError::Launchctl(detail)) +} + #[cfg(target_os = "linux")] fn is_systemd_available() -> bool { std::process::Command::new("systemctl") @@ -365,3 +484,136 @@ fn run_systemctl(args: &[&str]) -> Result<(), DaemonManagerError> { } } } + +#[cfg(all(test, target_os = "macos"))] +mod tests { + use super::{ + DaemonManagerError, LaunchAgentInstallAction, LaunchAgentState, + launch_agent_install_action, launchctl_output, parse_launch_agent_status, + }; + use std::os::unix::process::ExitStatusExt; + + #[test] + fn launch_agent_list_distinguishes_running_stopped_and_missing_jobs() { + assert_eq!( + parse_launch_agent_status("PID\tStatus\tLabel\n123\t0\tapp.spicetify.daemon\n"), + LaunchAgentState::Running + ); + assert_eq!( + parse_launch_agent_status("PID\tStatus\tLabel\n-\t1\tapp.spicetify.daemon\n"), + LaunchAgentState::LoadedStopped + ); + assert_eq!( + parse_launch_agent_status("PID\tStatus\tLabel\n123\t0\tapp.spicetify.daemon.other\n"), + LaunchAgentState::Unloaded + ); + } + + #[test] + fn launchctl_failures_report_command_and_stderr() { + let error = launchctl_output( + &["load", "-w"], + std::process::Output { + status: std::process::ExitStatus::from_raw(5 << 8), + stdout: Vec::new(), + stderr: b"Input/output error\n".to_vec(), + }, + ) + .expect_err("failed launchctl must not report success"); + assert!(matches!(error, DaemonManagerError::Launchctl(_))); + assert!(error.to_string().contains("load -w: Input/output error")); + } + + #[test] + fn launchctl_list_failure_is_not_an_unloaded_job() { + let error = launchctl_output( + &["list"], + std::process::Output { + status: std::process::ExitStatus::from_raw(1 << 8), + stdout: Vec::new(), + stderr: Vec::new(), + }, + ) + .expect_err("an unavailable launchd must not trigger daemon replacement"); + assert!(error.to_string().contains("list exited with")); + } + + #[test] + fn successful_launchctl_returns_the_job_list() { + let list = b"PID\tStatus\tLabel\n123\t0\tapp.spicetify.daemon\n"; + let output = launchctl_output( + &["list"], + std::process::Output { + status: std::process::ExitStatus::from_raw(0), + stdout: list.to_vec(), + stderr: Vec::new(), + }, + ) + .expect("launchctl succeeded"); + assert_eq!(output, list); + } + + #[test] + fn missing_plist_reloads_an_existing_registration() { + assert_eq!( + launch_agent_install_action(None, "desired plist", LaunchAgentState::Running), + LaunchAgentInstallAction::Reload + ); + } + + #[test] + fn first_install_loads_the_launch_agent() { + assert_eq!( + launch_agent_install_action(None, "desired plist", LaunchAgentState::Unloaded), + LaunchAgentInstallAction::Load + ); + } + + #[test] + fn unchanged_loaded_launch_agent_is_not_loaded_twice() { + assert_eq!( + launch_agent_install_action( + Some("desired plist"), + "desired plist", + LaunchAgentState::Running, + ), + LaunchAgentInstallAction::Noop + ); + } + + #[test] + fn unchanged_loaded_launch_agent_replaces_an_unmanaged_daemon() { + assert_eq!( + launch_agent_install_action( + Some("desired plist"), + "desired plist", + LaunchAgentState::LoadedStopped, + ), + LaunchAgentInstallAction::Reload + ); + } + + #[test] + fn unloaded_launch_agent_is_loaded_even_when_plist_is_unchanged() { + assert_eq!( + launch_agent_install_action( + Some("desired plist"), + "desired plist", + LaunchAgentState::Unloaded, + ), + LaunchAgentInstallAction::Load + ); + } + + #[test] + fn changed_loaded_launch_agent_is_reloaded() { + assert_eq!( + launch_agent_install_action( + Some("old plist"), + "desired plist", + LaunchAgentState::Running, + ), + LaunchAgentInstallAction::Reload + ); + } +} From 7297f2e74a41264e0f213a20ac921645fc1a6314 Mon Sep 17 00:00:00 2001 From: Afonso Jorge Ramos Date: Thu, 17 Sep 2026 01:22:26 +0200 Subject: [PATCH 2/3] fix(daemon): wait for complete shutdown before launchd handoff Wait for the previous daemon process and instance lock to exit before loading its launch agent. Keep the old lock inode open through shutdown, so a closed listener or unlinked pathname cannot falsely signal readiness. Block unmanaged startup when shutdown or launchd ownership is unresolved, while preserving the fallback for unsupported supervisors. --- rust/crates/spicetify/src/commands/apply.rs | 91 +++++++++++-- rust/crates/spicetify/src/daemon/manager.rs | 138 +++++++++++++++++++- 2 files changed, 215 insertions(+), 14 deletions(-) diff --git a/rust/crates/spicetify/src/commands/apply.rs b/rust/crates/spicetify/src/commands/apply.rs index 3f24fd55b4..d40278c36b 100644 --- a/rust/crates/spicetify/src/commands/apply.rs +++ b/rust/crates/spicetify/src/commands/apply.rs @@ -252,17 +252,33 @@ pub(crate) fn ensure_daemon(ctx: &AppContext) { // Registers auto-start with this CLI's own daemon binary. This runs after // any stop because stop unregisters; registering first left every // upgrade with the registration undone and the daemon back unsupervised. - if let Err(e) = super::daemon::install() { - tracing::warn!(error = %e, "could not enable the daemon at login"); + finish_daemon_startup( + super::daemon::install(), + || daemon_comes_up(std::time::Duration::from_secs(2)), + super::daemon::start, + ); +} + +fn finish_daemon_startup( + installation: Result<()>, + is_ready: impl FnOnce() -> bool, + start: impl FnOnce() -> Result<()>, +) { + if let Err(error) = installation { + tracing::warn!(%error, "could not enable the daemon at login"); + if error + .downcast_ref::() + .is_some_and(|error| !error.allows_unmanaged_fallback()) + { + tracing::warn!("daemon ownership is unresolved; skipping unmanaged startup"); + return; + } } - // A supervisor that starts what it registers (systemd, launchd) has the - // daemon up by now or within a moment; only spawn when nothing answers, - // so there is never a second, unsupervised copy beside the managed one. - if !daemon_comes_up(std::time::Duration::from_secs(2)) - && let Err(e) = super::daemon::start() + if !is_ready() + && let Err(error) = start() { - tracing::warn!(error = %e, "could not start the daemon"); + tracing::warn!(%error, "could not start the daemon"); } } @@ -914,3 +930,62 @@ mod tests { std::fs::remove_dir_all(root).expect("cleanup runtime fixture"); } } + +#[cfg(test)] +mod daemon_startup_tests { + use super::finish_daemon_startup; + use crate::daemon::DaemonManagerError; + use std::cell::Cell; + + #[test] + fn unresolved_supervisor_or_shutdown_never_starts_an_unmanaged_daemon() { + for error in [ + DaemonManagerError::Launchctl("cannot query launchd".to_owned()), + DaemonManagerError::ShutdownIncomplete("instance lock is still held".to_owned()), + ] { + let checked_ready = Cell::new(false); + let spawned = Cell::new(false); + finish_daemon_startup( + Err(error.into()), + || { + checked_ready.set(true); + false + }, + || { + spawned.set(true); + Ok(()) + }, + ); + assert!(!checked_ready.get()); + assert!(!spawned.get()); + } + } + + #[test] + fn unsupported_supervisor_still_allows_unmanaged_startup() { + let spawned = Cell::new(false); + finish_daemon_startup( + Err(DaemonManagerError::Unsupported.into()), + || false, + || { + spawned.set(true); + Ok(()) + }, + ); + assert!(spawned.get()); + } + + #[test] + fn ready_supervised_daemon_does_not_start_another_process() { + let spawned = Cell::new(false); + finish_daemon_startup( + Ok(()), + || true, + || { + spawned.set(true); + Ok(()) + }, + ); + assert!(!spawned.get()); + } +} diff --git a/rust/crates/spicetify/src/daemon/manager.rs b/rust/crates/spicetify/src/daemon/manager.rs index 32fe9fcf18..258ebce1a4 100644 --- a/rust/crates/spicetify/src/daemon/manager.rs +++ b/rust/crates/spicetify/src/daemon/manager.rs @@ -1,5 +1,5 @@ use std::path::PathBuf; -#[cfg(target_os = "linux")] +#[cfg(any(target_os = "linux", target_os = "macos"))] use std::time::Duration; use thiserror::Error; @@ -18,10 +18,19 @@ pub enum DaemonManagerError { #[error("launchctl error: {0}")] Launchctl(String), + #[error("daemon shutdown is incomplete: {0}")] + ShutdownIncomplete(String), + #[error("failed to spawn daemon: {0}")] Spawn(#[from] super::process::DaemonSpawnError), } +impl DaemonManagerError { + pub(crate) fn allows_unmanaged_fallback(&self) -> bool { + !matches!(self, Self::Launchctl(_) | Self::ShutdownIncomplete(_)) + } +} + #[derive(Debug, Clone, Copy)] pub enum DaemonManager { #[cfg(windows)] @@ -248,7 +257,7 @@ impl MacosDaemonManager { match action { LaunchAgentInstallAction::Noop => Ok(()), LaunchAgentInstallAction::Load => { - stop_unmanaged_daemon(); + stop_unmanaged_daemon()?; if plist_changed { std::fs::write(&plist_path, &plist)?; } @@ -256,7 +265,7 @@ impl MacosDaemonManager { } LaunchAgentInstallAction::Reload => { let _ = run_launchctl(&["remove", LAUNCH_AGENT_LABEL], None)?; - stop_unmanaged_daemon(); + stop_unmanaged_daemon()?; if plist_changed { std::fs::write(&plist_path, &plist)?; } @@ -397,10 +406,75 @@ fn parse_launch_agent_status(list: &str) -> LaunchAgentState { } #[cfg(target_os = "macos")] -fn stop_unmanaged_daemon() { - if super::is_daemon_running() { +fn stop_unmanaged_daemon() -> Result<(), DaemonManagerError> { + stop_unmanaged_daemon_and_wait() + .map_err(|error| DaemonManagerError::ShutdownIncomplete(error.to_string())) +} + +#[cfg(target_os = "macos")] +fn stop_unmanaged_daemon_and_wait() -> Result<(), DaemonManagerError> { + let path = crate::platform::default_spicetify_config_dir().join("spicetify-daemon.lock"); + // Keep the original inode open: the daemon unlinks this path before dropping its lock. + let lock_file = match std::fs::File::open(path) { + Ok(file) => Some(file), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => None, + Err(error) => return Err(error.into()), + }; + if super::is_daemon_running() || daemon_process_running()? { super::shutdown_daemon(); } + wait_for_daemon_exit(lock_file.as_ref(), Duration::from_secs(5), daemon_process_running) +} + +#[cfg(target_os = "macos")] +fn daemon_process_running() -> Result { + let status = std::process::Command::new("pgrep") + .args(["-x", super::daemon_binary_name()]) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .status()?; + match status.code() { + Some(0) => Ok(true), + Some(1) => Ok(false), + _ => { + Err(std::io::Error::other(format!("failed to inspect daemon process: {status}")).into()) + } + } +} + +#[cfg(target_os = "macos")] +fn wait_for_daemon_exit( + lock_file: Option<&std::fs::File>, + timeout: Duration, + mut process_running: impl FnMut() -> Result, +) -> Result<(), DaemonManagerError> { + let deadline = std::time::Instant::now() + timeout; + loop { + let lock_released = if let Some(file) = lock_file { + match fs4::FileExt::try_lock(file) { + Ok(()) => { + fs4::FileExt::unlock(file)?; + true + } + Err(fs4::TryLockError::WouldBlock) => false, + Err(fs4::TryLockError::Error(error)) => return Err(error.into()), + } + } else { + true + }; + if !process_running()? && lock_released { + return Ok(()); + } + let remaining = deadline.saturating_duration_since(std::time::Instant::now()); + if remaining.is_zero() { + return Err(std::io::Error::new( + std::io::ErrorKind::TimedOut, + "daemon did not exit before launch-agent startup; auto-start was not loaded", + ) + .into()); + } + std::thread::sleep(remaining.min(Duration::from_millis(50))); + } } #[cfg(target_os = "macos")] @@ -414,7 +488,10 @@ fn run_launchctl( let _ = command.arg(plist); } - launchctl_output(args, command.output()?) + let output = command + .output() + .map_err(|error| DaemonManagerError::Launchctl(format!("{}: {error}", args.join(" "))))?; + launchctl_output(args, output) } #[cfg(target_os = "macos")] @@ -490,9 +567,58 @@ mod tests { use super::{ DaemonManagerError, LaunchAgentInstallAction, LaunchAgentState, launch_agent_install_action, launchctl_output, parse_launch_agent_status, + wait_for_daemon_exit, }; use std::os::unix::process::ExitStatusExt; + #[test] + fn an_unlinked_instance_lock_still_blocks_loading_after_the_listener_closes() + -> anyhow::Result<()> { + let path = std::env::temp_dir().join(format!("spicetify-exit-lock-{}", std::process::id())); + let owner = std::fs::File::create(&path)?; + fs4::FileExt::try_lock(&owner)?; + let observer = std::fs::File::open(&path)?; + std::fs::remove_file(&path)?; + let result = wait_for_daemon_exit(Some(&observer), std::time::Duration::ZERO, || Ok(false)); + assert!( + matches!(result, Err(DaemonManagerError::Io(error)) if error.kind() == std::io::ErrorKind::TimedOut) + ); + drop(owner); + wait_for_daemon_exit(Some(&observer), std::time::Duration::ZERO, || Ok(false))?; + fs4::FileExt::try_lock(&observer)?; + fs4::FileExt::unlock(&observer)?; + Ok(()) + } + + #[test] + fn a_process_that_has_not_exited_blocks_loading_without_a_lock_file() { + let result = wait_for_daemon_exit(None, std::time::Duration::ZERO, || Ok(true)); + assert!( + matches!(result, Err(DaemonManagerError::Io(error)) if error.kind() == std::io::ErrorKind::TimedOut) + ); + } + + #[test] + fn daemon_exit_waits_until_the_process_is_gone() -> anyhow::Result<()> { + let mut probes = 0; + wait_for_daemon_exit(None, std::time::Duration::from_secs(1), || { + probes += 1; + Ok(probes < 2) + })?; + assert_eq!(probes, 2); + Ok(()) + } + + #[test] + fn process_inspection_failure_does_not_allow_loading() { + let result = wait_for_daemon_exit(None, std::time::Duration::ZERO, || { + Err(std::io::Error::from(std::io::ErrorKind::PermissionDenied).into()) + }); + assert!( + matches!(result, Err(DaemonManagerError::Io(error)) if error.kind() == std::io::ErrorKind::PermissionDenied) + ); + } + #[test] fn launch_agent_list_distinguishes_running_stopped_and_missing_jobs() { assert_eq!( From f541316e9f5c86349ac43cc7e9eaa00842d6b1f5 Mon Sep 17 00:00:00 2001 From: Afonso Jorge Ramos Date: Thu, 17 Sep 2026 01:25:55 +0200 Subject: [PATCH 3/3] fix(daemon): retain launchd ownership during delayed startup Do not start an unmanaged macOS daemon after launchd accepted registration but has not opened the listener within the readiness window. --- rust/crates/spicetify/src/commands/apply.rs | 28 ++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/rust/crates/spicetify/src/commands/apply.rs b/rust/crates/spicetify/src/commands/apply.rs index d40278c36b..f2223a5b84 100644 --- a/rust/crates/spicetify/src/commands/apply.rs +++ b/rust/crates/spicetify/src/commands/apply.rs @@ -264,6 +264,7 @@ fn finish_daemon_startup( is_ready: impl FnOnce() -> bool, start: impl FnOnce() -> Result<()>, ) { + let launchd_owns_startup = cfg!(target_os = "macos") && installation.is_ok(); if let Err(error) = installation { tracing::warn!(%error, "could not enable the daemon at login"); if error @@ -275,9 +276,16 @@ fn finish_daemon_startup( } } - if !is_ready() - && let Err(error) = start() - { + if is_ready() { + return; + } + if launchd_owns_startup { + tracing::warn!( + "launchd registered the daemon but startup is still pending; skipping unmanaged startup" + ); + return; + } + if let Err(error) = start() { tracing::warn!(%error, "could not start the daemon"); } } @@ -975,6 +983,20 @@ mod daemon_startup_tests { assert!(spawned.get()); } + #[test] + fn delayed_registered_supervisor_keeps_startup_ownership_on_macos() { + let spawned = Cell::new(false); + finish_daemon_startup( + Ok(()), + || false, + || { + spawned.set(true); + Ok(()) + }, + ); + assert_eq!(spawned.get(), !cfg!(target_os = "macos")); + } + #[test] fn ready_supervised_daemon_does_not_start_another_process() { let spawned = Cell::new(false);