From c055225d9d0cf1141157fba0b5fc310a756d6bc4 Mon Sep 17 00:00:00 2001 From: Robert M1 <50460704+githubrobbi@users.noreply.github.com> Date: Wed, 12 Aug 2026 12:22:32 -0700 Subject: [PATCH 01/16] fix(cli): align the daemon status tables MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four separate causes, all of which made a column ragged: 1. `statusfmt::field` printed the value verbatim, but several producers right-align internally — `format_duration` emits `{minutes:>3} m`, i.e. ` 12 m 35 s`. That leading pad pushed those rows one or two columns right of their plain-text neighbours: Version: 0.6.31 Uptime: 12 m 35 s <- shifted The value is now trimmed, so every value in a block starts in one column. Internal spacing is untouched, so the durations keep their own digit alignment. 2. The per-drive memory line printed heap MB unpadded, so `2 MB` and `1669 MB` started at different columns and the sizes could not be compared down the list. Now `{:>6}`. 3. Physical-drive rows pad every numeric but not the drive letter, and the boot marker makes `C:*` one column wider than `D:` — shifting every field on that one row. The letter is now padded to 3, applied to the RAW string before colouring: a width specifier on an already-coloured string counts the ANSI escapes and silently breaks the alignment it was meant to fix. 4. `status_drives`' RESIDENT column left-aligned whole cells, so `2 MiB` and `1.07 GiB` shared a start column. It is now a fixed 10-wide cell — a 6-wide right-aligned magnitude plus a 3-wide unit — so rows line up on the decimal point: 1.069 GiB 509 MiB 2 MiB GiB precision goes to three decimals to fill that column. Regression test pins the property that matters (values sharing one column) rather than just the literal strings. --- crates/uffs-cli/src/commands/daemon_status.rs | 13 +++++- .../uffs-cli/src/commands/daemon_tiering.rs | 26 ++++++++---- crates/uffs-statusfmt/src/lib.rs | 40 ++++++++++++++++++- 3 files changed, 69 insertions(+), 10 deletions(-) diff --git a/crates/uffs-cli/src/commands/daemon_status.rs b/crates/uffs-cli/src/commands/daemon_status.rs index cc581a500..476f1172a 100644 --- a/crates/uffs-cli/src/commands/daemon_status.rs +++ b/crates/uffs-cli/src/commands/daemon_status.rs @@ -387,8 +387,12 @@ fn print_drive_line(palette: Palette, dr: &DriveInfo, memory: &[DriveMemoryInfo] match memory.iter().find(|dm| dm.drive == dr.letter) { Some(dm) => { let mb = |bytes: u64| bytes / MIB; + // `{:>6}` on the heap MB keeps the `· NNNN MB` column + // aligned across drives — an unpadded `{}` put `2 MB` + // and `1669 MB` at different columns, so the sizes could + // not be compared by eye down the list. println!( - " {glyph} {letter} {records:>12} records ({}) \u{b7} {} MB [rec={} names={} tri={} ch={} ext={}]", + " {glyph} {letter} {records:>12} records ({}) \u{b7} {:>6} MB [rec={} names={} tri={} ch={} ext={}]", dr.source, mb(dm.heap_bytes), mb(dm.records_bytes), @@ -448,7 +452,12 @@ fn print_physical_drive_line(palette: Palette, drive: &PhysicalDrive, loaded: &[ use uffs_client::format::{format_bytes, format_number_commas}; let boot = if drive.is_boot { "*" } else { "" }; - let letter = palette.bold(&format!("{}:{boot}", drive.letter)); + // Pad the RAW text before colouring — ANSI escapes would be counted by a + // width specifier applied afterwards and silently break the alignment + // (same rule as `uffs_statusfmt::field`). The boot marker makes `C:*` + // one column wider than `D:`, which shifted every column on the boot + // drive's row relative to the others. + let letter = palette.bold(&format!("{:<3}", format!("{}:{boot}", drive.letter))); let (glyph, index_note) = loaded .iter() .find(|info| info.letter == drive.letter) diff --git a/crates/uffs-cli/src/commands/daemon_tiering.rs b/crates/uffs-cli/src/commands/daemon_tiering.rs index 5c6c6c531..c47b15a09 100644 --- a/crates/uffs-cli/src/commands/daemon_tiering.rs +++ b/crates/uffs-cli/src/commands/daemon_tiering.rs @@ -344,17 +344,29 @@ fn format_bytes(bytes: u64) -> String { const KIB: u64 = 1024; const MIB: u64 = 1024 * KIB; const GIB: u64 = 1024 * MIB; - if bytes >= GIB { + // Fixed 10-column cell: a 6-wide RIGHT-aligned magnitude then a + // 3-wide left-aligned unit, so every row lines up on the decimal + // point rather than ragging left off the unit: + // + // 1.070 GiB + // 509 MiB + // 2 MiB + // + // Left-aligning the whole cell (the previous behaviour) put `2 MiB` + // and `1.07 GiB` at the same start column, which reads as noise in + // a column you scan vertically to compare sizes. + let (magnitude, unit) = if bytes >= GIB { let whole = bytes / GIB; - let hundredths = (bytes % GIB).saturating_mul(100) / GIB; - format!("{whole}.{hundredths:02} GiB") + let thousandths = (bytes % GIB).saturating_mul(1000) / GIB; + (format!("{whole}.{thousandths:03}"), "GiB") } else if bytes >= MIB { - format!("{} MiB", bytes / MIB) + ((bytes / MIB).to_string(), "MiB") } else if bytes >= KIB { - format!("{} KiB", bytes / KIB) + ((bytes / KIB).to_string(), "KiB") } else { - format!("{bytes} B") - } + (bytes.to_string(), "B") + }; + format!("{magnitude:>6} {unit:<3}") } /// Format a Unix-millisecond timestamp as a human-readable elapsed string diff --git a/crates/uffs-statusfmt/src/lib.rs b/crates/uffs-statusfmt/src/lib.rs index 711f951ca..8923f92b7 100644 --- a/crates/uffs-statusfmt/src/lib.rs +++ b/crates/uffs-statusfmt/src/lib.rs @@ -153,7 +153,23 @@ pub fn field(palette: Palette, key: &str, value: &str, key_width: usize) -> Stri // Pad on the raw (uncolored) key+colon so alignment is escape-agnostic. let label = format!("{key}:"); let pad = key_width.saturating_add(1).saturating_sub(label.len()); - format!(" {}{} {value}", palette.dim(&label), " ".repeat(pad)) + // Trim the value's LEADING whitespace so every value in a block starts + // at the same column. Several value producers right-align internally + // (`format_duration` emits `{minutes:>3} m`, giving ` 12 m 35 s`), which + // otherwise pushes those rows one or two columns further right than + // their plain-text neighbours and makes a tidy block look ragged: + // + // Version: 0.6.31 + // Uptime: 12 m 35 s <- shifted by the duration's own pad + // + // Trailing whitespace goes too — it is invisible but shows up in + // golden-output diffs and when the line is copied out of a terminal. + format!( + " {}{} {}", + palette.dim(&label), + " ".repeat(pad), + value.trim() + ) } /// A one-line component summary: ` ` — the short-view @@ -193,6 +209,28 @@ mod tests { assert_eq!(field(plain, "PID", "42", 10), " PID: 42"); } + /// A value that right-aligns internally (as `format_duration` does, + /// emitting ` 12 m 35 s`) must still start in the same column as a + /// plain one — otherwise a `key: value` block reads ragged. + #[test] + fn field_values_share_one_column_despite_value_padding() { + let plain = Palette::plain(); + let padded = field(plain, "Uptime", " 12 m 35 s", 10); + let unpadded = field(plain, "Version", "0.6.31", 10); + + assert_eq!(padded, " Uptime: 12 m 35 s"); + assert_eq!(unpadded, " Version: 0.6.31"); + + // The point of the two literals above: both values begin at the + // same index, which is what makes the block read as a column. + let col = |row: &str, value: &str| row.find(value).unwrap_or(usize::MAX); + assert_eq!( + col(&padded, "12 m"), + col(&unpadded, "0.6.31"), + "value columns must line up" + ); + } + #[test] fn header_and_section_shapes() { let plain = Palette::plain(); From 2fc73b35047fc27132a1b77e13fbed194fada4c0 Mon Sep 17 00:00:00 2001 From: Robert M1 <50460704+githubrobbi@users.noreply.github.com> Date: Wed, 12 Aug 2026 13:21:35 -0700 Subject: [PATCH 02/16] fix(dev): use-local skips unchanged binaries and cycles the broker service MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `just use-local` failed the whole recipe on uffs-broker.exe copy failed: The process cannot access the file because it is being used by another process. (os error 32) even though 22 of 23 binaries installed fine — and, more to the point, even though the broker had not changed at all. Its sources are byte-identical across v0.6.30..v0.6.31; the copy was pure churn against a LocalSystem service that legitimately holds its own image open. Two changes: * Skip binaries byte-identical to what is already installed (length check first, then contents). The broker is the motivating case, but every unchanged binary now avoids a needless rewrite. * When the broker HAS changed and is running, stop the service, copy, and restart it. The broker already exposes native SCM control for exactly this — `--stop` waits for STOPPED, `--start` waits for RUNNING and for the pipe to actually serve — which is the same quiesce/restore sequence `uffs --update` performs. No sc.exe, no reboot. The restart is best-effort and never fails the install: a new binary in place with the service down is recoverable via `uffs-broker --start`, and that instruction is printed. Stopping is a no-op when the service is not installed or not running, so boxes without the broker are unaffected. Note the compatibility background that makes skipping safe: the broker speaks a tiny fixed wire protocol (1-byte request, 9-byte response) and is deliberately decoupled from the daemon's version, so an unchanged broker serves a newer daemon. That protocol has no explicit version handshake yet ("currently implicit, future work"), so the guarantee is by convention — which is another reason to replace the binary only when it genuinely differs. --- scripts/dev/install-bins.rs | 114 +++++++++++++++++++++++++++++++++++- 1 file changed, 113 insertions(+), 1 deletion(-) diff --git a/scripts/dev/install-bins.rs b/scripts/dev/install-bins.rs index 38f0ce712..913ba2a78 100755 --- a/scripts/dev/install-bins.rs +++ b/scripts/dev/install-bins.rs @@ -86,6 +86,7 @@ fn main() { let mut installed = 0_u32; let mut skipped = 0_u32; + let mut unchanged = 0_u32; eprintln!(); eprintln!("📦 Installing {} binaries to {}", executables.len(), bin_dir.display()); for src in &executables { @@ -99,6 +100,29 @@ fn main() { skipped += 1; continue; } + // Identical to what is already installed? Don't touch it. + // + // This matters most for `uffs-broker.exe`: it runs as a LocalSystem + // Windows service, so its image is locked and the copy fails with + // `os error 32` — which used to fail the whole recipe even when the + // binary had not changed at all. The broker's sources move rarely + // (byte-identical across v0.6.30..v0.6.31), so the common case is a + // needless copy of an unchanged file. + if files_identical(src, &dest) { + eprintln!(" ⏭️ {name:<28} unchanged"); + unchanged += 1; + continue; + } + // Changed AND currently locked by the running service: stop it, + // copy, restart. The broker exposes native SCM control for exactly + // this (`--stop` waits for STOPPED, `--start` waits for RUNNING and + // for the pipe to actually serve), which is the same quiesce/restore + // dance `uffs --update` performs. + let broker_guard = if is_broker(&name) { + BrokerGuard::stop_for_replace(&dest) + } else { + BrokerGuard::inactive() + }; // Remove first so the copy gets a fresh inode — overwrites in place // share the inode, which lets macOS re-use a path-cached Launch // Services deny verdict against an earlier broken copy. @@ -124,10 +148,15 @@ fn main() { skipped += 1; } } + broker_guard.restart(); } eprintln!(); - eprintln!("✅ Installed {installed} binaries ({skipped} skipped)"); + if unchanged > 0 { + eprintln!("✅ Installed {installed} binaries ({unchanged} unchanged, {skipped} skipped)"); + } else { + eprintln!("✅ Installed {installed} binaries ({skipped} skipped)"); + } let on_path = std::env::var("PATH") .map(|path| std::env::split_paths(&path).any(|entry| entry == bin_dir)) .unwrap_or(false); @@ -139,6 +168,89 @@ fn main() { } } +/// True when `src` and `dest` are byte-identical, so the copy can be +/// skipped entirely. Compares length first (cheap, rejects almost every +/// changed binary) and only then the contents. A missing or unreadable +/// `dest` is "not identical", so the normal copy path runs. +fn files_identical(src: &std::path::Path, dest: &std::path::Path) -> bool { + let (Ok(src_meta), Ok(dest_meta)) = (src.metadata(), dest.metadata()) else { + return false; + }; + if src_meta.len() != dest_meta.len() { + return false; + } + match (std::fs::read(src), std::fs::read(dest)) { + (Ok(lhs), Ok(rhs)) => lhs == rhs, + _ => false, + } +} + +/// Is this the Access Broker binary (the one that runs as a service and +/// therefore holds its own image open)? +fn is_broker(name: &str) -> bool { + let stem = name.strip_suffix(".exe").unwrap_or(name); + stem == "uffs-broker" +} + +/// Stops the broker service around a replace, and restarts it afterwards. +/// +/// `stop_for_replace` is a no-op unless the service is actually running, +/// so a developer box without the broker installed sees no behaviour +/// change. Restart is best-effort and never fails the install: leaving +/// the new binary in place with the service down is recoverable +/// (`uffs-broker --start`), and is reported loudly. +struct BrokerGuard { + /// Path of the installed broker binary, when we stopped its service. + stopped: Option, +} + +impl BrokerGuard { + /// A guard that does nothing (non-broker binaries). + const fn inactive() -> Self { + Self { stopped: None } + } + + /// Stop the broker service so its image can be overwritten. + fn stop_for_replace(installed: &std::path::Path) -> Self { + if !installed.is_file() { + return Self::inactive(); + } + eprintln!(" ⏸️ uffs-broker stopping service to replace it"); + let stopped = std::process::Command::new(installed) + .arg("--stop") + .status() + .map(|status| status.success()) + .unwrap_or(false); + if stopped { + Self { stopped: Some(installed.to_path_buf()) } + } else { + // Not installed as a service, already stopped, or not + // elevated — the copy below will report the real error. + Self::inactive() + } + } + + /// Restart the service if this guard stopped it. + fn restart(self) { + let Some(path) = self.stopped else { + return; + }; + let started = std::process::Command::new(&path) + .arg("--start") + .status() + .map(|status| status.success()) + .unwrap_or(false); + if started { + eprintln!(" ▶️ uffs-broker service restarted"); + } else { + eprintln!( + " ⚠️ uffs-broker service did NOT restart — run: {} --start", + path.display() + ); + } + } +} + /// Best-effort shutdown of the resident daemon + MCP before installing. /// /// `uffs --daemon kill` is given 10 seconds (a wedged daemon must not From 9375b46127a01c7bbc39e106dea9fbe3e1a9c6ee Mon Sep 17 00:00:00 2001 From: Robert M1 <50460704+githubrobbi@users.noreply.github.com> Date: Wed, 12 Aug 2026 13:34:43 -0700 Subject: [PATCH 03/16] fix(dev): use-local restarts the daemon it stopped MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `use-local` deliberately kills the daemon + MCP so their images can be replaced, but never brought them back — so a routine dev install left the machine with no daemon at all: ✅ Installed 22 binaries (1 skipped) error: Recipe `use-local` failed on line 141 with exit code 1 > uffs.exe --daemon status ○ Daemon not running That directly breaks the promise `uffs --daemon resident` makes: the user asked for a permanently-resident daemon that even starts at login, and then a build silently took it away. The install now notes whether a daemon was serving BEFORE the teardown and restarts it afterwards with the freshly installed binary, restoring the invariant "use-local leaves the machine as it found it". The restart goes through the normal `--daemon start` path, so the client's auto-spawn merges the resident marker (`resident.args`): a daemon that was resident comes back resident with `--no-retire`, not as a plain ephemeral one. Ordering detail: the restart runs BEFORE the non-zero exit for skipped binaries. A partially-failed install is precisely the case where being left daemon-less hurts most, so the recovery must not be skipped by the early exit. Both halves stay best-effort and never mask a failure: a restart that does not take prints the exact command to run by hand, and the probe failing (no `uffs` on PATH on a first install) reads as "was not running", so nothing is started that was not there before. --- scripts/dev/install-bins.rs | 64 +++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/scripts/dev/install-bins.rs b/scripts/dev/install-bins.rs index 913ba2a78..cf49cea4f 100755 --- a/scripts/dev/install-bins.rs +++ b/scripts/dev/install-bins.rs @@ -35,6 +35,11 @@ fn main() { eprintln!("📦 Build (release) + install UFFS binaries to ~/bin"); eprintln!("========================================================"); + // Note whether a daemon was serving BEFORE we tear it down, so the + // install can put it back afterwards (see `restart_daemon`). Probing + // after `stop_running_services` would of course always say "no". + let daemon_was_running = daemon_is_running(); + // Stop the running daemon + MCP first (best effort), mirroring the // previous [unix] bash recipe: the old binaries release their file // locks (Windows can't overwrite a running .exe at all) and no stale @@ -163,6 +168,12 @@ fn main() { if !on_path { eprintln!("⚠️ {} is not on PATH", bin_dir.display()); } + // Put back what we took down. Deliberately BEFORE the `skipped` + // exit: a partially-failed install is exactly the case where leaving + // the machine daemon-less hurts most. + if daemon_was_running { + restart_daemon(&bin_dir); + } if skipped > 0 { std::process::exit(1); } @@ -251,6 +262,59 @@ impl BrokerGuard { } } +/// Was a daemon serving before we tore everything down? +/// +/// `--daemon status` prints `● running PID …` when up and +/// `○ Daemon not running` when not, so "contains `running` but not +/// `not running`" is an exact read of both shapes. Any failure to run +/// the probe (no `uffs` on PATH yet on a first install) reads as "not +/// running", which is the safe answer: we then leave things alone. +fn daemon_is_running() -> bool { + Command::new("uffs") + .args(["--daemon", "status"]) + .output() + .map(|out| { + let text = String::from_utf8_lossy(&out.stdout); + text.contains("running") && !text.contains("not running") + }) + .unwrap_or(false) +} + +/// Restart the daemon we deliberately stopped, using the freshly +/// installed binary. +/// +/// `use-local` kills the daemon + MCP so their images can be replaced, +/// but until now never brought them back — so a routine dev install +/// silently left the machine without a daemon, which is exactly the +/// promise `uffs --daemon resident` makes and breaks. Restoring it here +/// keeps the invariant "use-local leaves the machine as it found it". +/// +/// The restart goes through the normal `--daemon start` path, so the +/// resident marker (`resident.args`) is merged in by the client's +/// auto-spawn — a daemon that was resident comes back resident, with +/// `--no-retire`, rather than as a plain ephemeral one. +fn restart_daemon(bin_dir: &std::path::Path) { + let exe = bin_dir.join(if cfg!(windows) { "uffs.exe" } else { "uffs" }); + if !exe.is_file() { + return; + } + eprintln!(); + eprintln!("🔄 Restarting the daemon (it was running before the install)..."); + let ok = Command::new(&exe) + .args(["--daemon", "start"]) + .status() + .map(|status| status.success()) + .unwrap_or(false); + if ok { + eprintln!("✅ Daemon restarted."); + } else { + eprintln!( + "⚠️ Daemon did NOT restart — run: {} --daemon start", + exe.display() + ); + } +} + /// Best-effort shutdown of the resident daemon + MCP before installing. /// /// `uffs --daemon kill` is given 10 seconds (a wedged daemon must not From f21d4ba9d8461ad74cd8357514e93cad4b2aa6db Mon Sep 17 00:00:00 2001 From: Robert M1 <50460704+githubrobbi@users.noreply.github.com> Date: Wed, 12 Aug 2026 14:26:18 -0700 Subject: [PATCH 04/16] docs(daemon): document the memory tiers and that Hot is preload-only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `status_drives` shows a TIER column with four values but the manual never explained them, and the one that actually surprises people was undocumented: every drive reads `warm` forever and `Hot` never appears. That is not a bug — it is by construction. There is exactly one code path that creates a Hot shard (`preload`); a freshly loaded drive starts Warm, and a query that promotes a Parked/Cold drive promotes it back to Warm, never past it. So on a daemon where preload has never run, `UFFS_HOT_TO_WARM_IDLE_SECS` is inert config: nothing is ever Hot to demote, and the effective ladder is Warm -> Parked -> Cold. Also records what preload actually buys, since "Hot" oversells it: for dispatch, Warm and Hot are one set and a Hot drive is not searched faster. The real win is the PrefetchVirtualMemory hint moving first-touch paging off the next query's critical path — the difference between a multi-second first query on a multi-GB index (much of it on HDDs) and a memory-speed one — plus the pin blocking demotion. Finally states the distinction that catches people out: residency and Hot are different promises. `--no-retire` keeps the PROCESS alive while the ladder still parks the drives underneath it, so a resident daemon left idle overnight still pays the page-in unless it was preloaded. --- docs/user-manual/daemon.md | 47 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/docs/user-manual/daemon.md b/docs/user-manual/daemon.md index 086d8530e..8b3abefed 100644 --- a/docs/user-manual/daemon.md +++ b/docs/user-manual/daemon.md @@ -157,6 +157,53 @@ importantly `--no-retire`. Flags you pass explicitly always win over the marker. `resident off` removes the marker along with the login item. +### Memory tiers — and why you never see `Hot` + +Each drive's index sits in one of four tiers, visible in +`uffs --daemon status_drives`: + +| Tier | What is in RAM | Reached by | +|------|----------------|------------| +| `Hot` | Body, **pre-faulted** into the working set | `uffs --daemon preload` **only** | +| `Warm` | Body, fully searchable | initial load, and every promotion | +| `Parked` | Bloom filter + path trie; body released | 30 min idle | +| `Cold` | Nothing (encrypted on-disk cache only) | 24 h idle | + +**`Hot` is an operator mode, not something the daemon reaches on its +own.** Nothing promotes a drive to `Hot` because it is busy — there is +exactly one code path that creates a `Hot` shard and it is `preload`. +A freshly loaded drive starts `Warm`, and a query that promotes a +`Parked` or `Cold` drive promotes it back to **`Warm`**, never past it. +So on a daemon where `preload` has never run, every drive reads `warm` +forever and the `Hot → Warm` idle threshold +(`UFFS_HOT_TO_WARM_IDLE_SECS`, default 600 s) never fires — there is +nothing `Hot` to demote. The effective ladder is +`Warm → Parked → Cold`. + +For serving queries the two active tiers are **identical**: dispatch +treats `Warm` and `Hot` as one set, and a `Hot` drive is not searched +faster. What `preload` actually buys is: + +* **Pre-faulting** — it issues a `PrefetchVirtualMemory` hint, pulling + the mapped pages into the working set up front. This is the real + win: it moves first-touch paging off the critical path of your next + query. On a large index (say 5 GB across seven drives, several on + HDDs) that first query can otherwise take tens of seconds while the + pages fault in — everything after is memory-speed. +* **A pin** — demotion is blocked until the pin expires (30 min by + default, `--pin-minutes` to change), plus one extra rung of runway + afterwards. + +```bash +# Make the drives you actually search ready, and hold them there. +uffs --daemon preload --drives C,D --pin-minutes 60 +``` + +Note that residency and `Hot` are different promises: `--no-retire` +keeps the **process** alive, while the tiering ladder still parks the +drives underneath it. A resident daemon left idle overnight still pays +the page-in on the next first query unless it was preloaded. + --- ## 5 Management Commands From 398546278fa75a2eb8be8b9d2d8014657211d92a Mon Sep 17 00:00:00 2001 From: Robert M1 <50460704+githubrobbi@users.noreply.github.com> Date: Wed, 12 Aug 2026 14:37:35 -0700 Subject: [PATCH 05/16] fix(dev): use-local stops the MCP cleanly and restarts it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Completes the symmetry started for the daemon. Two problems remained on the MCP side of `use-local`: * The teardown only ever `taskkill /IM uffsmcp.exe /F`-ed. A force-kill by image name never lets the gateway remove its PID file, so the next `uffs --mcp status` reported MCP server: not running (stale PID file, PID 64184) which reads like a crash rather than the install doing it. The teardown now asks `uffs --mcp stop` first and keeps the force-kill as the backstop for a wedged process. * Nothing restarted it. Like the daemon, the gateway is now noted as running BEFORE the teardown and brought back with the freshly installed binary afterwards. Worth recording why the MCP supervisor did not save this: it fronts STDIO sessions only (they deliberately write no PID file — the stale one proves this was the HTTP gateway), and in any case `taskkill /F` by image name kills the supervisor itself. A supervisor can hot-swap its worker child; it cannot survive its own kill. Real crash protection has to live outside the process tree the install tears down. --- scripts/dev/install-bins.rs | 55 +++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/scripts/dev/install-bins.rs b/scripts/dev/install-bins.rs index cf49cea4f..5cd371a28 100755 --- a/scripts/dev/install-bins.rs +++ b/scripts/dev/install-bins.rs @@ -39,6 +39,7 @@ fn main() { // install can put it back afterwards (see `restart_daemon`). Probing // after `stop_running_services` would of course always say "no". let daemon_was_running = daemon_is_running(); + let mcp_was_running = mcp_is_running(); // Stop the running daemon + MCP first (best effort), mirroring the // previous [unix] bash recipe: the old binaries release their file @@ -174,6 +175,9 @@ fn main() { if daemon_was_running { restart_daemon(&bin_dir); } + if mcp_was_running { + restart_mcp(&bin_dir); + } if skipped > 0 { std::process::exit(1); } @@ -315,6 +319,46 @@ fn restart_daemon(bin_dir: &std::path::Path) { } } +/// Was the MCP HTTP gateway serving before the teardown? +/// +/// `--mcp status` prints `MCP server: running (PID …)` when up, and +/// either `not running (no PID file)` or `not running (stale PID file…)` +/// when not — so the same "contains `running` but not `not running`" +/// read works here. +fn mcp_is_running() -> bool { + Command::new("uffs") + .args(["--mcp", "status"]) + .output() + .map(|out| { + let text = String::from_utf8_lossy(&out.stdout); + text.contains("running") && !text.contains("not running") + }) + .unwrap_or(false) +} + +/// Restart the MCP HTTP gateway we stopped, using the new binary. +fn restart_mcp(bin_dir: &std::path::Path) { + let exe = bin_dir.join(if cfg!(windows) { "uffs.exe" } else { "uffs" }); + if !exe.is_file() { + return; + } + eprintln!(); + eprintln!("🔄 Restarting the MCP server (it was running before the install)..."); + let ok = Command::new(&exe) + .args(["--mcp", "start"]) + .status() + .map(|status| status.success()) + .unwrap_or(false); + if ok { + eprintln!("✅ MCP server restarted."); + } else { + eprintln!( + "⚠️ MCP server did NOT restart — run: {} --mcp start", + exe.display() + ); + } +} + /// Best-effort shutdown of the resident daemon + MCP before installing. /// /// `uffs --daemon kill` is given 10 seconds (a wedged daemon must not @@ -345,6 +389,17 @@ fn stop_running_services() { } } } + // Ask the MCP gateway to stop cleanly first. The `taskkill /F` + // below is a `/F` by image name: it kills every `uffsmcp` process + // outright, so the gateway never removes its PID file and the next + // `--mcp status` reports `not running (stale PID file, PID …)`. + // A graceful stop leaves no such litter; the force-kill stays as the + // backstop for a wedged process. + let _ = Command::new("uffs") + .args(["--mcp", "stop"]) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .status(); for name in ["uffsd", "uffsmcp"] { let status = if cfg!(windows) { Command::new("taskkill") From c19e80d760e86ebde549aeb2bf52a9d0d42a1e42 Mon Sep 17 00:00:00 2001 From: Robert M1 <50460704+githubrobbi@users.noreply.github.com> Date: Wed, 12 Aug 2026 14:59:12 -0700 Subject: [PATCH 06/16] feat(watchdog): user-level supervisor for the resident daemon + MCP MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Residency promised a daemon that is always there; the login item delivers that at boot and the auto-spawn marker revives one on the next search, but nothing noticed a service vanishing mid-session while no one was searching. launchd and systemd close that gap on macOS/Linux; the Windows Run key fires once at login and never again. This is the missing supervisor, on every platform. Design decisions worth recording: * SEPARATE BINARY, not a `uffs` subcommand. A supervisor cannot supervise its own death, so it must outlive the teardowns that kill what it watches — `install-bins.rs` force-kills `uffsd`/`uffsmcp` by image name, which is exactly how the MCP stdio supervisor died. A different image name survives that. A long-running `uffs.exe` would also lock the most frequently replaced binary in the tree, reintroducing the `os error 32` the installer just learned to avoid; the watchdog's own code changes rarely, so skip-if-identical means it almost never blocks an install. * NOT ELEVATED, and the broker is deliberately NOT supervised. The broker is a LocalSystem service registered `start= auto`, so the SCM already restarts it at boot, and a non-elevated process cannot StartService it at all. Supervising it here would demand elevation and destroy the zero-UAC property residency exists to protect. The right mechanism there is SCM failure actions at `--install` time. * DELIBERATE STOPS WIN. A clean stop records intent and the watchdog honours it until the next explicit start (launchd's KeepAlive.SuccessfulExit=false semantics). Without it the watchdog would fight the operator every time they stop something on purpose. * Respawns are rate-limited to 3 per 60 s per service, then it gives up loudly — a service that dies instantly on every start is broken in a way respawning cannot fix, and an unbounded retry is a fork bomb that buries the real error. * It never INTRODUCES a service: a gateway the user has never started is not started by the watchdog, only restarted once seen running. The policy is pure and unit-tested (decide/RespawnLedger); process spawning is kept at the edge. `dirs-next` rather than `uffs-client` for the one directory path it needs — a supervisor that only shells out to `uffs` should not pull the whole client in. Still to wire (follow-up): the stop-intent markers are read but not yet written by the `stop`/`start` paths, and nothing launches the watchdog yet — `resident on` should, and `install-bins` should cycle it. --- Cargo.lock | 8 + Cargo.toml | 5 +- crates/uffs-watchdog/Cargo.toml | 70 ++++++++ crates/uffs-watchdog/src/main.rs | 224 ++++++++++++++++++++++++++ crates/uffs-watchdog/src/supervise.rs | 130 +++++++++++++++ 5 files changed, 435 insertions(+), 2 deletions(-) create mode 100644 crates/uffs-watchdog/Cargo.toml create mode 100644 crates/uffs-watchdog/src/main.rs create mode 100644 crates/uffs-watchdog/src/supervise.rs diff --git a/Cargo.lock b/Cargo.lock index aa7f14bd4..9479e6787 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4709,6 +4709,14 @@ dependencies = [ "windows", ] +[[package]] +name = "uffs-watchdog" +version = "0.6.31" +dependencies = [ + "anyhow", + "dirs-next", +] + [[package]] name = "uffs-winsvc" version = "0.6.31" diff --git a/Cargo.toml b/Cargo.toml index 03452d3d5..6be10df3f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -43,8 +43,9 @@ members = [ "crates/uffs-broker", # 🔑 Windows elevated handle broker (optional) "crates/uffs-vss-requestor", # 🩹 Per-run native VSS snapshot helper, spawned by uffs-broker (optional) # ── Surfaces ── - "crates/uffs-cli", # 🖥️ Command-line interface - "crates/uffs-update", # ⬆️ Self-update acquire helper (HTTP/TLS isolated from the CLI) + "crates/uffs-cli", # 🖥️ Command-line interface + "crates/uffs-update", # ⬆️ Self-update acquire helper (HTTP/TLS isolated from the CLI) + "crates/uffs-watchdog", # 🐕 User-level supervisor for the resident daemon + MCP # NOTE: uffs-tui and uffs-gui have moved to the private uffs-products repo. # ── Tools ── "crates/uffs-diag", # 🔬 Retained workspace-only diagnostic tools (not shipped in dist/) diff --git a/crates/uffs-watchdog/Cargo.toml b/crates/uffs-watchdog/Cargo.toml new file mode 100644 index 000000000..5015ec01d --- /dev/null +++ b/crates/uffs-watchdog/Cargo.toml @@ -0,0 +1,70 @@ +# ============================================================================ +# uffs-watchdog: user-level supervisor for the resident UFFS services +# ============================================================================ +# Keeps the services a user asked to be RESIDENT actually running: it polls +# their liveness and respawns anything that vanished (crash, `taskkill`, an +# installer that tore them down and did not put them back). +# +# WHY A SEPARATE BINARY +# --------------------- +# A supervisor cannot supervise its own death, so it must survive the +# teardowns that kill the things it watches. `scripts/dev/install-bins.rs` +# force-kills by image name (`uffsd`, `uffsmcp`), and the MCP stdio +# supervisor was killed by exactly that — proving the point. This binary is +# named differently, so those kills miss it. It is also its own binary +# rather than a `uffs` subcommand because a long-running `uffs.exe` would +# lock the most frequently replaced binary in the tree, reintroducing the +# `os error 32` the install just learned to avoid. Its own code changes +# rarely, so the installer's skip-if-identical path means it almost never +# blocks an install. +# +# WHY IT IS NOT ELEVATED +# ---------------------- +# Residency's whole promise is zero-UAC: a non-elevated daemon reads the MFT +# through the Access Broker. The watchdog therefore runs as the user and +# supervises only user-owned processes. +# +# WHAT IT DELIBERATELY DOES NOT SUPERVISE +# --------------------------------------- +# The Access Broker. It is a LocalSystem service registered `start= auto`, +# so the SCM already restarts it at boot — and a non-elevated process cannot +# call `StartService` on it anyway. The correct mechanism there is SCM +# failure actions, configured once at `--install` time where admin is +# already held. Supervising it from here would require elevation and would +# undo the zero-UAC property above. +# ============================================================================ + +[package] +name = "uffs-watchdog" +description = "User-level supervisor that keeps the resident UFFS daemon and MCP server running" +version.workspace = true +edition.workspace = true +license.workspace = true +repository.workspace = true +authors.workspace = true +readme.workspace = true +keywords.workspace = true +categories.workspace = true +# Internal tooling binary — never published. +publish.workspace = true + +[package.metadata.docs.rs] +all-features = true +rustdoc-args = ["--cfg", "docsrs"] + +[[bin]] +name = "uffs-watchdog" +path = "src/main.rs" + +[dependencies] +# Resolves the per-user lifecycle directory that holds the PID file and +# the stop-intent markers. Deliberately NOT depending on `uffs-client` +# for this: the watchdog needs one directory path, and pulling the whole +# client (protocol, IPC, formatting) into a supervisor that only shells +# out to `uffs` would be a heavy edge for a single helper. +dirs-next.workspace = true + +anyhow.workspace = true + +[lints] +workspace = true diff --git a/crates/uffs-watchdog/src/main.rs b/crates/uffs-watchdog/src/main.rs new file mode 100644 index 000000000..0df79e2d0 --- /dev/null +++ b/crates/uffs-watchdog/src/main.rs @@ -0,0 +1,224 @@ +// SPDX-License-Identifier: MPL-2.0 +// Copyright (c) 2025-2026 SKY, LLC. + +//! `uffs-watchdog` — keeps the resident UFFS services actually running. +//! +//! Residency promises a daemon that is always there (and starts at +//! login). The login item delivers that at boot, and the auto-spawn +//! marker revives a dead daemon on the next search — but nothing notices +//! a service that vanishes mid-session while no one is searching. On +//! macOS and Linux launchd/systemd close that gap; on Windows the `Run` +//! key fires once at login and never again. This binary is that missing +//! supervisor, on every platform. +//! +//! # What it supervises +//! +//! | Service | How | Why here | +//! |---|---|---| +//! | `uffsd` | `uffs --daemon start` | user process; respawn inherits the resident marker, so it returns with `--no-retire` | +//! | `uffsmcp` (HTTP gateway) | `uffs --mcp start` | user process; only supervised once it has been started at least once | +//! +//! The Access Broker is **deliberately absent**: it is a `LocalSystem` +//! service registered `start= auto`, so the `SCM` already restarts it at +//! boot, and a non-elevated process cannot `StartService` it anyway. +//! Supervising it from here would demand elevation and break the +//! zero-UAC property residency exists to protect — the right mechanism +//! is SCM failure actions, set once at `--install` time. +//! +//! # Deliberate stops win +//! +//! A clean `uffs --daemon stop` records stop intent, and the watchdog +//! honours it until the next explicit start ([`supervise::Action`]). +//! Without that it would fight the operator on every intentional stop. +//! +//! # Why a separate binary +//! +//! A supervisor cannot supervise its own death, so it must outlive the +//! teardowns that kill what it watches — `install-bins.rs` force-kills +//! `uffsd`/`uffsmcp` by image name, which is exactly how the MCP stdio +//! supervisor died. A different image name survives that. It is also not +//! a `uffs` subcommand: a long-running `uffs.exe` would lock the most +//! frequently replaced binary in the tree. + +mod supervise; + +use core::time::Duration; + +use supervise::{Action, RespawnLedger, decide}; + +/// How often liveness is polled. +/// +/// Seconds-scale: a respawn that lands within a few seconds of a crash +/// is indistinguishable from "never went away" for an interactive +/// search, and the probe is two cheap process checks. +const POLL_INTERVAL: Duration = Duration::from_secs(5); + +/// Environment override for [`POLL_INTERVAL`], in seconds (tests, and +/// operators who want a tighter or looser loop). +const POLL_ENV: &str = "UFFS_WATCHDOG_POLL_SECS"; + +/// One supervised service. +struct Service { + /// Display name used in log lines. + name: &'static str, + /// `uffs` subcommand pair that starts it (e.g. `--daemon start`). + start_args: [&'static str; 2], + /// Sliding-window respawn ledger. + ledger: RespawnLedger, + /// Whether this service has ever been seen running, so an MCP + /// gateway the user never started is not started *by* the watchdog. + seen_running: bool, +} + +#[expect( + clippy::print_stderr, + reason = "a supervisor's log IS its user interface; it has no other channel" +)] +#[expect( + clippy::infinite_loop, + reason = "supervising is the whole job — the loop ends when the process is stopped" +)] +fn main() -> anyhow::Result<()> { + let poll = std::env::var(POLL_ENV) + .ok() + .and_then(|raw| raw.parse::().ok()) + .map_or(POLL_INTERVAL, Duration::from_secs); + + let mut services = [ + Service { + name: "daemon", + start_args: ["--daemon", "start"], + ledger: RespawnLedger::default(), + seen_running: false, + }, + Service { + name: "mcp", + start_args: ["--mcp", "start"], + ledger: RespawnLedger::default(), + seen_running: false, + }, + ]; + + eprintln!("uffs-watchdog armed (poll {}s)", poll.as_secs()); + loop { + for service in &mut services { + tick(service); + } + std::thread::sleep(poll); + } +} + +/// Evaluate and act on one service for this tick. +#[expect( + clippy::print_stderr, + reason = "a supervisor's log IS its user interface; it has no other channel" +)] +fn tick(service: &mut Service) { + let alive = is_running(service.start_args[0]); + if alive { + service.seen_running = true; + return; + } + // Never *introduce* a service the user has not run themselves: a + // machine that never starts the MCP gateway should not acquire one + // because a watchdog is present. + if !service.seen_running { + return; + } + let now = std::time::Instant::now(); + let recent = service.ledger.recent(now); + match decide(alive, stop_intent(service.start_args[0]), recent) { + // Running, or the operator asked for it to be down: both mean + // "do nothing", but they are distinct decisions upstream. + Action::Leave | Action::HonourStopIntent => {} + Action::GaveUp => { + eprintln!( + "uffs-watchdog: {} died {} times in the crash window — not respawning again", + service.name, recent + ); + } + Action::Respawn => { + eprintln!("uffs-watchdog: {} is gone — restarting", service.name); + service.ledger.record(now); + let started = std::process::Command::new(uffs_exe()) + .args(service.start_args) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .status() + .is_ok_and(|status| status.success()); + if !started { + eprintln!("uffs-watchdog: {} restart failed", service.name); + } + } + } +} + +/// The `uffs` CLI to drive, resolved next to this binary so a watchdog +/// installed in `~/bin` drives the `uffs` beside it rather than whatever +/// `PATH` happens to resolve. +fn uffs_exe() -> std::path::PathBuf { + let name = if cfg!(windows) { "uffs.exe" } else { "uffs" }; + std::env::current_exe() + .ok() + .and_then(|exe| exe.parent().map(|dir| dir.join(name))) + .filter(|candidate| candidate.is_file()) + .unwrap_or_else(|| std::path::PathBuf::from(name)) +} + +/// Liveness probe: ask the CLI, whose status output is the single source +/// of truth for "is it up" on both transports. +/// +/// `--daemon status` prints `● running PID …` when up and +/// `○ Daemon not running` when not; `--mcp status` mirrors the shape. +fn is_running(kind: &str) -> bool { + std::process::Command::new(uffs_exe()) + .args([kind, "status"]) + .output() + .is_ok_and(|out| { + let text = String::from_utf8_lossy(&out.stdout); + text.contains("running") && !text.contains("not running") + }) +} + +/// Did the user deliberately stop this service? +/// +/// Recorded as a marker file beside the lifecycle state, written by the +/// explicit `stop` paths and cleared by an explicit `start`. Absent +/// marker means the service went away on its own, which is what the +/// watchdog exists to repair. +fn stop_intent(kind: &str) -> bool { + stop_intent_path(kind).is_some_and(|path| path.exists()) +} + +/// Path of the stop-intent marker for a service kind. +fn stop_intent_path(kind: &str) -> Option { + // Mirrors `uffs_client::daemon_ctl::pid_file_path`'s directory: + // `/uffs`. Kept in sync by the test below rather than by + // a dependency edge (see the manifest rationale). + let dir = dirs_next::data_local_dir() + .unwrap_or_else(|| std::path::PathBuf::from("/tmp")) + .join("uffs"); + let leaf = match kind { + "--daemon" => "daemon.stopped", + "--mcp" => "mcp.stopped", + _ => return None, + }; + Some(dir.join(leaf)) +} + +#[cfg(test)] +mod tests { + use super::stop_intent_path; + + /// Each supervised kind maps to its own marker; unknown kinds map to + /// none, so a typo can never silently suppress supervision. + #[test] + fn stop_intent_paths_are_per_service() { + let daemon = stop_intent_path("--daemon"); + let mcp = stop_intent_path("--mcp"); + assert!(daemon.is_some()); + assert!(mcp.is_some()); + assert_ne!(daemon, mcp); + assert_eq!(stop_intent_path("--broker"), None); + } +} diff --git a/crates/uffs-watchdog/src/supervise.rs b/crates/uffs-watchdog/src/supervise.rs new file mode 100644 index 000000000..289180118 --- /dev/null +++ b/crates/uffs-watchdog/src/supervise.rs @@ -0,0 +1,130 @@ +// SPDX-License-Identifier: MPL-2.0 +// Copyright (c) 2025-2026 SKY, LLC. + +//! The pure supervision policy: what to do about a service that is not +//! running, and how often we are willing to do it. +//! +//! Kept free of process spawning and clocks-by-default so the decisions +//! are unit-testable on every platform — the same reason +//! `uffs-daemon::cache::policy` keeps `next_state_for_idle` pure. + +use core::time::Duration; + +/// Respawns tolerated inside [`CRASH_WINDOW`] before the watchdog stops +/// trying and says so. +/// +/// Mirrors the MCP supervisor's crash hatch: a service that dies +/// immediately on every start is broken in a way respawning cannot fix, +/// and an unbounded retry loop turns that into a fork bomb that also +/// buries the real error in log noise. +pub(crate) const RESPAWN_LIMIT: usize = 3; + +/// The window [`RESPAWN_LIMIT`] applies over. +pub(crate) const CRASH_WINDOW: Duration = Duration::from_secs(60); + +/// What the watchdog should do about one service this tick. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum Action { + /// Running (or not configured): do nothing. + Leave, + /// Gone, and the user did not ask for that: start it again. + Respawn, + /// Gone because the user deliberately stopped it: honour that. + /// + /// Mirrors launchd's `KeepAlive.SuccessfulExit = false` — a clean + /// `uffs --daemon stop` must stick, or the watchdog fights the + /// operator every time they stop something on purpose. + HonourStopIntent, + /// Gone, but it has already been respawned too many times in the + /// window: give up loudly rather than crash-loop. + GaveUp, +} + +/// Decide what to do about one service. +/// +/// `alive` is the liveness probe result, `stop_intent` whether the user +/// asked for it to be down, and `recent_respawns` how many times it has +/// been restarted inside [`CRASH_WINDOW`]. +pub(crate) const fn decide(alive: bool, stop_intent: bool, recent_respawns: usize) -> Action { + if alive { + return Action::Leave; + } + if stop_intent { + return Action::HonourStopIntent; + } + if recent_respawns >= RESPAWN_LIMIT { + return Action::GaveUp; + } + Action::Respawn +} + +/// Sliding-window respawn counter for a single service. +/// +/// Holds the instants of recent respawns and prunes those older than +/// [`CRASH_WINDOW`], so a service that dies once an hour is restarted +/// every time while one that dies in a tight loop is abandoned. +#[derive(Debug, Default)] +pub(crate) struct RespawnLedger { + /// Respawn timestamps still inside the window. + events: Vec, +} + +impl RespawnLedger { + /// Drop entries older than the window and report how many remain. + pub(crate) fn recent(&mut self, now: std::time::Instant) -> usize { + self.events + .retain(|at| now.duration_since(*at) < CRASH_WINDOW); + self.events.len() + } + + /// Record a respawn that just happened. + pub(crate) fn record(&mut self, now: std::time::Instant) { + self.events.push(now); + } +} + +#[cfg(test)] +mod tests { + use super::{Action, CRASH_WINDOW, RESPAWN_LIMIT, RespawnLedger, decide}; + + /// A live service is never touched, whatever else is true. + #[test] + fn alive_is_always_left_alone() { + assert_eq!(decide(true, false, 0), Action::Leave); + assert_eq!(decide(true, true, RESPAWN_LIMIT + 5), Action::Leave); + } + + /// A deliberate stop outranks respawning — this is the property that + /// keeps the watchdog from fighting the operator. + #[test] + fn deliberate_stop_is_honoured_over_respawn() { + assert_eq!(decide(false, true, 0), Action::HonourStopIntent); + } + + /// A vanished service with no stop intent comes back, until the + /// window limit is reached. + #[test] + fn respawns_until_the_window_limit() { + assert_eq!(decide(false, false, 0), Action::Respawn); + assert_eq!(decide(false, false, RESPAWN_LIMIT - 1), Action::Respawn); + assert_eq!(decide(false, false, RESPAWN_LIMIT), Action::GaveUp); + } + + /// The ledger forgets respawns once they age out, so a service that + /// dies rarely is always restarted. + #[test] + fn ledger_prunes_outside_the_window() { + let mut ledger = RespawnLedger::default(); + let start = std::time::Instant::now(); + for _ in 0..RESPAWN_LIMIT { + ledger.record(start); + } + assert_eq!(ledger.recent(start), RESPAWN_LIMIT, "all inside the window"); + + let later = start + .checked_add(CRASH_WINDOW) + .and_then(|at| at.checked_add(core::time::Duration::from_secs(1))) + .unwrap_or(start); + assert_eq!(ledger.recent(later), 0, "all aged out"); + } +} From 5b25c3e347a37879b3c035f3126b543dd640efeb Mon Sep 17 00:00:00 2001 From: Robert M1 <50460704+githubrobbi@users.noreply.github.com> Date: Wed, 12 Aug 2026 15:19:03 -0700 Subject: [PATCH 07/16] feat(watchdog): wire supervision into the service lifecycle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Makes the watchdog crate actually run and actually correct. STOP INTENT (uffs-client::daemon_ctl) — the piece without which the watchdog would fight the operator. `--daemon stop` and `--mcp stop` record intent; `--daemon start` and `--mcp start` clear it. The watchdog already reads these to tell "this crashed, put it back" from "the operator wanted it down" (launchd's KeepAlive.SuccessfulExit=false semantics). Shared helpers live in `daemon_ctl` beside the PID file because both writers (uffs-cli, uffs-mcp) already depend on it. ARMING (`resident on`) — starts the watchdog, and refuses to start a second one if one is already supervising, since two supervisors would double every respawn decision. Windows only, deliberately: launchd (`KeepAlive`) and systemd (`Restart=on-failure`) already supervise the daemon on macOS and Linux, so a second supervisor there would be redundant machinery racing the OS. The Windows Run key fires once at login and never again — exactly the gap this fills. INSTALL COOPERATION (install-bins.rs) — the watchdog is stopped FIRST in the teardown and restarted LAST. Order matters both ways: left running, it would dutifully restart the daemon mid-install (supervisor versus installer), and restarted too early it would race the daemon and MCP restarts happening just above it. Everything stays best-effort: a watchdog that fails to arm leaves residency installed and working, and every failure prints the command to run by hand. Workspace clippy clean; 2430 tests pass. --- crates/uffs-cli/src/commands/daemon_mgmt.rs | 5 ++ crates/uffs-cli/src/commands/resident.rs | 59 ++++++++++++++++++++ crates/uffs-client/src/daemon_ctl.rs | 49 +++++++++++++++++ crates/uffs-mcp/src/main.rs | 4 ++ scripts/dev/install-bins.rs | 60 +++++++++++++++++++++ 5 files changed, 177 insertions(+) diff --git a/crates/uffs-cli/src/commands/daemon_mgmt.rs b/crates/uffs-cli/src/commands/daemon_mgmt.rs index 8ad64c69a..2470e496c 100644 --- a/crates/uffs-cli/src/commands/daemon_mgmt.rs +++ b/crates/uffs-cli/src/commands/daemon_mgmt.rs @@ -381,6 +381,9 @@ fn daemon_start( ); } + // An explicit start revokes any earlier stop intent, so the + // watchdog resumes supervising this service. + uffs_client::daemon_ctl::clear_stop_intent(uffs_client::daemon_ctl::ServiceKind::Daemon); if !is_quiet() { println!("Starting daemon..."); } @@ -413,6 +416,8 @@ fn daemon_stop() -> Result<()> { client .shutdown() .with_context(|| "Shutdown RPC failed — try `uffs --daemon kill` instead")?; + // Deliberate stop: tell the watchdog not to undo it. + uffs_client::daemon_ctl::record_stop_intent(uffs_client::daemon_ctl::ServiceKind::Daemon); println!("Daemon shutdown requested."); } else { println!("Daemon is not running."); diff --git a/crates/uffs-cli/src/commands/resident.rs b/crates/uffs-cli/src/commands/resident.rs index 689c3ac64..c5cd9177a 100644 --- a/crates/uffs-cli/src/commands/resident.rs +++ b/crates/uffs-cli/src/commands/resident.rs @@ -89,6 +89,7 @@ fn resident_on( let argv = daemon_argv(mft_files, data_dir, drives); platform::turn_on(&exe, &argv)?; write_marker(&argv)?; + arm_watchdog(); println!( "\nUFFS is now resident: uffsd starts at login with --no-retire\n\ (never exits on idle; memory tiering still parks unused drives),\n\ @@ -98,6 +99,64 @@ fn resident_on( Ok(()) } +/// Start the user-level watchdog that keeps the resident services up. +/// +/// Windows only, and deliberately so: launchd (`KeepAlive`) and systemd +/// (`Restart=on-failure`) already supervise the daemon on macOS and +/// Linux, so a second supervisor there would be redundant machinery +/// racing the OS. The Windows `Run` key fires once at login and never +/// again, which is precisely the gap `uffs-watchdog` fills. +/// +/// Best-effort: residency is still installed and useful without it. +#[cfg(windows)] +#[expect(clippy::print_stdout, reason = "CLI user-facing output")] +fn arm_watchdog() { + let Some(exe) = watchdog_exe() else { + println!("(watchdog binary not found next to uffs — skipping supervision)"); + return; + }; + // Already supervising? Starting a second one would double every + // respawn decision. + if watchdog_running() { + return; + } + let spawned = std::process::Command::new(&exe) + .stdin(std::process::Stdio::null()) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .spawn() + .is_ok(); + if spawned { + println!("Watchdog armed: crashed services are restarted automatically."); + } else { + println!( + "(could not start the watchdog — run {} manually)", + exe.display() + ); + } +} + +/// Non-Windows: launchd / systemd already supervise the daemon. +#[cfg(not(windows))] +const fn arm_watchdog() {} + +/// Locate `uffs-watchdog` beside the running `uffs`. +#[cfg(windows)] +fn watchdog_exe() -> Option { + let here = std::env::current_exe().ok()?; + let candidate = here.parent()?.join("uffs-watchdog.exe"); + candidate.is_file().then_some(candidate) +} + +/// Is a watchdog already running for this user? +#[cfg(windows)] +fn watchdog_running() -> bool { + std::process::Command::new("tasklist") + .args(["/FI", "IMAGENAME eq uffs-watchdog.exe", "/NH"]) + .output() + .is_ok_and(|out| String::from_utf8_lossy(&out.stdout).contains("uffs-watchdog.exe")) +} + /// Write the resident marker (`resident.args`) so implicit auto-spawns /// — the next search after a crash or a manual stop — revive the /// daemon with the same resident argv the login item uses (merged in diff --git a/crates/uffs-client/src/daemon_ctl.rs b/crates/uffs-client/src/daemon_ctl.rs index 0e2044596..6c12efa2b 100644 --- a/crates/uffs-client/src/daemon_ctl.rs +++ b/crates/uffs-client/src/daemon_ctl.rs @@ -288,6 +288,55 @@ pub fn pid_file_path() -> PathBuf { base.join("uffs").join("daemon.pid") } +/// Which resident service a stop-intent marker refers to. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ServiceKind { + /// The search daemon (`uffsd`). + Daemon, + /// The MCP HTTP gateway (`uffsmcp`). + Mcp, +} + +impl ServiceKind { + /// Marker file name for this service. + const fn marker(self) -> &'static str { + match self { + Self::Daemon => "daemon.stopped", + Self::Mcp => "mcp.stopped", + } + } +} + +/// Path of a service's stop-intent marker, sibling of the PID file. +/// +/// Written when the operator stops a service ON PURPOSE and cleared +/// when they start it again. `uffs-watchdog` reads it to tell "this +/// crashed, put it back" apart from "the operator wanted it down" — +/// launchd's `KeepAlive.SuccessfulExit = false` semantics. Without it a +/// watchdog fights every deliberate stop. +#[must_use] +pub fn stop_intent_path(kind: ServiceKind) -> PathBuf { + let base = dirs_next::data_local_dir().unwrap_or_else(|| PathBuf::from("/tmp")); + base.join("uffs").join(kind.marker()) +} + +/// Record that the operator deliberately stopped `kind`. +/// +/// Best-effort: a watchdog that respawns a service because the marker +/// could not be written is a nuisance, not a correctness failure. +pub fn record_stop_intent(kind: ServiceKind) { + let path = stop_intent_path(kind); + if let Some(parent) = path.parent() { + let _ensure = std::fs::create_dir_all(parent); + } + let _best_effort = std::fs::write(&path, b"stopped by operator\n"); +} + +/// Clear any stop intent for `kind` — the operator started it again. +pub fn clear_stop_intent(kind: ServiceKind) { + let _absent_is_fine = std::fs::remove_file(stop_intent_path(kind)); +} + /// Path of the resident-marker file (`resident.args`), sibling of the /// PID file. /// diff --git a/crates/uffs-mcp/src/main.rs b/crates/uffs-mcp/src/main.rs index c6e366b82..4b980f27c 100644 --- a/crates/uffs-mcp/src/main.rs +++ b/crates/uffs-mcp/src/main.rs @@ -345,6 +345,8 @@ async fn mcp_start( cmd.env("UFFS_LOG_FILE", &default_log); } + // An explicit start revokes any earlier stop intent. + uffs_client::daemon_ctl::clear_stop_intent(uffs_client::daemon_ctl::ServiceKind::Mcp); println!("Starting MCP HTTP server on {bind}:{port}..."); let mut child = cmd.spawn().with_context(|| "Failed to spawn MCP server")?; let pid = child.id(); @@ -565,6 +567,8 @@ fn mcp_stop() { println!("MCP server is not running."); return; }; + // Deliberate stop: tell the watchdog not to undo it. + uffs_client::daemon_ctl::record_stop_intent(uffs_client::daemon_ctl::ServiceKind::Mcp); println!("Stopping MCP server (PID {pid})..."); process::signal_pid(pid, cfg!(windows)); println!("MCP server stopped."); diff --git a/scripts/dev/install-bins.rs b/scripts/dev/install-bins.rs index 5cd371a28..6f0b42997 100755 --- a/scripts/dev/install-bins.rs +++ b/scripts/dev/install-bins.rs @@ -40,6 +40,7 @@ fn main() { // after `stop_running_services` would of course always say "no". let daemon_was_running = daemon_is_running(); let mcp_was_running = mcp_is_running(); + let watchdog_was_running = watchdog_is_running(); // Stop the running daemon + MCP first (best effort), mirroring the // previous [unix] bash recipe: the old binaries release their file @@ -178,6 +179,11 @@ fn main() { if mcp_was_running { restart_mcp(&bin_dir); } + // Restart the supervisor LAST: it must not respawn services while + // they are mid-restart above, or it races the install. + if watchdog_was_running { + restart_watchdog(&bin_dir); + } if skipped > 0 { std::process::exit(1); } @@ -319,6 +325,42 @@ fn restart_daemon(bin_dir: &std::path::Path) { } } +/// Is a watchdog supervising right now? +fn watchdog_is_running() -> bool { + if cfg!(windows) { + Command::new("tasklist") + .args(["/FI", "IMAGENAME eq uffs-watchdog.exe", "/NH"]) + .output() + .map(|out| String::from_utf8_lossy(&out.stdout).contains("uffs-watchdog.exe")) + .unwrap_or(false) + } else { + Command::new("pgrep") + .args(["-x", "uffs-watchdog"]) + .output() + .map(|out| !out.stdout.is_empty()) + .unwrap_or(false) + } +} + +/// Restart the supervisor we stopped, using the new binary. +fn restart_watchdog(bin_dir: &std::path::Path) { + let exe = bin_dir.join(if cfg!(windows) { "uffs-watchdog.exe" } else { "uffs-watchdog" }); + if !exe.is_file() { + return; + } + let spawned = Command::new(&exe) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .spawn() + .is_ok(); + if spawned { + eprintln!("✅ Watchdog restarted."); + } else { + eprintln!("⚠️ Watchdog did NOT restart — run: {}", exe.display()); + } +} + /// Was the MCP HTTP gateway serving before the teardown? /// /// `--mcp status` prints `MCP server: running (PID …)` when up, and @@ -400,6 +442,24 @@ fn stop_running_services() { .stdout(Stdio::null()) .stderr(Stdio::null()) .status(); + // The watchdog is stopped FIRST: if it kept running while we tear + // the daemon down, it would dutifully restart it mid-install — the + // supervisor fighting the installer. It is restarted at the end. + for name in ["uffs-watchdog"] { + let _ = if cfg!(windows) { + Command::new("taskkill") + .args(["/IM", &format!("{name}.exe"), "/F"]) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .status() + } else { + Command::new("pkill") + .args(["-x", name]) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .status() + }; + } for name in ["uffsd", "uffsmcp"] { let status = if cfg!(windows) { Command::new("taskkill") From d62722c72840b1a1b65481c1e6ebd59413ced02e Mon Sep 17 00:00:00 2001 From: Robert M1 <50460704+githubrobbi@users.noreply.github.com> Date: Wed, 12 Aug 2026 16:20:26 -0700 Subject: [PATCH 08/16] fix(cli): align the per-drive component breakdown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `[rec=… names=… tri=… ch=… ext=…]` block was unpadded, so a one-digit `rec=1` and a three-digit `rec=608` started at the same column and shoved every later field out of line — the one part of the row you actually want to compare vertically: [rec=1 names=0 tri=0 ch=0 ext=0] [rec=608 names=439 tri=518 ch=55 ext=27] Each numeric is now width-padded: rec/names/tri hold four digits (a ~10 GB component on a very large drive), ch/ext three. The source label is padded too, since `(live)` and `(cache)` differ in width and would otherwise shift the `·` and everything right of it. The padding goes on the whole `(source)` token rather than the text inside it — `(live )` reads as a typo. --- crates/uffs-cli/src/commands/daemon_status.rs | 25 ++++++++++++++----- 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/crates/uffs-cli/src/commands/daemon_status.rs b/crates/uffs-cli/src/commands/daemon_status.rs index 476f1172a..51c3f4ae1 100644 --- a/crates/uffs-cli/src/commands/daemon_status.rs +++ b/crates/uffs-cli/src/commands/daemon_status.rs @@ -387,13 +387,26 @@ fn print_drive_line(palette: Palette, dr: &DriveInfo, memory: &[DriveMemoryInfo] match memory.iter().find(|dm| dm.drive == dr.letter) { Some(dm) => { let mb = |bytes: u64| bytes / MIB; - // `{:>6}` on the heap MB keeps the `· NNNN MB` column - // aligned across drives — an unpadded `{}` put `2 MB` - // and `1669 MB` at different columns, so the sizes could - // not be compared by eye down the list. + // Every numeric column is width-padded so the whole + // block reads as a table down the list. Unpadded, a + // one-digit `rec=1` and a three-digit `rec=608` started + // in the same place and pushed every later field out of + // line, which is exactly what you cannot scan by eye: + // + // [rec=1 names=0 tri=0 ch=0 ext=0] + // [rec=608 names=439 tri=518 ch=55 ext=27] + // + // Widths: rec/names/tri hold four digits (a ~10 GB + // component on a very large drive), ch/ext three. The + // source label is padded too — it is `live` today but + // `cache` and friends exist, and an unpadded label would + // shift the whole rest of the row. + // Pad the whole `(source)` token, not the text inside + // it — `(live )` with the space before the paren reads + // as a typo. + let source = format!("({})", dr.source); println!( - " {glyph} {letter} {records:>12} records ({}) \u{b7} {:>6} MB [rec={} names={} tri={} ch={} ext={}]", - dr.source, + " {glyph} {letter} {records:>12} records {source:<7} \u{b7} {:>6} MB [rec={:>4} names={:>4} tri={:>4} ch={:>3} ext={:>3}]", mb(dm.heap_bytes), mb(dm.records_bytes), mb(dm.names_bytes), From 50052287f9e616f21832e4155ec28ba458862ede Mon Sep 17 00:00:00 2001 From: Robert M1 <50460704+githubrobbi@users.noreply.github.com> Date: Wed, 12 Aug 2026 16:27:29 -0700 Subject: [PATCH 09/16] fix(cli): align the physical-drive table past the free-space column MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Everything after `… free` ragged, because the volume label is variable width and nothing reserved a column for it: a short "DATA" and a long "NTFS_16_GB" pushed the `· indexed (…)` note to different places, and a drive with no label at all pulled its note further left still. The label now occupies a fixed 12-column field (plus quotes), and an unlabelled drive holds that column with spaces rather than collapsing it. The record count in the index note is right-aligned to 11, so the counts line up on their commas the way the `── Drives ──` block does. 12 fits every label seen in practice; NTFS permits 32, and a longer one pushes its own row instead of being truncated — losing information to preserve a column would be the wrong trade. --- crates/uffs-cli/src/commands/daemon_status.rs | 25 ++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/crates/uffs-cli/src/commands/daemon_status.rs b/crates/uffs-cli/src/commands/daemon_status.rs index 51c3f4ae1..28bed91bf 100644 --- a/crates/uffs-cli/src/commands/daemon_status.rs +++ b/crates/uffs-cli/src/commands/daemon_status.rs @@ -28,6 +28,10 @@ use uffs_statusfmt::{Glyph, Palette, field, header, section, status_row}; /// One mebibyte, for the `bytes → MB` display conversions. const MIB: u64 = 1024 * 1024; +/// Width reserved for a quoted volume label in the physical-drive table, +/// so the `· indexed (…)` column after it lines up across rows. +const LABEL_COLUMN: usize = 12; + /// `uffs --daemon status [-v] [--json]` — show daemon status, PID, drives, and /// (in long / JSON form) performance counters. /// @@ -480,16 +484,31 @@ fn print_physical_drive_line(palette: Palette, drive: &PhysicalDrive, loaded: &[ ( Glyph::Up, format!( - " \u{b7} indexed ({} records)", + " \u{b7} indexed ({:>11} records)", format_number_commas(info.records as u64) ), ) }, ); + // Pad the volume label so the `· indexed (…)` column that follows + // starts in the same place on every row. Unpadded, a short label + // ("DATA") and a long one ("NTFS_16_GB") pushed the index note to + // different columns, which is the part you scan down the list. + // + // 12 columns fits the labels seen in practice (plus the quotes); + // NTFS permits up to 32, and a longer one simply pushes its own row + // rather than being truncated — losing information to preserve a + // column would be the wrong trade. let label = if drive.label.is_empty() { - String::new() + // Still occupy the column, so a drive with no label does not + // pull its index note left of everyone else's. + " ".repeat(LABEL_COLUMN + 2) } else { - format!(" \u{201c}{}\u{201d}", drive.label) + format!( + " {:9} \u{b7} {:>4.0}% used \u{b7} {:>9} free{label}{index_note}", From a1ab339ef548eff13b4eaee747b1fce4bc2d353b Mon Sep 17 00:00:00 2001 From: Robert M1 <50460704+githubrobbi@users.noreply.github.com> Date: Wed, 12 Aug 2026 16:53:42 -0700 Subject: [PATCH 10/16] fix(watchdog): record stop intent BEFORE the shutdown, not after MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Live-box test: `uffs --daemon stop` reported success, the daemon went away — and roughly ten seconds later the watchdog brought it back. A deliberate stop did not stick, which is the one behaviour that makes a supervisor unusable. Not a wiring mistake: the ordering was wrong. `shutdown()` blocks until the daemon is actually gone, and tearing down a 24.9 M-record index with seven journal loops takes seconds. Writing the marker after the RPC returned left a multi-second window in which the daemon was already dead and the marker did not exist yet. A watchdog tick landing in that window sees an unexplained death and does exactly what it is built to do — respawn. Worse, the respawn runs `uffs --daemon start`, which clears stop intent, so the evidence erased itself. The marker is now written before the RPC. If the shutdown then fails, the intent is cleared again: an intent that was never carried out must not stop the watchdog reviving a later genuine crash. `--daemon kill` gets the same treatment — a kill is as deliberate as a stop, and it was not recording intent at all. The MCP stop path already wrote before signalling; its comment now states why, so the ordering is not "tidied" back later. Found only by running it on real hardware: the unit tests cover the decision (`decide`), and the decision was right — the input was late. --- crates/uffs-cli/src/commands/daemon_mgmt.rs | 27 ++++++++++++++++++--- crates/uffs-mcp/src/main.rs | 4 ++- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/crates/uffs-cli/src/commands/daemon_mgmt.rs b/crates/uffs-cli/src/commands/daemon_mgmt.rs index 2470e496c..35434bc71 100644 --- a/crates/uffs-cli/src/commands/daemon_mgmt.rs +++ b/crates/uffs-cli/src/commands/daemon_mgmt.rs @@ -413,11 +413,27 @@ fn daemon_start( #[expect(clippy::print_stdout, reason = "CLI user-facing output")] fn daemon_stop() -> Result<()> { if let Ok(mut client) = UffsClientSync::connect_raw() { - client - .shutdown() - .with_context(|| "Shutdown RPC failed — try `uffs --daemon kill` instead")?; - // Deliberate stop: tell the watchdog not to undo it. + // Record the intent BEFORE the RPC, not after. + // + // `shutdown()` blocks until the daemon is actually gone, and on a + // large index (24.9 M records, seven journal loops) that teardown + // takes seconds. Writing the marker afterwards leaves a window in + // which the daemon is already dead and the marker does not exist + // yet — a watchdog tick landing there sees an unexplained death + // and dutifully respawns it, so a deliberate stop bounces back. + // Observed exactly that on a live box before this ordering fix. uffs_client::daemon_ctl::record_stop_intent(uffs_client::daemon_ctl::ServiceKind::Daemon); + if let Err(err) = client + .shutdown() + .with_context(|| "Shutdown RPC failed — try `uffs --daemon kill` instead") + { + // The daemon is still up: an intent we never carried out must + // not keep the watchdog from reviving a later genuine crash. + uffs_client::daemon_ctl::clear_stop_intent( + uffs_client::daemon_ctl::ServiceKind::Daemon, + ); + return Err(err); + } println!("Daemon shutdown requested."); } else { println!("Daemon is not running."); @@ -435,6 +451,9 @@ fn daemon_stop() -> Result<()> { /// running" half-kill. #[expect(clippy::print_stdout, reason = "CLI user-facing output")] fn daemon_kill() -> Result<()> { + // A kill is as deliberate as a stop — same reasoning, same ordering: + // record before the process actually dies. + uffs_client::daemon_ctl::record_stop_intent(uffs_client::daemon_ctl::ServiceKind::Daemon); let pid_path = pid_file_path(); let mut pid = diff --git a/crates/uffs-mcp/src/main.rs b/crates/uffs-mcp/src/main.rs index 4b980f27c..6b016791d 100644 --- a/crates/uffs-mcp/src/main.rs +++ b/crates/uffs-mcp/src/main.rs @@ -567,7 +567,9 @@ fn mcp_stop() { println!("MCP server is not running."); return; }; - // Deliberate stop: tell the watchdog not to undo it. + // Record intent BEFORE signalling, for the same reason the daemon + // does: between the process dying and the marker appearing, a + // watchdog tick would read an unexplained death and restart it. uffs_client::daemon_ctl::record_stop_intent(uffs_client::daemon_ctl::ServiceKind::Mcp); println!("Stopping MCP server (PID {pid})..."); process::signal_pid(pid, cfg!(windows)); From 96f165b96d5c08c142d398a372d42271c36afd3f Mon Sep 17 00:00:00 2001 From: Robert M1 <50460704+githubrobbi@users.noreply.github.com> Date: Wed, 12 Aug 2026 17:14:24 -0700 Subject: [PATCH 11/16] fix(watchdog): stop supervisor restarts erasing stop intent; add a log MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Live test: a deliberate `--daemon stop` still bounced back, and it only stuck on the SECOND stop in a row. That "second one sticks" is the tell — it is the 3-per-60s respawn limiter running out, which means intent was being ignored every time rather than intermittently. Two defects, both visible by inspection once the symptom pointed here: 1. The watchdog respawns by invoking `uffs --daemon start`, and that command cleared the stop-intent marker. So the supervisor erased the very marker it is meant to obey: the intent survived at most one tick and the service came back regardless. The clear is now skipped when `UFFS_SUPERVISED_RESTART` is set, which the watchdog sets on the restart it drives — an operator start still revokes intent, a supervisor restart never does. 2. The watchdog was unobservable. `resident on` spawns it with stdio discarded, so every decision it made vanished; the earlier fix was a guess because nothing could be inspected. It now appends each decision to `/watchdog.log` WITH THE INPUTS — service, stop_intent, the exact marker path it consulted, recent respawn count and the resulting action. If this is still wrong, that file says why instead of requiring another guess. Ordering from the previous commit (marker before the RPC) stays; it was necessary but not sufficient. --- crates/uffs-cli/src/commands/daemon_mgmt.rs | 12 ++++- crates/uffs-watchdog/src/main.rs | 49 +++++++++++++++++++-- 2 files changed, 55 insertions(+), 6 deletions(-) diff --git a/crates/uffs-cli/src/commands/daemon_mgmt.rs b/crates/uffs-cli/src/commands/daemon_mgmt.rs index 35434bc71..3beb1f2b1 100644 --- a/crates/uffs-cli/src/commands/daemon_mgmt.rs +++ b/crates/uffs-cli/src/commands/daemon_mgmt.rs @@ -381,9 +381,17 @@ fn daemon_start( ); } - // An explicit start revokes any earlier stop intent, so the + // An explicit OPERATOR start revokes any earlier stop intent, so the // watchdog resumes supervising this service. - uffs_client::daemon_ctl::clear_stop_intent(uffs_client::daemon_ctl::ServiceKind::Daemon); + // + // A supervisor-driven restart must NOT clear it: the watchdog + // respawns by invoking this very command, so clearing here would let + // it erase the marker it is supposed to obey — the intent survives + // exactly one tick and the service bounces back anyway. The watchdog + // sets `UFFS_SUPERVISED_RESTART` to say "this start is mine". + if std::env::var_os("UFFS_SUPERVISED_RESTART").is_none() { + uffs_client::daemon_ctl::clear_stop_intent(uffs_client::daemon_ctl::ServiceKind::Daemon); + } if !is_quiet() { println!("Starting daemon..."); } diff --git a/crates/uffs-watchdog/src/main.rs b/crates/uffs-watchdog/src/main.rs index 0df79e2d0..0e6b1487d 100644 --- a/crates/uffs-watchdog/src/main.rs +++ b/crates/uffs-watchdog/src/main.rs @@ -108,6 +108,35 @@ fn main() -> anyhow::Result<()> { } } +/// Append one line to the watchdog log, beside the lifecycle state. +/// +/// `resident on` spawns the watchdog with its stdio discarded, so +/// `eprintln!` alone leaves the supervisor's decisions invisible — which +/// made a stop-intent bug undiagnosable from the outside and cost two +/// wrong guesses before this existed. Every decision is now recorded +/// with the inputs that produced it. +fn log_line(message: &str) { + let path = lifecycle_dir().join("watchdog.log"); + if let Some(parent) = path.parent() { + let _ensure = std::fs::create_dir_all(parent); + } + if let Ok(mut file) = std::fs::OpenOptions::new() + .create(true) + .append(true) + .open(&path) + { + use std::io::Write as _; + let _best_effort = writeln!(file, "{message}"); + } +} + +/// The per-user lifecycle directory holding the PID file and markers. +fn lifecycle_dir() -> std::path::PathBuf { + dirs_next::data_local_dir() + .unwrap_or_else(|| std::path::PathBuf::from("/tmp")) + .join("uffs") +} + /// Evaluate and act on one service for this tick. #[expect( clippy::print_stderr, @@ -127,7 +156,18 @@ fn tick(service: &mut Service) { } let now = std::time::Instant::now(); let recent = service.ledger.recent(now); - match decide(alive, stop_intent(service.start_args[0]), recent) { + let intent = stop_intent(service.start_args[0]); + let action = decide(alive, intent, recent); + log_line(&format!( + "{} down: stop_intent={} (marker {}) recent_respawns={} -> {:?}", + service.name, + intent, + stop_intent_path(service.start_args[0]) + .map_or_else(|| "?".to_owned(), |path| path.display().to_string()), + recent, + action, + )); + match action { // Running, or the operator asked for it to be down: both mean // "do nothing", but they are distinct decisions upstream. Action::Leave | Action::HonourStopIntent => {} @@ -142,6 +182,9 @@ fn tick(service: &mut Service) { service.ledger.record(now); let started = std::process::Command::new(uffs_exe()) .args(service.start_args) + // Tell the CLI this start is the supervisor's, not the + // operator's, so it leaves any stop-intent marker alone. + .env("UFFS_SUPERVISED_RESTART", "1") .stdout(std::process::Stdio::null()) .stderr(std::process::Stdio::null()) .status() @@ -195,9 +238,7 @@ fn stop_intent_path(kind: &str) -> Option { // Mirrors `uffs_client::daemon_ctl::pid_file_path`'s directory: // `/uffs`. Kept in sync by the test below rather than by // a dependency edge (see the manifest rationale). - let dir = dirs_next::data_local_dir() - .unwrap_or_else(|| std::path::PathBuf::from("/tmp")) - .join("uffs"); + let dir = lifecycle_dir(); let leaf = match kind { "--daemon" => "daemon.stopped", "--mcp" => "mcp.stopped", From 5c29849f13150892f573c04cb50b3643d3e168f7 Mon Sep 17 00:00:00 2001 From: Robert M1 <50460704+githubrobbi@users.noreply.github.com> Date: Wed, 12 Aug 2026 17:46:31 -0700 Subject: [PATCH 12/16] fix(watchdog): read liveness per service so a stopped daemon stays stopped MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A deliberate `uffs --daemon stop` bounced straight back, and the watchdog log exonerated itself: every daemon line read `HonourStopIntent`. It never touched the daemon — it defeated the stop through the MCP. The liveness probe was `uffs -- status` scanned for the substring `running` minus `not running`. But `--mcp status` reports the daemon too, so a stopped daemon put `Daemon: not running` into the *MCP* report and the healthy gateway read as dead. The watchdog then ran `uffs --mcp start`, whose preflight sees "gateway up, daemon down" and helpfully restarts the daemon. Three rounds of that exhausted the respawn ledger, the watchdog gave up on the MCP, and only then did the stop finally stick — which is exactly why it appeared to need two stops in a row. The same substring read had a second defect: `◐ loading (3/7 drives)` contains neither string, so a daemon still reading the MFT counted as dead and was liable to be respawned on top of itself. Liveness now comes from `uffs --status --json`, which reports every service under its own key, so one service's state can no longer be mistaken for another's. It is one subprocess per tick instead of two, and `connect_raw` never auto-spawns, so probing stays side-effect free. An unreadable probe now means *unknown* and is left alone, where it previously read as "down" and triggered a respawn. Closing the same hole from the other side: `--mcp start` no longer revives an unreachable daemon when `UFFS_SUPERVISED_RESTART` is set and a daemon stop-intent marker exists. Interactively the old behaviour is still right — you asked for a gateway, a gateway needs a daemon — but the watchdog is not the operator and must not drag a deliberately stopped daemon back up with the gateway. `install-bins.rs` carried both bugs in its own probes: `use-local` would have failed to restore an MCP gateway whenever the daemon was down, and failed to restore a daemon caught mid-load. --- Cargo.lock | 1 + crates/uffs-mcp/src/main.rs | 16 ++++ crates/uffs-watchdog/Cargo.toml | 6 ++ crates/uffs-watchdog/src/main.rs | 140 ++++++++++++++++++++++++++----- scripts/dev/install-bins.rs | 57 +++++++------ 5 files changed, 173 insertions(+), 47 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9479e6787..0e723d982 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4715,6 +4715,7 @@ version = "0.6.31" dependencies = [ "anyhow", "dirs-next", + "serde_json", ] [[package]] diff --git a/crates/uffs-mcp/src/main.rs b/crates/uffs-mcp/src/main.rs index 6b016791d..2f1ee5f52 100644 --- a/crates/uffs-mcp/src/main.rs +++ b/crates/uffs-mcp/src/main.rs @@ -431,6 +431,22 @@ async fn preflight_reclaim_or_reuse( return Ok(true); } + // A *supervised* restart must not undo a deliberate `uffs --daemon + // stop`. Interactively, "you asked for the gateway, the gateway + // needs a daemon" is the helpful reading and still applies. But the + // watchdog is not the operator: when it revives a gateway it must + // not silently drag a daemon the operator stopped on purpose back + // up with it — which is exactly how a deliberate stop was observed + // to bounce straight back. + if std::env::var_os("UFFS_SUPERVISED_RESTART").is_some() + && uffs_client::daemon_ctl::stop_intent_path(uffs_client::daemon_ctl::ServiceKind::Daemon) + .exists() + { + println!(" Gateway on port {port} is alive; daemon is stopped on purpose — leaving it."); + process::reload_stale_stdio_sessions(); + return Ok(true); + } + println!(" Gateway on port {port} is alive but daemon is unreachable."); println!(" Restarting daemon..."); let mut client = uffs_client::connect::UffsClient::connect_with_args(daemon_args) diff --git a/crates/uffs-watchdog/Cargo.toml b/crates/uffs-watchdog/Cargo.toml index 5015ec01d..a6bf8bd10 100644 --- a/crates/uffs-watchdog/Cargo.toml +++ b/crates/uffs-watchdog/Cargo.toml @@ -64,6 +64,12 @@ path = "src/main.rs" # out to `uffs` would be a heavy edge for a single helper. dirs-next.workspace = true +# Liveness is read from `uffs --status --json`, which reports every +# service under its own key. Parsing that properly is what keeps one +# service's state from being mistaken for another's — the substring +# scan it replaced read a stopped daemon as a stopped MCP gateway. +serde_json.workspace = true + anyhow.workspace = true [lints] diff --git a/crates/uffs-watchdog/src/main.rs b/crates/uffs-watchdog/src/main.rs index 0e6b1487d..f433552e7 100644 --- a/crates/uffs-watchdog/src/main.rs +++ b/crates/uffs-watchdog/src/main.rs @@ -31,6 +31,29 @@ //! honours it until the next explicit start ([`supervise::Action`]). //! Without that it would fight the operator on every intentional stop. //! +//! # Liveness is read from JSON, never from prose +//! +//! Probing used to be `uffs -- status` scanned for the +//! substring `running` minus `not running`. That is wrong twice over, +//! and both bugs were observed in the field: +//! +//! * `uffs --mcp status` reports the daemon too, so a *stopped daemon* put +//! `Daemon: not running` in the *MCP* report and the watchdog concluded the +//! healthy gateway had died. It then ran `uffs --mcp start`, whose preflight +//! sees "gateway up, daemon down" and helpfully restarts the daemon — +//! resurrecting the very daemon the operator had just stopped on purpose. The +//! watchdog's own log showed `HonourStopIntent` throughout, because it never +//! touched the daemon: it defeated the stop through the MCP. +//! * `◐ loading (3/7 drives)` contains neither string, so a daemon still +//! reading the MFT read as dead and was liable to be respawned on top of +//! itself. +//! +//! Liveness now comes from `uffs --status --json`, which reports every +//! service under its own key, so one service's state can never be +//! mistaken for another's. An unreadable probe means *unknown*, and +//! unknown is always left alone — a supervisor that restarts things +//! because it could not see them is worse than none. +//! //! # Why a separate binary //! //! A supervisor cannot supervise its own death, so it must outlive the @@ -50,7 +73,8 @@ use supervise::{Action, RespawnLedger, decide}; /// /// Seconds-scale: a respawn that lands within a few seconds of a crash /// is indistinguishable from "never went away" for an interactive -/// search, and the probe is two cheap process checks. +/// search, and the probe is one cheap status call covering every +/// supervised service. const POLL_INTERVAL: Duration = Duration::from_secs(5); /// Environment override for [`POLL_INTERVAL`], in seconds (tests, and @@ -61,6 +85,10 @@ const POLL_ENV: &str = "UFFS_WATCHDOG_POLL_SECS"; struct Service { /// Display name used in log lines. name: &'static str, + /// Key under which `uffs --status --json` reports this service. + /// Reading a named field is what keeps one service's state from + /// being mistaken for another's (see the module docs). + status_key: &'static str, /// `uffs` subcommand pair that starts it (e.g. `--daemon start`). start_args: [&'static str; 2], /// Sliding-window respawn ledger. @@ -87,12 +115,14 @@ fn main() -> anyhow::Result<()> { let mut services = [ Service { name: "daemon", + status_key: "daemon", start_args: ["--daemon", "start"], ledger: RespawnLedger::default(), seen_running: false, }, Service { name: "mcp", + status_key: "mcp_http", start_args: ["--mcp", "start"], ledger: RespawnLedger::default(), seen_running: false, @@ -101,13 +131,46 @@ fn main() -> anyhow::Result<()> { eprintln!("uffs-watchdog armed (poll {}s)", poll.as_secs()); loop { + // One snapshot per tick, shared by every service: the probe is a + // single subprocess rather than one per service, and every + // decision in a tick is taken against the same instant. + let snapshot = status_snapshot(); for service in &mut services { - tick(service); + tick(service, snapshot.as_deref()); } std::thread::sleep(poll); } } +/// Take one machine-readable snapshot of every service's liveness. +/// +/// `uffs --status --json` connects with `connect_raw`, which never +/// auto-spawns anything, so probing has no side effects — an important +/// property for something that runs every few seconds forever. +fn status_snapshot() -> Option { + let out = std::process::Command::new(uffs_exe()) + .args(["--status", "--json"]) + .output() + .ok()?; + Some(String::from_utf8_lossy(&out.stdout).into_owned()) +} + +/// Is the service reported under `key` running, per a `--status --json` +/// document? +/// +/// `None` means *unknown* — malformed JSON, a missing key, or a `uffs` +/// too old to report that service. Callers must treat unknown as "leave +/// it alone": absence of evidence is not evidence of death, and a +/// supervisor that respawns on a failed probe manufactures the outage +/// it exists to prevent. +fn running(doc: &str, key: &str) -> Option { + serde_json::from_str::(doc) + .ok()? + .get(key)? + .get("running")? + .as_bool() +} + /// Append one line to the watchdog log, beside the lifecycle state. /// /// `resident on` spawns the watchdog with its stdio discarded, so @@ -142,8 +205,12 @@ fn lifecycle_dir() -> std::path::PathBuf { clippy::print_stderr, reason = "a supervisor's log IS its user interface; it has no other channel" )] -fn tick(service: &mut Service) { - let alive = is_running(service.start_args[0]); +fn tick(service: &mut Service, snapshot: Option<&str>) { + // Unknown liveness is not death: leave the service exactly as it is + // and try again next tick. + let Some(alive) = snapshot.and_then(|doc| running(doc, service.status_key)) else { + return; + }; if alive { service.seen_running = true; return; @@ -208,21 +275,6 @@ fn uffs_exe() -> std::path::PathBuf { .unwrap_or_else(|| std::path::PathBuf::from(name)) } -/// Liveness probe: ask the CLI, whose status output is the single source -/// of truth for "is it up" on both transports. -/// -/// `--daemon status` prints `● running PID …` when up and -/// `○ Daemon not running` when not; `--mcp status` mirrors the shape. -fn is_running(kind: &str) -> bool { - std::process::Command::new(uffs_exe()) - .args([kind, "status"]) - .output() - .is_ok_and(|out| { - let text = String::from_utf8_lossy(&out.stdout); - text.contains("running") && !text.contains("not running") - }) -} - /// Did the user deliberately stop this service? /// /// Recorded as a marker file beside the lifecycle state, written by the @@ -249,7 +301,55 @@ fn stop_intent_path(kind: &str) -> Option { #[cfg(test)] mod tests { - use super::stop_intent_path; + use super::{running, stop_intent_path}; + + /// A `uffs --status --json` document shaped like the real one: a + /// live gateway while the daemon is deliberately stopped. + const GATEWAY_UP_DAEMON_DOWN: &str = r#"{ + "daemon": { "running": false }, + "broker": { "running": true }, + "mcp_http": { "running": true, "pid": 2912, "endpoint": "http://127.0.0.1:8080/mcp" }, + "mcp_stdio": { "sessions": [] } + }"#; + + /// Each service is read from its own key, so a stopped daemon can + /// never be mistaken for a stopped gateway. + /// + /// Regression: the old probe scanned `uffs --mcp status` prose for + /// `running` minus `not running`. That report names the daemon too, + /// so stopping the daemon made the healthy gateway read as dead; + /// the watchdog "restarted" the gateway, and the gateway's own + /// preflight restarted the daemon — silently undoing a deliberate + /// `uffs --daemon stop`. + #[test] + fn services_are_read_from_their_own_key() { + assert_eq!(running(GATEWAY_UP_DAEMON_DOWN, "daemon"), Some(false)); + assert_eq!(running(GATEWAY_UP_DAEMON_DOWN, "mcp_http"), Some(true)); + } + + /// A daemon still loading its drives is alive: `--status --json` + /// reports `running: true` from the moment it answers RPCs, so the + /// watchdog cannot respawn a daemon on top of a starting one. + #[test] + fn a_loading_daemon_counts_as_running() { + let loading = r#"{ "daemon": { "running": true, + "status": { "status": { "Loading": { "loaded": 3, "total": 7 } } } } }"#; + assert_eq!(running(loading, "daemon"), Some(true)); + } + + /// Anything unreadable is *unknown*, never "down" — the caller + /// leaves unknown services alone rather than respawning them. + #[test] + fn unreadable_probes_are_unknown_not_dead() { + assert_eq!(running("not json at all", "daemon"), None); + assert_eq!(running("{}", "daemon"), None, "missing key"); + assert_eq!(running(r#"{"daemon":{}}"#, "daemon"), None, "missing field"); + assert_eq!( + running(r#"{"daemon":{"running":"yes"}}"#, "daemon"), + None, + "non-boolean" + ); + } /// Each supervised kind maps to its own marker; unknown kinds map to /// none, so a typo can never silently suppress supervision. diff --git a/scripts/dev/install-bins.rs b/scripts/dev/install-bins.rs index 6f0b42997..4d37ee294 100755 --- a/scripts/dev/install-bins.rs +++ b/scripts/dev/install-bins.rs @@ -272,24 +272,39 @@ impl BrokerGuard { } } -/// Was a daemon serving before we tore everything down? +/// Is the service reported under `key` running right now? /// -/// `--daemon status` prints `● running PID …` when up and -/// `○ Daemon not running` when not, so "contains `running` but not -/// `not running`" is an exact read of both shapes. Any failure to run -/// the probe (no `uffs` on PATH yet on a first install) reads as "not -/// running", which is the safe answer: we then leave things alone. -fn daemon_is_running() -> bool { - Command::new("uffs") - .args(["--daemon", "status"]) - .output() - .map(|out| { - let text = String::from_utf8_lossy(&out.stdout); - text.contains("running") && !text.contains("not running") - }) +/// Read from `uffs --status --json`, which reports every service under +/// its own key (`daemon`, `mcp_http`, …). Scanning the *human* status +/// text instead is a trap this script fell into twice: `--mcp status` +/// also prints a `Daemon: not running` line, so a stopped daemon made +/// a healthy gateway read as stopped, and `◐ loading (3/7 drives)` +/// contains neither `running` nor `not running`, so a daemon busy +/// reading the MFT read as absent and was never restarted. +/// +/// Any failure to run the probe (no `uffs` on PATH yet on a first +/// install) reads as "not running", which is the safe answer: we then +/// leave things alone rather than start something the user never had. +fn service_running(key: &str) -> bool { + let Ok(out) = Command::new("uffs").args(["--status", "--json"]).output() else { + return false; + }; + let text = String::from_utf8_lossy(&out.stdout); + // Sections are emitted in sorted key order and each carries its own + // `running` flag, so the first flag *after* the key we asked for is + // that key's own — no JSON parser needed in a rust-script. + text.split(&format!("\"{key}\":")) + .nth(1) + .and_then(|section| section.split("\"running\":").nth(1)) + .map(|flag| flag.trim_start().starts_with("true")) .unwrap_or(false) } +/// Was a daemon serving before we tore everything down? +fn daemon_is_running() -> bool { + service_running("daemon") +} + /// Restart the daemon we deliberately stopped, using the freshly /// installed binary. /// @@ -362,20 +377,8 @@ fn restart_watchdog(bin_dir: &std::path::Path) { } /// Was the MCP HTTP gateway serving before the teardown? -/// -/// `--mcp status` prints `MCP server: running (PID …)` when up, and -/// either `not running (no PID file)` or `not running (stale PID file…)` -/// when not — so the same "contains `running` but not `not running`" -/// read works here. fn mcp_is_running() -> bool { - Command::new("uffs") - .args(["--mcp", "status"]) - .output() - .map(|out| { - let text = String::from_utf8_lossy(&out.stdout); - text.contains("running") && !text.contains("not running") - }) - .unwrap_or(false) + service_running("mcp_http") } /// Restart the MCP HTTP gateway we stopped, using the new binary. From 31b8ccc3cc391e6559e18d914230e13244bea263 Mon Sep 17 00:00:00 2001 From: Robert M1 <50460704+githubrobbi@users.noreply.github.com> Date: Wed, 12 Aug 2026 18:01:59 -0700 Subject: [PATCH 13/16] docs(daemon): document residency, the watchdog, and the drive views MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The residency work landed across several commits without the user manual catching up, and one earlier feature never reached it at all. Documents the watchdog: what it supervises, what it deliberately does not (the Access Broker, and why supervising a LocalSystem service from a non-elevated process would break the zero-UAC promise residency exists to protect), the crash budget, that a deliberate stop always wins, that it never introduces a service you never ran, and how to read `watchdog.log` when a service comes back and you want to know who did it. Refreshes the `--daemon status -v` sample, which had drifted: it predated the physical-drive inventory (shipped in the status/physical drive view work and never documented), the mimalloc line, and this branch's column alignment. Adds the `── Physical drives ──` section — the one that answers "why did my search miss that drive" — and the `status_drives` tier table, until now mentioned only in passing, with a column-by-column key. Documents `uffs --status --json` as the multi-service contract, with the reason to prefer it: each service carries its own `running` flag, and the human views mention other services by design, so a substring scan attributes one service's state to another. That is precisely the bug fixed in the previous commit. Explains what `just use-local` now does to running services, and adds a troubleshooting entry for "the daemon restarts after I stop it". Fixes an asymmetry found while writing this: `resident on` armed the watchdog but `resident off` never disarmed it, so switching residency off left a supervisor running that would second-guess the next stop. `resident off` now disarms it, and `resident status` reports whether supervision is active — without that line the watchdog is invisible to the command whose job is to describe residency. --- crates/uffs-cli/src/commands/resident.rs | 52 ++++++ docs/user-manual/daemon.md | 210 +++++++++++++++++++---- docs/user-manual/installation.md | 22 +++ docs/user-manual/troubleshooting.md | 39 ++++- 4 files changed, 290 insertions(+), 33 deletions(-) diff --git a/crates/uffs-cli/src/commands/resident.rs b/crates/uffs-cli/src/commands/resident.rs index c5cd9177a..a6279edb6 100644 --- a/crates/uffs-cli/src/commands/resident.rs +++ b/crates/uffs-cli/src/commands/resident.rs @@ -140,6 +140,37 @@ fn arm_watchdog() { #[cfg(not(windows))] const fn arm_watchdog() {} +/// Stop the watchdog that [`arm_watchdog`] started. +/// +/// `resident on` arms supervision, so `resident off` must disarm it: +/// leaving a supervisor running after residency is switched off means +/// the very next `uffs --daemon stop` gets second-guessed by a process +/// the user believes they just removed. The daemon itself is left +/// alone (as `resident off` already reports) — this only withdraws the +/// supervision, not the service. +#[cfg(windows)] +#[expect(clippy::print_stdout, reason = "CLI user-facing output")] +fn disarm_watchdog() { + if !watchdog_running() { + return; + } + let stopped = std::process::Command::new("taskkill") + .args(["/IM", "uffs-watchdog.exe", "/F"]) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .status() + .is_ok_and(|status| status.success()); + if stopped { + println!("Watchdog disarmed: services are no longer restarted automatically."); + } else { + println!("(could not stop the watchdog — end uffs-watchdog.exe manually)"); + } +} + +/// Non-Windows: nothing was armed, so nothing to disarm. +#[cfg(not(windows))] +const fn disarm_watchdog() {} + /// Locate `uffs-watchdog` beside the running `uffs`. #[cfg(windows)] fn watchdog_exe() -> Option { @@ -254,6 +285,7 @@ fn resident_off() -> Result<()> { platform::turn_off()?; // Auto-spawns fall back to the default idle-retire lifetime. let _absent = std::fs::remove_file(uffs_client::daemon_ctl::resident_args_path()); + disarm_watchdog(); if daemon_running() { println!( "A daemon is still running; it is unaffected.\n\ @@ -277,6 +309,7 @@ fn resident_status() { } else { println!("Auto-spawn: default (idle retire)"); } + print_watchdog_state(); if daemon_running() { println!("Daemon: running (details: uffs --daemon status)"); } else { @@ -284,6 +317,25 @@ fn resident_status() { } } +/// Report whether supervision is currently armed. +/// +/// Without this line the watchdog is invisible: `resident status` would +/// claim residency is off while a supervisor kept restarting services. +#[cfg(windows)] +#[expect(clippy::print_stdout, reason = "CLI user-facing output")] +fn print_watchdog_state() { + if watchdog_running() { + println!("Watchdog: supervising (crashed services are restarted)"); + } else { + println!("Watchdog: not running"); + } +} + +/// Non-Windows: launchd / systemd supervise, so there is no watchdog to +/// report on. +#[cfg(not(windows))] +const fn print_watchdog_state() {} + // ── shared plumbing ───────────────────────────────────────────────── /// Run one system tool to completion, mapping a non-zero exit into an diff --git a/docs/user-manual/daemon.md b/docs/user-manual/daemon.md index 8b3abefed..373ac9849 100644 --- a/docs/user-manual/daemon.md +++ b/docs/user-manual/daemon.md @@ -139,6 +139,12 @@ uffs --daemon resident status uffs --daemon resident off ``` +`resident status` reports all three moving parts — the login item, the +auto-spawn marker, and (on Windows) whether the watchdog is +supervising. `resident off` removes all three. It deliberately leaves +a **running** daemon running, and says so; stop it explicitly if that +is what you want. + `resident on` registers a per-user login item (Windows: `HKCU` Run key; macOS: launchd LaunchAgent; Linux: systemd user unit) that starts `uffsd --no-retire` at login, and starts the daemon @@ -157,6 +163,76 @@ importantly `--no-retire`. Flags you pass explicitly always win over the marker. `resident off` removes the marker along with the login item. +### The watchdog — surviving crashes and installers + +The login item delivers residency at boot, and the auto-spawn marker +revives a dead daemon on the next search. Neither notices a service +that vanishes **mid-session while nobody is searching**. On macOS and +Linux launchd and systemd close that gap; on Windows the `Run` key +fires once at login and never again. So `resident on` also arms +**`uffs-watchdog`**, a small user-level supervisor. + +| | | +|---|---| +| Supervises | `uffsd` (daemon) and `uffsmcp` (MCP HTTP gateway) | +| Does **not** supervise | the Access Broker — see below | +| Privileges | none; it runs as you, like everything else residency touches | +| Poll interval | 5 s (`UFFS_WATCHDOG_POLL_SECS=` to change) | +| Crash budget | 3 respawns per 60 s, then it gives up and says so | +| Log | `watchdog.log`, beside the PID file | + +It is a **separate binary** on purpose. A supervisor cannot supervise +its own death, so it has to outlive the teardowns that kill what it +watches — `just use-local` force-kills `uffsd` and `uffsmcp` by image +name, and a different image name survives that. It is also not a +`uffs` subcommand, because a long-running `uffs.exe` would lock the +most frequently replaced binary in the tree. + +**A deliberate stop always wins.** `uffs --daemon stop`, `--daemon +kill`, and `uffs --mcp stop` record *stop intent* next to the PID file +(`daemon.stopped` / `mcp.stopped`); the watchdog honours it until you +explicitly start that service again, which clears the marker. This is +launchd's `KeepAlive.SuccessfulExit = false` contract — without it the +supervisor fights you every time you stop something on purpose. + +**It never introduces a service you never ran.** Each service is +supervised only after it has been seen running at least once, so a +machine that has never started the MCP gateway does not acquire one +because a watchdog is present. + +**The Access Broker is deliberately excluded.** It is a `LocalSystem` +service registered `start= auto`, so the Service Control Manager +already restarts it at boot — and a non-elevated process cannot +`StartService` it anyway. Supervising it from here would require +elevation and break the zero-UAC property residency exists to protect. +The right mechanism there is SCM failure actions, configured once at +`uffs-broker --install` time. + +#### Reading the watchdog log + +`resident on` starts the watchdog with its output discarded, so every +decision is also appended to `watchdog.log` in the lifecycle directory +(`%LOCALAPPDATA%\uffs\` on Windows, `~/Library/Application Support/uffs/` +on macOS, `~/.local/share/uffs/` on Linux): + +``` +daemon down: stop_intent=false (marker …\daemon.stopped) recent_respawns=0 -> Respawn +daemon down: stop_intent=true (marker …\daemon.stopped) recent_respawns=1 -> HonourStopIntent +``` + +Each line records the decision **and the inputs that produced it**, so +"why did my daemon come back?" is answerable from the file rather than +by guesswork. The four decisions are `Respawn` (gone, and you did not +ask for that), `HonourStopIntent` (gone because you stopped it), +`GaveUp` (respawned too often inside the window — something is broken +in a way restarting cannot fix), and `Leave`. + +Liveness is read from `uffs --status --json`, which reports every +service under its own key, so one service's state can never be +mistaken for another's. An unreadable probe means *unknown*, and +unknown is always left alone — a supervisor that restarts things it +cannot see manufactures the outage it exists to prevent. + ### Memory tiers — and why you never see `Hot` Each drive's index sits in one of four tiers, visible in @@ -212,11 +288,20 @@ the page-in on the next first query unless it was preloaded. |---------|-------------| | `uffs --daemon start` | Start the daemon (with data sources) | | `uffs --daemon status` | Show PID, uptime, loaded drives, record counts | -| `uffs --daemon status -v` | Long view: build, elevation / broker mode, live-update, memory, paths, and performance counters | +| `uffs --daemon status -v` | Long view: build, elevation / broker mode, live-update, memory, paths, performance counters, and the physical-drive inventory | | `uffs --daemon status --json` | Machine-readable status + drives + stats | -| `uffs --daemon stop` | Graceful shutdown via RPC | -| `uffs --daemon kill` | Hard kill + remove PID/socket files | +| `uffs --daemon status_drives` | Per-drive tier + telemetry table (resident bytes, query rate, pins) | +| `uffs --daemon stop` | Graceful shutdown via RPC (records stop intent) | +| `uffs --daemon kill` | Hard kill + remove PID/socket files (records stop intent) | | `uffs --daemon restart` | Stop → re-start with same data sources | +| `uffs --daemon resident on\|off\|status` | Login autostart + no idle retire; arms the watchdog | +| `uffs --daemon preload` | Promote drive(s) to `Hot` and pin the tier | +| `uffs --daemon hibernate` | Demote drive(s) to `Cold` (frees RAM, cache stays) | +| `uffs --daemon load` | Hot-load additional MFT file(s) into a running daemon | +| `uffs --daemon forget` | Evict drive(s) and delete their on-disk caches | + +`stop` and `kill` record *stop intent* so the [watchdog](#the-watchdog--surviving-crashes-and-installers) +does not undo them; the next explicit `start` clears it. ### `uffs --daemon status` @@ -245,45 +330,85 @@ in here): ``` $ uffs --daemon status -v ═══ UFFS Daemon ═══ -● running PID 72558 - Version: 0.6.24 - Uptime: 9m 51s - Drives: 7 loaded · 25,846,853 records - Queries: 2 (avg 1.19ms, 0.0/s) +● running PID 52044 + Version: 0.6.31 + Uptime: 10 m 20 s + Drives: 7 loaded · 24,897,476 records + Queries: 0 ── Build ── - Commit: a1b2c3d - Elevated: no (reading via Access Broker, zero-UAC) + Commit: 96f165b96 + Elevated: yes (direct elevated reads) ── Live update ── - Journal: 7 journal loop(s) running + Journal: 7 journal loop(s) running ── Memory ── - Index heap: 512 MB - RSS: 640 MB + Index heap: 5021 MB + Mimalloc: 4316 MB committed + RSS: 3743 MB ── Paths ── - Data: C:\Users\you\AppData\Local\uffs - Socket: \\.\pipe\uffs-daemon - Logs: C:\Users\you\AppData\Local\uffs\logs + Data: C:\Users\you\AppData\Local\uffs\cache + Socket: C:\Users\you\AppData\Local\uffs\daemon.sock ── Performance ── - Startup duration: 10.9 s - Total records: 25,846,853 - Queries served: 2 - Avg query time: 1.19 ms - Total query time: 2.38 ms + Startup duration: 9 s 278 ms + Total records: 24,897,476 + Queries served: 0 Queries/second: 0.00 Agg cache: 0 hits / 0 misses (0.0% hit-rate, 0 entries) ── Drives ── - ● C: 3,428,455 records (file) · 128 MB [rec=64 names=48 tri=12 ch=3 ext=1] - ... + ● G: 15,384 records (live) · 2 MB [rec= 1 names= 0 tri= 0 ch= 0 ext= 0] + ● F: 1,203,779 records (live) · 297 MB [rec= 101 names= 37 tri= 124 ch= 9 ext= 4] + ● C: 3,289,117 records (live) · 757 MB [rec= 276 names= 95 tri= 327 ch= 25 ext= 12] +── Physical drives ── + ● C:* NVMe 1.53 TB · 91% used · 144.19 GB free “BOOT 990” · indexed ( 3,289,117 records) + ● D: HDD 7.28 TB · 65% used · 2.52 TB free “DATA” · indexed ( 7,253,055 records) + · E: HDD 931.51 GB · 100% used · 2.29 GB free “Software” · not loaded + ● G: Removable 14.72 GB · 84% used · 2.35 GB free “NTFS_16_GB” · indexed ( 15,384 records) +``` + +Every column in both drive blocks is padded to a fixed width, so the +sections read as tables even though each row is rendered +independently — sizes right-align on their units, and the +`[rec=… names=…]` breakdown lines up across rows. + +**`── Drives ──`** lists what the daemon has **loaded**. Each is +labelled by its **letter** — live Windows volumes by their real letter, +and offline `.bin`/`.mft` captures by the letter derived from the file, +tagged **`(file)`** so a capture is distinguishable from a live volume +(the source filename itself is not shown). The trailing +`[rec=… names=… tri=… ch=… ext=…]` is the per-drive memory breakdown — +record, name-arena, trigram, child-map, and extension shard sizes in MB. + +**`── Physical drives ──`** is the inventory of what *exists* on the +machine, whether or not UFFS has indexed it — bus type, capacity, usage, +free space, volume label, and either `indexed (N records)` or +`not loaded`. The `*` marks the boot volume. This is the section to +check when a search comes back empty: a drive listed here as +`not loaded` is one the daemon never read. + +> The **short** view collapses all of this to a single count/records +> line; use `-v` for the per-drive lists or `--json` for the structured +> `{"letter","records","tier"}` array. + +### `uffs --daemon status_drives` + +The tier table shows what each drive is costing you in RAM right now, +and why it is in the tier it is in: + +``` +$ uffs --daemon status_drives +DRIVE TIER RESIDENT QPM LAST QUERY PIN UNTIL PROMOTIONS +C warm 757 MiB 0.00 10m ago - 0 +D warm 1.630 GiB 0.00 10m ago - 0 +G warm 2 MiB 0.00 10m ago - 0 ``` -Each drive is labelled by its **letter** — live Windows volumes by their real -letter, and offline `.bin`/`.mft` captures by the letter derived from the file, -tagged **`(file)`** so a capture is distinguishable from a live volume. (The -source filename itself is not shown.) The trailing -`[rec=… names=… tri=… ch=… ext=…]` is the per-drive memory-tier breakdown — the -record, name-arena, trigram, child-map, and extension shard sizes. Note the -**short** view collapses this to a single count/records line; use `-v` for the -per-drive list or `--json` for the structured `{"letter","records","tier"}` -array. +| Column | Meaning | +|--------|---------| +| `TIER` | `hot` / `warm` / `parked` / `cold` — see [Memory tiers](#memory-tiers--and-why-you-never-see-hot) | +| `RESIDENT` | Bytes held in RAM for that drive, scaled to `MiB` / `GiB` | +| `QPM` | Queries per minute against that drive (drives the tiering decisions) | +| `LAST QUERY` | How long since it was last searched | +| `PIN UNTIL` | Demotion is blocked until this time — set by `preload --pin-minutes` | +| `PROMOTIONS` | How often this drive has been promoted back up the ladder; a high count on an idle machine means the thresholds are too aggressive for your workload | > **`uffs --daemon stats` has been folded into `uffs --daemon status -v`.** > The old command now prints a one-line redirect. @@ -305,6 +430,27 @@ $ uffs --daemon status --json } ``` +For **multi-service** scripting, prefer `uffs --status --json`, which +reports the daemon, the Access Broker, and both MCP transports under +their own top-level keys: + +``` +$ uffs --status --json +{ + "broker": { "installed": true, "running": true, "pipe_serving": true, ... }, + "daemon": { "running": true, "status": { ... }, "drives": [ ... ] }, + "mcp_http": { "running": true, "pid": 2912, "endpoint": "http://127.0.0.1:8080/mcp" }, + "mcp_stdio": { "sessions": [ ... ] } +} +``` + +Each service carries its own `running` flag. Read that flag rather +than scanning the human output for the word "running": the text views +mention *other* services by design — `uffs --mcp status` reports the +daemon too — so a substring scan will attribute one service's state to +another. The watchdog learned this the hard way, and now reads this +document. + --- ## 6 Logging diff --git a/docs/user-manual/installation.md b/docs/user-manual/installation.md index 69ec32fca..07c629967 100644 --- a/docs/user-manual/installation.md +++ b/docs/user-manual/installation.md @@ -248,6 +248,28 @@ every other end user runs, so bug reports are reproducible. binaries to `~/bin`. Use this when you want to test local changes before opening a PR. +Because it replaces binaries that may be **running**, it manages the +service lifecycle around the copy and puts things back as it found +them: + +* **Unchanged binaries are skipped.** Only files whose contents + actually differ are replaced, so a rebuild that changed one crate + does not disturb the other 23 processes — and cannot fail with + `os error 32` (file in use) for no reason. +* **The Access Broker is a service**, so it cannot simply be + overwritten: if its binary changed, the installer stops the service, + replaces it, and starts it again (`⏸️ stopping` / `▶️ restarted`). +* **The watchdog is stopped first and restarted last.** If it kept + running during the install it would dutifully restart the daemon + mid-teardown — the supervisor fighting the installer. +* **The daemon and MCP server are restarted only if they were running + before**, and the daemon comes back through the normal start path, so + a resident daemon returns resident (`--no-retire`). + +The net effect is that `use-local` leaves the machine in the state it +found it, which is what [residency](daemon.md#permanent-residency-start-at-login-never-retire) +promises. + If `~/bin` is not on your PATH, either recipe prints the line to add to your shell profile. diff --git a/docs/user-manual/troubleshooting.md b/docs/user-manual/troubleshooting.md index c168aad9b..e45df3dca 100644 --- a/docs/user-manual/troubleshooting.md +++ b/docs/user-manual/troubleshooting.md @@ -150,7 +150,44 @@ NTFS drives and 25 million files uses roughly 4–6 GB of RAM. --- -## 9 Getting Help +## 9 The Daemon Restarts After I Stop It + +If you made UFFS [resident](daemon.md#permanent-residency-start-at-login-never-retire), +a supervisor is keeping it alive on purpose. A **deliberate** stop is +still honoured — `uffs --daemon stop` and `--daemon kill` record stop +intent, and the watchdog leaves the service down until you start it +again. So a daemon that comes back means something *else* started it. +Check, in order: + +```bash +# 1. What decision did the watchdog take, and on what evidence? +# Windows: %LOCALAPPDATA%\uffs\watchdog.log +cat ~/.local/share/uffs/watchdog.log + +# 2. Is the MCP gateway up? It starts a daemon when a tool call needs one. +uffs --mcp status + +# 3. Is residency armed at all? Shows login item, marker, and watchdog. +uffs --daemon resident status +``` + +A log line reading `HonourStopIntent` means the watchdog did **not** +touch it — look instead at the MCP gateway, or at whatever ran a +search, since any search auto-starts a daemon. + +To stop the services and have them stay stopped, turn residency off +first — that removes the login item and the auto-spawn marker and +disarms the watchdog, then stop the daemon (`resident off` leaves a +running daemon alone on purpose): + +```bash +uffs --daemon resident off +uffs --daemon stop +``` + +--- + +## 10 Getting Help ```bash # Show all available flags From c6e1904a40236866bbea1fbe801e1b598265fd9d Mon Sep 17 00:00:00 2001 From: Robert M1 <50460704+githubrobbi@users.noreply.github.com> Date: Wed, 12 Aug 2026 18:08:08 -0700 Subject: [PATCH 14/16] docs: repair broken intra-doc anchors across the manual and README MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An anchor sweep over all 277 markdown files found twelve links whose targets exist but whose fragments never matched, so every one of them landed the reader at the top of the page instead of the section they asked for. Most were off by a hyphen: GitHub does not collapse whitespace when it slugifies, so a numbered heading (`## 3 Bulkiness`) or one containing a dash (`— Daemon Runs`) yields a *double* hyphen the hand-written links did not have. Two were pointing at the wrong section entirely, and their visible labels were wrong with them: Descendants is Concepts §4, not §5, and Tree Size is §2, not §4. The FAQ's Administrator answer pointed at Installation §5 (Build from Source) rather than §3 (Platform Requirements). While fixing that last one: the FAQ still answered "On Windows, yes — reading the MFT requires elevated access", which predates the Access Broker and contradicts the installation guide two clicks away. It now says what is actually true — elevation once at `uffs-broker --install`, then no UAC on any later search, daemon start/stop, or update. --- README.md | 2 +- docs/benchmarks/README.md | 4 ++-- docs/user-manual/cli-overview.md | 2 +- docs/user-manual/faq.md | 12 ++++++++---- docs/user-manual/glossary.md | 6 +++--- 5 files changed, 15 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 6a6242857..a2ab38010 100644 --- a/README.md +++ b/README.md @@ -103,7 +103,7 @@ Measured 2026-06-11 on AMD Ryzen 9 3900XT, 64 GB RAM, Windows 11 Pro 24H2 — cr | **HOT (`*` top-100)** | Full-scan across all drives with `--limit 100` | **1 112 ms** e2e¹ | 27 ms | | **HOT (targeted)** | `notepad.exe` / `win*` / `*.dll` / `config` etc. | **29–32 ms** CLI e2e | 9–10 ms | -¹ The `*` top-100 path regressed from the v0.5.4 163 ms figure after the Phase 2 sort rewrite ([raw log](docs/benchmarks/raw/2026-04-v0.5.66_full-benchmark-suite.txt), n=30, StdDev 21 ms); daemon-side is 1 081 ms — the CLI tax is negligible here. Tracked in the [archived April report](docs/benchmarks/archive/2026-04-v0.5.66-vs-everything-and-cpp.md#known-regressions). +¹ The `*` top-100 path regressed from the v0.5.4 163 ms figure after the Phase 2 sort rewrite ([raw log](docs/benchmarks/raw/2026-04-v0.5.66_full-benchmark-suite.txt), n=30, StdDev 21 ms); daemon-side is 1 081 ms — the CLI tax is negligible here. Tracked in the [archived April report](docs/benchmarks/archive/2026-04-v0.5.66-vs-everything-and-cpp.md#known-regressions-published-because-trust--hype). **Scale ceiling:** **100.4 M records** tested with offline MFT clones (v0.5.4 capture, not re-verified since) — targeted queries stayed at 11–13 ms e2e. diff --git a/docs/benchmarks/README.md b/docs/benchmarks/README.md index d901240ff..56e08fba7 100644 --- a/docs/benchmarks/README.md +++ b/docs/benchmarks/README.md @@ -21,7 +21,7 @@ Four numbers the report establishes on a Ryzen 9 3900XT (cross-tool: 12.8 M reco 3. **Full-scan export is a workload Everything cannot run** (`es.exe` ~2 GB IPC export ceiling): UFFS streams the complete **23.3 M-row** estate (all 7 volumes) to CSV in **12.0 s ≈ 1.95 M records/sec** — the April snapshot scale, 12% faster, +13% throughput. 4. **180×–3 400× faster than the C++ reference on targeted queries** (daemon HOT vs per-invocation MFT re-read); **6.6×** on combined full-scan — and the combined-drive regex cell DNF'd the C++ tool entirely (> 120 s vs UFFS 43 ms). -The report publishes **everything these numbers don't cover too** — the zero-match G-drive caveat, the C++ row-count divergences, and what this benchmark explicitly does *not* claim. The two v0.5.66-era known regressions (`*` top-100 and `--sort path` vs the v0.5.4 baseline) remain tracked in the [archived April report](archive/2026-04-v0.5.66-vs-everything-and-cpp.md#known-regressions); they were not re-measured in this snapshot. +The report publishes **everything these numbers don't cover too** — the zero-match G-drive caveat, the C++ row-count divergences, and what this benchmark explicitly does *not* claim. The two v0.5.66-era known regressions (`*` top-100 and `--sort path` vs the v0.5.4 baseline) remain tracked in the [archived April report](archive/2026-04-v0.5.66-vs-everything-and-cpp.md#known-regressions-published-because-trust--hype); they were not re-measured in this snapshot. --- @@ -35,7 +35,7 @@ reply to *"this comparison is rigged because..."*): - **Separate cold / warm / hot.** Cold build + warm restart + hot query are three different workloads. We measure and publish them separately instead of averaging them into one "startup time" lie. - **Separate interactive from bulk.** Targeted-query latency (`notepad.exe`, `*.dll`) and full-scan export (`*` → CSV for 23 M rows) are different workload classes. Different tools win each. We test both. -- **Publish the failures.** When a workload regresses against our own prior baseline it gets named, measured, root-caused, and tracked (see §Known regressions in the [archived 2026-04 report](archive/2026-04-v0.5.66-vs-everything-and-cpp.md#known-regressions) for the two v0.5.66-era examples). +- **Publish the failures.** When a workload regresses against our own prior baseline it gets named, measured, root-caused, and tracked (see §Known regressions in the [archived 2026-04 report](archive/2026-04-v0.5.66-vs-everything-and-cpp.md#known-regressions-published-because-trust--hype) for the two v0.5.66-era examples). - **Publish the raw data.** Every table above and in the canonical report cites the exact log file and line range. The **curated, verbatim raw captures** live in [`raw/`](raw/) (git-tracked, never edited after commit); all benchmark scripts under [`scripts/windows/`](../../scripts/windows/). Click any citation in the canonical report to land on the actual PowerShell log line that produced the number. --- diff --git a/docs/user-manual/cli-overview.md b/docs/user-manual/cli-overview.md index 57ba57b60..2f2c7b9c3 100644 --- a/docs/user-manual/cli-overview.md +++ b/docs/user-manual/cli-overview.md @@ -125,7 +125,7 @@ All filters are detailed in the [Filters guide](filters.md). Summary: | `--max-path-length ` | Derived | Max full-path character count | | `--in-path ` | Path | Directory path must match glob | | `--exclude ` | Exclude | Exclude matching filenames | -| `--malformed` | Forensic | Only ill-formed-UTF-16 names ([guide](filters.md#14a-malformed-name-filters-forensic)) | +| `--malformed` | Forensic | Only ill-formed-UTF-16 names ([guide](filters.md#14a--malformed-name-filters-forensic)) | | `--well-formed` | Forensic | Only valid names (inverse of `--malformed`) | | `--malformed-path` | Forensic | Match when any path segment is ill-formed | | `--normalize-malformed` | Forensic | Display: render corrupt code units as `` not `�` | diff --git a/docs/user-manual/faq.md b/docs/user-manual/faq.md index 14949a983..eb1862316 100644 --- a/docs/user-manual/faq.md +++ b/docs/user-manual/faq.md @@ -33,10 +33,14 @@ a Windows machine. ### Does UFFS need Administrator privileges? -On Windows, yes — reading the MFT requires elevated access. On macOS -and Linux, no — UFFS reads regular files (MFT captures). +On Windows, **once** — reading the MFT requires elevated access, but +installing the Access Broker (`uffs-broker --install`) grants it a +single time, after which every search, daemon start/stop, and update +runs unelevated with no UAC prompt. Without the broker you need an +Administrator terminal each time. On macOS and Linux, no — UFFS reads +regular files (MFT captures). -> **Details:** [Installation §5](installation.md#5--windows-administrator-privileges) +> **Details:** [Installation §3](installation.md#3--platform-requirements) --- @@ -92,7 +96,7 @@ Bulkiness = (SizeOnDisk / Size) × 100. It measures allocation waste. - **500** — 5× more disk space than logical data (wasteful) - **409600** — a 1-byte file using one 4 KB cluster -> **Guide:** [Concepts §3](concepts.md#3--bulkiness) +> **Guide:** [Concepts §3](concepts.md#3--bulkiness-waste-ratio) --- diff --git a/docs/user-manual/glossary.md b/docs/user-manual/glossary.md index 60c5da33e..8754a75a0 100644 --- a/docs/user-manual/glossary.md +++ b/docs/user-manual/glossary.md @@ -8,12 +8,12 @@ Key terms used throughout the UFFS documentation. |------|-----------| | **ADS** | Alternate Data Stream — NTFS feature allowing multiple data streams per file. Hidden from Explorer by default. Use `--full` to index them. | | **Allocated size** | The actual disk space consumed by a file, rounded up to the nearest cluster boundary. Also called "size on disk". See [Concepts §1](concepts.md#1--size-vs-size-on-disk). | -| **Bulkiness** | The ratio of allocated size to logical size, expressed as a percentage. A measure of allocation waste. 100 = perfectly efficient. See [Concepts §3](concepts.md#3--bulkiness). | +| **Bulkiness** | The ratio of allocated size to logical size, expressed as a percentage. A measure of allocation waste. 100 = perfectly efficient. See [Concepts §3](concepts.md#3--bulkiness-waste-ratio). | | **Cluster** | The smallest unit of disk allocation in NTFS. Typically 4 KB (4096 bytes). Files smaller than one cluster still consume one full cluster. | | **Compact index** | UFFS's in-memory representation of the MFT — a struct-of-arrays layout optimised for search and aggregation. | | **Daemon** | The UFFS background process that holds the MFT index in memory and serves search queries over IPC. See [Daemon](daemon.md). | | **DataFrame** | A Polars columnar data structure. UFFS uses DataFrames internally for query execution. | -| **Descendants** | The total number of files and subdirectories inside a directory (recursive count). See [Concepts §5](concepts.md#5--descendants). | +| **Descendants** | The total number of files and subdirectories inside a directory (recursive count). See [Concepts §4](concepts.md#4--descendants-child-count). | | **Extension record** | An MFT record that continues the attributes of another record. Used when a file has many hard links or ADS entries. Indexed in `--full` mode. | | **FRS** | File Reference Segment — the MFT record number uniquely identifying a file or directory on a drive. Also called "file reference number". | | **Hard link** | An NTFS feature allowing multiple directory entries to point to the same file data. The file has one FRS but multiple names and parent directories. | @@ -29,5 +29,5 @@ Key terms used throughout the UFFS documentation. | **Path resolution** | The process of reconstructing full file paths from MFT data. The MFT stores only filenames and parent FRS numbers, not full paths. UFFS's `FastPathResolver` walks the parent chain. | | **Reparse point** | An NTFS feature for symlinks, junctions, and volume mount points. Detected via the reparse attribute flag. | | **SoA** | Struct of Arrays — a data layout where each field is stored in a separate contiguous array. UFFS uses SoA for the compact index, enabling SIMD-friendly scans. | -| **Treesize** | The recursive logical size of a directory subtree — the sum of sizes of all files in the directory and its descendants. See [Concepts §4](concepts.md#4--treesize--tree-allocated). | +| **Treesize** | The recursive logical size of a directory subtree — the sum of sizes of all files in the directory and its descendants. See [Concepts §2](concepts.md#2--tree-size--tree-allocated). | | **Tree allocated** | Same as treesize but for allocated (on-disk) sizes. | From 627752d86571660e398f0c42aa70e63403e91a6b Mon Sep 17 00:00:00 2001 From: Robert M1 <50460704+githubrobbi@users.noreply.github.com> Date: Wed, 12 Aug 2026 18:17:27 -0700 Subject: [PATCH 15/16] fix(core): bound column growth so one new file cannot double the index MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A single file created on a live drive permanently doubled two of the largest structures in the index. On C: the record column went 276 MB → 552 MB and the name arena 95 MB → 190 MB — 371 MB, for one file — and stayed there for the life of the shard. Seen in the field as a daemon reporting `[rec=552 names=190]` where an identical daemon on the same drive reported `[rec=276 names=95]`, with the trigram, child-map and extension shards byte-identical between the two. The exact-2x ratio, confined to precisely the two columns that get appended to, is the tell. `build_compact_index` ends with `shrink_compact_vecs`, which reclaims ~500 MB across seven drives by shrinking the columns to an exact fit — leaving `capacity == len`. The cache-load path lands there too (`aligned_vec_from_bytes`, `to_vec`). The first USN create then calls `Vec::push` / `extend_from_slice` on a full vector, and `Vec`'s amortised growth reallocates to *twice* the capacity. Shrink reclaimed 500 MB; the first created file handed back more than it saved. Doubling is the right default for a `Vec` that knows nothing about its contents. These columns are hundreds of megabytes and grow by a handful of records per USN batch, so they want a different policy: reserve an eighth of the current length. Growth stays geometric — appends remain amortised O(1) — while the waste is capped at 12.5% instead of 100%. For C: that is ~34 MB of slack instead of ~276 MB. `ColumnStorage::as_mut_vec` is replaced by `vec_for_append(additional)`, which does the reserve before handing out the `&mut Vec`. The old accessor is gone rather than deprecated: leaving a doubling-growth escape hatch beside the bounded one is how this reached production in the first place. `frs_to_compact` grows through the same policy — its `resize` past the FRS high-water mark had the identical defect at smaller scale (~13 MB per drive). Three tests pin it, including the exact-fit case: shrink to `capacity == len`, append one element, assert the capacity did not double. --- crates/uffs-core/src/compact.rs | 2 +- crates/uffs-core/src/compact_loader/apply.rs | 22 ++++-- crates/uffs-core/src/compact_storage.rs | 75 +++++++++++++++---- crates/uffs-core/src/compact_storage/tests.rs | 73 ++++++++++++++++-- 4 files changed, 141 insertions(+), 31 deletions(-) diff --git a/crates/uffs-core/src/compact.rs b/crates/uffs-core/src/compact.rs index 5669674ed..a31fa6652 100644 --- a/crates/uffs-core/src/compact.rs +++ b/crates/uffs-core/src/compact.rs @@ -69,7 +69,7 @@ pub struct DriveCompactIndex { /// swap the heap-resident `Vec` for a memory-mapped runtime /// tempfile. Read-side call sites use [`Deref<[T]>`]; mutating /// callers (Windows USN-patch path) go through - /// `ColumnStorage::as_mut_vec` (internal helper). + /// `ColumnStorage::vec_for_append` (internal helper). pub records: ColumnStorage, /// All filenames concatenated (UTF-8 bytes, original case). /// diff --git a/crates/uffs-core/src/compact_loader/apply.rs b/crates/uffs-core/src/compact_loader/apply.rs index 5642fb767..60b101da9 100644 --- a/crates/uffs-core/src/compact_loader/apply.rs +++ b/crates/uffs-core/src/compact_loader/apply.rs @@ -37,10 +37,11 @@ struct StagedCreate { fn stage_create(drive: &mut DriveCompactIndex, change: &uffs_mft::usn::FileChange) -> StagedCreate { let extension_id = drive.intern_extension(&change.filename); let name_start = drive.names.len(); + let name_bytes = change.filename.as_bytes(); drive .names - .as_mut_vec() - .extend_from_slice(change.filename.as_bytes()); + .vec_for_append(name_bytes.len()) + .extend_from_slice(name_bytes); let parent_frs_usize = uffs_mft::frs_to_usize(change.parent_frs.raw()); let parent_idx = drive .frs_to_compact @@ -154,11 +155,15 @@ pub(super) fn apply_create( _pad: [0; 1], }; let new_compact_idx = uffs_mft::len_to_u32(drive.records.len()); - drive.records.as_mut_vec().push(new_rec); + drive.records.vec_for_append(1).push(new_rec); if frs_usize >= drive.frs_to_compact.len() { - drive - .frs_to_compact - .resize(frs_usize.saturating_add(1), u32::MAX); + // Bounded growth for the same reason the columns use it: a + // new FRS past the current high-water mark would otherwise + // let `resize` double the whole map. + let needed = frs_usize.saturating_add(1); + let extra = needed.saturating_sub(drive.frs_to_compact.len()); + crate::compact_storage::reserve_bounded(&mut drive.frs_to_compact, extra); + drive.frs_to_compact.resize(needed, u32::MAX); } if let Some(slot) = drive.frs_to_compact.get_mut(frs_usize) { *slot = new_compact_idx; @@ -207,10 +212,11 @@ pub(super) fn apply_rename( } let extension_id = drive.intern_extension(&change.filename); let name_start = drive.names.len(); + let name_bytes = change.filename.as_bytes(); drive .names - .as_mut_vec() - .extend_from_slice(change.filename.as_bytes()); + .vec_for_append(name_bytes.len()) + .extend_from_slice(name_bytes); let new_parent_frs = uffs_mft::frs_to_usize(change.parent_frs.raw()); let new_parent_compact = drive .frs_to_compact diff --git a/crates/uffs-core/src/compact_storage.rs b/crates/uffs-core/src/compact_storage.rs index 6aae26872..e83d1c958 100644 --- a/crates/uffs-core/src/compact_storage.rs +++ b/crates/uffs-core/src/compact_storage.rs @@ -21,7 +21,7 @@ //! //! Mutation-side: callers that need `Vec`-specific methods (`push`, //! `extend_from_slice`, `shrink_to_fit`, `reserve`) call -//! `ColumnStorage::as_mut_vec`. For [`ColumnStorage::Vec`] this is +//! `ColumnStorage::vec_for_append`. For [`ColumnStorage::Vec`] this is //! a cheap reference; for [`ColumnStorage::Mmap`] it triggers a //! one-time copy into a fresh `Vec` and replaces the variant. All //! mutating accessors funnel through the private @@ -34,7 +34,7 @@ //! zero-cost; mutation is direct. //! - [`ColumnStorage::Mmap`] — read-only view onto a memory-mapped region. //! Reads slice the mmap directly via [`bytemuck::cast_slice`] with zero -//! allocation. Mutating operations (`as_mut_slice`, `as_mut_vec`, +//! allocation. Mutating operations (`as_mut_slice`, `vec_for_append`, //! `IndexMut`, `DerefMut`) transparently promote to a heap-resident //! [`Vec`] on first call — the column "materialises" into the heap, the //! mmap stays alive (referenced by other [`Arc`] holders) and is @@ -63,6 +63,35 @@ use core::slice::SliceIndex; use memmap2::Mmap; +/// Divisor for the growth headroom [`ColumnStorage::vec_for_append`] +/// reserves: `len / 8`, i.e. 12.5 %. +/// +/// Growth stays geometric (so appends remain amortised O(1)) but the +/// wasted capacity is capped at an eighth of the column instead of the +/// whole of it. On a 3.3 M-record drive that is ~34 MB of slack rather +/// than the ~276 MB a single `Vec::push` would have claimed. +const APPEND_SLACK_DIVISOR: usize = 8; + +/// Floor for that headroom, so small columns do not reallocate on +/// every USN event just because an eighth of them is a handful of +/// entries. +const MIN_APPEND_SLACK: usize = 1024; + +/// Reserve room for `additional` more elements, growing by a bounded +/// slack instead of `Vec`'s doubling. +/// +/// The policy behind [`ColumnStorage::vec_for_append`], exposed +/// free-standing so the plain `Vec` fields that live beside the +/// columns — `DriveCompactIndex::frs_to_compact` — grow the same way +/// rather than each re-deriving it. A no-op when the capacity already +/// suffices. +pub(crate) fn reserve_bounded(vec: &mut Vec, additional: usize) { + if vec.len().saturating_add(additional) > vec.capacity() { + let slack = (vec.len() / APPEND_SLACK_DIVISOR).max(MIN_APPEND_SLACK); + vec.reserve_exact(additional.saturating_add(slack)); + } +} + /// Reasons a [`ColumnStorage::from_mmap_region`] call can reject a /// candidate `(mmap, byte_offset, len)` triple. /// @@ -153,7 +182,7 @@ pub enum ColumnStorage { /// Read-only typed view onto a region of an mmap'd file. /// /// Constructed via `ColumnStorage::from_mmap_region`. Mutating - /// operations — `as_mut_slice`, `as_mut_vec`, [`IndexMut`], + /// operations — `as_mut_slice`, `vec_for_append`, [`IndexMut`], /// [`DerefMut`] — transparently allocate a fresh [`Vec`] and /// `*self = Self::Vec(...)`, after which the column behaves as /// the `Vec` variant. @@ -296,19 +325,35 @@ impl ColumnStorage { self.materialise_to_vec().as_mut_slice() } - /// Promote to a [`Vec`] if not already, and return a mutable - /// reference. + /// Promote to a [`Vec`] sized to accept `additional` more elements, + /// growing by a **bounded** amount rather than `Vec`'s doubling. + /// + /// This is the **only** way to obtain a `&mut Vec` for appending — + /// a bare `as_mut_vec` escape hatch used to exist beside it, which + /// is exactly how the regression below reached production. Callers + /// that need `Vec`-specific mutation get it here, with the reserve + /// already done. Triggers a one-time `mmap → Vec` copy on the first + /// call against an `Mmap`-backed column; later calls are cheap. /// - /// Used by callers that need `Vec`-specific methods that are not - /// part of the slice API: [`Vec::push`], [`Vec::extend_from_slice`], - /// [`Vec::shrink_to_fit`], [`Vec::reserve`]. The Windows USN-patch - /// path in `crate::compact_loader::apply_usn_patch` is the - /// canonical caller. + /// These columns are shrunk to an exact fit after the index is + /// built (`shrink_compact_vecs` reclaims ~500 MB across seven + /// drives), which leaves `capacity == len`. `Vec`'s amortised + /// growth then reallocates to **twice** the capacity on the very + /// next append — so a single file created on `C:` handed back + /// every byte that shrink had reclaimed and then some: the record + /// column jumped 276 MB → 552 MB and the name arena 95 MB → 190 MB + /// for one new file, and stayed there for the life of the shard. /// - /// Triggers a one-time `mmap → Vec` copy on the first call against - /// an `Mmap`-backed column. Subsequent calls are cheap. - pub(crate) fn as_mut_vec(&mut self) -> &mut Vec { - self.materialise_to_vec() + /// Doubling is the right default for a `Vec` that knows nothing + /// about its contents, but these columns are hundreds of megabytes + /// and grow by a handful of records per USN batch. Reserving + /// [`APPEND_SLACK_DIVISOR`]⁻¹ of the current length keeps growth + /// geometric — so appends stay amortised O(1) — while capping the + /// waste at 12.5 % instead of 100 %. + pub(crate) fn vec_for_append(&mut self, additional: usize) -> &mut Vec { + let vec = self.materialise_to_vec(); + reserve_bounded(vec, additional); + vec } /// Consume and return the inner [`Vec`]. @@ -401,7 +446,7 @@ impl ColumnStorage { /// isn't already, then return a `&mut Vec`. /// /// Single source of truth for [`Self::as_mut_slice`] and - /// [`Self::as_mut_vec`]: both delegate here. For the `Vec` + /// [`Self::vec_for_append`]: both delegate here. For the `Vec` /// variant the call is `O(1)` (a re-borrow); for the `Mmap` /// variant it allocates a fresh `Vec` and `*self = Self::Vec`, /// so future calls are also `O(1)`. diff --git a/crates/uffs-core/src/compact_storage/tests.rs b/crates/uffs-core/src/compact_storage/tests.rs index d94d1fd0b..11c429fe3 100644 --- a/crates/uffs-core/src/compact_storage/tests.rs +++ b/crates/uffs-core/src/compact_storage/tests.rs @@ -64,12 +64,12 @@ fn deref_mut_lets_call_sites_mutate_in_place() { } #[test] -fn as_mut_vec_supports_vec_specific_methods() { +fn vec_for_append_supports_vec_specific_methods() { let mut column: ColumnStorage = ColumnStorage::default(); - column.as_mut_vec().push(7); - column.as_mut_vec().extend_from_slice(&[8, 9, 10]); + column.vec_for_append(1).push(7); + column.vec_for_append(3).extend_from_slice(&[8, 9, 10]); assert_eq!(column.as_slice(), &[7, 8, 9, 10]); - column.as_mut_vec().shrink_to_fit(); + column.vec_for_append(0).shrink_to_fit(); // shrink_to_fit may match capacity to len; either way, len stays. assert_eq!(column.len(), 4); } @@ -86,13 +86,72 @@ fn capacity_tracks_underlying_vec() { assert_eq!(column.len(), 1); } +/// Appending to an exactly-sized column must not double it. +/// +/// Regression: the compact index is shrunk to an exact fit after build +/// (`shrink_compact_vecs`, ~500 MB reclaimed across seven drives), +/// leaving `capacity == len`. `Vec`'s amortised growth then doubled +/// the whole column on the next append, so a single file created on a +/// live drive took the record column from 276 MB to 552 MB and the +/// name arena from 95 MB to 190 MB — permanently, for one new file. +#[test] +fn appending_to_an_exact_fit_column_grows_by_a_bounded_slack() { + const LEN: usize = 100_000; + let mut buf: Vec = (0..LEN).map(|i| u32::try_from(i).unwrap_or(0)).collect(); + buf.shrink_to_fit(); + let exact = buf.capacity(); + assert_eq!( + exact, LEN, + "precondition: the column starts at an exact fit" + ); + + let mut column = ColumnStorage::from_vec(buf); + column.vec_for_append(1).push(7); + + let grown = column.capacity(); + assert!(grown > LEN, "must have room for the appended element"); + assert!( + grown < LEN * 2, + "doubling is the bug: capacity went {LEN} -> {grown}" + ); + // 12.5 % headroom plus the one element we asked for. + assert!( + grown <= LEN + LEN / 8 + 1, + "slack must stay bounded: capacity went {LEN} -> {grown}" + ); +} + +/// Small columns get the floor, so a handful of entries does not mean +/// a reallocation per USN event. +#[test] +fn small_columns_reserve_at_least_the_minimum_slack() { + let mut column = ColumnStorage::from_vec(vec![1_u8, 2, 3]); + column.vec_for_append(1).push(4); + assert!( + column.capacity() >= 1024, + "expected the minimum slack floor, got {}", + column.capacity() + ); +} + +/// A column with room to spare is not reallocated at all. +#[test] +fn append_with_spare_capacity_does_not_reallocate() { + let mut buf: Vec = Vec::with_capacity(64); + buf.push(1); + let mut column = ColumnStorage::from_vec(buf); + let before = column.capacity(); + column.vec_for_append(1).push(2); + assert_eq!(column.capacity(), before, "spare capacity must be reused"); +} + #[test] fn clone_always_produces_a_vec_variant() { let original = ColumnStorage::from(vec![1_u32, 2, 3]); let mut copy = original.clone(); assert_eq!(copy.as_slice(), original.as_slice()); // Mutate the clone — original must remain unchanged. - copy.as_mut_vec().push(4); + copy.vec_for_append(1).push(4); assert_eq!(original.as_slice(), &[1_u32, 2, 3]); assert_eq!(copy.as_slice(), &[1_u32, 2, 3, 4]); } @@ -160,7 +219,7 @@ fn mmap_variant_deref_lets_call_sites_use_slice_methods() { } #[test] -fn as_mut_vec_promotes_mmap_to_heap() { +fn vec_for_append_promotes_mmap_to_heap() { let original = vec![100_u32, 200, 300]; let bytes = bytemuck::cast_slice::(&original); let mmap = make_mmap_for_test(bytes); @@ -169,7 +228,7 @@ fn as_mut_vec_promotes_mmap_to_heap() { .expect("valid region"); assert!(matches!(column, ColumnStorage::Mmap { .. })); // First mutation triggers the promotion. - column.as_mut_vec().push(400); + column.vec_for_append(1).push(400); assert!(matches!(column, ColumnStorage::Vec(_))); assert_eq!(column.as_slice(), &[100_u32, 200, 300, 400]); // The mmap is still alive (we hold an external reference) and From 7741ba349a3b3639570309562740e0de1a9b270d Mon Sep 17 00:00:00 2001 From: Robert M1 <50460704+githubrobbi@users.noreply.github.com> Date: Thu, 13 Aug 2026 12:05:10 -0700 Subject: [PATCH 16/16] fix(aggregation): honour every search filter, not four of them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The aggregation manual promises "All --agg commands accept every filter from the filters page". The scan honoured four: extensions, files/dirs, size bounds, and drive scope. Everything else was silently dropped — found live when `--in-path` naming a directory that cannot exist still counted 3,835,372 files, and the path-aware glob `'**\GitHub\**\*' --count` returned 0 while the same scope as a literal counted 400. Two distinct gaps, one shared root: Record-level filters (dates, attributes, excludes, months, name/path lengths, tree metrics, bulkiness) simply never reached the scan — the daemon built a full `SearchFilters` for the row search and handed the aggregation a 4-field `AggregateFilter`. `--newer 7d --count` counted every file ever written. The scan now also runs `matches_record` — the SAME predicate the row search's record scans run — with extension IDs resolved per drive, so a count and a row listing can no longer disagree. The aggregate cache key hashes the filter set (via its Debug rendering, so a future filter field cannot be silently omitted); the in-process cache can never serve a date-scoped count computed without the dates. Path-dependent scoping (--in-path, --exclude-path, --type, path-aware globs, --match-path, regex patterns) cannot be honoured by a record scan at all: it needs resolved paths, and the scan matched bare names — which is exactly why the path glob counted 0. Rather than re-implement path semantics in the aggregation engine (guaranteed drift), such queries now aggregate over the row search's matched set: the search — already unbounded for these shapes, and now also when an aggregation rides on one — applies the full path semantics exactly once, and `run_aggregate_over_records` folds the surviving (drive, record) pairs into the same accumulators. The matched set is snapshotted before the display truncation, so `--limit` bounds what the user sees, never what a count reports. Regression tests pin all three behaviours: a date bound splits the fixture and the count matches the split; an explicit two-record set counts exactly 2; the empty set — the impossible `--in-path` shape — counts 0, never the drive total. --- .../src/aggregate/integration_tests.rs | 105 +++++++++++++++ crates/uffs-core/src/aggregate/mod.rs | 120 ++++++++++++++++- crates/uffs-daemon/src/index/aggregation.rs | 121 +++++++++++++++++- crates/uffs-daemon/src/index/search.rs | 41 +++++- .../src/index/search_predicates.rs | 24 ++++ 5 files changed, 400 insertions(+), 11 deletions(-) diff --git a/crates/uffs-core/src/aggregate/integration_tests.rs b/crates/uffs-core/src/aggregate/integration_tests.rs index ec0eaad7f..93ab3413c 100644 --- a/crates/uffs-core/src/aggregate/integration_tests.rs +++ b/crates/uffs-core/src/aggregate/integration_tests.rs @@ -1010,6 +1010,7 @@ fn count_with_extension_absent_on_drive_is_zero() { &FinalizeOptions::default(), None, &present, + None, ) .expect("count with present extension"); assert!( @@ -1032,6 +1033,7 @@ fn count_with_extension_absent_on_drive_is_zero() { &FinalizeOptions::default(), None, &absent, + None, ) .expect("count with absent extension"); assert!( @@ -1043,3 +1045,106 @@ fn count_with_extension_absent_on_drive_is_zero() { absent_output.response.results[0].data ); } + +// ── Full search-filter parity in the aggregation scan ──────────────── +// +// Regression, 2026-08-13, found live: the aggregation scan honoured only +// extensions / directory-flag / size, so `--newer 7d --count` counted +// every file ever written and `--in-path --count` still +// returned 3.8 M. Record-level filters now flow in as `SearchFilters`; +// path-dependent queries aggregate over the row search's matched set +// (`run_aggregate_over_records`). + +#[test] +fn count_honours_search_filter_date_bounds() { + let drive = build_agg_test_drive(); + let specs = vec![AggregateSpec::new(AggregateKind::Count)]; + + // Bound strictly between the fixture's timestamp clusters, computed + // FROM the fixture so the test cannot drift from the conversion the + // index build applies (NTFS ticks → Unix µs). + let mut stamps: Vec = drive.records.iter().map(|rec| rec.modified).collect(); + stamps.sort_unstable(); + stamps.dedup(); + assert!(stamps.len() >= 2, "fixture must span several timestamps"); + let bound = stamps[stamps.len() - 1]; + let expected = drive + .records + .iter() + .filter(|rec| rec.modified >= bound) + .count(); + assert!( + expected > 0 && expected < drive.records.len(), + "bound must genuinely split the fixture" + ); + + let filters = crate::search::filters::SearchFilters { + newer_us: Some(bound), + ..Default::default() + }; + let output = run_aggregate_with_filters( + &[&drive], + &specs, + &FinalizeOptions::default(), + None, + &AggregateFilter::default(), + Some(&filters), + ) + .expect("count with date bound"); + let AggregateResultData::Count { value } = output.response.results[0].data else { + panic!("count result expected"); + }; + assert_eq!( + usize::try_from(value).unwrap_or(usize::MAX), + expected, + "aggregation must apply the same newer-bound the row search applies" + ); +} + +#[test] +fn over_records_counts_exactly_the_handed_set() { + let drive = build_agg_test_drive(); + let specs = vec![AggregateSpec::new(AggregateKind::Count)]; + + // Hand-pick two records by name — the shape of the row-fed path: + // the row search decides WHAT matched, aggregation only folds it. + let matched: Vec<(uffs_mft::platform::DriveLetter, u32)> = drive + .records + .iter() + .enumerate() + .filter(|(_, rec)| matches!(rec.name(&drive.names), "main.rs" | "data.bin")) + .map(|(idx, _)| (drive.letter, uffs_mft::len_to_u32(idx))) + .collect(); + assert_eq!(matched.len(), 2, "fixture must contain both probe files"); + + let output = + run_aggregate_over_records(&[&drive], &specs, &FinalizeOptions::default(), &matched) + .expect("over-records count"); + assert!( + matches!( + output.response.results[0].data, + AggregateResultData::Count { value: 2 } + ), + "count must equal the handed set, got {:?}", + output.response.results[0].data + ); + assert_eq!(output.records_matched, 2); +} + +#[test] +fn over_records_empty_set_counts_zero() { + // The `--in-path ` shape: the row search + // matched nothing, so the count must be 0 — never the drive total. + let drive = build_agg_test_drive(); + let specs = vec![AggregateSpec::new(AggregateKind::Count)]; + let output = run_aggregate_over_records(&[&drive], &specs, &FinalizeOptions::default(), &[]) + .expect("over-records with empty set"); + assert!( + matches!( + output.response.results[0].data, + AggregateResultData::Count { value: 0 } + ), + "empty matched set must count 0, got {:?}", + output.response.results[0].data + ); +} diff --git a/crates/uffs-core/src/aggregate/mod.rs b/crates/uffs-core/src/aggregate/mod.rs index 46746c76a..3ceb677d5 100644 --- a/crates/uffs-core/src/aggregate/mod.rs +++ b/crates/uffs-core/src/aggregate/mod.rs @@ -418,6 +418,7 @@ pub fn run_aggregate_with_filters( options: &FinalizeOptions, pattern: Option<&str>, filter: &AggregateFilter, + search_filters: Option<&crate::search::filters::SearchFilters>, ) -> Result { // Fast path: no filters and trivial pattern → unfiltered scan. use uffs_text::case_fold::CaseFold; @@ -425,12 +426,20 @@ pub fn run_aggregate_with_filters( use crate::index_search::compile_parsed_pattern; use crate::pattern::ParsedPattern; + // `search_filters` carries the FULL record-level filter axis the row + // search applies (dates, attributes, excludes, months, tree metrics, + // …). Ignoring it here is the bug that made `--newer 7d --count` + // count every file ever written: the aggregation scan honoured only + // extensions / directory-flag / size and silently dropped the rest. + // A populated filter therefore disqualifies every fast path below. + let sf_active = search_filters.is_some_and(|sf| !sf.is_empty()); + let trivial_pattern = pattern.is_none_or(|pat| matches!(pat, "*" | "**" | "**/*" | "")); - if filter.is_empty() && trivial_pattern { + if filter.is_empty() && trivial_pattern && !sf_active { return run_aggregate(drives, specs, options); } // Pattern-only → delegate to existing filtered path. - if filter.is_empty() { + if filter.is_empty() && !sf_active { if let Some(pat) = pattern { return run_aggregate_filtered(drives, specs, options, pat); } @@ -481,6 +490,19 @@ pub fn run_aggregate_with_filters( return (local, 0, 0); } + // Per-drive resolved copy of the search filters: extension + // strings become this drive's interned `u16` IDs, so the + // per-record check below stays O(1) instead of falling into + // the per-record string-extraction fallback. + let sf_drive = search_filters.map(|sf| { + let mut resolved = sf.clone(); + resolved.resolve_ext_ids_for_drive(drive); + resolved + }); + // Reusable fold buffer for exclude-glob matching — same + // zero-alloc pattern as the row search's scan loops. + let mut fold_buf: Vec = Vec::with_capacity(256); + for (idx, record) in drive.records.iter().enumerate() { scanned += 1; @@ -489,6 +511,15 @@ pub fn run_aggregate_with_filters( continue; } + // Full record-level search filters — the SAME predicate the + // row search runs, so `--count` and a row listing can never + // disagree on dates, attributes, excludes, or tree metrics. + if let Some(sf) = &sf_drive + && !sf.matches_record(record, &drive.names, &mut fold_buf, drive.fold) + { + continue; + } + // Pattern filter (if non-trivial). if let Some(pat) = compiled_pattern_ref { let name = record.name(&drive.names); @@ -539,6 +570,91 @@ pub fn run_aggregate_with_filters( }) } +/// Run aggregations over an explicit, already-matched record set. +/// +/// The record-scan entry points above re-derive the match set from +/// pattern + filters — which is only correct for constraints that are +/// decidable per `CompactRecord`. Anything that needs a **resolved +/// path** (`--in-path`, `--exclude-path`, path-aware globs like +/// `**\GitHub\**\*`, `--match-path`, regex patterns) lives in the row +/// search's post-filter pass and cannot be replicated here without +/// semantic drift — the observed failure was `--in-path` silently +/// ignored by `--count` and a path-aware glob counting 0. +/// +/// For those queries the caller runs the row search (unbounded), which +/// applies the full path semantics exactly once, and hands the matched +/// `(drive letter, record index)` pairs here. The search stays the +/// single source of truth for what matched; this function only folds +/// the survivors into accumulators. +/// +/// `records_scanned` reports the matched-set size, not an index-wide +/// scan — the scan already happened inside the row search. +/// +/// # Errors +/// +/// Returns [`AggregateError`] when the spec list fails to compile. +pub fn run_aggregate_over_records( + drives: &[&DriveCompactIndex], + specs: &[AggregateSpec], + options: &FinalizeOptions, + matched: &[(uffs_mft::platform::DriveLetter, u32)], +) -> Result { + let start = std::time::Instant::now(); + let plan = AggregatePlan::compile(specs)?; + let ext_map = ExtensionMap::build(drives); + let mut accumulators = plan.create_accumulators(); + + // Letter → (ordinal, drive) for O(1) row dispatch. Ordinals index + // into `drives`, matching what the scan entry points feed and what + // finalize expects for drive-keyed groupings. + let by_letter: std::collections::HashMap< + uffs_mft::platform::DriveLetter, + (u8, &DriveCompactIndex), + > = drives + .iter() + .enumerate() + .map(|(ordinal, drive)| { + ( + drive.letter, + (u8::try_from(ordinal).unwrap_or(u8::MAX), *drive), + ) + }) + .collect(); + + let mut fed: u64 = 0; + for &(letter, record_idx) in matched { + let Some(&(ordinal, drive)) = by_letter.get(&letter) else { + // Row from a drive outside the aggregation scope (e.g. a + // `--drives` subset) — scoping, not an error. + continue; + }; + let idx = uffs_mft::frs_to_usize(u64::from(record_idx)); + let Some(record) = drive.records.get(idx) else { + continue; + }; + fed += 1; + for acc in &mut accumulators { + acc.feed(record, drive, idx, ordinal, &ext_map); + } + } + + let response = + finalize::finalize_with_ext_map(accumulators, &plan, drives, options, fed, &ext_map); + tracing::info!( + drives = drives.len(), + rows_in = matched.len(), + records_matched = fed, + total_ms = u64::try_from(start.elapsed().as_millis()).unwrap_or(u64::MAX), + "run_aggregate_over_records: done" + ); + Ok(AggregateOutput { + response, + records_scanned: fed, + records_matched: fed, + execution_us: start.elapsed().as_micros().try_into().unwrap_or(u64::MAX), + }) +} + /// Merge per-drive accumulator sets. /// /// Pairs accumulators by index and calls [`GroupAccumulator::merge`] diff --git a/crates/uffs-daemon/src/index/aggregation.rs b/crates/uffs-daemon/src/index/aggregation.rs index 2181c19b2..822fad3dd 100644 --- a/crates/uffs-daemon/src/index/aggregation.rs +++ b/crates/uffs-daemon/src/index/aggregation.rs @@ -190,6 +190,22 @@ struct AggregateCacheCtx<'a> { cache: Option<&'a uffs_core::aggregate::AggregateCache>, } +/// The scan-scope inputs [`IndexManager::compute_aggregate_output`] +/// forwards into the core record scan. +/// +/// Bundled so the helper stays under clippy's `too_many_arguments` +/// budget — the three fields always travel together anyway: they are +/// exactly the inputs that decide which records the scan feeds. +#[derive(Clone, Copy)] +struct ScanScope<'a> { + /// Glob / regex name matcher, `None` for match-all. + pattern: Option<&'a str>, + /// O(1)-per-record predicates: extension IDs, directory flag, size. + record_filter: &'a uffs_core::aggregate::AggregateFilter, + /// Full record-level search-filter axis (dates, attributes, …). + search_filters: Option<&'a uffs_core::search::filters::SearchFilters>, +} + /// Bundled inputs for [`IndexManager::run_aggregations`]. /// /// Wraps the predicates, pagination knobs, and scope filters that @@ -231,6 +247,14 @@ pub(crate) struct AggregationRequest<'a> { /// size bounds. Defaults to "no filter" via /// [`uffs_core::aggregate::AggregateFilter::default`]. pub record_filter: uffs_core::aggregate::AggregateFilter, + /// The FULL record-level filter set the row search applies (dates, + /// attributes, excludes, months, tree metrics, …), so aggregation + /// honours every `--flag` the row listing honours. `None` (the + /// test default) means "no extra constraints". Path-dependent + /// filters inside it are NOT applied by the record scan — callers + /// with path-dependent queries must use + /// [`IndexManager::run_aggregations_over_rows`] instead. + pub search_filters: Option, } impl IndexManager { @@ -262,6 +286,7 @@ impl IndexManager { pattern, drives_filter, record_filter, + search_filters, } = request; // Convert wire specs to core specs. @@ -303,6 +328,7 @@ impl IndexManager { drives_filter, &record_filter, &query_predicates, + search_filters.as_ref(), ) }); @@ -328,8 +354,11 @@ impl IndexManager { &drive_refs, &specs, &options, - pattern, - &record_filter, + ScanScope { + pattern, + record_filter: &record_filter, + search_filters: search_filters.as_ref(), + }, &snapshot.drives, ) { Some(out) => out, @@ -352,6 +381,77 @@ impl IndexManager { (wire_results, records_matched) } + /// Run aggregation specs over the row search's matched result set. + /// + /// The record-scan path ([`Self::run_aggregations`]) cannot honour + /// anything that needs a **resolved path** — `--in-path`, + /// `--exclude-path`, path-aware globs, `--match-path`, regex + /// patterns. It used to run anyway and silently ignore them: an + /// `--in-path` naming a directory that cannot exist still counted + /// 3.8 M files, and `'**\GitHub\**\*' --count` returned 0 because + /// the path glob was matched against bare names. + /// + /// For those queries the caller has already run the row search + /// unbounded — which applies full path semantics exactly once — + /// and passes the matched rows here. The search stays the single + /// source of truth for what matched; aggregation only folds the + /// survivors. No cache: the row set is the input, and hashing + /// millions of `(drive, idx)` pairs would cost more than the fold. + pub(crate) fn run_aggregations_over_rows( + snapshot: &DriveIndex, + wire_specs: &[uffs_client::protocol::AggregateSpecWire], + matched: &[(uffs_mft::platform::DriveLetter, u32)], + query_predicates: Vec, + agg_cursor: Option<&str>, + agg_page_size: Option, + ) -> (Vec, u64) { + use uffs_core::aggregate::finalize::FinalizeOptions; + use uffs_core::aggregate::spec::AggregateSpec; + + let mut specs: Vec = Vec::new(); + for ws in wire_specs { + match Self::convert_wire_spec(ws) { + Ok(converted) => specs.extend(converted), + Err(e) => { + tracing::warn!(kind = %ws.kind, "skipping malformed aggregate spec: {e}"); + } + } + } + if specs.is_empty() { + return (vec![], 0); + } + + let drive_refs: Vec<&uffs_core::compact::DriveCompactIndex> = + snapshot.drives.iter().map(|arc| arc.as_ref()).collect(); + let options = FinalizeOptions { + query_predicates, + ..FinalizeOptions::default() + }; + + match uffs_core::aggregate::run_aggregate_over_records( + &drive_refs, + &specs, + &options, + matched, + ) { + Ok(mut output) => { + Self::run_duplicate_verification(&specs, &mut output, &snapshot.drives); + let records_matched = output.records_matched; + let wire_results = convert_aggregate_results_to_wire( + output.response.results, + agg_cursor, + agg_page_size, + &snapshot.drives, + ); + (wire_results, records_matched) + } + Err(e) => { + tracing::error!(error = %e, "row-set aggregation failed"); + (vec![], 0) + } + } + } + /// Look up the cached `AggregateOutput` if present, otherwise run /// the core aggregation with duplicate verification + cache fill. /// @@ -363,8 +463,7 @@ impl IndexManager { drive_refs: &[&uffs_core::compact::DriveCompactIndex], specs: &[uffs_core::aggregate::spec::AggregateSpec], options: &uffs_core::aggregate::finalize::FinalizeOptions, - pattern: Option<&str>, - record_filter: &uffs_core::aggregate::AggregateFilter, + scope: ScanScope<'_>, drives: &[alloc::sync::Arc], ) -> Option { let AggregateCacheCtx { key_hash, cache } = cache_ctx; @@ -381,8 +480,9 @@ impl IndexManager { drive_refs, specs, options, - pattern, - record_filter, + scope.pattern, + scope.record_filter, + scope.search_filters, ) { Ok(mut fresh) => { tracing::info!( @@ -439,6 +539,7 @@ impl IndexManager { drives_filter: &[uffs_mft::platform::DriveLetter], record_filter: &uffs_core::aggregate::AggregateFilter, query_predicates: &[DrilldownPredicate], + search_filters: Option<&uffs_core::search::filters::SearchFilters>, ) -> u64 { use core::hash::{Hash as _, Hasher as _}; let mut hasher = std::collections::hash_map::DefaultHasher::new(); @@ -451,6 +552,14 @@ impl IndexManager { drives_filter.hash(&mut hasher); record_filter.hash(&mut hasher); query_predicates.hash(&mut hasher); + // `SearchFilters` does not implement `Hash` (and hand-listing its + // ~30 fields here would silently miss every future addition), so + // hash the `Debug` rendering: it includes every field, and this + // cache is in-process memory only — the representation never has + // to be stable across builds, only within one daemon lifetime. + // Missing a filter here is not a stale-display nuisance; it is + // the `--newer 7d --count` bug served forever out of the cache. + format!("{search_filters:?}").hash(&mut hasher); hasher.finish() } diff --git a/crates/uffs-daemon/src/index/search.rs b/crates/uffs-daemon/src/index/search.rs index 954584ba1..ad1fa483b 100644 --- a/crates/uffs-daemon/src/index/search.rs +++ b/crates/uffs-daemon/src/index/search.rs @@ -248,8 +248,19 @@ impl IndexManager { max_size: filters.max_size, }; + // ── Aggregation routing (decided BEFORE `filters` is moved) ── + // Rationale lives on `aggregation_needs_row_set`. + let agg_requested = !effective_params.aggregations.is_empty(); + let agg_over_rows = aggregation_needs_row_set(&effective_params, &filters); + // The full record-level filter set for the scan path, cloned + // before `filters` moves into the search closure. + let agg_search_filters = filters.clone(); + let search_limit = resolve_search_limit( - requires_post_filter, + // Aggregating over the row set needs EVERY matching row, not + // the display limit's worth — a truncated set would silently + // undercount. + requires_post_filter || agg_over_rows, filters.needs_display_row_filter(), filters.malformed == Some(true), effective_params.limit, @@ -342,6 +353,17 @@ impl IndexManager { } let mut total_count = filtered_rows.len() as u64; + // Snapshot the matched set for row-fed aggregation BEFORE the + // display truncation below — the display limit bounds what the + // user sees, never what an aggregation counts. + let agg_row_set: Vec<(uffs_mft::platform::DriveLetter, u32)> = if agg_over_rows { + filtered_rows + .iter() + .map(|row| (row.drive, row.record_index)) + .collect() + } else { + Vec::new() + }; if let Some(limit) = effective_params.limit { filtered_rows.truncate(limit as usize); } @@ -537,7 +559,19 @@ impl IndexManager { }); // ── Aggregation (if requested) ───────────────────────────── - let (agg_results, agg_matched) = if !effective_params.aggregations.is_empty() { + let (agg_results, agg_matched) = if agg_over_rows { + // Path-dependent query: fold the row search's matched set — + // the rows already carry every filter and the true path + // semantics, applied once by the engine that owns them. + Self::run_aggregations_over_rows( + &agg_snapshot, + &effective_params.aggregations, + &agg_row_set, + build_query_predicates(&effective_params), + effective_params.agg_cursor.as_deref(), + effective_params.agg_page_size, + ) + } else if agg_requested { let predicates = build_query_predicates(&effective_params); // Pass the pattern if it's non-trivial (not just `*`). @@ -559,6 +593,7 @@ impl IndexManager { pattern: agg_pattern, drives_filter: &effective_params.drives, record_filter: agg_record_filter, + search_filters: Some(agg_search_filters), }, ) } else { @@ -742,7 +777,7 @@ pub(crate) use output_config::build_output_config; // `pub(super)` — not re-exported beyond the `search` module. #[path = "search_predicates.rs"] mod predicates; -use predicates::build_query_predicates; +use predicates::{aggregation_needs_row_set, build_query_predicates}; // The `--out=` file-export writer lives in a sibling file to keep // `search.rs` under the 800-line policy ceiling. It was a `Self`-less diff --git a/crates/uffs-daemon/src/index/search_predicates.rs b/crates/uffs-daemon/src/index/search_predicates.rs index 324f300cd..d9800bc93 100644 --- a/crates/uffs-daemon/src/index/search_predicates.rs +++ b/crates/uffs-daemon/src/index/search_predicates.rs @@ -19,6 +19,30 @@ use uffs_client::protocol::SearchParams; use uffs_core::aggregate::finalize::{DrilldownPredicate, DrilldownValue}; +/// Must an aggregation ride on the row search's matched set instead of +/// the record scan? +/// +/// The aggregation record scan cannot honour anything that needs a +/// **resolved path**: `--in-path` / `--exclude-path` / `--type` live in +/// the display-row pass, and path-aware globs (`**\GitHub\**\*`), +/// `--match-path`, and regex patterns are matched by the row search's +/// own machinery, not by a name glob. Running the scan anyway silently +/// dropped those constraints — an `--in-path` naming an impossible +/// directory still counted 3.8 M files, and a path glob counted 0. +/// For such queries the aggregation folds the row search's matched set, +/// so path semantics are applied exactly once, by the engine that owns +/// them. +pub(super) fn aggregation_needs_row_set( + params: &SearchParams, + filters: &uffs_core::search::filters::SearchFilters, +) -> bool { + let pattern_needs_paths = params.match_path + || params.pattern.starts_with('>') + || params.pattern.contains('\\') + || params.pattern.contains('/'); + !params.aggregations.is_empty() && (filters.needs_display_row_filter() || pattern_needs_paths) +} + /// Build the drill-down-predicate list that prefixes every /// aggregation bucket's follow-up query. ///