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

@@ -12,16 +12,25 @@ pub enum DaemonCmd {
 	Status,
 }
 
-pub async fn run(data_dir: PathBuf, instance: Option<String>, cmd: DaemonCmd) -> Result<()> {
+pub async fn run(
+	data_dir: PathBuf,
+	base_data_dir: PathBuf,
+	instance: Option<String>,
+	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<String>) -> Result<()> {
+async fn install_launchd_service(
+	data_dir: PathBuf,
+	base_data_dir: PathBuf,
+	instance: Option<String>,
+) -> Result<()> {
 	use std::fs;
 	use std::io::Write;
 
@@ -64,7 +73,7 @@ async fn install_launchd_service(data_dir: PathBuf, instance: Option<String>) ->
 	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<String>) -> Result<()> {
 }
 
 #[cfg(target_os = "linux")]
-async fn install_launchd_service(data_dir: PathBuf, instance: Option<String>) -> Result<()> {
+async fn install_launchd_service(
+	_data_dir: PathBuf,
+	base_data_dir: PathBuf,
+	instance: Option<String>,
+) -> Result<()> {
 	use std::fs;
 	use std::io::Write;
 
@@ -284,7 +297,7 @@ async fn install_launchd_service(data_dir: PathBuf, instance: Option<String>) ->
 	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<String>) -> Result<()> {
 }
 
 #[cfg(not(any(target_os = "macos", target_os = "linux")))]
-async fn install_launchd_service(_data_dir: PathBuf, _instance: Option<String>) -> Result<()> {
+async fn install_launchd_service(
+	_data_dir: PathBuf,
+	_base_data_dir: PathBuf,
+	_instance: Option<String>,
+) -> Result<()> {
 	Err(anyhow::anyhow!(
 		"Daemon auto-start is currently only supported on macOS and Linux."
 	))

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<String>,
 	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<String>,
 	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,
 	};

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<String>, cmd: TimerCmd) -> Result<()> {
+pub async fn run_timer(
+	data_dir: PathBuf,
+	base_data_dir: PathBuf,
+	instance: Option<String>,
+	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<Vec<String>> {
+fn update_command(base_data_dir: &Path, instance: Option<&str>) -> Result<Vec<String>> {
 	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<String> {
 }
 
 #[cfg(target_os = "macos")]
-fn install(data_dir: PathBuf, instance: Option<String>, interval: u32) -> Result<()> {
+fn install(
+	data_dir: PathBuf,
+	base_data_dir: PathBuf,
+	instance: Option<String>,
+	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<String>, 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<string>{}</string>", arg))
 		.collect::<Vec<_>>()
@@ -215,13 +225,18 @@ fn unit_paths(instance: Option<&str>) -> Result<(PathBuf, String)> {
 }
 
 #[cfg(target_os = "linux")]
-fn install(data_dir: PathBuf, instance: Option<String>, interval: u32) -> Result<()> {
+fn install(
+	_data_dir: PathBuf,
+	base_data_dir: PathBuf,
+	instance: Option<String>,
+	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<String>) -> Result<()> {
 }
 
 #[cfg(not(any(target_os = "macos", target_os = "linux")))]
-fn install(_data_dir: PathBuf, _instance: Option<String>, _interval: u32) -> Result<()> {
+fn install(
+	_data_dir: PathBuf,
+	_base_data_dir: PathBuf,
+	_instance: Option<String>,
+	_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<String>) -> 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"]
+		);
+	}
+}

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 },

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<String>,
 	/// 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<Command> {
 		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/<name>` 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")
+		);
+	}
+}