From a1cda2410b30c9bd67fa8085b35e3d1bcb87d13c Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Wed, 3 Jun 2026 10:19:44 -0500 Subject: [PATCH] fix(desktop): self-update rebuilds and relaunches cleanly on macOS The macOS DMG / in-app update could leave Hermes unable to relaunch: the staged updater rebuilt the desktop without managed Node on PATH ("npm not found"), never installed the rebuilt bundle over the running app, and could race itself on `git stash`. Child install scripts also inherited a deleted cwd from the .app bundle replaced during self-update. - update.rs: prepend $HERMES_HOME/node/bin + venv bin to the rebuild PATH; read --branch / --target-app from args; add a macOS "install" stage that dittos the rebuilt bundle over the target app, clears quarantine, and relaunches via `open` (rolling back on a failed swap); guard start_update with an AtomicBool so concurrent startUpdate() calls can't race git stash. - main.cjs: pass --branch and --target-app to the staged updater, and spawn it with HERMES_HOME + managed Node/venv on PATH and cwd=HERMES_HOME. - bootstrap.rs: launch the desktop via `open .app` on macOS instead of exec'ing Contents/MacOS/Hermes, avoiding cwd/quarantine issues post-rebuild. - powershell.rs: pin child install scripts to a stable cwd so they don't emit getcwd errors when the launching .app is replaced mid-install. - failure.tsx: in update mode show "Update didn't finish" / "Retry update" and retry via startUpdate() instead of re-running the installer bootstrap. --- .../src-tauri/src/bootstrap.rs | 85 ++++- .../src-tauri/src/powershell.rs | 25 ++ .../src-tauri/src/update.rs | 336 +++++++++++++++++- .../src/routes/failure.tsx | 17 +- apps/desktop/electron/main.cjs | 22 +- 5 files changed, 457 insertions(+), 28 deletions(-) diff --git a/apps/bootstrap-installer/src-tauri/src/bootstrap.rs b/apps/bootstrap-installer/src-tauri/src/bootstrap.rs index 9b5897168..4aef5ca84 100644 --- a/apps/bootstrap-installer/src-tauri/src/bootstrap.rs +++ b/apps/bootstrap-installer/src-tauri/src/bootstrap.rs @@ -179,9 +179,11 @@ pub async fn launch_hermes_desktop( tracing::info!(?exe_path, "launching Hermes desktop"); - // Detach from us — the installer is about to exit. - let mut cmd = tokio::process::Command::new(&exe_path); - cmd.current_dir(exe_path.parent().unwrap_or(&install_root)); + // Detach from us — the installer is about to exit. On macOS launch the + // bundle through LaunchServices instead of exec'ing Contents/MacOS/Hermes + // directly; this matches user double-click/open behavior and avoids cwd / + // quarantine oddities after a self-update rebuild. + let mut cmd = desktop_launch_command(&exe_path, &install_root); #[cfg(target_os = "windows")] { use std::os::windows::process::CommandExt; @@ -232,6 +234,24 @@ pub(crate) fn resolve_hermes_desktop_exe(install_root: &std::path::Path) -> Opti None } +pub(crate) fn resolve_hermes_desktop_app(install_root: &std::path::Path) -> Option { + let exe = resolve_hermes_desktop_exe(install_root)?; + #[cfg(target_os = "macos")] + { + // .../Hermes.app/Contents/MacOS/Hermes -> .../Hermes.app + let app = exe.parent()?.parent()?.parent()?.to_path_buf(); + if app.extension().and_then(|e| e.to_str()) == Some("app") && app.is_dir() { + return Some(app); + } + } + #[cfg(not(target_os = "macos"))] + { + return Some(exe); + } + #[allow(unreachable_code)] + None +} + /// True when a prior install completed (bootstrap-complete marker present) AND a /// launchable desktop app exists on disk. Used by the installer's launcher fast /// path so a bare re-open just opens Hermes instead of re-running setup. @@ -247,8 +267,7 @@ pub(crate) fn spawn_installed_desktop(install_root: &std::path::Path) -> std::io let exe = resolve_hermes_desktop_exe(install_root).ok_or_else(|| { std::io::Error::new(std::io::ErrorKind::NotFound, "no built Hermes desktop app") })?; - let mut cmd = std::process::Command::new(&exe); - cmd.current_dir(exe.parent().unwrap_or(install_root)); + let mut cmd = desktop_launch_command_std(&exe, install_root); #[cfg(target_os = "windows")] { use std::os::windows::process::CommandExt; @@ -261,6 +280,62 @@ pub(crate) fn spawn_installed_desktop(install_root: &std::path::Path) -> std::io cmd.spawn().map(|_child| ()) } +#[cfg(target_os = "macos")] +pub(crate) fn open_macos_app_detached(app_bundle: &std::path::Path) -> std::io::Result<()> { + let mut cmd = std::process::Command::new("/usr/bin/open"); + cmd.arg(app_bundle); + cmd.current_dir(crate::paths::hermes_home()); + cmd.spawn().map(|_child| ()) +} + +#[cfg(target_os = "macos")] +fn app_bundle_for_exe(exe: &std::path::Path) -> Option { + let app = exe.parent()?.parent()?.parent()?.to_path_buf(); + if app.extension().and_then(|e| e.to_str()) == Some("app") && app.is_dir() { + Some(app) + } else { + None + } +} + +fn desktop_launch_command( + exe_path: &std::path::Path, + install_root: &std::path::Path, +) -> tokio::process::Command { + #[cfg(target_os = "macos")] + { + if let Some(app_bundle) = app_bundle_for_exe(exe_path) { + let mut cmd = tokio::process::Command::new("/usr/bin/open"); + cmd.arg(app_bundle); + cmd.current_dir(crate::paths::hermes_home()); + return cmd; + } + } + + let mut cmd = tokio::process::Command::new(exe_path); + cmd.current_dir(exe_path.parent().unwrap_or(install_root)); + cmd +} + +fn desktop_launch_command_std( + exe_path: &std::path::Path, + install_root: &std::path::Path, +) -> std::process::Command { + #[cfg(target_os = "macos")] + { + if let Some(app_bundle) = app_bundle_for_exe(exe_path) { + let mut cmd = std::process::Command::new("/usr/bin/open"); + cmd.arg(app_bundle); + cmd.current_dir(crate::paths::hermes_home()); + return cmd; + } + } + + let mut cmd = std::process::Command::new(exe_path); + cmd.current_dir(exe_path.parent().unwrap_or(install_root)); + cmd +} + // --------------------------------------------------------------------------- // Bootstrap implementation // --------------------------------------------------------------------------- diff --git a/apps/bootstrap-installer/src-tauri/src/powershell.rs b/apps/bootstrap-installer/src-tauri/src/powershell.rs index c85d0ee55..b24e26959 100644 --- a/apps/bootstrap-installer/src-tauri/src/powershell.rs +++ b/apps/bootstrap-installer/src-tauri/src/powershell.rs @@ -45,6 +45,14 @@ pub async fn run_script( ) -> Result { let mut cmd = build_command(script_path, args); + // The installer can be launched from a .app bundle that is later replaced + // during self-update. Pin child scripts to a stable directory so bash/zsh + // never starts from a deleted cwd and emits getcwd/job-working-directory + // errors at the end of an otherwise successful install. + if let Some(cwd) = stable_script_cwd(script_path, hermes_home_override) { + cmd.current_dir(cwd); + } + if let Some(home) = hermes_home_override { cmd.env("HERMES_HOME", home); } @@ -146,6 +154,16 @@ pub async fn run_script( }) } +fn stable_script_cwd<'a>(script_path: &'a Path, hermes_home_override: Option<&'a str>) -> Option<&'a Path> { + if let Some(home) = hermes_home_override { + let path = Path::new(home); + if path.is_dir() { + return Some(path); + } + } + script_path.parent().filter(|p| p.is_dir()) +} + async fn recv_cancel(rx: &mut Option) { match rx { Some(r) => { @@ -264,4 +282,11 @@ info line assert!(parse_stage_result("just banner\n").is_none()); assert!(parse_manifest("just banner\n").is_none()); } + + #[test] + fn stable_script_cwd_prefers_existing_hermes_home() { + let script = Path::new("/tmp/install.sh"); + let cwd = stable_script_cwd(script, Some("/")); + assert_eq!(cwd, Some(Path::new("/"))); + } } diff --git a/apps/bootstrap-installer/src-tauri/src/update.rs b/apps/bootstrap-installer/src-tauri/src/update.rs index b7f147020..03bbb5bc0 100644 --- a/apps/bootstrap-installer/src-tauri/src/update.rs +++ b/apps/bootstrap-installer/src-tauri/src/update.rs @@ -19,8 +19,11 @@ //! the no-window creation flag — both already cfg-gated. Keep new logic //! OS-agnostic so the mac/linux port stays "fill in the paths". +use std::env; +use std::ffi::OsString; use std::path::{Path, PathBuf}; use std::process::Stdio; +use std::sync::atomic::{AtomicBool, Ordering}; use std::time::{Duration, Instant}; use anyhow::{anyhow, Result}; @@ -40,10 +43,27 @@ const UPDATE_EXIT_CONCURRENT: i32 = 2; const DESKTOP_EXIT_WAIT: Duration = Duration::from_secs(20); const DESKTOP_EXIT_POLL: Duration = Duration::from_millis(500); +/// Guards against concurrent update runs. The frontend kicks `startUpdate()` +/// from a mount effect, which can fire more than once (React strict-mode +/// double-invokes effects in dev; a window reload or stray re-init can do it +/// in prod). Two `run_update` tasks racing on `git stash` corrupt the working +/// tree — one stashes the changes the other then can't find. Exactly one task +/// may hold this flag at a time. +static UPDATE_RUNNING: AtomicBool = AtomicBool::new(false); + /// Frontend → Rust: kick off the update flow. Mirrors `start_bootstrap`'s /// fire-and-forget shape; progress arrives on the `bootstrap` event channel. #[tauri::command] pub async fn start_update(app: AppHandle) -> Result<(), String> { + // Re-entrancy guard (see UPDATE_RUNNING). compare_exchange lets exactly one + // caller flip false→true; any concurrent caller no-ops instead of spawning + // a second racing update. + if UPDATE_RUNNING + .compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst) + .is_err() + { + return Ok(()); + } tokio::spawn(async move { if let Err(err) = run_update(app.clone()).await { // run_update already emits a Failed event on the paths that matter; @@ -56,6 +76,7 @@ pub async fn start_update(app: AppHandle) -> Result<(), String> { }, ); } + UPDATE_RUNNING.store(false, Ordering::SeqCst); }); Ok(()) } @@ -63,6 +84,10 @@ pub async fn start_update(app: AppHandle) -> Result<(), String> { async fn run_update(app: AppHandle) -> Result<()> { let hermes_home = crate::paths::hermes_home(); let install_root = hermes_home.join("hermes-agent"); + let update_branch = update_branch_from_args(std::env::args().skip(1)) + .or_else(|| option_env_string("BUILD_PIN_BRANCH")) + .unwrap_or_else(|| "main".to_string()); + let target_app = target_app_from_args(std::env::args().skip(1)); let hermes = resolve_hermes(&install_root).ok_or_else(|| { let msg = format!( @@ -81,13 +106,18 @@ async fn run_update(app: AppHandle) -> Result<()> { })?; // Synthetic manifest so the existing progress UI renders our two stages. + let mut stages = vec![ + stage_info("update", "Updating Hermes"), + stage_info("rebuild", "Rebuilding the desktop app"), + ]; + if cfg!(target_os = "macos") && target_app.is_some() { + stages.push(stage_info("install", "Installing the updated app")); + } + emit( &app, BootstrapEvent::Manifest { - stages: vec![ - stage_info("update", "Updating Hermes"), - stage_info("rebuild", "Rebuilding the desktop app"), - ], + stages, protocol_version: None, }, ); @@ -107,12 +137,16 @@ async fn run_update(app: AppHandle) -> Result<()> { // reports "already up to date" against the wrong branch. The desktop // detected the update against this same branch, so we must update against // it too. - let pin_branch = option_env_string("BUILD_PIN_BRANCH"); - let mut update_args: Vec<&str> = vec!["update", "--yes", "--gateway"]; - if let Some(b) = pin_branch.as_deref() { - update_args.push("--branch"); - update_args.push(b); - } + emit_log( + &app, + Some("update"), + &format!("[update] updating against branch {update_branch}"), + ); + let child_env = update_child_env(&install_root); + let mut update_args: Vec = + vec!["update".into(), "--yes".into(), "--gateway".into()]; + update_args.push("--branch".into()); + update_args.push(update_branch); emit_stage(&app, "update", StageState::Running, None, None); let started = Instant::now(); @@ -121,6 +155,7 @@ async fn run_update(app: AppHandle) -> Result<()> { &hermes, &update_args, &install_root, + &child_env, Some("update"), ) .await?; @@ -182,11 +217,13 @@ async fn run_update(app: AppHandle) -> Result<()> { // repo-root deps with --workspaces=false). This is the rebuild it skips. emit_stage(&app, "rebuild", StageState::Running, None, None); let started = Instant::now(); + let rebuild_args: Vec = vec!["desktop".into(), "--build-only".into()]; let rebuild = run_streamed( &app, &hermes, - &["desktop", "--build-only"], + &rebuild_args, &install_root, + &child_env, Some("rebuild"), ) .await?; @@ -217,6 +254,43 @@ async fn run_update(app: AppHandle) -> Result<()> { } emit_stage(&app, "rebuild", StageState::Succeeded, Some(rebuild_ms), None); + let launch_target = if let Some(target_app) = target_app { + let started = Instant::now(); + emit_stage(&app, "install", StageState::Running, None, None); + match install_macos_app_update(&app, &install_root, &target_app).await { + Ok(installed_app) => { + emit_stage( + &app, + "install", + StageState::Succeeded, + Some(started.elapsed().as_millis() as u64), + None, + ); + Some(installed_app) + } + Err(err) => { + let msg = format!("{err:#}"); + emit_stage( + &app, + "install", + StageState::Failed, + Some(started.elapsed().as_millis() as u64), + Some(msg.clone()), + ); + emit( + &app, + BootstrapEvent::Failed { + stage: Some("install".into()), + error: msg.clone(), + }, + ); + return Err(anyhow!(msg)); + } + } + } else { + None + }; + // ---- done: signal complete, then launch the fresh desktop ------------ emit( &app, @@ -226,10 +300,16 @@ async fn run_update(app: AppHandle) -> Result<()> { }, ); - // Reuse the same detached-launch + app.exit(0) used post-install. - if let Err(err) = - crate::bootstrap::launch_hermes_desktop(app.clone(), install_root.to_string_lossy().into_owned()) - .await + if let Some(target_app) = launch_target { + if let Err(err) = launch_macos_app_and_exit(&app, &target_app).await { + emit_log( + &app, + None, + &format!("[update] could not auto-launch desktop: {err}. Launch Hermes manually."), + ); + } + } else if let Err(err) = + crate::bootstrap::launch_hermes_desktop(app.clone(), install_root.to_string_lossy().into_owned()).await { // Launch failed: don't hard-fail the update (it succeeded); surface a // log line so the success screen can still tell the user to launch @@ -289,8 +369,9 @@ fn is_locked(path: &Path) -> bool { async fn run_streamed( app: &AppHandle, program: &Path, - args: &[&str], + args: &[String], cwd: &Path, + envs: &[(String, OsString)], stage: Option<&str>, ) -> Result { let mut cmd = Command::new(program); @@ -299,6 +380,9 @@ async fn run_streamed( .stdin(Stdio::null()) .stdout(Stdio::piped()) .stderr(Stdio::piped()); + for (key, value) in envs { + cmd.env(key, value); + } #[cfg(target_os = "windows")] { @@ -378,6 +462,204 @@ fn resolve_hermes(install_root: &Path) -> Option { None } +fn update_child_env(install_root: &Path) -> Vec<(String, OsString)> { + let hermes_home = crate::paths::hermes_home(); + let mut envs = vec![( + "HERMES_HOME".to_string(), + hermes_home.as_os_str().to_os_string(), + )]; + if let Some(path) = path_with_prepended_entries(&[ + hermes_home.join("node").join("bin"), + venv_bin_dir(install_root), + ]) { + envs.push(("PATH".to_string(), path)); + } + envs +} + +fn venv_bin_dir(install_root: &Path) -> PathBuf { + if cfg!(target_os = "windows") { + install_root.join("venv").join("Scripts") + } else { + install_root.join("venv").join("bin") + } +} + +fn path_with_prepended_entries(entries: &[PathBuf]) -> Option { + let mut parts: Vec = entries.to_vec(); + if let Some(existing) = env::var_os("PATH") { + parts.extend(env::split_paths(&existing)); + } + env::join_paths(parts).ok() +} + +fn update_branch_from_args(args: I) -> Option +where + I: IntoIterator, + S: AsRef, +{ + arg_value_from_args(args, "--branch") + .map(|s| s.trim().to_string()) + .filter(|s| !s.is_empty()) +} + +fn target_app_from_args(args: I) -> Option +where + I: IntoIterator, + S: AsRef, +{ + arg_value_from_args(args, "--target-app") + .map(PathBuf::from) + .filter(|p| p.extension().and_then(|e| e.to_str()) == Some("app")) +} + +fn arg_value_from_args(args: I, name: &str) -> Option +where + I: IntoIterator, + S: AsRef, +{ + let mut iter = args.into_iter().map(|s| s.as_ref().to_string()).peekable(); + while let Some(arg) = iter.next() { + if arg == name { + return iter.next(); + } + if let Some(value) = arg.strip_prefix(&format!("{name}=")) { + return Some(value.to_string()); + } + } + None +} + +#[cfg(target_os = "macos")] +async fn install_macos_app_update( + app: &AppHandle, + install_root: &Path, + target_app: &Path, +) -> Result { + if target_app.extension().and_then(|e| e.to_str()) != Some("app") { + return Err(anyhow!( + "refusing to install update into non-app path: {}", + target_app.display() + )); + } + + let rebuilt_app = crate::bootstrap::resolve_hermes_desktop_app(install_root).ok_or_else(|| { + anyhow!( + "desktop rebuild succeeded but no Hermes.app was found under {}", + install_root.join("apps").join("desktop").join("release").display() + ) + })?; + + let same = match (rebuilt_app.canonicalize(), target_app.canonicalize()) { + (Ok(a), Ok(b)) => a == b, + _ => rebuilt_app == target_app, + }; + if same { + emit_log( + app, + Some("install"), + &format!( + "[update] rebuilt app is already the launch target: {}", + target_app.display() + ), + ); + return Ok(target_app.to_path_buf()); + } + + emit_log( + app, + Some("install"), + &format!( + "[update] installing rebuilt app {} -> {}", + rebuilt_app.display(), + target_app.display() + ), + ); + + if let Some(parent) = target_app.parent() { + tokio::fs::create_dir_all(parent).await?; + } + let tmp = PathBuf::from(format!("{}.hermes-update-new", target_app.display())); + let old = PathBuf::from(format!("{}.hermes-update-old", target_app.display())); + remove_dir_if_exists(&tmp).await; + remove_dir_if_exists(&old).await; + + let ditto = Command::new("/usr/bin/ditto") + .arg(&rebuilt_app) + .arg(&tmp) + .current_dir(crate::paths::hermes_home()) + .status() + .await + .map_err(|e| anyhow!("running ditto: {e}"))?; + if !ditto.success() { + return Err(anyhow!( + "ditto failed while copying updated app into {}", + tmp.display() + )); + } + + let moved_old = if target_app.exists() { + match tokio::fs::rename(target_app, &old).await { + Ok(()) => true, + Err(_) => { + remove_dir_if_exists(target_app).await; + false + } + } + } else { + false + }; + if let Err(err) = tokio::fs::rename(&tmp, target_app).await { + if moved_old { + let _ = tokio::fs::rename(&old, target_app).await; + } + return Err(anyhow!( + "installing updated app at {}: {err}", + target_app.display() + )); + } + remove_dir_if_exists(&old).await; + + let _ = Command::new("/usr/bin/xattr") + .arg("-dr") + .arg("com.apple.quarantine") + .arg(target_app) + .current_dir(crate::paths::hermes_home()) + .status() + .await; + + Ok(target_app.to_path_buf()) +} + +#[cfg(not(target_os = "macos"))] +async fn install_macos_app_update( + _app: &AppHandle, + _install_root: &Path, + target_app: &Path, +) -> Result { + Ok(target_app.to_path_buf()) +} + +async fn remove_dir_if_exists(path: &Path) { + if path.exists() { + let _ = tokio::fs::remove_dir_all(path).await; + } +} + +#[cfg(target_os = "macos")] +async fn launch_macos_app_and_exit(app: &AppHandle, target_app: &Path) -> Result<()> { + crate::bootstrap::open_macos_app_detached(target_app) + .map_err(|e| anyhow!("launching {}: {e}", target_app.display()))?; + tokio::time::sleep(std::time::Duration::from_millis(150)).await; + app.exit(0); + Ok(()) +} + +#[cfg(not(target_os = "macos"))] +async fn launch_macos_app_and_exit(_app: &AppHandle, _target_app: &Path) -> Result<()> { + Ok(()) +} + // --------------------------------------------------------------------------- // Event helpers — keep emit shape identical to bootstrap.rs so the UI is reused // --------------------------------------------------------------------------- @@ -459,4 +741,26 @@ mod tests { fn missing_file_is_not_locked() { assert!(!is_locked(Path::new("/nonexistent/does/not/exist/xyz"))); } + + #[test] + fn parses_update_branch_from_space_or_equals_args() { + assert_eq!( + update_branch_from_args(["--update", "--branch", "bb/test"]), + Some("bb/test".to_string()) + ); + assert_eq!( + update_branch_from_args(["--update", "--branch=main"]), + Some("main".to_string()) + ); + assert_eq!(update_branch_from_args(["--update"]), None); + } + + #[test] + fn parses_only_app_targets() { + assert_eq!( + target_app_from_args(["--update", "--target-app", "/Applications/Hermes.app"]), + Some(PathBuf::from("/Applications/Hermes.app")) + ); + assert_eq!(target_app_from_args(["--target-app", "/tmp/not-an-app"]), None); + } } diff --git a/apps/bootstrap-installer/src/routes/failure.tsx b/apps/bootstrap-installer/src/routes/failure.tsx index f781d8602..4125e0b5b 100644 --- a/apps/bootstrap-installer/src/routes/failure.tsx +++ b/apps/bootstrap-installer/src/routes/failure.tsx @@ -3,8 +3,10 @@ import { useStore } from '@nanostores/react' import { Button } from '../components/button' import { $logPath, + $mode, openLogDir, startInstall, + startUpdate, type BootstrapStateModel } from '../store' import { RefreshCw, FileText } from 'lucide-react' @@ -22,6 +24,8 @@ interface FailureProps { */ export default function Failure({ bootstrap }: FailureProps) { const logPath = useStore($logPath) + const mode = useStore($mode) + const isUpdate = mode === 'update' return (
@@ -37,24 +41,27 @@ export default function Failure({ bootstrap }: FailureProps) { } > - Install didn’t finish + {isUpdate ? 'Update didn\u2019t finish' : 'Install didn\u2019t finish'} - +

- {bootstrap.error ?? 'Something went wrong during installation.'} + {bootstrap.error ?? + (isUpdate + ? 'Something went wrong during the update.' + : 'Something went wrong during installation.')}