diff --git a/crates/hephd/src/selfupdate.rs b/crates/hephd/src/selfupdate.rs index fcaa181..6a3300b 100644 --- a/crates/hephd/src/selfupdate.rs +++ b/crates/hephd/src/selfupdate.rs @@ -182,14 +182,39 @@ impl Installer for CargoInstaller { } } -/// Apply a detected update: install the binaries for `tag`. The blocking install -/// runs on the blocking pool so it never stalls the async runtime. (Restarting -/// onto the new binary is layered on by the self-restart step.) -pub async fn apply_update(installer: Arc, tag: &str) -> Result<()> { - let tag = tag.to_string(); - tokio::task::spawn_blocking(move || installer.install(&tag)) +/// Hands off to the freshly-installed binary. Injectable so the apply path is +/// testable without actually exiting the test process (real: [`ProcessRestarter`]). +pub trait Restarter: Send + Sync + 'static { + /// Restart onto the new binary. The production impl does not return. + fn restart(&self) -> Result<()>; +} + +/// The production restarter: exit cleanly so the OS service manager (launchd +/// `KeepAlive` / systemd `Restart=always`) respawns the new binary. In-flight +/// RPC connections simply drop; clients reconnect (the nvim plugin already does). +pub struct ProcessRestarter; + +impl Restarter for ProcessRestarter { + fn restart(&self) -> Result<()> { + tracing::info!("self-update: exiting to let the service manager start the new binary"); + std::process::exit(0); + } +} + +/// Apply a detected update: install the binaries for `tag`, then restart onto +/// them. The blocking install runs on the blocking pool so it never stalls the +/// async runtime; the restart only happens if the install succeeded. +pub async fn apply_update( + installer: Arc, + restarter: Arc, + tag: &str, +) -> Result<()> { + let owned = tag.to_string(); + tokio::task::spawn_blocking(move || installer.install(&owned)) .await - .context("self-update install task panicked")? + .context("self-update install task panicked")??; + tracing::info!(%tag, "self-update: installed; restarting into the new binary"); + restarter.restart() } /// The background poll loop: tick on `interval`, check for a newer release, and @@ -197,6 +222,7 @@ pub async fn apply_update(installer: Arc, tag: &str) -> Result<() pub async fn run_poll_loop( source: S, installer: Arc, + restarter: Arc, interval: Duration, current: &'static str, ) { @@ -206,9 +232,10 @@ pub async fn run_poll_loop( match check_release(&source, current).await { CheckOutcome::UpdateAvailable(tag) => { tracing::info!(%tag, current, "self-update: newer release available, applying"); - match apply_update(installer.clone(), &tag).await { - Ok(()) => tracing::info!(%tag, "self-update: installed new binaries"), - Err(e) => tracing::error!("self-update: install failed for {tag}: {e}"), + // On success the restarter exits the process, so this only + // returns on failure — log it and keep polling. + if let Err(e) = apply_update(installer.clone(), restarter.clone(), &tag).await { + tracing::error!("self-update: failed for {tag}: {e}"); } } CheckOutcome::UpToDate => tracing::debug!(current, "self-update: up to date"), @@ -245,22 +272,47 @@ mod tests { } } - #[tokio::test] - async fn apply_update_invokes_the_installer_with_the_tag() { - let inst = Arc::new(FakeInstaller::default()); - apply_update(inst.clone(), "v1.0.4").await.unwrap(); - assert_eq!(*inst.installed.lock().unwrap(), vec!["v1.0.4".to_string()]); + /// Records whether a restart was requested (instead of exiting the process). + #[derive(Default)] + struct FakeRestarter { + restarted: std::sync::Mutex, + } + impl Restarter for FakeRestarter { + fn restart(&self) -> Result<()> { + *self.restarted.lock().unwrap() = true; + Ok(()) + } } #[tokio::test] - async fn apply_update_propagates_install_failure() { + async fn apply_update_installs_then_restarts_on_success() { + let inst = Arc::new(FakeInstaller::default()); + let restart = Arc::new(FakeRestarter::default()); + apply_update(inst.clone(), restart.clone(), "v1.0.4") + .await + .unwrap(); + assert_eq!(*inst.installed.lock().unwrap(), vec!["v1.0.4".to_string()]); + assert!( + *restart.restarted.lock().unwrap(), + "should restart on success" + ); + } + + #[tokio::test] + async fn apply_update_does_not_restart_when_install_fails() { let inst = Arc::new(FakeInstaller { fail: true, ..Default::default() }); - assert!(apply_update(inst.clone(), "v1.0.4").await.is_err()); - // It still attempted the install for the right tag. + let restart = Arc::new(FakeRestarter::default()); + assert!(apply_update(inst.clone(), restart.clone(), "v1.0.4") + .await + .is_err()); assert_eq!(*inst.installed.lock().unwrap(), vec!["v1.0.4".to_string()]); + assert!( + !*restart.restarted.lock().unwrap(), + "must NOT restart after a failed install" + ); } #[tokio::test] diff --git a/crates/hephd/src/server.rs b/crates/hephd/src/server.rs index 757a935..16e7494 100644 --- a/crates/hephd/src/server.rs +++ b/crates/hephd/src/server.rs @@ -133,13 +133,22 @@ impl Daemon { let source = selfupdate::ForgeReleaseSource::new(self.ctx.http.clone()); let installer: std::sync::Arc = std::sync::Arc::new(selfupdate::CargoInstaller); + let restarter: std::sync::Arc = + std::sync::Arc::new(selfupdate::ProcessRestarter); tracing::info!( interval_secs = cfg.interval.as_secs(), current = heph_core::VERSION, "self-update enabled" ); tokio::spawn(async move { - selfupdate::run_poll_loop(source, installer, cfg.interval, heph_core::VERSION).await; + selfupdate::run_poll_loop( + source, + installer, + restarter, + cfg.interval, + heph_core::VERSION, + ) + .await; }); }