diff --git a/crates/bsk-cli/skill/SKILL.md b/crates/bsk-cli/skill/SKILL.md index 9a1b6a8..b88fffa 100644 --- a/crates/bsk-cli/skill/SKILL.md +++ b/crates/bsk-cli/skill/SKILL.md @@ -108,6 +108,8 @@ Do **not** call `bsk get-html` or `bsk screenshot` first just to inspect a page. | `--quiet` | Suppress informational stderr | | `-v` / `-vv` | More verbose logging | +Auto-update is **on by default** — the background daemon upgrades `bsk` itself when a new release is available (postponed while a session is active). Set `BSK_AUTO_UPDATE=off` to disable it and upgrade manually with `bsk update`. + Command-specific flags (timeouts, `--tab-id`, `--wait-until`, …): **`bsk --help`** ## CLI command reference (one line each) diff --git a/crates/bsk-cli/src/cli/update.rs b/crates/bsk-cli/src/cli/update.rs index fc93ba6..d827a3e 100644 --- a/crates/bsk-cli/src/cli/update.rs +++ b/crates/bsk-cli/src/cli/update.rs @@ -35,6 +35,11 @@ pub(crate) const UPDATE_CHECK_INTERVAL: Duration = Duration::from_secs(30 * 60); pub(crate) const DAEMON_REFRESH_WINDOW: Duration = Duration::from_secs(UPDATE_CHECK_INTERVAL.as_secs() * 5 / 6); +/// Environment variable that switches daemon-side auto-upgrade off. +/// Unset, or any value other than `off` (compared case-insensitively, +/// surrounding whitespace ignored), keeps auto-upgrade on. +pub(crate) const AUTO_UPDATE_ENV: &str = "BSK_AUTO_UPDATE"; + #[derive(Debug, Clone)] pub struct UpdateManifest { pub version: Version, @@ -332,16 +337,7 @@ fn install_candidate_with_client( restart_daemon: bool, client: &reqwest::blocking::Client, ) -> Result { - let expected_sha = candidate.asset.sha256.as_deref().with_context(|| { - format!( - "release {} does not include a sha256 checksum; cannot safely auto-update", - candidate.tag - ) - })?; - let archive = fetch_bytes_with_client(client, &candidate.asset.url)?; - verify_sha256(&archive, expected_sha)?; - let kind = ArchiveKind::from_url(&candidate.asset.url)?; - let binary = extract_bsk_binary(&archive, kind)?; + let binary = download_candidate_binary(candidate, client)?; let target = std::env::current_exe().context("locate current bsk executable")?; let daemon_was_running = restart_daemon && crate::daemon::info::read_valid()?.is_some(); @@ -359,6 +355,90 @@ fn install_candidate_with_client( Ok(action) } +/// Download the candidate's release archive, verify its sha256 checksum, +/// and extract the `bsk` binary. Archives without a checksum are +/// refused — auto-update never installs unverifiable bytes. +pub(crate) fn download_candidate_binary( + candidate: &UpdateCandidate, + client: &reqwest::blocking::Client, +) -> Result> { + let expected_sha = candidate.asset.sha256.as_deref().with_context(|| { + format!( + "release {} does not include a sha256 checksum; cannot safely auto-update", + candidate.tag + ) + })?; + let archive = fetch_bytes_with_client(client, &candidate.asset.url)?; + verify_sha256(&archive, expected_sha)?; + let kind = ArchiveKind::from_url(&candidate.asset.url)?; + extract_bsk_binary(&archive, kind) +} + +/// Daemon-side install: download, verify, and replace the executable at +/// `target` (on Windows: stage the replacement next to it). Unlike the +/// CLI path this never stops or starts the daemon — the daemon drives +/// its own restart once the binary is replaced. +/// +/// `target` must be captured *before* any replacement happens: on Linux +/// `std::env::current_exe` starts returning a ` (deleted)`-suffixed +/// path once the running binary has been replaced on disk. +pub(crate) fn self_install_candidate( + candidate: &UpdateCandidate, + target: &Path, +) -> Result { + let client = update_http_client(ARCHIVE_FETCH_TIMEOUT)?; + let binary = download_candidate_binary(candidate, &client)?; + replace_binary_at_path(target, &binary) +} + +/// Outcome of one daemon auto-update step (see [`auto_update_step`]). +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) enum AutoUpdateOutcome { + /// The manifest names no newer version. + UpToDate, + /// A newer version exists but auto-update is switched off — the + /// refreshed cache still feeds the CLI hint. + Disabled { latest: String }, + /// Live agent sessions block replacing the binary; the next tick + /// retries. + PostponedSessions { latest: String, sessions: usize }, + /// The new binary replaced the old one; the daemon should now + /// restart into it. + Replaced { latest: String }, + /// Windows staged the replacement next to the running binary; a + /// daemon/terminal restart finishes it. + Staged { latest: String }, +} + +/// The daemon's auto-update step for one tick: decide whether the +/// fetched `candidate` may be installed (auto-update on, no live agent +/// sessions) and, only then, run `install`. The installer is injectable +/// so tests never touch a real binary. +pub(crate) fn auto_update_step( + candidate: Option<&UpdateCandidate>, + auto_update_enabled: bool, + active_sessions: usize, + install: impl FnOnce(&UpdateCandidate) -> Result, +) -> Result { + let Some(candidate) = candidate else { + return Ok(AutoUpdateOutcome::UpToDate); + }; + let latest = candidate.latest.to_string(); + if !auto_update_enabled { + return Ok(AutoUpdateOutcome::Disabled { latest }); + } + if active_sessions > 0 { + return Ok(AutoUpdateOutcome::PostponedSessions { + latest, + sessions: active_sessions, + }); + } + Ok(match install(candidate)? { + InstallAction::Replaced => AutoUpdateOutcome::Replaced { latest }, + InstallAction::Staged => AutoUpdateOutcome::Staged { latest }, + }) +} + pub fn verify_sha256(bytes: &[u8], expected_hex: &str) -> Result<()> { let actual = hex_sha256(bytes); let expected = expected_hex.trim().to_ascii_lowercase(); @@ -372,28 +452,51 @@ fn manifest_url() -> String { std::env::var("BSK_UPDATE_MANIFEST_URL").unwrap_or_else(|_| DEFAULT_MANIFEST_URL.to_string()) } +/// Whether daemon-side auto-upgrade is enabled. On by default; only +/// [`AUTO_UPDATE_ENV`]`=off` disables it. This is the single place the +/// switch is interpreted — the daemon periodic task and the CLI hint +/// both go through it so they always agree. +pub(crate) fn auto_update_enabled() -> bool { + auto_update_enabled_from(std::env::var(AUTO_UPDATE_ENV).ok().as_deref()) +} + +fn auto_update_enabled_from(value: Option<&str>) -> bool { + !matches!(value, Some(value) if value.trim().eq_ignore_ascii_case("off")) +} + pub fn update_hint_for_manifest( manifest: &UpdateManifest, current_version: &str, platform_key: &str, + auto_update: bool, ) -> Result> { Ok(manifest .update_candidate(current_version, platform_key)? - .map(|candidate| { - format!( - "A new bsk version is available: {} -> {}. Run `bsk update`.", - candidate.current, candidate.latest - ) - })) + .map(|candidate| update_hint_text(&candidate.current, &candidate.latest, auto_update))) } -fn update_hint_for_cache(cache: &UpdateCheckCache, current_version: &str) -> Option { +fn update_hint_for_cache( + cache: &UpdateCheckCache, + current_version: &str, + auto_update: bool, +) -> Option { let latest = cache.latest_version.as_str(); let current = Version::parse(current_version.trim_start_matches('v')).ok()?; let latest_version = Version::parse(latest.trim_start_matches('v')).ok()?; - (latest_version > current).then(|| { - format!("A new bsk version is available: {current} -> {latest_version}. Run `bsk update`.") - }) + (latest_version > current).then(|| update_hint_text(¤t, &latest_version, auto_update)) +} + +/// CLI hint wording. With auto-update on, the daemon upgrades bsk +/// itself so the hint only announces that; with it off, the hint keeps +/// pointing at the manual `bsk update`. +fn update_hint_text(current: &Version, latest: &Version, auto_update: bool) -> String { + if auto_update { + format!( + "A new bsk version is available: {current} -> {latest}. The daemon will upgrade bsk automatically." + ) + } else { + format!("A new bsk version is available: {current} -> {latest}. Run `bsk update`.") + } } pub fn read_update_cache(path: &Path) -> Result> { @@ -458,12 +561,12 @@ pub(crate) fn cache_needs_refresh( } /// Fetch the update manifest and rewrite the cache. Returns the update -/// hint when a newer version exists. Blocking: call from a blocking -/// context (the daemon wraps it in `spawn_blocking`). -pub(crate) fn refresh_update_cache(cache_path: &Path) -> Result> { +/// candidate when the manifest names a newer version. Blocking: call +/// from a blocking context (the daemon wraps it in `spawn_blocking`). +pub(crate) fn refresh_update_cache(cache_path: &Path) -> Result> { let manifest = fetch_manifest(&manifest_url())?; let platform = current_platform_key()?; - let hint = update_hint_for_manifest(&manifest, env!("CARGO_PKG_VERSION"), platform)?; + let candidate = manifest.update_candidate(env!("CARGO_PKG_VERSION"), platform)?; write_update_cache( cache_path, &UpdateCheckCache { @@ -471,7 +574,7 @@ pub(crate) fn refresh_update_cache(cache_path: &Path) -> Result> latest_version: manifest.version.to_string(), }, )?; - Ok(hint) + Ok(candidate) } /// Print the cached "new version available" hint, if there is one. @@ -496,7 +599,12 @@ pub fn print_update_hint_from_cache(flags: &super::GlobalFlags, command: &super: let Ok(cache_path) = crate::daemon::paths::update_check_path() else { return; }; - match cached_update_hint(&cache_path, env!("CARGO_PKG_VERSION"), now_epoch_secs()) { + match cached_update_hint( + &cache_path, + env!("CARGO_PKG_VERSION"), + now_epoch_secs(), + auto_update_enabled(), + ) { Ok(Some(hint)) => eprintln!("{hint}"), Ok(None) => {} Err(err) => { @@ -511,6 +619,7 @@ fn cached_update_hint( cache_path: &Path, current_version: &str, now_epoch_secs: u64, + auto_update: bool, ) -> Result> { let Some(cache) = read_update_cache(cache_path)? else { return Ok(None); @@ -518,7 +627,7 @@ fn cached_update_hint( if !cache.is_fresh(now_epoch_secs, UPDATE_CHECK_INTERVAL) { return Ok(None); } - Ok(update_hint_for_cache(&cache, current_version)) + Ok(update_hint_for_cache(&cache, current_version, auto_update)) } fn confirm_update(candidate: &UpdateCandidate) -> Result { @@ -864,13 +973,22 @@ mod tests { ) .unwrap(); - let hint = update_hint_for_manifest(&manifest, "0.1.7", "linux-x64").unwrap(); + // Auto-update off: the hint keeps pointing at `bsk update`. + let hint = update_hint_for_manifest(&manifest, "0.1.7", "linux-x64", false).unwrap(); assert_eq!( hint.as_deref(), Some("A new bsk version is available: 0.1.7 -> 0.2.0. Run `bsk update`.") ); + // Auto-update on: the daemon upgrades bsk itself. + let hint = update_hint_for_manifest(&manifest, "0.1.7", "linux-x64", true).unwrap(); + assert_eq!( + hint.as_deref(), + Some( + "A new bsk version is available: 0.1.7 -> 0.2.0. The daemon will upgrade bsk automatically." + ) + ); assert!( - update_hint_for_manifest(&manifest, "0.2.0", "linux-x64") + update_hint_for_manifest(&manifest, "0.2.0", "linux-x64", true) .unwrap() .is_none() ); @@ -958,18 +1076,28 @@ mod tests { let now = now_epoch_secs(); // Missing cache -> no hint. - assert_eq!(cached_update_hint(&path, "0.1.7", now).unwrap(), None); + assert_eq!( + cached_update_hint(&path, "0.1.7", now, false).unwrap(), + None + ); - // Fresh cache with a newer version -> hint. + // Fresh cache with a newer version -> hint, worded by the + // auto-update switch. let fresh_newer = UpdateCheckCache { checked_at_epoch_secs: now, latest_version: "0.2.0".to_string(), }; write_update_cache(&path, &fresh_newer).unwrap(); assert_eq!( - cached_update_hint(&path, "0.1.7", now).unwrap(), + cached_update_hint(&path, "0.1.7", now, false).unwrap(), Some("A new bsk version is available: 0.1.7 -> 0.2.0. Run `bsk update`.".to_string()) ); + assert_eq!( + cached_update_hint(&path, "0.1.7", now, true).unwrap(), + Some( + "A new bsk version is available: 0.1.7 -> 0.2.0. The daemon will upgrade bsk automatically.".to_string() + ) + ); // Fresh cache without a newer version -> no hint. let fresh_current = UpdateCheckCache { @@ -977,7 +1105,7 @@ mod tests { latest_version: "0.1.7".to_string(), }; write_update_cache(&path, &fresh_current).unwrap(); - assert_eq!(cached_update_hint(&path, "0.1.7", now).unwrap(), None); + assert_eq!(cached_update_hint(&path, "0.1.7", now, true).unwrap(), None); // Stale cache, even with a newer version -> no hint. let stale_newer = UpdateCheckCache { @@ -985,11 +1113,114 @@ mod tests { latest_version: "0.2.0".to_string(), }; write_update_cache(&path, &stale_newer).unwrap(); - assert_eq!(cached_update_hint(&path, "0.1.7", now).unwrap(), None); + assert_eq!(cached_update_hint(&path, "0.1.7", now, true).unwrap(), None); // Corrupt cache file -> error surfaced to the caller, no panic. std::fs::write(&path, b"not json").unwrap(); - assert!(cached_update_hint(&path, "0.1.7", now).is_err()); + assert!(cached_update_hint(&path, "0.1.7", now, true).is_err()); + } + + #[test] + fn auto_update_toggle_defaults_on_and_only_off_disables() { + assert!(auto_update_enabled_from(None)); + assert!(auto_update_enabled_from(Some("on"))); + assert!(auto_update_enabled_from(Some("1"))); + assert!(auto_update_enabled_from(Some(""))); + assert!(!auto_update_enabled_from(Some("off"))); + assert!(!auto_update_enabled_from(Some("OFF"))); + assert!(!auto_update_enabled_from(Some(" Off "))); + } + + fn test_candidate() -> UpdateCandidate { + UpdateCandidate { + current: Version::parse("0.1.7").unwrap(), + latest: Version::parse("0.2.0").unwrap(), + tag: "cli-v0.2.0".to_string(), + release_url: None, + asset: ManifestAsset { + url: "https://example.test/bsk.tar.gz".to_string(), + sha256: Some("abc123".to_string()), + }, + } + } + + #[test] + fn auto_update_step_reports_up_to_date_without_candidate() { + let outcome = auto_update_step(None, true, 0, |_| panic!("install must not run")).unwrap(); + assert_eq!(outcome, AutoUpdateOutcome::UpToDate); + } + + #[test] + fn auto_update_step_keeps_cache_only_when_disabled() { + let candidate = test_candidate(); + let outcome = auto_update_step(Some(&candidate), false, 0, |_| { + panic!("install must not run") + }) + .unwrap(); + assert_eq!( + outcome, + AutoUpdateOutcome::Disabled { + latest: "0.2.0".to_string() + } + ); + } + + #[test] + fn auto_update_step_postpones_with_active_sessions() { + let candidate = test_candidate(); + let outcome = auto_update_step(Some(&candidate), true, 2, |_| { + panic!("install must not run") + }) + .unwrap(); + assert_eq!( + outcome, + AutoUpdateOutcome::PostponedSessions { + latest: "0.2.0".to_string(), + sessions: 2, + } + ); + } + + #[test] + fn auto_update_step_installs_when_no_sessions() { + let candidate = test_candidate(); + let installs = std::cell::Cell::new(0); + let outcome = auto_update_step(Some(&candidate), true, 0, |candidate| { + installs.set(installs.get() + 1); + assert_eq!(candidate.latest.to_string(), "0.2.0"); + Ok(InstallAction::Replaced) + }) + .unwrap(); + assert_eq!(installs.get(), 1); + assert_eq!( + outcome, + AutoUpdateOutcome::Replaced { + latest: "0.2.0".to_string() + } + ); + } + + #[test] + fn auto_update_step_staged_outcome_requests_no_restart() { + // The Windows shape: the replacement is staged next to the + // running binary, so the outcome must not ask for the immediate + // self-restart that `Replaced` triggers. + let candidate = test_candidate(); + let outcome = + auto_update_step(Some(&candidate), true, 0, |_| Ok(InstallAction::Staged)).unwrap(); + assert_eq!( + outcome, + AutoUpdateOutcome::Staged { + latest: "0.2.0".to_string() + } + ); + } + + #[test] + fn auto_update_step_propagates_install_errors() { + let candidate = test_candidate(); + let result = auto_update_step(Some(&candidate), true, 0, |_| bail!("boom")); + assert!(result.is_err()); } fn tar_gz_with_bsk(binary: &[u8]) -> Vec { diff --git a/crates/bsk-cli/src/daemon/start.rs b/crates/bsk-cli/src/daemon/start.rs index f0c2a3c..582cb4c 100644 --- a/crates/bsk-cli/src/daemon/start.rs +++ b/crates/bsk-cli/src/daemon/start.rs @@ -12,6 +12,7 @@ //! falls through to `run_foreground`. use std::net::{IpAddr, Ipv4Addr, SocketAddr}; +use std::path::Path; use std::sync::Arc; use std::sync::Mutex; use std::sync::atomic::{AtomicUsize, Ordering}; @@ -34,6 +35,12 @@ use crate::daemon::{ /// to indicate "you are the daemon, detach yourself and run". pub(crate) const DAEMONIZED_ENV: &str = "BSK_DAEMONIZED"; +/// Internal env-var contract for daemon self-restart after an +/// auto-update: the outgoing daemon spawns its replacement with this set +/// to its own pid; the child waits for that pid to exit — releasing the +/// daemon lock, IPC socket, and WS port — before taking over. +pub(crate) const DAEMON_REPLACEMENT_WAIT_ENV: &str = "BSK_DAEMON_REPLACES_PID"; + /// Concrete daemon configuration resolved from CLI flags / defaults. #[derive(Debug, Clone)] pub struct DaemonConfig { @@ -108,6 +115,7 @@ pub fn run_start(args: StartArgs) -> Result<()> { // Detached child mode (set by parent before spawn). if is_daemonized_child() { + wait_for_replaced_daemon(); detach_stdio()?; return run_foreground(cfg); } @@ -220,7 +228,12 @@ pub fn run_foreground(cfg: DaemonConfig) -> Result<()> { let state = Arc::new(DaemonState::new(cfg.clone())); let session_idle_task = spawn_session_idle_reaper(Arc::clone(&state)); let browser_liveness_task = spawn_browser_liveness_reaper(Arc::clone(&state)); - let update_check_task = spawn_update_check_task(); + // Fired by the update check task after a successful auto-update: + // the replacement daemon has already been spawned, so this + // process should shut down and let it take over. + let restart_notify = Arc::new(tokio::sync::Notify::new()); + let update_check_task = + spawn_update_check_task(Arc::clone(&state), Arc::clone(&restart_notify)); let ws_addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::LOCALHOST), cfg.ws_port); let ws_handle = ws::WsServer::new(Arc::clone(&state)) .bind(ws_addr) @@ -367,6 +380,11 @@ pub fn run_foreground(cfg: DaemonConfig) -> Result<()> { }; drop(idle_rx); + // Created before `select!` so a `notify_one` from the update + // check task is never missed (Notify stores one permit). + let restart_notified = restart_notify.notified(); + tokio::pin!(restart_notified); + tokio::select! { _ = wait_for_shutdown() => { info!("bsk daemon shutting down (signal)"); @@ -376,6 +394,9 @@ pub fn run_foreground(cfg: DaemonConfig) -> Result<()> { info!("bsk daemon shutting down (idle)"); } } + _ = &mut restart_notified => { + info!("bsk daemon shutting down (auto-update restart)"); + } } let _ = ipc_shutdown_tx.send(()); @@ -509,7 +530,21 @@ pub(crate) fn spawn_session_idle_reaper(state: Arc) -> tokio::task: /// loops forever; shutdown aborts it like the other background tasks, so /// it never delays daemon exit (an in-flight fetch is bounded by the /// update client's own timeout and detached on abort). -pub(crate) fn spawn_update_check_task() -> tokio::task::JoinHandle<()> { +/// +/// When a tick finds a newer version the daemon also *installs* it +/// (auto-update, on by default; [`crate::cli::update::AUTO_UPDATE_ENV`] +/// `=off` disables it and keeps the check cache/hint-only). Safety +/// gate: while any agent session is live the tick postpones the +/// install and retries next time. Once the new binary is in place the +/// task spawns the replacement daemon (see +/// [`DAEMON_REPLACEMENT_WAIT_ENV`]) and fires `restart` so this process +/// shuts down and the new version takes over; on Windows the +/// replacement can only be staged, so it just logs that a restart is +/// needed. +pub(crate) fn spawn_update_check_task( + state: Arc, + restart: Arc, +) -> tokio::task::JoinHandle<()> { use crate::cli::update; tokio::spawn(async move { @@ -520,6 +555,17 @@ pub(crate) fn spawn_update_check_task() -> tokio::task::JoinHandle<()> { return; } }; + // Capture our own executable path once, up front: after an + // auto-update replaces the binary, `current_exe` on Linux starts + // returning a " (deleted)"-suffixed path that can neither be + // replaced again nor spawned. + let exe_path = match std::env::current_exe() { + Ok(exe) => Some(exe), + Err(err) => { + warn!(error = %err, "auto-update install disabled: cannot locate current executable"); + None + } + }; let mut ticker = tokio::time::interval(update::UPDATE_CHECK_INTERVAL); ticker.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Delay); @@ -544,18 +590,97 @@ pub(crate) fn spawn_update_check_task() -> tokio::task::JoinHandle<()> { let result = { let cache_path = cache_path.clone(); - tokio::task::spawn_blocking(move || update::refresh_update_cache(&cache_path)).await + let state = Arc::clone(&state); + let exe_path = exe_path.clone(); + tokio::task::spawn_blocking(move || { + let candidate = update::refresh_update_cache(&cache_path)?; + // The session gate is read after the fetch, as late + // as possible before the binary gets replaced. + let active_sessions = state.sessions.len(); + let auto_update = update::auto_update_enabled() && exe_path.is_some(); + update::auto_update_step( + candidate.as_ref(), + auto_update, + active_sessions, + |candidate| { + let target = + exe_path.as_deref().context("current executable unknown")?; + update::self_install_candidate(candidate, target) + }, + ) + }) + .await }; - match result { - Ok(Ok(Some(hint))) => info!(%hint, "periodic update check found a new version"), - Ok(Ok(None)) => info!("periodic update check refreshed cache (already up to date)"), - Ok(Err(err)) => warn!(error = %err, "periodic update check failed"), - Err(err) => warn!(error = %err, "periodic update check task panicked"), + let outcome = match result { + Ok(Ok(outcome)) => outcome, + Ok(Err(err)) => { + warn!(error = %err, "periodic update check failed"); + continue; + } + Err(err) => { + warn!(error = %err, "periodic update check task panicked"); + continue; + } + }; + match outcome { + update::AutoUpdateOutcome::UpToDate => { + info!("periodic update check refreshed cache (already up to date)") + } + update::AutoUpdateOutcome::Disabled { latest } => info!( + %latest, + "periodic update check found a new version; auto-update off, CLI hint only" + ), + update::AutoUpdateOutcome::PostponedSessions { latest, sessions } => info!( + %latest, + sessions, + "auto-update postponed: agent session(s) active; will retry on the next tick" + ), + update::AutoUpdateOutcome::Staged { latest } => warn!( + %latest, + "auto-update staged the new binary but cannot replace the running daemon in place; restart the daemon (or terminal) to finish the upgrade" + ), + update::AutoUpdateOutcome::Replaced { latest } => { + info!( + current = env!("CARGO_PKG_VERSION"), + %latest, + "auto-update installed the new bsk binary; restarting daemon" + ); + // `exe_path` is always Some here: the install only + // runs when it was captured. + if let Some(exe) = &exe_path { + let args = restart_start_args(&state.config); + match spawn_detached_at(exe, &args, Some(std::process::id())) { + Ok(()) => { + info!( + pid = std::process::id(), + "replacement daemon spawned; exiting so it can take over" + ); + restart.notify_one(); + return; + } + Err(err) => warn!( + error = %err, + "auto-update replaced the binary but failed to spawn the replacement daemon; the next daemon start picks up the new version" + ), + } + } + } } } }) } +/// Rebuild the `StartArgs` for the replacement daemon from the running +/// config so the respawn keeps the same port and idle timeouts. +fn restart_start_args(cfg: &DaemonConfig) -> StartArgs { + StartArgs { + port: Some(cfg.ws_port), + foreground: false, + session_idle: Some(cfg.session_idle), + daemon_idle: Some(cfg.daemon_idle), + } +} + fn record_activity(activity: &Arc>) { if let Ok(mut a) = activity.lock() { *a = Instant::now(); @@ -627,6 +752,24 @@ fn is_daemonized_child() -> bool { std::env::var(DAEMONIZED_ENV).as_deref() == Ok("1") } +/// Self-update handoff: when the outgoing daemon spawned us as its +/// replacement ([`DAEMON_REPLACEMENT_WAIT_ENV`]), wait for its pid to +/// exit so the daemon lock, IPC socket, and WS port are free before we +/// try to take them over. Bounded on purpose — the lockfile is the real +/// backstop if the predecessor somehow lingers. +fn wait_for_replaced_daemon() { + let pid = std::env::var(DAEMON_REPLACEMENT_WAIT_ENV) + .ok() + .and_then(|raw| raw.parse::().ok()); + let Some(pid) = pid else { + return; + }; + let deadline = Instant::now() + Duration::from_secs(30); + while lockfile::pid_alive(pid) && Instant::now() < deadline { + std::thread::sleep(Duration::from_millis(50)); + } +} + #[cfg(unix)] fn detach_stdio() -> Result<()> { use std::fs::OpenOptions; @@ -670,14 +813,27 @@ fn detach_stdio() -> Result<()> { #[cfg(unix)] fn spawn_detached(args: &StartArgs) -> Result<()> { - use std::os::unix::process::CommandExt; let exe = std::env::current_exe().context("current_exe")?; + spawn_detached_at(&exe, args, None) +} + +/// Spawn a detached daemon child running the binary at `exe`. When +/// `predecessor_pid` is set, the child first waits for that process to +/// exit ([`DAEMON_REPLACEMENT_WAIT_ENV`]) — used by the auto-update +/// self-restart, where the on-disk binary has already been replaced, so +/// the child runs the new version. +#[cfg(unix)] +fn spawn_detached_at(exe: &Path, args: &StartArgs, predecessor_pid: Option) -> Result<()> { + use std::os::unix::process::CommandExt; let mut cmd = std::process::Command::new(exe); apply_start_args(&mut cmd, args); cmd.stdin(std::process::Stdio::null()) .stdout(std::process::Stdio::null()) .stderr(std::process::Stdio::null()) .env(DAEMONIZED_ENV, "1"); + if let Some(pid) = predecessor_pid { + cmd.env(DAEMON_REPLACEMENT_WAIT_ENV, pid.to_string()); + } // Place the child into its own session before exec to fully detach. unsafe { cmd.pre_exec(|| { @@ -692,17 +848,26 @@ fn spawn_detached(args: &StartArgs) -> Result<()> { #[cfg(windows)] fn spawn_detached(args: &StartArgs) -> Result<()> { + let exe = std::env::current_exe().context("current_exe")?; + spawn_detached_at(&exe, args, None) +} + +/// Windows counterpart of the unix [`spawn_detached_at`]; see its docs. +#[cfg(windows)] +fn spawn_detached_at(exe: &Path, args: &StartArgs, predecessor_pid: Option) -> Result<()> { use std::os::windows::process::CommandExt; const DETACHED_PROCESS: u32 = 0x0000_0008; const CREATE_NEW_PROCESS_GROUP: u32 = 0x0000_0200; - let exe = std::env::current_exe().context("current_exe")?; let mut cmd = std::process::Command::new(exe); apply_start_args(&mut cmd, args); cmd.stdin(std::process::Stdio::null()) .stdout(std::process::Stdio::null()) .stderr(std::process::Stdio::null()) - .env(DAEMONIZED_ENV, "1") - .creation_flags(DETACHED_PROCESS | CREATE_NEW_PROCESS_GROUP); + .env(DAEMONIZED_ENV, "1"); + if let Some(pid) = predecessor_pid { + cmd.env(DAEMON_REPLACEMENT_WAIT_ENV, pid.to_string()); + } + cmd.creation_flags(DETACHED_PROCESS | CREATE_NEW_PROCESS_GROUP); let _child = cmd.spawn().context("spawn detached daemon child")?; Ok(()) } @@ -714,6 +879,13 @@ fn spawn_detached(_args: &StartArgs) -> Result<()> { )) } +#[cfg(not(any(unix, windows)))] +fn spawn_detached_at(_exe: &Path, _args: &StartArgs, _predecessor_pid: Option) -> Result<()> { + Err(anyhow::anyhow!( + "detached daemon spawn is not supported on this platform" + )) +} + fn apply_start_args(cmd: &mut std::process::Command, args: &StartArgs) { cmd.arg("daemon").arg("start"); if let Some(p) = args.port { @@ -880,4 +1052,19 @@ mod tests { assert_eq!(format_duration(Duration::from_secs(5)), "5s"); assert_eq!(format_duration(Duration::from_millis(750)), "750ms"); } + + #[test] + fn restart_start_args_preserve_the_running_config() { + let cfg = DaemonConfig { + ws_port: 1234, + session_idle: Duration::from_secs(11), + daemon_idle: Duration::from_secs(22), + ..DaemonConfig::new(0) + }; + let args = restart_start_args(&cfg); + assert_eq!(args.port, Some(1234)); + assert!(!args.foreground); + assert_eq!(args.session_idle, Some(Duration::from_secs(11))); + assert_eq!(args.daemon_idle, Some(Duration::from_secs(22))); + } } diff --git a/skill/SKILL.md b/skill/SKILL.md index 9a1b6a8..b88fffa 100644 --- a/skill/SKILL.md +++ b/skill/SKILL.md @@ -108,6 +108,8 @@ Do **not** call `bsk get-html` or `bsk screenshot` first just to inspect a page. | `--quiet` | Suppress informational stderr | | `-v` / `-vv` | More verbose logging | +Auto-update is **on by default** — the background daemon upgrades `bsk` itself when a new release is available (postponed while a session is active). Set `BSK_AUTO_UPDATE=off` to disable it and upgrade manually with `bsk update`. + Command-specific flags (timeouts, `--tab-id`, `--wait-until`, …): **`bsk --help`** ## CLI command reference (one line each)