diff --git a/crates/allmystuff-updater/src/lib.rs b/crates/allmystuff-updater/src/lib.rs index c7e4c40d..80a1bc4a 100644 --- a/crates/allmystuff-updater/src/lib.rs +++ b/crates/allmystuff-updater/src/lib.rs @@ -314,6 +314,14 @@ pub enum CheckOutcome { Staged { version: String, }, + /// A newer release exists, but this install can't swap its own binaries — + /// a package-managed copy, or a per-machine install this process can't + /// write to. Reported rather than swallowed so the app can still *tell* + /// you an update is out and how to take it. + ManualUpdateAvailable { + current: String, + latest: String, + }, } #[derive(Debug, Clone, Serialize)] @@ -773,17 +781,33 @@ pub async fn check_now(force: bool) -> Result { if !au.enabled || env_disabled() { return Ok(CheckOutcome::Disabled); } - if detect_install_kind() == InstallKind::PackageManager { - return Ok(CheckOutcome::PackageManager); - } if !force && !is_due(au.check_interval_hours) { return Ok(CheckOutcome::NotDue); } - stamp_check_now(); + + // A package-managed / unwritable install can't stage or apply anything, + // but it can still *look*. Checking anyway is what turns "self-update is + // off here" into "0.2.50 is out, update it the way you installed it" — + // previously this returned before the fetch, so such an install never + // contacted the release feed at all and silently reported nothing forever. + let managed = detect_install_kind() == InstallKind::PackageManager; let release = fetch_release(&au).await?; let latest = release_tag(&release)?; let current = current_version().to_string(); + // Stamped only now: a failed fetch must not burn the interval, or one + // offline moment costs a full `check_interval_hours` before the next try. + stamp_check_now(); + + if managed { + return Ok( + if compare_semver(¤t, &latest) == std::cmp::Ordering::Less { + CheckOutcome::ManualUpdateAvailable { current, latest } + } else { + CheckOutcome::UpToDate { current, latest } + }, + ); + } // Which halves are behind `latest` — the CLI gauged by the running binary's // version, each installed sibling (GUI, node) by its recorded stamp. Gating @@ -834,21 +858,62 @@ fn wanted_artifacts(current: &str, latest: &str) -> Vec { .collect() } -/// One attended check, logging the outcome. Shared by the launch check and -/// every interval tick of [`tick_forever`]. `force` skips only the interval -/// cooldown ([`is_due`]); the enabled flag and package-manager guard inside -/// [`check_now`] still apply, so a forced launch check no-ops cleanly when -/// auto-update is off or the install is package-managed. -async fn run_attended_check(force: bool) { +/// One attended check: log the outcome, then hand it to `notify` so the app +/// can surface it. Shared by the launch check and every interval tick of +/// [`tick_forever`]. `force` skips only the interval cooldown ([`is_due`]); +/// the enabled flag inside [`check_now`] still applies, so a forced launch +/// check no-ops cleanly when auto-update is off. A package-managed or +/// unwritable install still checks — it reports +/// [`CheckOutcome::ManualUpdateAvailable`] rather than staging. +async fn run_attended_check(force: bool, notify: &(dyn Fn(&CheckOutcome) + Send + Sync)) { match check_now(force).await { - Ok(CheckOutcome::Staged { version }) => { - tracing::info!("self-update staged {version}; applies on next launch"); + Ok(outcome) => { + log_check_outcome(&outcome); + notify(&outcome); } - Ok(_) => {} Err(e) => tracing::warn!("self-update check failed: {e}"), } } +/// Log what a check decided — *every* outcome, not just the interesting one. +/// These used to be swallowed by a bare `Ok(_) => {}`, which meant a ticker +/// that was silently disabled (package-managed install, auto-update off) and a +/// ticker that was running fine and finding nothing looked exactly alike: no +/// output at all, at any log level. That is the single hardest part of "it +/// never checks" to diagnose, so each branch now says so. +fn log_check_outcome(outcome: &CheckOutcome) { + match outcome { + CheckOutcome::Staged { version } => { + tracing::info!("self-update staged {version}; applies on next launch"); + } + CheckOutcome::ManualUpdateAvailable { current, latest } => { + tracing::info!( + "self-update: {latest} is available (running {current}), but this install is \ + package-managed or not writable by this process — update it the way you \ + installed it" + ); + } + CheckOutcome::UpToDate { current, latest } => { + tracing::debug!("self-update: up to date (running {current}, latest {latest})"); + } + CheckOutcome::PolicyBlocked { + current, + latest, + policy, + } => { + tracing::info!( + "self-update: {latest} is available (running {current}) but the '{policy}' \ + apply policy holds it back — apply it from Settings → Updates" + ); + } + CheckOutcome::NotDue => tracing::debug!("self-update: not due yet, skipping this tick"), + CheckOutcome::Disabled => tracing::debug!("self-update: disabled, skipping this tick"), + CheckOutcome::PackageManager => { + tracing::debug!("self-update: package-managed install, skipping this tick") + } + } +} + /// Background auto-update ticker — the half of self-update that makes it /// "set and forget". Runs forever: a **launch check** fires shortly after the /// process starts and *always* checks when auto-update is enabled — it ignores @@ -857,9 +922,11 @@ async fn run_attended_check(force: bool) { /// used to no-op on the cooldown and wait the full interval). It then starts /// the 24h timer: another check every `check_interval_hours` (re-read each loop /// so a settings change takes effect without a restart). Each tick is gated on -/// the enabled flag, the package-manager guard, and the apply policy — so -/// spawning it in a disabled or package-managed install simply no-ops, launch -/// check included. Whatever it stages applies on the next launch (see +/// the enabled flag and the apply policy, so spawning it in a disabled install +/// no-ops. A package-managed or unwritable install still *checks* — it just +/// reports [`CheckOutcome::ManualUpdateAvailable`] instead of staging, so the +/// app can say a release is out even where it can't install it itself. +/// Whatever it does stage applies on the next launch (see /// [`apply_pending_if_any`]). /// /// Every long-lived process that links the updater spawns this: the desktop @@ -869,17 +936,32 @@ async fn run_attended_check(force: bool) { /// on its own. The short-lived CLI subcommands don't spawn it: they act once /// and exit. pub async fn tick_forever() { + tick_forever_notify(|_| {}).await +} + +/// [`tick_forever`], plus a callback fired with the outcome of every check. +/// +/// The ticker is otherwise mute: it logs, and a staged update sits on disk +/// until someone thinks to open Settings → Updates, because the background +/// task holds no handle to the UI and so cannot tell it anything. The desktop +/// app passes a closure here that emits a Tauri event, which is what makes an +/// "update ready" actually reach the user — the notify half of an update +/// notification system. +pub async fn tick_forever_notify(notify: F) +where + F: Fn(&CheckOutcome) + Send + Sync + 'static, +{ // Let a freshly launched app/node settle (bind sockets, bring the session // online) before the first network hit. tokio::time::sleep(Duration::from_secs(30)).await; // The launch check: force past the interval cooldown so opening the app is - // its own check (still gated on enabled + package manager). Then the 24h - // timer takes over for the rest of the run. - run_attended_check(true).await; + // its own check (still gated on the enabled flag). Then the 24h timer takes + // over for the rest of the run. + run_attended_check(true, ¬ify).await; loop { let hours = load_auto_update().check_interval_hours.max(1); tokio::time::sleep(Duration::from_secs(hours as u64 * 3600)).await; - run_attended_check(false).await; + run_attended_check(false, ¬ify).await; } } @@ -941,7 +1023,7 @@ async fn run_unattended_check(force: bool, relaunch: fn() -> !) { Err(e) => tracing::warn!("self-update apply failed: {e}; retrying next tick"), } } - Ok(_) => {} + Ok(other) => log_check_outcome(&other), Err(e) => tracing::warn!("self-update check failed: {e}"), } } @@ -1086,12 +1168,49 @@ fn staged_version() -> Option { // ---- install-kind detection ------------------------------------------ pub fn detect_install_kind() -> InstallKind { - let path = std::env::current_exe() - .map(|p| p.to_string_lossy().to_string()) - .unwrap_or_default(); - detect_install_kind_from_path(&path) + let Ok(exe) = std::env::current_exe() else { + return InstallKind::Raw; + }; + // A foreign package manager owns the binary outright — never self-update. + if detect_install_kind_from_path(&exe.to_string_lossy()) == InstallKind::PackageManager { + return InstallKind::PackageManager; + } + // Otherwise the only question left is whether we can actually perform the + // swap. A per-machine install (`C:\Program Files\…` from the MSI, `/opt/…`) + // is owned by Administrators/root, so an unelevated process can stage a + // download it will never be able to apply — retrying, and failing, forever. + // Treating that as package-managed keeps the *check* running (so a new + // release is still reported) while routing the install itself back through + // the installer that owns it. + match exe.parent() { + Some(dir) if !dir_is_writable(dir) => InstallKind::PackageManager, + _ => InstallKind::Raw, + } +} + +/// Whether `dir` accepts writes from this process. Probing by *doing* it is +/// the only reliable test on Windows, where ACLs (and virtualisation) make a +/// permissions read meaningless — and it's what the swap itself will attempt. +fn dir_is_writable(dir: &Path) -> bool { + let probe = dir.join(format!(".allmystuff-write-probe-{}", std::process::id())); + match std::fs::File::create(&probe) { + Ok(_) => { + let _ = std::fs::remove_file(&probe); + true + } + Err(_) => false, + } } +/// Path-only classification: is this binary owned by a *foreign* package +/// manager (Homebrew, apt, Chocolatey, Scoop) that must do the updating? +/// +/// Deliberately **not** a test for "somewhere under Program Files". Our own +/// Windows MSI installs to `C:\Program Files\AllMyStuff\`, and treating that +/// as package-managed silently disabled self-update — check included — for +/// every machine that took the MSI rather than the NSIS setup. Whether such an +/// install can be *written* is a separate question, answered by +/// [`dir_is_writable`] in [`detect_install_kind`]. fn detect_install_kind_from_path(path_str: &str) -> InstallKind { // Homebrew (macOS/Linux). if path_str.contains("/Cellar/") @@ -1105,14 +1224,9 @@ fn detect_install_kind_from_path(path_str: &str) -> InstallKind { if path_str.starts_with("/usr/bin/") || path_str.starts_with("/usr/sbin/") { return InstallKind::PackageManager; } - #[cfg(target_os = "windows")] { let lower = path_str.to_lowercase(); - if lower.contains("\\program files\\") - || lower.contains("\\program files (x86)\\") - || lower.contains("\\chocolatey\\lib\\") - || lower.contains("\\scoop\\apps\\") - { + if lower.contains("\\chocolatey\\lib\\") || lower.contains("\\scoop\\apps\\") { return InstallKind::PackageManager; } } @@ -1481,6 +1595,54 @@ mod tests { ); } + #[test] + fn windows_package_managers_are_detected_but_program_files_is_not() { + // Chocolatey and Scoop genuinely own their copy — they must update it. + assert_eq!( + detect_install_kind_from_path( + r"C:\ProgramData\chocolatey\lib\allmystuff\allmystuff.exe" + ), + InstallKind::PackageManager + ); + assert_eq!( + detect_install_kind_from_path( + r"C:\Users\me\scoop\apps\allmystuff\current\allmystuff.exe" + ), + InstallKind::PackageManager + ); + // `C:\Program Files\AllMyStuff` is where our OWN MSI lands. Classifying + // it as package-managed is what silently switched self-update off — the + // check included — for every MSI install, so the path alone must not + // decide it. Whether that install is writable is a separate question + // (`dir_is_writable`), asked at runtime rather than guessed from text. + assert_eq!( + detect_install_kind_from_path(r"C:\Program Files\AllMyStuff\allmystuff-gui.exe"), + InstallKind::Raw + ); + assert_eq!( + detect_install_kind_from_path(r"C:\Program Files (x86)\AllMyStuff\allmystuff.exe"), + InstallKind::Raw + ); + // The per-user NSIS location was always fine and stays fine. + assert_eq!( + detect_install_kind_from_path( + r"C:\Users\me\AppData\Local\AllMyStuff\allmystuff-gui.exe" + ), + InstallKind::Raw + ); + } + + #[test] + fn write_probe_tells_a_writable_dir_from_a_missing_one() { + let tmp = tempfile::tempdir().expect("tempdir"); + assert!(dir_is_writable(tmp.path())); + // The probe must clean up after itself — a stray file next to the + // installed binary on every status() call would be its own bug report. + assert_eq!(std::fs::read_dir(tmp.path()).unwrap().count(), 0); + // A directory that doesn't exist can't be written to. + assert!(!dir_is_writable(&tmp.path().join("nope"))); + } + #[test] fn os_bundle_paths_are_left_alone() { // A binary inside a macOS .app must never be swapped in place — the diff --git a/gui/src-tauri/Cargo.lock b/gui/src-tauri/Cargo.lock index bb33db4c..8de8cdb8 100644 --- a/gui/src-tauri/Cargo.lock +++ b/gui/src-tauri/Cargo.lock @@ -19,7 +19,7 @@ dependencies = [ [[package]] name = "allmystuff-bridge" -version = "0.2.48" +version = "0.2.49" dependencies = [ "allmystuff-graph", "allmystuff-inventory", @@ -28,7 +28,7 @@ dependencies = [ [[package]] name = "allmystuff-cec-consent" -version = "0.2.48" +version = "0.2.49" dependencies = [ "allmystuff-cec-protocol", "serde", @@ -38,7 +38,7 @@ dependencies = [ [[package]] name = "allmystuff-cec-protocol" -version = "0.2.48" +version = "0.2.49" dependencies = [ "serde", "serde_json", @@ -47,7 +47,7 @@ dependencies = [ [[package]] name = "allmystuff-graph" -version = "0.2.48" +version = "0.2.49" dependencies = [ "serde", "serde_json", @@ -92,7 +92,7 @@ dependencies = [ [[package]] name = "allmystuff-inventory" -version = "0.2.48" +version = "0.2.49" dependencies = [ "serde", "serde_json", @@ -101,7 +101,7 @@ dependencies = [ [[package]] name = "allmystuff-node" -version = "0.2.48" +version = "0.2.49" dependencies = [ "allmystuff-bridge", "allmystuff-cec-consent", @@ -151,7 +151,7 @@ version = "0.1.0" [[package]] name = "allmystuff-protocol" -version = "0.2.48" +version = "0.2.49" dependencies = [ "allmystuff-graph", "serde", @@ -160,7 +160,7 @@ dependencies = [ [[package]] name = "allmystuff-service" -version = "0.2.48" +version = "0.2.49" dependencies = [ "anyhow", "dirs 5.0.1", @@ -169,7 +169,7 @@ dependencies = [ [[package]] name = "allmystuff-session" -version = "0.2.48" +version = "0.2.49" dependencies = [ "allmystuff-graph", "allmystuff-protocol", @@ -180,7 +180,7 @@ dependencies = [ [[package]] name = "allmystuff-updater" -version = "0.2.48" +version = "0.2.49" dependencies = [ "dirs 5.0.1", "flate2", diff --git a/gui/src-tauri/src/main.rs b/gui/src-tauri/src/main.rs index 5c86601f..401bae3e 100644 --- a/gui/src-tauri/src/main.rs +++ b/gui/src-tauri/src/main.rs @@ -2730,10 +2730,25 @@ fn main() { }); // Self-update ticker — the first check fires shortly after launch, // then at the configured interval. Spawned unconditionally: - // `check_now` no-ops when auto-update is off or this is a - // package-managed install. Without this the in-app updater only - // ever checks when the user clicks "Check now". - tauri::async_runtime::spawn(allmystuff_updater::tick_forever()); + // `check_now` no-ops when auto-update is off. Without this the + // in-app updater only ever checks when the user clicks "Check now". + // + // Every outcome is forwarded to the webview as `update://checked`, + // so a release found in the background actually reaches the user. + // The ticker used to run mute — a staged update sat on disk until + // someone happened to open Settings → Updates, which is what made + // an app that *was* checking look like one that never did. + let update_handle = app.handle().clone(); + tauri::async_runtime::spawn(allmystuff_updater::tick_forever_notify( + move |outcome| match serde_json::to_value(outcome) { + Ok(payload) => { + if let Err(e) = update_handle.emit("update://checked", payload) { + tracing::warn!("couldn't emit the self-update outcome: {e}"); + } + } + Err(e) => tracing::warn!("couldn't serialise the self-update outcome: {e}"), + }, + )); Ok(()) }) .build(tauri::generate_context!()) diff --git a/gui/src/store.svelte.ts b/gui/src/store.svelte.ts index eb64d8bb..0c930a28 100644 --- a/gui/src/store.svelte.ts +++ b/gui/src/store.svelte.ts @@ -103,6 +103,7 @@ import { meshRosterRemove, onOwned, onOwnership, + onUpdateChecked, onRoom, onRoomLocal, onSession, @@ -1492,6 +1493,11 @@ class AppStore { await this.loadSites(); await onNodeSites((e) => this.applyNodeSites(e)); await this.loadUpdateStatus(); + // The background ticker's verdict. Registered here (not in the Updates + // pane) so a release found while the user is anywhere in the app still + // surfaces — the pane only mounts when you go looking for it, which is + // exactly the trap that made auto-update feel like it never ran. + await onUpdateChecked((o) => this.applyUpdateChecked(o)); await this.loadDisabledNetworks(); this.startMeshPolling(); await onSubscription((s) => { @@ -7798,6 +7804,37 @@ class AppStore { } } + /** A background check reported in. Only the outcomes that mean "there is + * something newer than what you're running" are worth interrupting for — + * the routine up-to-date / not-due / disabled ticks refresh the Updates + * pane quietly and say nothing. */ + private applyUpdateChecked(o: CheckOutcome) { + this.updateOutcome = o; + void this.loadUpdateStatus(); + switch (o.outcome) { + case "staged": + this.toast( + "info", + `Update ${o.version} is ready — relaunch to run it (Settings → Updates)`, + ); + break; + case "manual_update_available": + this.toast( + "info", + `${o.latest} is available — update the way you installed AllMyStuff`, + ); + break; + case "policy_blocked": + this.toast( + "info", + `${o.latest} is available — approve it in Settings → Updates`, + ); + break; + default: + break; + } + } + /** Check the release feed now and stage anything permitted. */ async checkUpdates() { if (!isTauri()) { @@ -8251,6 +8288,8 @@ class AppStore { return "You're on the latest version"; case "policy_blocked": return `${o.latest} is available but held by your auto-apply setting`; + case "manual_update_available": + return `${o.latest} is available, but this install can't update itself — update it the way you installed it`; case "package_manager": return "Installed via a package manager — update through it"; case "disabled": diff --git a/gui/src/tauri.ts b/gui/src/tauri.ts index 94780ab8..642bc0eb 100644 --- a/gui/src/tauri.ts +++ b/gui/src/tauri.ts @@ -1890,6 +1890,19 @@ export function updateStatus(): Promise { return tryInvoke("update_status"); } +/** The background auto-update ticker reporting what a check decided — the + * launch check (~30s after start) and then every `check_interval_hours`. + * Without this the ticker was mute: it staged updates that nobody was told + * about, so auto-update looked like it wasn't running at all. No-op listener + * in web mode. */ +export async function onUpdateChecked( + cb: (o: CheckOutcome) => void, +): Promise<() => void> { + if (!isTauri()) return () => {}; + const { listen } = await import("@tauri-apps/api/event"); + return listen("update://checked", (e) => cb(e.payload)); +} + export function updateCheck(): Promise { return tryInvoke("update_check"); } diff --git a/gui/src/types.ts b/gui/src/types.ts index c606e54b..a9b4a74c 100644 --- a/gui/src/types.ts +++ b/gui/src/types.ts @@ -538,7 +538,9 @@ export interface CheckOutcome { | "not_due" | "up_to_date" | "policy_blocked" - | "staged"; + | "staged" + /** Newer release exists, but this install can't swap its own binaries. */ + | "manual_update_available"; current?: string; latest?: string; policy?: string; diff --git a/gui/src/ui/settings/UpdatesSection.svelte b/gui/src/ui/settings/UpdatesSection.svelte index 6c750a70..48da49f7 100644 --- a/gui/src/ui/settings/UpdatesSection.svelte +++ b/gui/src/ui/settings/UpdatesSection.svelte @@ -83,7 +83,9 @@ {pkg ? "Installed via a package manager" : "Installed as a standalone binary"} - @@ -95,8 +97,11 @@ {#if pkg}

- AllMyStuff was installed through a package manager (Homebrew, apt, MSI…), - so self-update is off — update it the same way you installed it. + This copy of AllMyStuff can't replace its own files — it was installed + through a package manager (Homebrew, apt, Chocolatey, Scoop), or into a + location that needs an administrator to write to. It still checks for + new releases and will tell you when one lands; installing it is done + the same way you installed AllMyStuff.

{:else} diff --git a/node/Cargo.lock b/node/Cargo.lock index 1a6d8f40..7964543c 100644 --- a/node/Cargo.lock +++ b/node/Cargo.lock @@ -19,7 +19,7 @@ dependencies = [ [[package]] name = "allmystuff-bridge" -version = "0.2.48" +version = "0.2.49" dependencies = [ "allmystuff-graph", "allmystuff-inventory", @@ -28,7 +28,7 @@ dependencies = [ [[package]] name = "allmystuff-cec-consent" -version = "0.2.48" +version = "0.2.49" dependencies = [ "allmystuff-cec-protocol", "serde", @@ -38,7 +38,7 @@ dependencies = [ [[package]] name = "allmystuff-cec-protocol" -version = "0.2.48" +version = "0.2.49" dependencies = [ "serde", "serde_json", @@ -47,7 +47,7 @@ dependencies = [ [[package]] name = "allmystuff-graph" -version = "0.2.48" +version = "0.2.49" dependencies = [ "serde", "serde_json", @@ -56,7 +56,7 @@ dependencies = [ [[package]] name = "allmystuff-inventory" -version = "0.2.48" +version = "0.2.49" dependencies = [ "serde", "serde_json", @@ -116,7 +116,7 @@ version = "0.1.0" [[package]] name = "allmystuff-protocol" -version = "0.2.48" +version = "0.2.49" dependencies = [ "allmystuff-graph", "serde", @@ -125,7 +125,7 @@ dependencies = [ [[package]] name = "allmystuff-session" -version = "0.2.48" +version = "0.2.49" dependencies = [ "allmystuff-graph", "allmystuff-protocol", @@ -136,7 +136,7 @@ dependencies = [ [[package]] name = "allmystuff-updater" -version = "0.2.48" +version = "0.2.49" dependencies = [ "dirs", "flate2", diff --git a/node/src/bin/serve.rs b/node/src/bin/serve.rs index 01f31933..9a58949b 100644 --- a/node/src/bin/serve.rs +++ b/node/src/bin/serve.rs @@ -36,6 +36,7 @@ //! [`allmystuff_updater::tick_forever_unattended`]. use std::future::Future; +use std::io::IsTerminal; use std::process::ExitCode; use std::sync::Arc; @@ -525,7 +526,15 @@ fn init_logging(as_service: bool) { .with_ansi(false) .with_writer(make) }); - let stdout_layer = tracing_subscriber::fmt::layer().with_target(false); + // Colour only when a human is actually looking. Every *file* layer here + // already sets `with_ansi(false)`; stdout was left on the default, which is + // unconditionally ON. Under systemd that stdout is captured by + // journald/rsyslog, so each line lands in /var/log/syslog wrapped in + // escape sequences (`#033[2m…#033[0m`) — tens of wasted bytes per line on + // a box that logs continuously, and grep stops matching what you can see. + let stdout_layer = tracing_subscriber::fmt::layer() + .with_target(false) + .with_ansi(std::io::stdout().is_terminal()); // The field-test log: a second file, in the directory the node was // launched from, at full debug verbosity for our crates regardless of