diff --git a/crates/uffs-broker/src/broker/service.rs b/crates/uffs-broker/src/broker/service.rs index f7c03d862..a453ca223 100644 --- a/crates/uffs-broker/src/broker/service.rs +++ b/crates/uffs-broker/src/broker/service.rs @@ -193,6 +193,13 @@ pub(super) fn install_service() -> anyhow::Result<()> { reason = "CLI admin command — stdout is the user-visible result channel" )] pub(super) fn uninstall_service() -> anyhow::Result<()> { + // Checking existence is a non-elevated SCM query. If the service is already + // absent, the requested end state holds — a no-op success, and there is no + // reason to demand Administrator for work that would not happen. + if !uffs_winsvc::is_installed(SERVICE_NAME) { + println!("Broker service is not installed — nothing to remove."); + return Ok(()); + } if !super::is_elevated() { anyhow::bail!( "removing the broker service requires Administrator.\n\ @@ -213,12 +220,37 @@ pub(super) fn uninstall_service() -> anyhow::Result<()> { if output.status.success() { println!("Service uninstalled."); - } else { - // AUDIT-OK(bytes): `sc` command output surfaced verbatim in an error - // message for the operator — display only, no decision. - anyhow::bail!("Uninstall failed: {}", sc_output(&output)); + return Ok(()); } - Ok(()) + + // The service already not existing IS the requested end state, so treat it + // as a no-op success rather than a loud failure. `sc delete` on a missing + // service returns ERROR_SERVICE_DOES_NOT_EXIST (1060). + if service_already_absent(&output) { + println!("Broker service is not installed — nothing to remove."); + return Ok(()); + } + + // AUDIT-OK(bytes): `sc` command output surfaced verbatim in an error + // message for the operator — display only, no decision. + anyhow::bail!("Uninstall failed: {}", sc_output(&output)); +} + +/// Whether an `sc delete` failure is just "the service does not exist" +/// (`ERROR_SERVICE_DOES_NOT_EXIST`, 1060) — i.e. already uninstalled, so the +/// uninstall request is already satisfied. +fn service_already_absent(output: &std::process::Output) -> bool { + // AUDIT-OK(bytes): the `sc` output is inspected only to classify the "already + // gone" case; the exit code is the primary signal, the text a fallback. + absent_service_signal(output.status.code(), &sc_output(output)) +} + +/// Pure classifier: `sc delete` reports a missing service as +/// `ERROR_SERVICE_DOES_NOT_EXIST` (1060) — via the process exit code (primary) +/// or its text (fallback). Split out so the "already gone → success" decision +/// is unit-testable without spawning `sc`. +fn absent_service_signal(exit_code: Option, sc_text: &str) -> bool { + exit_code == Some(1060) || sc_text.contains("1060") } // ── FU-1: Windows Service control dispatcher ──────────────────────────────── @@ -385,3 +417,29 @@ fn signal_stop() { } } } + +#[cfg(test)] +mod tests { + use super::absent_service_signal; + + #[test] + fn absent_when_sc_reports_1060_by_code_or_text() { + // Exit code is the primary signal. + assert!(absent_service_signal(Some(1060_i32), "")); + // Text is the fallback when the code is not surfaced. + assert!(absent_service_signal( + None, + "[SC] OpenService FAILED 1060:\n\nThe specified service does not exist" + )); + } + + #[test] + fn other_failures_are_not_treated_as_absent() { + // Access denied (5) is a real failure, not "already gone". + assert!(!absent_service_signal( + Some(5_i32), + "[SC] DeleteService FAILED 5:\n\nAccess is denied." + )); + assert!(!absent_service_signal(None, "some unrelated error")); + } +} diff --git a/crates/uffs-cli/src/commands.rs b/crates/uffs-cli/src/commands.rs index 0bc69ba3b..8f00d7284 100644 --- a/crates/uffs-cli/src/commands.rs +++ b/crates/uffs-cli/src/commands.rs @@ -34,6 +34,12 @@ pub(crate) mod mcp_mgmt; pub mod output; /// Search command implementation. pub mod search; +/// Braille spinner for blocking steps with no incremental progress of their own +/// (winget upgrade, a cold-cache daemon start). Windows-only for now — its only +/// callers (winget orchestration, uninstall coverage reload) are +/// `#[cfg(windows)]`. +#[cfg(windows)] +pub(crate) mod spinner; /// Stats subcommand implementation. pub mod stats; /// Combined `uffs --status` command. diff --git a/crates/uffs-cli/src/commands/elevation.rs b/crates/uffs-cli/src/commands/elevation.rs index 3604053c0..5fc93525f 100644 --- a/crates/uffs-cli/src/commands/elevation.rs +++ b/crates/uffs-cli/src/commands/elevation.rs @@ -1,20 +1,24 @@ // SPDX-License-Identifier: MPL-2.0 // Copyright (c) 2025-2026 SKY, LLC. -//! Shared elevation gate for the mutating flows. +//! Shared elevation gate for the mutating flows (`uffs --uninstall`, +//! `uffs --update`). //! //! 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). +//! Administrator and decides once. Surfacing this before any mutation is what +//! keeps a flow from failing halfway through. Both flows drive this identically +//! so their elevation UX stays in lockstep. //! -//! 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. +//! The one difference is whether the flow can elevate **in place**: +//! +//! - `uffs --uninstall` has a one-shot elevated helper it spawns via a single +//! UAC prompt at removal time, so on Windows it offers a 3-way choice — +//! elevate now / continue without / abort (`offer_inflow_elevation = true`). +//! - `uffs --update` has no in-flow elevation (and `winget` itself *refuses* to +//! upgrade a user package elevated), so its elevated path is simply "re-run +//! elevated" and the choice stays binary (`offer_inflow_elevation = false`). +//! +//! Non-Windows has no UAC to request, so the choice is always binary there. use anyhow::{Context as _, Result, bail}; @@ -34,8 +38,18 @@ pub(crate) trait ElevatablePlan { /// Flow-specific wording woven into the gate prompt. pub(crate) struct GateWording { - /// The command to re-run elevated — `"uffs --update"` / `"uffs - /// --uninstall"`. + /// The mutating action — `"removal"` / `"update"` — used in + /// "elevate at <action> time" (only on the in-flow-elevation prompt). + #[cfg_attr( + not(windows), + expect( + dead_code, + reason = "read only by the Windows in-flow-elevation prompt" + ) + )] + pub(crate) action: &'static str, + /// The command to re-run elevated — `"uffs --uninstall"` / `"uffs + /// --update"`. pub(crate) rerun_cmd: &'static str, } @@ -43,6 +57,15 @@ pub(crate) struct GateWording { pub(crate) enum ElevationChoice { /// Elevated, dry-run, or nothing needs Administrator — plan untouched. NotNeeded, + /// Windows in-flow elevation: keep the admin items; the mutating step + /// routes them through a one-shot elevated helper (a single UAC + /// prompt). Produced only when the caller offers in-flow elevation AND + /// on Windows. + #[cfg_attr( + not(windows), + expect(dead_code, reason = "constructed only on the Windows in-flow-UAC path") + )] + ElevateLater, /// Continuing without the admin items: they were dropped from the plan; /// carries their descriptions for the final summary. ContinueWithout(Vec), @@ -51,7 +74,9 @@ pub(crate) enum ElevationChoice { /// 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). +/// scripted run must never block on a prompt it cannot answer, nor trigger a +/// surprise UAC). `offer_inflow_elevation` enables the Windows 3-way choice for +/// flows that can elevate in place. /// /// # Errors /// @@ -60,6 +85,7 @@ pub(crate) fn elevation_gate( plan: &mut impl ElevatablePlan, dry_run: bool, assume_yes: bool, + offer_inflow_elevation: bool, wording: &GateWording, ) -> Result { if dry_run || !plan.requires_elevation() || uffs_mft::platform::is_elevated() { @@ -71,6 +97,52 @@ pub(crate) fn elevation_gate( plan.drop_elevation_required(), )); } + platform_elevation_choice(plan, offer_inflow_elevation, wording) +} + +/// Windows: the 3-way choice when the flow can elevate in place (`e` records +/// the decision — the single UAC prompt appears later, when the mutating step +/// runs); otherwise the binary continue-without / abort. +#[cfg(windows)] +fn platform_elevation_choice( + plan: &mut impl ElevatablePlan, + offer_inflow_elevation: bool, + wording: &GateWording, +) -> Result { + if !offer_inflow_elevation { + return binary_choice(plan, wording); + } + let choice = prompt_line(&format!( + "\n e = elevate at {} time (Windows shows one UAC prompt)\n\ + \x20 c = continue without it (the item(s) above are left as-is)\n\ + \x20 a = abort\n\n\ + Choice [e/c/A]: ", + wording.action, + ))?; + match choice.as_str() { + "e" | "elevate" => Ok(ElevationChoice::ElevateLater), + "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 + ), + } +} + +/// Non-Windows: there is no UAC to request, so the choice is always binary. +#[cfg(not(windows))] +fn platform_elevation_choice( + plan: &mut impl ElevatablePlan, + _offer_inflow_elevation: bool, + wording: &GateWording, +) -> Result { + binary_choice(plan, wording) +} + +/// The continue-without / abort choice (no in-flow elevation available). +fn binary_choice(plan: &mut impl ElevatablePlan, wording: &GateWording) -> Result { let choice = prompt_line( "\n c = continue without it (the item(s) above are left as-is)\n\ \x20 a = abort\n\n\ diff --git a/crates/uffs-cli/src/commands/spinner.rs b/crates/uffs-cli/src/commands/spinner.rs new file mode 100644 index 000000000..464532071 --- /dev/null +++ b/crates/uffs-cli/src/commands/spinner.rs @@ -0,0 +1,44 @@ +// SPDX-License-Identifier: MPL-2.0 +// Copyright (c) 2025-2026 SKY, LLC. + +//! A braille spinner for blocking CLI steps that have no incremental progress +//! of their own (a `winget upgrade`, a cold-cache daemon start, …). Runs the +//! work on a scoped thread and animates until it finishes, then erases the +//! line — so a ~90 s wait reads as "working", not "hung". + +use core::time::Duration; +use std::io::Write as _; + +/// Spinner frame interval. +const POLL: Duration = Duration::from_millis(120); + +/// Braille spinner frames. +const FRAMES: [&str; 10] = ["⠋", "⠙", "⠹", "⠸", "⠼", "⠴", "⠦", "⠧", "⠇", "⠏"]; + +/// Run `body` on a scoped thread while animating a spinner after `label`, then +/// erase the spinner line so the caller's next output lands clean. Returns +/// whatever `body` returns. For a blocking call that prints nothing itself — +/// drive the underlying work in its *quiet* form so the spinner owns the line. +#[expect(clippy::print_stdout, reason = "interactive progress spinner")] +pub(crate) fn spinner_while(label: &str, body: impl FnOnce() -> T + Send) -> T { + // Width of the line drawn each frame ("