From 399e7b49f86e277f5df46dd733c532bd6c3429f4 Mon Sep 17 00:00:00 2001 From: Robert Nio <50460704+githubrobbi@users.noreply.github.com> Date: Tue, 7 Jul 2026 18:18:59 -0700 Subject: [PATCH 1/6] docs(readme): soften Everything-CLI export claim to the tested ~2 GB IPC limit (#543) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * docs(readme): add YouTube demos playlist link Link the full uncut CLI/MCP/TUI demo reels above the inline GIFs. Co-Authored-By: Claude Opus 4.8 * docs(readme): soften Everything-CLI export claim to the tested ~2 GB IPC limit Replace the absolute 'Everything's CLI cannot/can't run this' with the factual, scoped claim ('hit a ~2 GB IPC limit at this scale') in both the blog-link paragraph and the benchmark highlights β€” matching the softened blog post so the exact claim most likely to be scrutinised is consistent across README and blog. Co-Authored-By: Claude Opus 4.8 --------- Co-authored-by: Claude Opus 4.8 --- README.md | 4 +- crates/uffs-cli/src/commands/daemon_mgmt.rs | 116 +++++++++++++++----- crates/uffs-client/src/daemon_ctl.rs | 40 +++++++ 3 files changed, 128 insertions(+), 32 deletions(-) diff --git a/README.md b/README.md index 0b6e2a6a1..6a6242857 100644 --- a/README.md +++ b/README.md @@ -79,14 +79,14 @@ Every clip runs the **real binary** against real NTFS data with unedited timings ## Benchmark snapshot (v0.5.120 Β· June 2026) -πŸ“– **The story behind these numbers:** [*I benchmarked my Rust file search engine against Everything until I ran out of excuses*](https://uffs.io/blog/benchmarking-against-everything/) β€” the methodology I had to fix first, the bulk-export workload Everything's CLI can't run, and the two regressions published anyway. +πŸ“– **The story behind these numbers:** [*I benchmarked my Rust file search engine against Everything until I ran out of excuses*](https://uffs.io/blog/benchmarking-against-everything/) β€” the methodology I had to fix first, the bulk-export workload where `es.exe` hit a ~2 GB IPC limit at this scale, and the two regressions published anyway. Measured 2026-06-11 on AMD Ryzen 9 3900XT, 64 GB RAM, Windows 11 Pro 24H2 β€” cross-tool on four NTFS volumes (C/D/F/G, 12.8 M records, the Everything-RAM-budget-negotiated set), full-scan on all seven (25.9 M records; that workload is UFFS-only, so the negotiation doesn't constrain it). Raw data: [`cross-tool-summary.csv`](docs/benchmarks/raw/2026-06-v0.5.120_cross-tool-summary.csv) Β· [`full-scan-all-drives.csv`](docs/benchmarks/raw/2026-06-v0.5.120_full-scan-all-drives.csv). Publication-grade report: [**docs/benchmarks/**](docs/benchmarks/). **vs the competition** (10 rounds per cell, p50, file sink): - **30/30 head-to-head cells faster than Everything** β€” median ratio **0.36Γ— (~2.8Γ— faster)** across C/D/F/G + the combined index; every cell from the April snapshot improved (median βˆ’33%) -- **Full-scan export across all 7 drives: 23.3 M rows β†’ CSV in 12.0 s β‰ˆ 1.95 M rec/s** (+13% throughput vs April at the same scale) β€” a workload Everything's CLI export cannot run (~2 GB IPC ceiling) +- **Full-scan export across all 7 drives: 23.3 M rows β†’ CSV in 12.0 s β‰ˆ 1.95 M rec/s** (+13% throughput vs April at the same scale) β€” a workload where Everything's CLI export hit a ~2 GB IPC limit at this scale - **180×–3 400Γ— vs the UFFS C++ reference** on targeted queries (daemon HOT vs per-invocation MFT re-read); 6.6Γ— on combined full-scan **Latency shape** (v0.5.120): diff --git a/crates/uffs-cli/src/commands/daemon_mgmt.rs b/crates/uffs-cli/src/commands/daemon_mgmt.rs index 95c6cdf24..72d23c995 100644 --- a/crates/uffs-cli/src/commands/daemon_mgmt.rs +++ b/crates/uffs-cli/src/commands/daemon_mgmt.rs @@ -147,10 +147,7 @@ pub(crate) fn daemon(action: &DaemonAction) -> Result<()> { ), DaemonAction::Status { verbose, json } => daemon_status::daemon_status(*verbose, *json), DaemonAction::Stop => daemon_stop(), - DaemonAction::Kill => { - daemon_kill(); - Ok(()) - } + DaemonAction::Kill => daemon_kill(), DaemonAction::Restart => daemon_restart(), DaemonAction::Load { mft_file, @@ -413,8 +410,15 @@ fn daemon_stop() -> Result<()> { } /// `uffs --daemon kill` β€” hard kill via PID file or socket discovery + cleanup. +/// +/// The PID file / socket are removed **only after** the process is confirmed +/// gone. If the terminate did not take effect β€” almost always because the +/// daemon was started elevated and this is a non-elevated shell β€” the PID file +/// is left in place (so the daemon stays discoverable) and an error explains +/// how to finish. This closes the old "PID file deleted, process still +/// running" half-kill. #[expect(clippy::print_stdout, reason = "CLI user-facing output")] -fn daemon_kill() { +fn daemon_kill() -> Result<()> { let pid_path = pid_file_path(); let mut pid = @@ -428,41 +432,93 @@ fn daemon_kill() { pid = Some(status.pid); } - if let Some(target_pid) = pid { + let Some(target_pid) = pid else { + // Nothing running β†’ sweep any stale files (no-op if already absent). + drop(std::fs::remove_file(&pid_path)); + drop(std::fs::remove_file(socket_path())); + if !is_quiet() { + println!("No daemon found (no PID file, no socket connection)."); + } + return Ok(()); + }; + + // A PID file that outlived its process (crash / reboot / recycled PID): the + // daemon is already gone, so there is nothing to kill β€” just clean up. This + // avoids reporting a "kill" when nothing was running. + if !uffs_client::daemon_ctl::is_pid_alive(target_pid) { + drop(std::fs::remove_file(&pid_path)); + drop(std::fs::remove_file(socket_path())); if !is_quiet() { - println!("Killing daemon (PID {target_pid})..."); + println!("No running daemon (stale PID {target_pid}); cleaned up PID file + socket."); } - kill_pid(target_pid); - } else if !is_quiet() { - println!("No daemon found (no PID file, no socket connection)."); + return Ok(()); } - // Always clean up stale files. - drop(std::fs::remove_file(&pid_path)); - drop(std::fs::remove_file(socket_path())); - if pid.is_some() && !is_quiet() { - println!("Daemon killed. PID file and socket cleaned up."); + if !is_quiet() { + println!("Killing daemon (PID {target_pid})..."); + } + + if kill_pid(target_pid) { + // Confirmed gone β€” safe to drop the discovery files. + drop(std::fs::remove_file(&pid_path)); + drop(std::fs::remove_file(socket_path())); + if !is_quiet() { + println!("Daemon killed. PID file and socket cleaned up."); + } + return Ok(()); } + + // Still alive β†’ do NOT remove the PID file (no half-kill; keep it + // discoverable). Almost always: an elevated daemon vs a non-elevated shell. + #[cfg(windows)] + anyhow::bail!( + "Could not terminate the daemon (PID {target_pid}) β€” it is still running.\n\n\ + It was most likely started from an elevated (Administrator) terminal, so a\n\ + non-elevated process cannot stop it. The PID file was left in place so the\n\ + daemon stays discoverable.\n\n\ + Re-run from an elevated terminal:\n\ + \x20 uffs --daemon kill" + ); + #[cfg(not(windows))] + anyhow::bail!( + "Could not terminate the daemon (PID {target_pid}) β€” it is still running.\n\n\ + It was most likely started as root, so a non-root process cannot stop it.\n\ + The PID file was left in place so the daemon stays discoverable.\n\n\ + Re-run with elevated privileges:\n\ + \x20 sudo uffs --daemon kill" + ); } -/// Send SIGKILL (Unix) or taskkill (Windows) to a process. -fn kill_pid(pid: u32) { +/// Terminate `pid` (SIGKILL on Unix, `taskkill /F` on Windows), then +/// **confirm** it is actually gone. Returns `true` only when the process is no +/// longer alive. +/// +/// The OS call's exit code is unreliable for our case β€” a non-elevated +/// `taskkill` of an elevated daemon is denied β€” so we verify via +/// [`uffs_client::daemon_ctl::is_pid_alive`], polling briefly (a `/F` kill is +/// near-instant but not synchronous). +fn kill_pid(pid: u32) -> bool { #[cfg(unix)] - { - drop( - std::process::Command::new("kill") - .args(["-9", &pid.to_string()]) - .output(), - ); - } + drop( + std::process::Command::new("kill") + .args(["-9", &pid.to_string()]) + .output(), + ); #[cfg(windows)] - { - drop( - std::process::Command::new("taskkill") - .args(["/F", "/PID", &pid.to_string()]) - .output(), - ); + drop( + std::process::Command::new("taskkill") + .args(["/F", "/PID", &pid.to_string()]) + .output(), + ); + + // Poll for the process to disappear (fast path returns on the first check). + for _ in 0..10_u8 { + if !uffs_client::daemon_ctl::is_pid_alive(pid) { + return true; + } + std::thread::sleep(core::time::Duration::from_millis(100)); } + false } /// `uffs --daemon restart` β€” stop, capture data sources, then re-launch. diff --git a/crates/uffs-client/src/daemon_ctl.rs b/crates/uffs-client/src/daemon_ctl.rs index 4098078ce..86a8eea4f 100644 --- a/crates/uffs-client/src/daemon_ctl.rs +++ b/crates/uffs-client/src/daemon_ctl.rs @@ -261,6 +261,46 @@ pub fn parse_pid_file(path: &std::path::Path) -> Option<(u32, u64, u64, String)> Some((pid, ts, hash, nonce)) } +/// Whether a process with `pid` is currently alive. +/// +/// A bare liveness probe used by `uffs --daemon kill` to confirm a terminate +/// actually took effect **before** the PID file is removed. A non-elevated +/// `taskkill /F` against an *elevated* daemon fails silently (Access denied), +/// so the caller must verify rather than trust the exit code β€” otherwise it +/// deletes the PID file while the process keeps running (an orphaned daemon). +/// +/// Unix uses `kill(pid, 0)` (existence check, no signal delivered). Windows +/// filters `tasklist` by PID and confirms the image is `uffsd`, so a recycled +/// PID that is now some *other* process reads as "not our daemon" β†’ dead. +#[must_use] +pub fn is_pid_alive(pid: u32) -> bool { + #[cfg(unix)] + { + // Unix PIDs fit in i32 by spec; the saturating fallback is unreachable. + let pid_i32 = i32::try_from(pid).unwrap_or(i32::MAX); + // SAFETY: `kill(pid, 0)` performs a permission/existence check without + // delivering a signal; `pid` comes from our own PID file. + #[expect(unsafe_code, reason = "kill(pid, 0) is a safe existence check")] + let rc = unsafe { libc::kill(pid_i32, 0) }; + rc == 0 + } + #[cfg(not(unix))] + { + std::process::Command::new("tasklist") + .args(["/FI", &format!("PID eq {pid}"), "/NH"]) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::null()) + .output() + .is_ok_and(|output| { + // AUDIT-OK(bytes): liveness probe via substring match; a lossy + // decode can only FAIL the match β†’ reads as "not alive" + // (fail-safe: we never wrongly claim a live daemon is dead). + let text = String::from_utf8_lossy(&output.stdout); + text.contains(&pid.to_string()) && text.contains("uffsd") + }) + } +} + /// Find the `uffs` CLI executable. /// /// The `$PATH` fallback carries the `.exe` extension on Windows so a bare From c2ca8cca70d3425eba04802b5a21d394e7ce8285 Mon Sep 17 00:00:00 2001 From: Robert M1 <50460704+githubrobbi@users.noreply.github.com> Date: Tue, 7 Jul 2026 22:16:27 -0700 Subject: [PATCH 2/6] fix(update): elevation gate up front + prune, no mid-flow rollback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `uffs --update` hit a privilege catch-22: an elevated shell can't run `winget upgrade` (winget refuses a user package elevated), while a non-elevated shell can't stop an elevated daemon β€” so quiesce shelled `daemon stop`, ignored the gated failure, waited 20s, and rolled the whole update back on an install the caller could never have updated. Mirror the uninstall flow instead: build a per-root plan, run a shared elevation gate up front (continue-without / abort), then feed the journaled execute a report PRUNED to just the doable roots so quiesce/apply/winget never touch a dropped root. - New shared `commands::elevation` gate (trait `ElevatablePlan`): surface admin-only work once, before any mutation. Update consumes it; the working uninstall flow is deliberately left on its own gate for now (migrating it is a safe follow-up β€” no risk to a flow that works). - New `commands::update::plan` β€” per-root classification + elevation model: unmanagedβ†’writable-probe, winget-userβ†’must-run-non-elevated (the winget inversion, surfaced as a delegation note, not the gate), winget-machineβ†’needs elevation, dev-build/unknownβ†’skip. + unit tests. Also holds the plan render/prune helpers (keeps update/mod.rs under the 800-LOC ceiling). - `run_automatic_update`: gate β†’ split winget-when-elevated β†’ prune report β†’ reuse the existing acquire/quiesce/apply/winget execute. Compile-verified native + x86_64-pc-windows-msvc; 144 uffs-cli tests green. Windows runtime behavior pending user validation. Co-Authored-By: Claude Opus 4.8 --- crates/uffs-cli/src/commands.rs | 4 + crates/uffs-cli/src/commands/elevation.rs | 105 ++++++ crates/uffs-cli/src/commands/update/mod.rs | 95 +++-- crates/uffs-cli/src/commands/update/plan.rs | 386 ++++++++++++++++++++ 4 files changed, 559 insertions(+), 31 deletions(-) create mode 100644 crates/uffs-cli/src/commands/elevation.rs create mode 100644 crates/uffs-cli/src/commands/update/plan.rs diff --git a/crates/uffs-cli/src/commands.rs b/crates/uffs-cli/src/commands.rs index bdfeb3412..0bc69ba3b 100644 --- a/crates/uffs-cli/src/commands.rs +++ b/crates/uffs-cli/src/commands.rs @@ -23,6 +23,10 @@ pub(crate) mod daemon_status; /// under the 800-LOC policy ceiling. Forward-looking: 8-D `forget` /// and 8-E `status_drives` will land their shims here as well. pub(crate) mod daemon_tiering; +/// Shared elevation gate for the mutating flows (uninstall / update): surface +/// admin-only work up front and decide once (elevate / continue-without / +/// abort) instead of failing mid-flow. Keeps both flows' elevation UX aligned. +pub(crate) mod elevation; // Index and info subcommands were merged into other modules. /// MCP server management subcommands. pub(crate) mod mcp_mgmt; diff --git a/crates/uffs-cli/src/commands/elevation.rs b/crates/uffs-cli/src/commands/elevation.rs new file mode 100644 index 000000000..3604053c0 --- /dev/null +++ b/crates/uffs-cli/src/commands/elevation.rs @@ -0,0 +1,105 @@ +// SPDX-License-Identifier: MPL-2.0 +// Copyright (c) 2025-2026 SKY, LLC. + +//! Shared elevation gate for the mutating flows. +//! +//! A non-elevated run that has admin-only work is told **up front** what needs +//! Administrator and decides once β€” continue without the admin items (they are +//! dropped so the summary never lists work that will not happen), or abort and +//! re-run from an elevated terminal. Surfacing this before any mutation is what +//! keeps a flow from failing halfway through (the uninstall flow pioneered it; +//! `uffs --update` reuses this gate so both behave alike). +//! +//! Unlike the uninstall flow β€” which has a one-shot elevated helper it can +//! spawn via a single UAC prompt at removal time β€” the update flow has no +//! in-flow elevation (and `winget` itself *refuses* to upgrade a user package +//! from an elevated shell), so the elevated path here is simply "re-run +//! elevated". The choice is therefore binary on every platform. + +use anyhow::{Context as _, Result, bail}; + +/// A plan the elevation gate can reason about: some of its work needs +/// Administrator, and those items can be dropped to "continue without". +pub(crate) trait ElevatablePlan { + /// Whether any item in the plan needs elevation. + fn requires_elevation(&self) -> bool; + + /// Drop the elevation-requiring items, returning their human-readable + /// descriptions (for the "not done in this run" summary). + fn drop_elevation_required(&mut self) -> Vec; + + /// Print the flow-specific "these items need Administrator" preamble. + fn render_elevation_needed(&self); +} + +/// Flow-specific wording woven into the gate prompt. +pub(crate) struct GateWording { + /// The command to re-run elevated β€” `"uffs --update"` / `"uffs + /// --uninstall"`. + pub(crate) rerun_cmd: &'static str, +} + +/// What the elevation gate decided for this run. +pub(crate) enum ElevationChoice { + /// Elevated, dry-run, or nothing needs Administrator β€” plan untouched. + NotNeeded, + /// Continuing without the admin items: they were dropped from the plan; + /// carries their descriptions for the final summary. + ContinueWithout(Vec), +} + +/// The elevation gate β€” the first question, before any mutation. Returns +/// [`ElevationChoice::NotNeeded`] when elevated, under `dry_run`, or when +/// nothing needs Administrator. `assume_yes` continues without asking (a +/// scripted run must never block on a prompt it cannot answer). +/// +/// # Errors +/// +/// Propagates a prompt I/O error, or aborts (bails) when the user declines. +pub(crate) fn elevation_gate( + plan: &mut impl ElevatablePlan, + dry_run: bool, + assume_yes: bool, + wording: &GateWording, +) -> Result { + if dry_run || !plan.requires_elevation() || uffs_mft::platform::is_elevated() { + return Ok(ElevationChoice::NotNeeded); + } + plan.render_elevation_needed(); + if assume_yes { + return Ok(ElevationChoice::ContinueWithout( + plan.drop_elevation_required(), + )); + } + let choice = prompt_line( + "\n c = continue without it (the item(s) above are left as-is)\n\ + \x20 a = abort\n\n\ + Choice [c/A]: ", + )?; + match choice.as_str() { + "c" | "continue" => Ok(ElevationChoice::ContinueWithout( + plan.drop_elevation_required(), + )), + _ => bail!( + "aborted β€” re-run `{}` from an elevated (Administrator) terminal to include the item(s) above", + wording.rerun_cmd + ), + } +} + +/// Read a line from stdin after printing `prompt`, trimmed + lowercased. +/// +/// # Errors +/// +/// Fails if stdout cannot be flushed or stdin cannot be read. +#[expect(clippy::print_stdout, reason = "interactive CLI prompt")] +fn prompt_line(prompt: &str) -> Result { + use std::io::Write as _; + print!("{prompt}"); + std::io::stdout().flush().context("flushing the prompt")?; + let mut line = String::new(); + std::io::stdin() + .read_line(&mut line) + .context("reading input")?; + Ok(line.trim().to_ascii_lowercase()) +} diff --git a/crates/uffs-cli/src/commands/update/mod.rs b/crates/uffs-cli/src/commands/update/mod.rs index 1eb45ea3d..18bfb3672 100644 --- a/crates/uffs-cli/src/commands/update/mod.rs +++ b/crates/uffs-cli/src/commands/update/mod.rs @@ -26,6 +26,7 @@ pub(crate) mod binaries; pub(crate) mod channel; mod doctor; pub(crate) mod model; +mod plan; pub(crate) mod procinfo; mod report; mod self_heal; @@ -38,6 +39,8 @@ use std::path::{Path, PathBuf}; use anyhow::{Result, bail}; use model::{Anchor, Channel, Component, DetectionReport, InstallRoot, RunningProcess, Scope}; +use crate::commands::elevation; + /// Run `uffs --update []` β€” uniform `-- [action] [--options]` /// grammar (design: `docs/architecture/cli-grammar.md`). The action is the /// first positional token: @@ -140,7 +143,7 @@ pub(crate) fn run_update(args: &[String]) -> Result<()> { // release feed). With `--repair` we run it; interactively we ask; piped // we just point. if !args.iter().any(|arg| arg == "--offline") - && matches!(assess(&report), UpdatePlan::Available { .. }) + && matches!(assess(&report), UpdateAssessment::Available { .. }) { if repair || prompt_yes_no("Run `uffs --update` now to fix this?") { return run_automatic_update(&report, verbose); @@ -189,7 +192,7 @@ pub(crate) fn run_update(args: &[String]) -> Result<()> { } /// Whether an update is warranted, and the target tag. -enum UpdatePlan { +enum UpdateAssessment { /// Installed matches the latest release and there is no version skew. UpToDate { /// The latest release tag (e.g. `v0.6.5`). @@ -207,14 +210,14 @@ enum UpdatePlan { /// Compare the detected install against the latest release (one non-mutating /// metadata fetch via the helper) to decide whether an update is warranted. -fn assess(report: &DetectionReport) -> UpdatePlan { +fn assess(report: &DetectionReport) -> UpdateAssessment { let installed = report::distinct_versions(report); let skewed = installed.len() > 1; // A core binary missing from a real install root makes it *incomplete* β€” // an update reconciles the full core set back into place. let incomplete = has_missing_core(report); let Some(latest) = acquire::latest_version() else { - return UpdatePlan::Offline; + return UpdateAssessment::Offline; }; let newer = match installed.as_slice() { // A single clean version: update only if the release is different. @@ -223,9 +226,9 @@ fn assess(report: &DetectionReport) -> UpdatePlan { _ => true, }; if skewed || newer || incomplete { - UpdatePlan::Available { latest } + UpdateAssessment::Available { latest } } else { - UpdatePlan::UpToDate { latest } + UpdateAssessment::UpToDate { latest } } } @@ -295,41 +298,71 @@ fn prompt_yes_no(question: &str) -> bool { /// Run the full end-to-end update when one is needed; otherwise report the /// install is current. Journaled + auto-rollback (delegated to `apply`). fn run_automatic_update(report: &DetectionReport, verbose: bool) -> Result<()> { - match assess(report) { - UpdatePlan::UpToDate { latest } => { + let latest = match assess(report) { + UpdateAssessment::UpToDate { latest } => { print_already_current(&latest); - Ok(()) + return Ok(()); } - UpdatePlan::Offline => { + UpdateAssessment::Offline => { print_offline_notice(); - Ok(()) - } - UpdatePlan::Available { latest } => { - print_updating(&latest); - let snapshot_path = snapshot::write_snapshot(report, Some(&latest))?; - acquire::spawn(&snapshot_path, None, verbose)?; - // Quiesce-first: on a winget-managed install, stop the daemon + - // (package) broker up front β€” ONE UAC β€” so BOTH the hand-rolled - // ~\bin update and the winget upgrade run against a stopped - // install (a bare `winget upgrade` fails on locked images). No-op - // for a plain unmanaged install (apply keeps its own daemon stop). - let quiesce = winget::quiesce(report)?; - apply::spawn(&snapshot_path, verbose)?; - let outcome = winget::run_upgrade(report, &latest, &quiesce)?; - winget::resume(quiesce, outcome); - print_updated(&latest); - Ok(()) + return Ok(()); } + UpdateAssessment::Available { latest } => latest, + }; + + // Mirror `uffs --uninstall`: build the per-root plan and run the SHARED + // elevation gate up front β€” surface admin-only roots, decide once + // (continue-without / abort) β€” so nothing fails mid-swap. The per-root + // elevation model lives in `plan`; the gate in `commands::elevation`. + let mut plan = plan::UpdatePlan::build(report); + if let elevation::ElevationChoice::ContinueWithout(dropped) = + elevation::elevation_gate(&mut plan, false, false, &elevation::GateWording { + rerun_cmd: "uffs --update", + })? + { + plan::print_skipped_elevation(&dropped); + } + + // Elevated shells cannot run `winget upgrade` for a user-scope package + // (winget refuses) β€” split those out as a delegation note instead of a + // mid-flow failure. + let delegated = plan.split_off_non_elevated_when_elevated(uffs_mft::platform::is_elevated()); + plan::print_winget_delegation(&delegated); + + if !plan.has_work() { + plan::print_no_local_work(); + return Ok(()); } + + // Feed the journaled execute a report PRUNED to just the roots the gated + // plan will actually touch, so quiesce/apply/winget never operate on a + // dropped (elevation-required or delegated) root β€” this removes the old + // stop-timeout β†’ rollback on an install the caller could never update. + let doable = plan::prune_report(report, &plan); + + print_updating(&latest); + let snapshot_path = snapshot::write_snapshot(&doable, Some(&latest))?; + acquire::spawn(&snapshot_path, None, verbose)?; + // Quiesce-first: on a winget-managed install, stop the daemon + (package) + // broker up front β€” ONE UAC β€” so BOTH the hand-rolled ~\bin update and the + // winget upgrade run against a stopped install (a bare `winget upgrade` + // fails on locked images). No-op for a plain unmanaged install (apply keeps + // its own daemon stop). + let quiesce = winget::quiesce(&doable)?; + apply::spawn(&snapshot_path, verbose)?; + let outcome = winget::run_upgrade(&doable, &latest, &quiesce)?; + winget::resume(quiesce, outcome); + print_updated(&latest); + Ok(()) } /// Print the verdict for the non-mutating `uffs --update check`. #[expect(clippy::print_stdout, reason = "CLI user-facing output")] -fn report_assessment(plan: &UpdatePlan) { +fn report_assessment(plan: &UpdateAssessment) { match plan { - UpdatePlan::UpToDate { latest } => println!("\n\u{2713} Up to date ({latest})."), - UpdatePlan::Offline => print_offline_notice(), - UpdatePlan::Available { latest } => { + UpdateAssessment::UpToDate { latest } => println!("\n\u{2713} Up to date ({latest})."), + UpdateAssessment::Offline => print_offline_notice(), + UpdateAssessment::Available { latest } => { println!("\n\u{2b06} Update available: {latest} β€” run `uffs --update` to install."); } } diff --git a/crates/uffs-cli/src/commands/update/plan.rs b/crates/uffs-cli/src/commands/update/plan.rs new file mode 100644 index 000000000..48bbb8e36 --- /dev/null +++ b/crates/uffs-cli/src/commands/update/plan.rs @@ -0,0 +1,386 @@ +// SPDX-License-Identifier: MPL-2.0 +// Copyright (c) 2025-2026 SKY, LLC. + +//! Update execution plan β€” the per-root "what will actually happen", derived +//! from Phase-A detection. This mirrors the uninstall flow's `RemovalPlan`: +//! every discovered root becomes an item with an action and a `needs_elevation` +//! verdict, so the shared elevation gate ([`crate::commands::elevation`]) can +//! surface admin-only work **up front** and the final summary never lists work +//! that will not happen. +//! +//! ## Elevation model (per root) +//! +//! | Root | Action | `needs_elevation` | +//! |------------------|------------------|---------------------------------------| +//! | dev-build | skip (never) | β€” (skipped) | +//! | unmanaged | replace in place | the dir is not writable by this user | +//! | winget (user) | `winget upgrade` | no β€” but must run **non-elevated** | +//! | winget (machine) | `winget upgrade` | yes | +//! | unknown | skip | β€” (skipped) | +//! +//! Stopping running components before the swap: the broker (a `LocalSystem` +//! service) always needs elevation; the daemon/mcp are stoppable without +//! elevation when broker-launched. The winget-user-scope-while-elevated case is +//! **not** an elevation item β€” winget *refuses* to upgrade a user package from +//! an elevated shell β€” so it is surfaced as a "run from a normal terminal" +//! delegation note, the inverse of the elevation gate. + +use std::path::Path; + +use super::model::{Channel, DetectionReport, InstallRoot, Scope}; +use crate::commands::elevation::ElevatablePlan; + +/// What the update will do to a given root. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum UpdateAction { + /// Replace the binaries in place from the freshly-downloaded release. + ReplaceBinaries, + /// Delegate to `winget upgrade` (a winget-managed root). + WingetUpgrade, + /// A dev-build (`target/{debug,release}`) tree β€” never auto-updated. + SkipDevBuild, + /// The path could not be classified β€” left untouched. + SkipUnknown, +} + +impl UpdateAction { + /// Whether this action actually mutates the root (vs. a skip). + pub(crate) const fn is_mutating(self) -> bool { + matches!(self, Self::ReplaceBinaries | Self::WingetUpgrade) + } + + /// Short human label for report output. + pub(crate) const fn label(self) -> &'static str { + match self { + Self::ReplaceBinaries => "replace binaries", + Self::WingetUpgrade => "winget upgrade", + Self::SkipDevBuild => "skip (dev-build)", + Self::SkipUnknown => "skip (unclassified)", + } + } +} + +/// One planned update action against a discovered root. +#[derive(Debug, Clone)] +pub(crate) struct UpdateItem { + /// The root directory this item targets (for display + apply). + pub(crate) dir: std::path::PathBuf, + /// The install channel that placed the binaries. + pub(crate) channel: Channel, + /// The install scope (meaningful for winget). + pub(crate) scope: Scope, + /// What will happen to this root. + pub(crate) action: UpdateAction, + /// True when the action cannot proceed without Administrator (a + /// non-writable target, or a machine-scope winget upgrade). + pub(crate) needs_elevation: bool, + /// True when this is a winget-user root that must be upgraded from a + /// **non-elevated** terminal β€” the inverse of `needs_elevation`. Surfaced + /// as a delegation note, never routed through the elevation gate. + pub(crate) needs_non_elevated: bool, +} + +impl UpdateItem { + /// Human-readable one-line description for the elevation gate / summary. + pub(crate) fn describe(&self) -> String { + format!( + "{} ({}{}) β€” {}", + self.dir.display(), + self.channel.label(), + match self.scope { + Scope::Unknown => String::new(), + scope @ (Scope::User | Scope::Machine) => format!("/{}", scope.label()), + }, + self.action.label(), + ) + } +} + +/// The full update execution plan: a per-root item list. The target tag is +/// carried by the caller ([`super::run_automatic_update`]), not duplicated +/// here. +#[derive(Debug, Clone)] +pub(crate) struct UpdatePlan { + /// Per-root planned actions. + pub(crate) items: Vec, +} + +impl UpdatePlan { + /// Build the plan by classifying every detected root. + pub(crate) fn build(report: &DetectionReport) -> Self { + let items = report.roots.iter().map(classify_root).collect::>(); + Self { items } + } + + /// The items that will actually mutate a root (drop the skips). + pub(crate) fn mutating_items(&self) -> impl Iterator { + self.items.iter().filter(|item| item.action.is_mutating()) + } + + /// When running **elevated**, split out the winget-user roots β€” `winget` + /// refuses to upgrade a user package from an elevated shell β€” returning + /// them for the "run from a normal terminal" delegation note and + /// removing them from the plan. Non-elevated: nothing to split (they + /// upgrade fine here). + pub(crate) fn split_off_non_elevated_when_elevated( + &mut self, + elevated: bool, + ) -> Vec { + if !elevated { + return Vec::new(); + } + let (split, keep): (Vec<_>, Vec<_>) = core::mem::take(&mut self.items) + .into_iter() + .partition(|item| item.needs_non_elevated); + self.items = keep; + split + } + + /// Whether the plan has any real work left after skips. + pub(crate) fn has_work(&self) -> bool { + self.mutating_items().next().is_some() + } +} + +/// Classify one root into a planned action + elevation verdict per the module's +/// elevation model. +fn classify_root(root: &InstallRoot) -> UpdateItem { + let (action, needs_elevation, needs_non_elevated) = match root.channel { + Channel::DevBuild => (UpdateAction::SkipDevBuild, false, false), + Channel::Unknown => (UpdateAction::SkipUnknown, false, false), + Channel::Unmanaged => ( + UpdateAction::ReplaceBinaries, + !dir_writable(&root.dir), + false, + ), + Channel::WinGet => match root.scope { + // winget refuses to upgrade a user package from an elevated shell, + // so a user root never *needs* elevation β€” it needs the absence of + // it. Machine scope is the opposite: winget requires Administrator. + Scope::Machine => (UpdateAction::WingetUpgrade, true, false), + Scope::User | Scope::Unknown => (UpdateAction::WingetUpgrade, false, true), + }, + }; + UpdateItem { + dir: root.dir.clone(), + channel: root.channel, + scope: root.scope, + action, + needs_elevation, + needs_non_elevated, + } +} + +/// Probe whether `dir` is writable by the current user without elevation, by +/// creating and immediately removing a uniquely-named temp file. A missing dir +/// or any I/O error is treated as "not writable" β€” the caller then routes the +/// item through the elevation gate rather than failing mid-apply. +fn dir_writable(dir: &Path) -> bool { + let probe = dir.join(format!(".uffs-write-probe-{}", std::process::id())); + std::fs::File::create(&probe).is_ok_and(|file| { + drop(file); + drop(std::fs::remove_file(&probe)); + true + }) +} + +impl ElevatablePlan for UpdatePlan { + fn requires_elevation(&self) -> bool { + self.items.iter().any(|item| item.needs_elevation) + } + + fn drop_elevation_required(&mut self) -> Vec { + let dropped = self + .items + .iter() + .filter(|item| item.needs_elevation) + .map(UpdateItem::describe) + .collect(); + self.items.retain(|item| !item.needs_elevation); + dropped + } + + fn render_elevation_needed(&self) { + render_elevation_needed(self); + } +} + +/// Print the "these roots need Administrator" preamble for the elevation gate. +#[expect(clippy::print_stdout, reason = "CLI user-facing output")] +fn render_elevation_needed(plan: &UpdatePlan) { + println!( + "\nThis terminal is not elevated (Administrator). The following can only\n\ + be updated from an elevated terminal:\n" + ); + for item in plan.items.iter().filter(|item| item.needs_elevation) { + println!(" - {}", item.describe()); + } +} + +/// Summary for roots dropped at the elevation gate (the user chose to continue +/// without them). Nothing is printed when none were dropped. +#[expect(clippy::print_stdout, reason = "CLI user-facing output")] +pub(crate) fn print_skipped_elevation(dropped: &[String]) { + if dropped.is_empty() { + return; + } + println!("\nLeft for an elevated run (re-run `uffs --update` as Administrator):"); + for item in dropped { + println!(" - {item}"); + } +} + +/// Note the winget-user roots that must be upgraded from a NON-elevated +/// terminal (winget refuses a user package when the shell is elevated). Nothing +/// is printed when there are none (i.e. this terminal is not elevated). +#[expect(clippy::print_stdout, reason = "CLI user-facing output")] +pub(crate) fn print_winget_delegation(delegated: &[UpdateItem]) { + if delegated.is_empty() { + return; + } + println!( + "\nThis terminal is elevated; `winget` will not upgrade a user-scope\n\ + package from an elevated shell. Re-run `uffs --update` from a normal\n\ + (non-elevated) terminal to update:" + ); + for item in delegated { + println!(" - {}", item.describe()); + } +} + +/// Printed when the gate + winget delegation left nothing this terminal can do. +#[expect(clippy::print_stdout, reason = "CLI user-facing output")] +pub(crate) fn print_no_local_work() { + println!("\nNothing left to update in this terminal β€” see the note(s) above."); +} + +/// A detection report pruned to just the roots the gated `plan` will touch, so +/// the journaled execute (snapshot β†’ quiesce β†’ apply β†’ winget) never operates +/// on a dropped (elevation-required or winget-delegated) root. +pub(crate) fn prune_report(report: &DetectionReport, plan: &UpdatePlan) -> DetectionReport { + let keep: std::collections::HashSet<&Path> = plan + .mutating_items() + .map(|item| item.dir.as_path()) + .collect(); + let roots = report + .roots + .iter() + .filter(|root| keep.contains(root.dir.as_path())) + .cloned() + .collect(); + let running = report + .running + .iter() + .filter(|process| { + process + .image_path + .as_deref() + .and_then(Path::parent) + .is_some_and(|dir| keep.contains(dir)) + }) + .cloned() + .collect(); + DetectionReport { roots, running } +} + +#[cfg(test)] +mod tests { + use super::{UpdateAction, UpdatePlan, classify_root}; + use crate::commands::elevation::ElevatablePlan as _; + use crate::commands::update::model::{Channel, InstallRoot, Scope}; + + /// A root with the given channel/scope and no binaries/anchors. + fn root(dir: &str, channel: Channel, scope: Scope) -> InstallRoot { + InstallRoot { + dir: dir.into(), + channel, + scope, + anchored_by: Vec::new(), + binaries: Vec::new(), + } + } + + /// A path string for a directory the current user can write without + /// elevation (the OS temp dir), so `dir_writable` returns `true`. + fn writable_dir() -> String { + std::env::temp_dir().to_string_lossy().into_owned() + } + + #[test] + fn dev_build_and_unknown_are_skipped() { + let dev = classify_root(&root("/x", Channel::DevBuild, Scope::Unknown)); + assert_eq!(dev.action, UpdateAction::SkipDevBuild); + assert!(!dev.needs_elevation); + assert!(!dev.needs_non_elevated); + + let unknown = classify_root(&root("/x", Channel::Unknown, Scope::Unknown)); + assert_eq!(unknown.action, UpdateAction::SkipUnknown); + } + + #[test] + fn winget_user_needs_non_elevated_never_elevation() { + let item = classify_root(&root("/x", Channel::WinGet, Scope::User)); + assert_eq!(item.action, UpdateAction::WingetUpgrade); + assert!(item.needs_non_elevated, "user scope must run non-elevated"); + assert!( + !item.needs_elevation, + "winget refuses a user package elevated" + ); + } + + #[test] + fn winget_machine_needs_elevation() { + let item = classify_root(&root("/x", Channel::WinGet, Scope::Machine)); + assert!(item.needs_elevation); + assert!(!item.needs_non_elevated); + } + + #[test] + fn unmanaged_writable_dir_needs_no_elevation() { + let item = classify_root(&root(&writable_dir(), Channel::Unmanaged, Scope::Unknown)); + assert_eq!(item.action, UpdateAction::ReplaceBinaries); + assert!(!item.needs_elevation); + } + + #[test] + fn split_off_non_elevated_only_when_elevated() { + let build = || UpdatePlan { + items: vec![ + classify_root(&root("/a", Channel::WinGet, Scope::User)), + classify_root(&root(&writable_dir(), Channel::Unmanaged, Scope::Unknown)), + ], + }; + + // Non-elevated: nothing is split off β€” winget upgrades fine here. + let mut plan_non_elevated = build(); + assert!( + plan_non_elevated + .split_off_non_elevated_when_elevated(false) + .is_empty() + ); + assert_eq!(plan_non_elevated.items.len(), 2); + + // Elevated: the winget-user item moves to the delegation list. + let mut plan_elevated = build(); + let split = plan_elevated.split_off_non_elevated_when_elevated(true); + assert_eq!(split.len(), 1); + assert_eq!(plan_elevated.items.len(), 1); + } + + #[test] + fn drop_elevation_required_prunes_but_keeps_doable_work() { + let mut plan = UpdatePlan { + items: vec![ + classify_root(&root("/m", Channel::WinGet, Scope::Machine)), + classify_root(&root(&writable_dir(), Channel::Unmanaged, Scope::Unknown)), + ], + }; + assert!(plan.requires_elevation()); + assert_eq!(plan.drop_elevation_required().len(), 1); + assert!(!plan.requires_elevation()); + assert!( + plan.has_work(), + "the writable unmanaged root is still doable" + ); + } +} From 0ff4c50936f60b58cb72584ef1b5bbb5dff108c1 Mon Sep 17 00:00:00 2001 From: Robert M1 <50460704+githubrobbi@users.noreply.github.com> Date: Wed, 8 Jul 2026 05:51:02 -0700 Subject: [PATCH 3/6] fix(update): model daemon-stop elevation + skip already-current roots MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Windows testing exposed the real gap: a non-elevated `uffs --update` with an elevated daemon running from `C:\Users\rnio\bin` still hit the 20s stop-timeout β†’ rollback. The plan judged `~\bin` "doable" purely from dir-writability, so it never saw that the elevated daemon there must be stopped first (which needs Administrator) β€” the gate stayed silent, `apply` shelled `daemon stop`, hit the gate prose, and the helper's own quiesce blindly waited 20s before rolling back. Two rules the plan was missing: 1. **Stop cost is the root's cost** β€” `classify_root` now checks whether a component running from the root can be stopped without elevation, reusing fix #1's exact gate (`mutating_management_needs_elevation`, now `pub(crate)`): broker β†’ always needs elevation; daemon/mcp β†’ needs it when the daemon is elevated with no serving broker. If so, the root is elevation-required and the gate surfaces it up front. 2. **Already current** β€” a root whose every binary is at the target version is a pure skip, so an already-current `~\bin` hosting an elevated daemon is never dragged in (and its daemon never stopped). `UpdatePlan::build` now takes `latest` for the per-root version compare. Also: `run_automatic_update` skips `apply` entirely when the pruned plan has no unmanaged root, so no daemon is ever stopped for work that isn't there. +2 unit tests covering the exact scenario (elevated daemon in an already-current root β†’ skip; out-of-date root with an unstoppable daemon β†’ forces elevation). 146 uffs-cli tests green; native + windows-msvc clippy + rustdoc clean. Windows runtime behavior pending re-validation. Co-Authored-By: Claude Opus 4.8 --- crates/uffs-cli/src/commands/daemon_mgmt.rs | 6 +- crates/uffs-cli/src/commands/update/mod.rs | 17 +- crates/uffs-cli/src/commands/update/plan.rs | 209 +++++++++++++++++--- 3 files changed, 195 insertions(+), 37 deletions(-) diff --git a/crates/uffs-cli/src/commands/daemon_mgmt.rs b/crates/uffs-cli/src/commands/daemon_mgmt.rs index 72d23c995..4bc747d41 100644 --- a/crates/uffs-cli/src/commands/daemon_mgmt.rs +++ b/crates/uffs-cli/src/commands/daemon_mgmt.rs @@ -180,7 +180,7 @@ pub(crate) fn daemon(action: &DaemonAction) -> Result<()> { /// (Windows uses a broker-aware variant β€” see the `#[cfg(windows)]` impl /// below.) #[cfg(unix)] -fn mutating_management_needs_elevation() -> bool { +pub(crate) fn mutating_management_needs_elevation() -> bool { daemon_owner_needs_elevation(&pid_file_path(), uffs_mft::current_euid()) } @@ -211,7 +211,7 @@ fn daemon_owner_needs_elevation(pid_file: &std::path::Path, caller_euid: u32) -> /// /// Otherwise (an elevated daemon, no broker) managing it needs Administrator. #[cfg(windows)] -fn mutating_management_needs_elevation() -> bool { +pub(crate) fn mutating_management_needs_elevation() -> bool { /// Short pipe probe β€” this gate runs once per management command. const BROKER_GATE_PROBE_MS: u32 = 600; @@ -244,7 +244,7 @@ fn launch_state_says_non_elevated(pid_path: &std::path::Path) -> bool { /// Other non-Unix targets (WASM, bare-metal β€” not real deployments): keep the /// conservative default of always requiring elevation. #[cfg(not(any(unix, windows)))] -const fn mutating_management_needs_elevation() -> bool { +pub(crate) const fn mutating_management_needs_elevation() -> bool { true } diff --git a/crates/uffs-cli/src/commands/update/mod.rs b/crates/uffs-cli/src/commands/update/mod.rs index 18bfb3672..a12441f08 100644 --- a/crates/uffs-cli/src/commands/update/mod.rs +++ b/crates/uffs-cli/src/commands/update/mod.rs @@ -314,7 +314,7 @@ fn run_automatic_update(report: &DetectionReport, verbose: bool) -> Result<()> { // elevation gate up front β€” surface admin-only roots, decide once // (continue-without / abort) β€” so nothing fails mid-swap. The per-root // elevation model lives in `plan`; the gate in `commands::elevation`. - let mut plan = plan::UpdatePlan::build(report); + let mut plan = plan::UpdatePlan::build(report, &latest); if let elevation::ElevationChoice::ContinueWithout(dropped) = elevation::elevation_gate(&mut plan, false, false, &elevation::GateWording { rerun_cmd: "uffs --update", @@ -340,6 +340,15 @@ fn run_automatic_update(report: &DetectionReport, verbose: bool) -> Result<()> { // stop-timeout β†’ rollback on an install the caller could never update. let doable = plan::prune_report(report, &plan); + // Only hand-placed (unmanaged) roots go through the in-process replace + // (`apply`, which stops the daemon). If the gate/prune left only winget + // roots, skip `apply` entirely so no daemon is stopped for work that isn't + // there β€” the crux of the elevated-daemon 20s-rollback fix. + let has_unmanaged = doable + .roots + .iter() + .any(|root| matches!(root.channel, Channel::Unmanaged)); + print_updating(&latest); let snapshot_path = snapshot::write_snapshot(&doable, Some(&latest))?; acquire::spawn(&snapshot_path, None, verbose)?; @@ -349,7 +358,9 @@ fn run_automatic_update(report: &DetectionReport, verbose: bool) -> Result<()> { // fails on locked images). No-op for a plain unmanaged install (apply keeps // its own daemon stop). let quiesce = winget::quiesce(&doable)?; - apply::spawn(&snapshot_path, verbose)?; + if has_unmanaged { + apply::spawn(&snapshot_path, verbose)?; + } let outcome = winget::run_upgrade(&doable, &latest, &quiesce)?; winget::resume(quiesce, outcome); print_updated(&latest); @@ -369,7 +380,7 @@ fn report_assessment(plan: &UpdateAssessment) { } /// Strip a leading `v` from a release tag so `v0.6.5` compares to `0.6.5`. -fn normalize_tag(tag: &str) -> &str { +pub(crate) fn normalize_tag(tag: &str) -> &str { tag.strip_prefix('v').unwrap_or(tag) } diff --git a/crates/uffs-cli/src/commands/update/plan.rs b/crates/uffs-cli/src/commands/update/plan.rs index 48bbb8e36..7736a32fa 100644 --- a/crates/uffs-cli/src/commands/update/plan.rs +++ b/crates/uffs-cli/src/commands/update/plan.rs @@ -10,24 +10,35 @@ //! //! ## Elevation model (per root) //! -//! | Root | Action | `needs_elevation` | -//! |------------------|------------------|---------------------------------------| -//! | dev-build | skip (never) | β€” (skipped) | -//! | unmanaged | replace in place | the dir is not writable by this user | -//! | winget (user) | `winget upgrade` | no β€” but must run **non-elevated** | -//! | winget (machine) | `winget upgrade` | yes | -//! | unknown | skip | β€” (skipped) | +//! | Root | Action | `needs_elevation` | +//! |-------------------|------------------|--------------------------------------| +//! | already current | skip | β€” (skipped, no daemon stop) | +//! | dev-build/unknown | skip (never) | β€” (skipped) | +//! | unmanaged | replace in place | dir not writable **or** a component here can't be stopped without elevation | +//! | winget (user) | `winget upgrade` | no β€” but must run **non-elevated** | +//! | winget (machine) | `winget upgrade` | yes | //! -//! Stopping running components before the swap: the broker (a `LocalSystem` -//! service) always needs elevation; the daemon/mcp are stoppable without -//! elevation when broker-launched. The winget-user-scope-while-elevated case is -//! **not** an elevation item β€” winget *refuses* to upgrade a user package from -//! an elevated shell β€” so it is surfaced as a "run from a normal terminal" -//! delegation note, the inverse of the elevation gate. +//! Two rules keep an install the caller can't fully touch from failing mid-swap +//! (the elevated-daemon 20 s-rollback bug): +//! +//! 1. **Already current** β€” a root whose every binary is at the target version +//! is a pure skip, so an already-current `~\bin` hosting an *elevated* +//! daemon is never dragged in (its daemon is never stopped). +//! 2. **Stop cost is the root's cost** β€” replacing/upgrading a root means +//! stopping any component running from it first. The broker (a `LocalSystem` +//! service) always needs elevation; the daemon/mcp inherit fix #1's gate +//! (`mutating_management_needs_elevation` β€” an elevated daemon with no +//! serving broker needs Administrator). If the stop needs elevation, the +//! root does. +//! +//! The winget-user-scope-while-elevated case is **not** an elevation item β€” +//! winget *refuses* to upgrade a user package from an elevated shell β€” so it is +//! surfaced as a "run from a normal terminal" delegation note, the inverse of +//! the elevation gate. use std::path::Path; -use super::model::{Channel, DetectionReport, InstallRoot, Scope}; +use super::model::{Channel, Component, DetectionReport, InstallRoot, Scope}; use crate::commands::elevation::ElevatablePlan; /// What the update will do to a given root. @@ -41,6 +52,9 @@ pub(crate) enum UpdateAction { SkipDevBuild, /// The path could not be classified β€” left untouched. SkipUnknown, + /// Every binary here is already at the target version β€” nothing to do (and, + /// crucially, no running daemon here needs stopping). + AlreadyCurrent, } impl UpdateAction { @@ -56,6 +70,7 @@ impl UpdateAction { Self::WingetUpgrade => "winget upgrade", Self::SkipDevBuild => "skip (dev-build)", Self::SkipUnknown => "skip (unclassified)", + Self::AlreadyCurrent => "skip (already current)", } } } @@ -106,9 +121,19 @@ pub(crate) struct UpdatePlan { } impl UpdatePlan { - /// Build the plan by classifying every detected root. - pub(crate) fn build(report: &DetectionReport) -> Self { - let items = report.roots.iter().map(classify_root).collect::>(); + /// Build the plan by classifying every detected root against `latest`. + pub(crate) fn build(report: &DetectionReport, latest: &str) -> Self { + let latest_norm = super::normalize_tag(latest); + let elevated = uffs_mft::platform::is_elevated(); + // fix #1's exact gate: can this terminal stop/restart the resident + // daemon at all? An elevated daemon with no serving broker β†’ no. + let daemon_stop_needs_elev = + crate::commands::daemon_mgmt::mutating_management_needs_elevation(); + let items = report + .roots + .iter() + .map(|root| classify_root(root, latest_norm, report, elevated, daemon_stop_needs_elev)) + .collect::>(); Self { items } } @@ -144,21 +169,43 @@ impl UpdatePlan { /// Classify one root into a planned action + elevation verdict per the module's /// elevation model. -fn classify_root(root: &InstallRoot) -> UpdateItem { +fn classify_root( + root: &InstallRoot, + latest_norm: &str, + report: &DetectionReport, + elevated: bool, + daemon_stop_needs_elev: bool, +) -> UpdateItem { + // Already at the target version β†’ never touched, and (crucially) its running + // daemon is never stopped. This is what keeps an already-current `~\bin` + // hosting an elevated daemon out of the update entirely. + if root_already_current(root, latest_norm) { + return skip_item(root, UpdateAction::AlreadyCurrent); + } + + // Replacing/upgrading a root means stopping any component running from it + // first β€” so if that stop needs elevation, the whole root does. + let stop_needs_elev = root_stop_needs_elevation(root, report, elevated, daemon_stop_needs_elev); + let (action, needs_elevation, needs_non_elevated) = match root.channel { Channel::DevBuild => (UpdateAction::SkipDevBuild, false, false), Channel::Unknown => (UpdateAction::SkipUnknown, false, false), Channel::Unmanaged => ( UpdateAction::ReplaceBinaries, - !dir_writable(&root.dir), + stop_needs_elev || !dir_writable(&root.dir), false, ), Channel::WinGet => match root.scope { // winget refuses to upgrade a user package from an elevated shell, // so a user root never *needs* elevation β€” it needs the absence of // it. Machine scope is the opposite: winget requires Administrator. + // A component that needs elevation to stop overrides either way. Scope::Machine => (UpdateAction::WingetUpgrade, true, false), - Scope::User | Scope::Unknown => (UpdateAction::WingetUpgrade, false, true), + Scope::User | Scope::Unknown => ( + UpdateAction::WingetUpgrade, + stop_needs_elev, + !stop_needs_elev, + ), }, }; UpdateItem { @@ -171,6 +218,48 @@ fn classify_root(root: &InstallRoot) -> UpdateItem { } } +/// A non-mutating item (a skip) β€” never needs elevation either way. +fn skip_item(root: &InstallRoot, action: UpdateAction) -> UpdateItem { + UpdateItem { + dir: root.dir.clone(), + channel: root.channel, + scope: root.scope, + action, + needs_elevation: false, + needs_non_elevated: false, + } +} + +/// Whether every binary in `root` already reports the target version β€” an +/// update would be a pointless swap (and a pointless daemon stop). +fn root_already_current(root: &InstallRoot, latest_norm: &str) -> bool { + !root.binaries.is_empty() + && root + .binaries + .iter() + .all(|bin| bin.version.as_deref() == Some(latest_norm)) +} + +/// Whether a component running from `root` cannot be stopped without elevation: +/// the broker (a `LocalSystem` service) always needs it; the daemon/mcp inherit +/// fix #1's gate (`daemon_stop_needs_elev` β€” an elevated daemon with no serving +/// broker). The root must be stopped before it can be replaced, so this cost is +/// the root's cost. +fn root_stop_needs_elevation( + root: &InstallRoot, + report: &DetectionReport, + elevated: bool, + daemon_stop_needs_elev: bool, +) -> bool { + report.running.iter().any(|process| { + process.image_path.as_deref().and_then(Path::parent) == Some(root.dir.as_path()) + && match process.component { + Component::Broker => !elevated, + Component::Daemon | Component::Mcp => daemon_stop_needs_elev, + } + }) +} + /// Probe whether `dir` is writable by the current user without elevation, by /// creating and immediately removing a uniquely-named temp file. A missing dir /// or any I/O error is treated as "not writable" β€” the caller then routes the @@ -285,9 +374,11 @@ pub(crate) fn prune_report(report: &DetectionReport, plan: &UpdatePlan) -> Detec #[cfg(test)] mod tests { - use super::{UpdateAction, UpdatePlan, classify_root}; + use super::{UpdateAction, UpdateItem, UpdatePlan, classify_root}; use crate::commands::elevation::ElevatablePlan as _; - use crate::commands::update::model::{Channel, InstallRoot, Scope}; + use crate::commands::update::model::{ + BinaryInfo, Channel, Component, DetectionReport, InstallRoot, RunningProcess, Scope, + }; /// A root with the given channel/scope and no binaries/anchors. fn root(dir: &str, channel: Channel, scope: Scope) -> InstallRoot { @@ -300,26 +391,54 @@ mod tests { } } + /// One binary at a version, for the already-current check. + fn bin(name: &str, version: &str) -> BinaryInfo { + BinaryInfo { + name: name.to_owned(), + version: Some(version.to_owned()), + } + } + + /// A report with a single daemon running from `dir`. + fn report_with_daemon(dir: &std::path::Path) -> DetectionReport { + DetectionReport { + roots: Vec::new(), + running: vec![RunningProcess { + component: Component::Daemon, + pid: 1, + image_path: Some(dir.join("uffsd.exe")), + command_line: None, + version: None, + }], + } + } + /// A path string for a directory the current user can write without /// elevation (the OS temp dir), so `dir_writable` returns `true`. fn writable_dir() -> String { std::env::temp_dir().to_string_lossy().into_owned() } + /// Classify with no running components, non-elevated, and a target no empty + /// root can already match β€” the pure channel/scope path. + fn classify(root: &InstallRoot) -> UpdateItem { + classify_root(root, "9.9.9", &DetectionReport::default(), false, false) + } + #[test] fn dev_build_and_unknown_are_skipped() { - let dev = classify_root(&root("/x", Channel::DevBuild, Scope::Unknown)); + let dev = classify(&root("/x", Channel::DevBuild, Scope::Unknown)); assert_eq!(dev.action, UpdateAction::SkipDevBuild); assert!(!dev.needs_elevation); assert!(!dev.needs_non_elevated); - let unknown = classify_root(&root("/x", Channel::Unknown, Scope::Unknown)); + let unknown = classify(&root("/x", Channel::Unknown, Scope::Unknown)); assert_eq!(unknown.action, UpdateAction::SkipUnknown); } #[test] fn winget_user_needs_non_elevated_never_elevation() { - let item = classify_root(&root("/x", Channel::WinGet, Scope::User)); + let item = classify(&root("/x", Channel::WinGet, Scope::User)); assert_eq!(item.action, UpdateAction::WingetUpgrade); assert!(item.needs_non_elevated, "user scope must run non-elevated"); assert!( @@ -330,24 +449,52 @@ mod tests { #[test] fn winget_machine_needs_elevation() { - let item = classify_root(&root("/x", Channel::WinGet, Scope::Machine)); + let item = classify(&root("/x", Channel::WinGet, Scope::Machine)); assert!(item.needs_elevation); assert!(!item.needs_non_elevated); } #[test] fn unmanaged_writable_dir_needs_no_elevation() { - let item = classify_root(&root(&writable_dir(), Channel::Unmanaged, Scope::Unknown)); + let item = classify(&root(&writable_dir(), Channel::Unmanaged, Scope::Unknown)); assert_eq!(item.action, UpdateAction::ReplaceBinaries); assert!(!item.needs_elevation); } + #[test] + fn root_already_at_target_is_a_pure_skip_even_with_an_elevated_daemon() { + let mut install = root(&writable_dir(), Channel::Unmanaged, Scope::Unknown); + install.binaries = vec![bin("uffs", "0.6.24"), bin("uffsd", "0.6.24")]; + // Elevated daemon running here + non-elevated caller: still a pure skip, + // because there is nothing to update β€” so the daemon is never stopped. + let report = report_with_daemon(&install.dir); + let item = classify_root(&install, "0.6.24", &report, false, true); + assert_eq!(item.action, UpdateAction::AlreadyCurrent); + assert!(!item.needs_elevation); + } + + #[test] + fn out_of_date_root_hosting_unstoppable_daemon_forces_elevation() { + let mut install = root(&writable_dir(), Channel::Unmanaged, Scope::Unknown); + install.binaries = vec![bin("uffs", "0.6.18")]; // out of date β†’ needs updating + let report = report_with_daemon(&install.dir); + // daemon_stop_needs_elev = true (elevated daemon, no serving broker): + // the writable dir no longer makes it doable β€” stopping the daemon to + // swap the binaries needs Administrator. + let item = classify_root(&install, "0.6.24", &report, false, true); + assert_eq!(item.action, UpdateAction::ReplaceBinaries); + assert!( + item.needs_elevation, + "an unstoppable daemon in the root forces elevation" + ); + } + #[test] fn split_off_non_elevated_only_when_elevated() { let build = || UpdatePlan { items: vec![ - classify_root(&root("/a", Channel::WinGet, Scope::User)), - classify_root(&root(&writable_dir(), Channel::Unmanaged, Scope::Unknown)), + classify(&root("/a", Channel::WinGet, Scope::User)), + classify(&root(&writable_dir(), Channel::Unmanaged, Scope::Unknown)), ], }; @@ -371,8 +518,8 @@ mod tests { fn drop_elevation_required_prunes_but_keeps_doable_work() { let mut plan = UpdatePlan { items: vec![ - classify_root(&root("/m", Channel::WinGet, Scope::Machine)), - classify_root(&root(&writable_dir(), Channel::Unmanaged, Scope::Unknown)), + classify(&root("/m", Channel::WinGet, Scope::Machine)), + classify(&root(&writable_dir(), Channel::Unmanaged, Scope::Unknown)), ], }; assert!(plan.requires_elevation()); From 8f334e14db9ac54451db0c27c84637e8cde05ccb Mon Sep 17 00:00:00 2001 From: Robert M1 <50460704+githubrobbi@users.noreply.github.com> Date: Wed, 8 Jul 2026 06:25:09 -0700 Subject: [PATCH 4/6] fix(update): skip acquire too (not just apply) on a winget-only plan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the daemon-stop fix: when the gate/prune leaves only winget roots, `acquire` (the uffs-update helper's binary download for hand-placed roots) errors with "no unmanaged binaries to acquire" and aborts the whole update. Gate `acquire` on `has_unmanaged` exactly like `apply` β€” the winget root is upgraded by `winget::run_upgrade` (which shells `winget upgrade` directly and needs neither acquire nor apply). 146 uffs-cli tests green; native + windows-msvc clippy + rustdoc clean. Co-Authored-By: Claude Opus 4.8 --- crates/uffs-cli/src/commands/update/mod.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/crates/uffs-cli/src/commands/update/mod.rs b/crates/uffs-cli/src/commands/update/mod.rs index a12441f08..36d351433 100644 --- a/crates/uffs-cli/src/commands/update/mod.rs +++ b/crates/uffs-cli/src/commands/update/mod.rs @@ -340,10 +340,12 @@ fn run_automatic_update(report: &DetectionReport, verbose: bool) -> Result<()> { // stop-timeout β†’ rollback on an install the caller could never update. let doable = plan::prune_report(report, &plan); - // Only hand-placed (unmanaged) roots go through the in-process replace - // (`apply`, which stops the daemon). If the gate/prune left only winget - // roots, skip `apply` entirely so no daemon is stopped for work that isn't - // there β€” the crux of the elevated-daemon 20s-rollback fix. + // The `uffs-update` helper (acquire + apply) only handles hand-placed + // (unmanaged) roots β€” winget roots are upgraded separately by + // `winget::run_upgrade`. So when the gate/prune left only winget roots, + // skip BOTH acquire and apply: the helper errors with "no unmanaged + // binaries to acquire", and apply would needlessly stop the daemon. This is + // the other half of the elevated-daemon fix. let has_unmanaged = doable .roots .iter() @@ -351,7 +353,9 @@ fn run_automatic_update(report: &DetectionReport, verbose: bool) -> Result<()> { print_updating(&latest); let snapshot_path = snapshot::write_snapshot(&doable, Some(&latest))?; - acquire::spawn(&snapshot_path, None, verbose)?; + if has_unmanaged { + acquire::spawn(&snapshot_path, None, verbose)?; + } // Quiesce-first: on a winget-managed install, stop the daemon + (package) // broker up front β€” ONE UAC β€” so BOTH the hand-rolled ~\bin update and the // winget upgrade run against a stopped install (a bare `winget upgrade` From fb2ab61ac9ba0c60f4495a8249740e1fca007272 Mon Sep 17 00:00:00 2001 From: Robert M1 <50460704+githubrobbi@users.noreply.github.com> Date: Wed, 8 Jul 2026 06:59:34 -0700 Subject: [PATCH 5/6] =?UTF-8?q?fix(update):=20don't=20gate=20winget=20root?= =?UTF-8?q?s=20on=20daemon-stop=20=E2=80=94=20quiesce=20handles=20it?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The winget-root corner exposed a dead-end: a daemon running FROM a winget-user root got the root flagged elevation-required, so a non-elevated `uffs --update` printed "re-run as Administrator" β€” which winget can't honor (it refuses a user package elevated). Neither continue-without nor re-run updated the root. Root cause: I applied the daemon-stop-elevation rule to winget roots too. But `winget::quiesce` stops a daemon inside the package *cooperatively* β€” `connect_raw().shutdown()`, an IPC shutdown the daemon honors regardless of its own elevation (same-user pipe), NOT the policy-gated CLI `daemon stop`. So a winget root never needs elevation for a running component; it only needs winget's own privilege level (user = non-elevated, machine = elevated). Restrict `root_stop_needs_elevation` to unmanaged roots (where the uffs-update helper does use the gated `daemon stop`, so an elevated daemon there really does need Administrator). A winget-user root with a daemon now classifies as (WingetUpgrade, non-elevated) β†’ no gate β†’ quiesce stops the daemon β†’ winget upgrades non-elevated. +1 unit test (winget-user root hosting a daemon is not gated). 147 uffs-cli tests green; native + windows-msvc clippy + rustdoc clean. Co-Authored-By: Claude Opus 4.8 --- crates/uffs-cli/src/commands/update/plan.rs | 61 ++++++++++++++------- 1 file changed, 41 insertions(+), 20 deletions(-) diff --git a/crates/uffs-cli/src/commands/update/plan.rs b/crates/uffs-cli/src/commands/update/plan.rs index 7736a32fa..d074de552 100644 --- a/crates/uffs-cli/src/commands/update/plan.rs +++ b/crates/uffs-cli/src/commands/update/plan.rs @@ -24,12 +24,15 @@ //! 1. **Already current** β€” a root whose every binary is at the target version //! is a pure skip, so an already-current `~\bin` hosting an *elevated* //! daemon is never dragged in (its daemon is never stopped). -//! 2. **Stop cost is the root's cost** β€” replacing/upgrading a root means -//! stopping any component running from it first. The broker (a `LocalSystem` -//! service) always needs elevation; the daemon/mcp inherit fix #1's gate -//! (`mutating_management_needs_elevation` β€” an elevated daemon with no -//! serving broker needs Administrator). If the stop needs elevation, the -//! root does. +//! 2. **Stop cost β€” but only where the stop is gated.** Replacing an +//! **unmanaged** root means the uffs-update helper stops the daemon via the +//! *gated* CLI `daemon stop` (no UAC), so an elevated daemon (or a broker) +//! hosted there makes the root elevation-required (fix #1's +//! `mutating_management_needs_elevation`). A **winget** root is different: +//! `winget::quiesce` stops a daemon inside the package *cooperatively* (an +//! IPC shutdown the daemon honors regardless of elevation β€” not the gated +//! CLI path) and cycles the package broker via its own one-UAC path, so a +//! winget root is never gated for a running component. //! //! The winget-user-scope-while-elevated case is **not** an elevation item β€” //! winget *refuses* to upgrade a user package from an elevated shell β€” so it is @@ -183,29 +186,28 @@ fn classify_root( return skip_item(root, UpdateAction::AlreadyCurrent); } - // Replacing/upgrading a root means stopping any component running from it - // first β€” so if that stop needs elevation, the whole root does. - let stop_needs_elev = root_stop_needs_elevation(root, report, elevated, daemon_stop_needs_elev); - let (action, needs_elevation, needs_non_elevated) = match root.channel { Channel::DevBuild => (UpdateAction::SkipDevBuild, false, false), Channel::Unknown => (UpdateAction::SkipUnknown, false, false), + // Unmanaged: the uffs-update helper stops the daemon via the *gated* CLI + // `daemon stop` (no UAC), so an elevated daemon (or a broker) hosted here + // genuinely needs elevation β€” as does a non-writable dir. Channel::Unmanaged => ( UpdateAction::ReplaceBinaries, - stop_needs_elev || !dir_writable(&root.dir), + !dir_writable(&root.dir) + || root_stop_needs_elevation(root, report, elevated, daemon_stop_needs_elev), false, ), + // winget's own `quiesce` stops a daemon inside the package + // **cooperatively** (an IPC shutdown the daemon honors regardless of its + // elevation β€” not the gated CLI `daemon stop`) and cycles the package + // broker via its own one-UAC path. So a winget root is never gated here; + // it only needs the privilege level winget itself demands: user scope + // must run non-elevated (winget refuses a user package elevated), machine + // scope requires Administrator. Channel::WinGet => match root.scope { - // winget refuses to upgrade a user package from an elevated shell, - // so a user root never *needs* elevation β€” it needs the absence of - // it. Machine scope is the opposite: winget requires Administrator. - // A component that needs elevation to stop overrides either way. Scope::Machine => (UpdateAction::WingetUpgrade, true, false), - Scope::User | Scope::Unknown => ( - UpdateAction::WingetUpgrade, - stop_needs_elev, - !stop_needs_elev, - ), + Scope::User | Scope::Unknown => (UpdateAction::WingetUpgrade, false, true), }, }; UpdateItem { @@ -473,6 +475,25 @@ mod tests { assert!(!item.needs_elevation); } + #[test] + fn winget_user_root_hosting_a_daemon_is_not_gated() { + // A daemon running from a winget-user root (even elevated) must NOT be + // gated: winget::quiesce stops it cooperatively (IPC shutdown, no + // elevation), so the upgrade runs non-elevated. Gating it would give the + // dead-end "re-run elevated" hint winget can't honor. + let dir = writable_dir(); + let mut install = root(&dir, Channel::WinGet, Scope::User); + install.binaries = vec![bin("uffs", "0.6.18")]; // out of date + let report = report_with_daemon(&install.dir); + let item = classify_root(&install, "0.6.24", &report, false, true); + assert_eq!(item.action, UpdateAction::WingetUpgrade); + assert!( + !item.needs_elevation, + "winget::quiesce stops the daemon cooperatively β€” no elevation" + ); + assert!(item.needs_non_elevated); + } + #[test] fn out_of_date_root_hosting_unstoppable_daemon_forces_elevation() { let mut install = root(&writable_dir(), Channel::Unmanaged, Scope::Unknown); From 84accb6f782cd531f90b33e5ae063c5d75922fc5 Mon Sep 17 00:00:00 2001 From: Robert M1 <50460704+githubrobbi@users.noreply.github.com> Date: Wed, 8 Jul 2026 11:51:44 -0700 Subject: [PATCH 6/6] feat(update): tell the user when the daemon couldn't restart (no broker) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After a winget upgrade, `resume` stops+restarts a daemon that lived in the package. When that daemon needs elevation to read the MFT and there is no broker, the non-elevated restart silently fails and the daemon is left down β€” confusing after a "βœ“ updated" message. Check whether the daemon actually came back (`daemon_ctl` PID file + live process) and, if not, print the one-time fix instead of a silent stop: ⚠ The index daemon was stopped for the update and couldn't restart non-elevated (no broker). Run `uffs-broker --install` for zero-UAC restarts, or start it from an elevated terminal. 147 uffs-cli tests green; native + windows-msvc clippy + rustdoc clean. Co-Authored-By: Claude Opus 4.8 --- crates/uffs-cli/src/commands/update/winget.rs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/crates/uffs-cli/src/commands/update/winget.rs b/crates/uffs-cli/src/commands/update/winget.rs index d15c70b9d..0fe933b3d 100644 --- a/crates/uffs-cli/src/commands/update/winget.rs +++ b/crates/uffs-cli/src/commands/update/winget.rs @@ -382,6 +382,16 @@ mod windows_impl { // ~69s on a cold cache), so animate a spinner β€” a silent minute is // the very "looks hung" symptom this rework set out to remove. spinner_while("Restarting the index daemon", restart_daemon); + // A daemon that needs elevation to read the MFT (no broker) cannot + // come back from this non-elevated restart. Say so with the one-time + // fix instead of leaving a silently-stopped daemon. + if !daemon_running() { + println!( + "\u{26a0} The index daemon was stopped for the update and couldn't restart\n\ + \x20 non-elevated (no broker). Run `uffs-broker --install` for zero-UAC\n\ + \x20 restarts, or start it from an elevated terminal." + ); + } } } @@ -482,6 +492,15 @@ mod windows_impl { true } + /// Whether the resident daemon is up: a fresh PID file naming a live + /// process. Used after the best-effort restart to tell the user when a + /// no-broker elevated daemon could not come back non-elevated. + fn daemon_running() -> bool { + let path = uffs_client::daemon_ctl::pid_file_path(); + uffs_client::daemon_ctl::parse_pid_file(&path) + .is_some_and(|(pid, ..)| uffs_client::daemon_ctl::is_pid_alive(pid)) + } + /// Best-effort daemon restart after the upgrade (auto-start on the next /// search covers any failure). fn restart_daemon() {