From f07010c85018fea67c6f91c7d1ce4094c0863fed Mon Sep 17 00:00:00 2001 From: Jamie Pine <32987599+jamiepine@users.noreply.github.com> Date: Thu, 5 Nov 2026 12:06:39 +0900 Subject: [PATCH] Launch the daemon with the base data dir so a named instance nests once (#3171) Co-authored-by: capy-ai-staging[bot] <305915985+capy-ai-staging[bot]@users.noreply.github.com> --- apps/cli/src/domains/daemon/mod.rs | 31 +++++++++++---- apps/cli/src/domains/update/mod.rs | 9 +++-- apps/cli/src/domains/update/timer.rs | 59 +++++++++++++++++++++++----- apps/cli/src/main.rs | 11 +++--- crates/sd-client/src/daemon.rs | 39 ++++++++++++++++-- 5 files changed, 121 insertions(+), 28 deletions(-) diff --git a/apps/cli/src/domains/daemon/mod.rs b/apps/cli/src/domains/daemon/mod.rs index 5919ea3f2..c5bbdbd2d 100644 --- a/apps/cli/src/domains/daemon/mod.rs +++ b/apps/cli/src/domains/daemon/mod.rs @@ -12,16 +12,25 @@ pub enum DaemonCmd { Status, } -pub async fn run(data_dir: PathBuf, instance: Option, cmd: DaemonCmd) -> Result<()> { +pub async fn run( + data_dir: PathBuf, + base_data_dir: PathBuf, + instance: Option, + cmd: DaemonCmd, +) -> Result<()> { match cmd { - DaemonCmd::Install => install_launchd_service(data_dir, instance).await, + DaemonCmd::Install => install_launchd_service(data_dir, base_data_dir, instance).await, DaemonCmd::Uninstall => uninstall_launchd_service(instance).await, DaemonCmd::Status => check_launchd_status(instance).await, } } #[cfg(target_os = "macos")] -async fn install_launchd_service(data_dir: PathBuf, instance: Option) -> Result<()> { +async fn install_launchd_service( + data_dir: PathBuf, + base_data_dir: PathBuf, + instance: Option, +) -> Result<()> { use std::fs; use std::io::Write; @@ -64,7 +73,7 @@ async fn install_launchd_service(data_dir: PathBuf, instance: Option) -> let mut program_args = vec![ daemon_path.to_string_lossy().to_string(), "--data-dir".to_string(), - data_dir.to_string_lossy().to_string(), + base_data_dir.to_string_lossy().to_string(), ]; if let Some(ref inst) = instance { @@ -247,7 +256,11 @@ async fn check_launchd_status(instance: Option) -> Result<()> { } #[cfg(target_os = "linux")] -async fn install_launchd_service(data_dir: PathBuf, instance: Option) -> Result<()> { +async fn install_launchd_service( + _data_dir: PathBuf, + base_data_dir: PathBuf, + instance: Option, +) -> Result<()> { use std::fs; use std::io::Write; @@ -284,7 +297,7 @@ async fn install_launchd_service(data_dir: PathBuf, instance: Option) -> let mut exec_start = format!( "{} --data-dir {}", daemon_path.display(), - data_dir.display() + base_data_dir.display() ); if let Some(ref inst) = instance { exec_start.push_str(&format!(" --instance {}", inst)); @@ -466,7 +479,11 @@ async fn check_launchd_status(instance: Option) -> Result<()> { } #[cfg(not(any(target_os = "macos", target_os = "linux")))] -async fn install_launchd_service(_data_dir: PathBuf, _instance: Option) -> Result<()> { +async fn install_launchd_service( + _data_dir: PathBuf, + _base_data_dir: PathBuf, + _instance: Option, +) -> Result<()> { Err(anyhow::anyhow!( "Daemon auto-start is currently only supported on macOS and Linux." )) diff --git a/apps/cli/src/domains/update/mod.rs b/apps/cli/src/domains/update/mod.rs index 9b7f0f87f..9c1105505 100644 --- a/apps/cli/src/domains/update/mod.rs +++ b/apps/cli/src/domains/update/mod.rs @@ -137,6 +137,7 @@ impl UpdateState { pub async fn run( data_dir: PathBuf, + base_data_dir: PathBuf, instance: Option, socket_addr: String, options: UpdateOptions, @@ -152,6 +153,7 @@ pub async fn run( let mut available = None; let outcome = attempt( &data_dir, + &base_data_dir, instance, socket_addr, &config, @@ -208,6 +210,7 @@ fn record_attempt(data_dir: &Path, state: &UpdateState) -> Result<()> { async fn attempt( data_dir: &Path, + base_data_dir: &Path, instance: Option, socket_addr: String, config: &CliConfig, @@ -343,7 +346,7 @@ async fn attempt( if daemon_was_running { println!("Starting daemon..."); - restart_daemon(&client, &daemon_path, data_dir, instance.as_deref()).await?; + restart_daemon(&client, &daemon_path, base_data_dir, instance.as_deref()).await?; } println!(); @@ -428,7 +431,7 @@ fn format_age(age: chrono::Duration) -> String { async fn restart_daemon( client: &sd_client::CoreClient, daemon_path: &Path, - data_dir: &Path, + base_data_dir: &Path, instance: Option<&str>, ) -> Result<()> { #[cfg(target_os = "macos")] @@ -451,7 +454,7 @@ async fn restart_daemon( let launch = sd_client::DaemonLaunchConfig { daemon_path: daemon_path.to_path_buf(), - data_dir: data_dir.to_path_buf(), + base_data_dir: base_data_dir.to_path_buf(), instance: instance.map(str::to_string), no_default_sources: false, }; diff --git a/apps/cli/src/domains/update/timer.rs b/apps/cli/src/domains/update/timer.rs index 0ebbfa067..999de5de6 100644 --- a/apps/cli/src/domains/update/timer.rs +++ b/apps/cli/src/domains/update/timer.rs @@ -3,7 +3,7 @@ use anyhow::Result; use clap::Subcommand; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; #[derive(Subcommand, Debug)] pub enum TimerCmd { @@ -21,9 +21,14 @@ pub enum TimerCmd { Status, } -pub async fn run_timer(data_dir: PathBuf, instance: Option, cmd: TimerCmd) -> Result<()> { +pub async fn run_timer( + data_dir: PathBuf, + base_data_dir: PathBuf, + instance: Option, + cmd: TimerCmd, +) -> Result<()> { match cmd { - TimerCmd::InstallTimer { interval } => install(data_dir, instance, interval), + TimerCmd::InstallTimer { interval } => install(data_dir, base_data_dir, instance, interval), TimerCmd::UninstallTimer => uninstall(instance), TimerCmd::TimerStatus => print_status(instance.as_deref()), TimerCmd::Status => super::status(&data_dir, instance.as_deref()), @@ -43,13 +48,13 @@ fn label(instance: Option<&str>) -> String { /// The command the service runs. The absolute path of the running executable /// is used because launchd and systemd start services with a minimal PATH. -fn update_command(data_dir: &PathBuf, instance: Option<&str>) -> Result> { +fn update_command(base_data_dir: &Path, instance: Option<&str>) -> Result> { let exe = std::env::current_exe()?; let exe = exe.canonicalize().unwrap_or(exe); let mut args = vec![ exe.to_string_lossy().to_string(), "--data-dir".to_string(), - data_dir.to_string_lossy().to_string(), + base_data_dir.to_string_lossy().to_string(), ]; if let Some(inst) = instance { args.push("--instance".to_string()); @@ -95,7 +100,12 @@ pub fn loaded_daemon_agent(instance: Option<&str>) -> Option { } #[cfg(target_os = "macos")] -fn install(data_dir: PathBuf, instance: Option, interval: u32) -> Result<()> { +fn install( + data_dir: PathBuf, + base_data_dir: PathBuf, + instance: Option, + interval: u32, +) -> Result<()> { let label = label(instance.as_deref()); let plist_path = launch_agent_path(&label)?; std::fs::create_dir_all(plist_path.parent().unwrap())?; @@ -104,7 +114,7 @@ fn install(data_dir: PathBuf, instance: Option, interval: u32) -> Result std::fs::create_dir_all(&log_dir)?; let log_path = log_dir.join("update.log"); - let program_args = update_command(&data_dir, instance.as_deref())? + let program_args = update_command(&base_data_dir, instance.as_deref())? .iter() .map(|arg| format!("\t\t{}", arg)) .collect::>() @@ -215,13 +225,18 @@ fn unit_paths(instance: Option<&str>) -> Result<(PathBuf, String)> { } #[cfg(target_os = "linux")] -fn install(data_dir: PathBuf, instance: Option, interval: u32) -> Result<()> { +fn install( + _data_dir: PathBuf, + base_data_dir: PathBuf, + instance: Option, + interval: u32, +) -> Result<()> { let (dir, name) = unit_paths(instance.as_deref())?; std::fs::create_dir_all(&dir)?; // systemd splits ExecStart on whitespace and expands `%` specifiers, so // each argument is quoted and `%` doubled. - let exec_start = update_command(&data_dir, instance.as_deref())? + let exec_start = update_command(&base_data_dir, instance.as_deref())? .iter() .map(|arg| { format!( @@ -308,7 +323,12 @@ fn status(instance: Option) -> Result<()> { } #[cfg(not(any(target_os = "macos", target_os = "linux")))] -fn install(_data_dir: PathBuf, _instance: Option, _interval: u32) -> Result<()> { +fn install( + _data_dir: PathBuf, + _base_data_dir: PathBuf, + _instance: Option, + _interval: u32, +) -> Result<()> { Err(anyhow::anyhow!( "The update timer is only available on macOS (launchd) and Linux (systemd)" )) @@ -325,3 +345,22 @@ fn status(_instance: Option) -> Result<()> { println!("Update timer is not supported on this platform."); Ok(()) } + +#[cfg(test)] +mod tests { + use super::*; + + /// The timer re-runs the CLI, which resolves the instance itself, so it + /// has to be handed the base directory rather than the instance's own. + #[test] + fn the_update_command_carries_the_base_data_dir() { + let base = PathBuf::from("/data/spacedrive"); + let args = update_command(&base, Some("smoke")).unwrap(); + let at = args.iter().position(|arg| arg == "--data-dir").unwrap(); + assert_eq!(args[at + 1], base.to_string_lossy()); + assert_eq!( + args[args.len() - 4..], + ["--instance", "smoke", "update", "--yes"] + ); + } +} diff --git a/apps/cli/src/main.rs b/apps/cli/src/main.rs index 4f0f8c7c4..266e6a223 100644 --- a/apps/cli/src/main.rs +++ b/apps/cli/src/main.rs @@ -278,7 +278,7 @@ async fn main() -> Result<()> { // Same rule the daemon applies, so a client addressing an instance reads // the state that instance actually owns. let data_dir = - sd_core::infra::daemon::addr::instance_data_dir(base_data_dir, instance.as_deref()); + sd_core::infra::daemon::addr::instance_data_dir(base_data_dir.clone(), instance.as_deref()); let socket_addr = daemon_socket_addr(instance.as_deref()).to_string(); match cli.command { @@ -296,7 +296,7 @@ async fn main() -> Result<()> { .parent() .unwrap() .join(sd_client::daemon_binary_name()), - data_dir: data_dir.clone(), + base_data_dir: base_data_dir.clone(), instance: instance.clone(), no_default_sources, }; @@ -429,7 +429,7 @@ async fn main() -> Result<()> { .parent() .unwrap() .join(sd_client::daemon_binary_name()), - data_dir: data_dir.clone(), + base_data_dir: base_data_dir.clone(), instance: instance.clone(), no_default_sources, }; @@ -485,7 +485,7 @@ async fn main() -> Result<()> { } Commands::Daemon(cmd) => { // Daemon management doesn't need the client, handle directly - daemon::run(data_dir, instance, cmd).await?; + daemon::run(data_dir, base_data_dir, instance, cmd).await?; } Commands::Mcp => { // Serves without a daemon so an MCP client can list tools; each @@ -493,10 +493,11 @@ async fn main() -> Result<()> { domains::mcp::run(data_dir, socket_addr, cli.device).await?; } Commands::Update { force, yes, timer } => match timer { - Some(cmd) => update::run_timer(data_dir, instance, cmd).await?, + Some(cmd) => update::run_timer(data_dir, base_data_dir, instance, cmd).await?, None => { update::run( data_dir, + base_data_dir, instance, socket_addr, update::UpdateOptions { force, yes }, diff --git a/crates/sd-client/src/daemon.rs b/crates/sd-client/src/daemon.rs index 5fefb216e..fcb88c5db 100644 --- a/crates/sd-client/src/daemon.rs +++ b/crates/sd-client/src/daemon.rs @@ -19,8 +19,10 @@ pub fn daemon_binary_name() -> String { pub struct DaemonLaunchConfig { /// Path to the daemon executable. pub daemon_path: PathBuf, - /// Data directory passed to the daemon via `--data-dir`. - pub data_dir: PathBuf, + /// Base data directory passed via `--data-dir`. The daemon resolves + /// `instance` under it itself, so this is never the instance's own + /// directory; passing that nests the instance a second time. + pub base_data_dir: PathBuf, /// Instance name passed via `--instance`, if any. pub instance: Option, /// Passed as `--no-default-sources`: index only the sources added to the @@ -33,7 +35,7 @@ impl DaemonLaunchConfig { /// leaving stdio configuration to the caller. pub fn command(&self) -> std::io::Result { let mut command = Command::new(&self.daemon_path); - command.arg("--data-dir").arg(&self.data_dir); + command.arg("--data-dir").arg(&self.base_data_dir); if let Some(instance) = &self.instance { command.arg("--instance").arg(instance); } @@ -95,3 +97,34 @@ pub async fn ensure_daemon( responsive: is_daemon_running(client).await, }) } + +#[cfg(test)] +mod tests { + use super::*; + + use sd_core::infra::daemon::addr::instance_data_dir; + + /// A named instance's daemon resolves its directory from the base it is + /// given; handed the instance's own directory it nested a second + /// `instances/` and its state left the CLI's view. + #[test] + fn the_daemon_resolves_the_instance_once() { + let base = PathBuf::from("/data/spacedrive"); + let launch = DaemonLaunchConfig { + daemon_path: PathBuf::from("sd-daemon"), + base_data_dir: base.clone(), + instance: Some("smoke".to_string()), + no_default_sources: false, + }; + let command = launch.command().unwrap(); + let args: Vec<_> = command.get_args().collect(); + assert_eq!( + args, + ["--data-dir", "/data/spacedrive", "--instance", "smoke"].map(std::ffi::OsStr::new) + ); + assert_eq!( + instance_data_dir(PathBuf::from(args[1]), Some("smoke")), + base.join("instances").join("smoke") + ); + } +}